net: Fix the fib trie iterator to work with a single entry routing tables

PATCH New Eric W. Biederman 3 participants 7 messages Jan 24, 2007 – Jan 27, 2007

Subsystems (3)

Networking Network drivers IPv4/IPv6

lore

Share & tools

lore

Eric W. Biederman

author

2007-01-24 19:56 UTC #0 lore raw

PATCH 1/1 net: Fix the fib trie iterator to work with a single entry routing tables

In a kernel with trie routing enabled I had a simple routing setup with only a single route to the outside world and no default route. "ip route table list main" showed my the route just fine but /proc/net/route was an empty file. What was going on?

Thinking it was a bug in something I did and I looked deeper. Eventually I setup a second route and everything looked correct, huh? Finally I realized that the it was just the iterator pair in fib_trie_get_first, fib_trie_get_next just could not handle a routing table with a single entry.

So to save myself and others further confusion, here is a simple fix for the fib proc iterator so it works even when there is only a single route in a routing table.

Signed-off-by Eric W. Biederman <[email protected]>

---
 net/ipv4/fib_trie.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

net/ipv4/fib_trie.c +16 -5

index cfb249c..13307c0 100644
@@ -1989,6 +1989,10 @@ static struct node *fib_trie_get_next(struct fib_trie_iter *iter)
 	unsigned cindex = iter->index;
 	struct tnode *p;
 
+	/* A single entry routing table */
+	if (!tn)
+		return NULL;
+
 	pr_debug("get_next iter={node=%p index=%d depth=%d}\n",
 		 iter->tnode, iter->index, iter->depth);
 rescan:

@@ -2037,11 +2041,18 @@ static struct node *fib_trie_get_first(struct fib_trie_iter *iter,
 	if(!iter)
 		return NULL;
 
-	if (n && IS_TNODE(n)) {
-		iter->tnode = (struct tnode *) n;
-		iter->trie = t;
-		iter->index = 0;
-		iter->depth = 1;
+	if (n) {
+		if (IS_TNODE(n)) {
+			iter->tnode = (struct tnode *) n;
+			iter->trie = t;
+			iter->index = 0;
+			iter->depth = 1;
+		} else {
+			iter->tnode = NULL;
+			iter->trie  = t;
+			iter->index = 0;
+			iter->depth = 0;
+		}
 		return n;
 	}
 	return NULL;

Signature · 1 line

1.4.4.1.g278f

Robert Olsson

2007-01-24 22:21 UTC #1 lore raw

PATCH 1/1 net: Fix the fib trie iterator to work with a single entry routing tables

Hello!

Yes the case when the trie is just a single leaf got wrong with the iterator and your patchs cures it. I think we have a similar problem with /proc/net/fib_trie

Cheers

							--ro

Signed-off-by Robert Olsson <[email protected]>

Quoted 61 lines · Eric W. Biederman

In a kernel with trie routing enabled I had a simple routing setup with only a single route to the outside world and no default route. "ip route table list main" showed my the route just fine but /proc/net/route was an empty file. What was going on?
Thinking it was a bug in something I did and I looked deeper. Eventually I setup a second route and everything looked correct, huh? Finally I realized that the it was just the iterator pair in fib_trie_get_first, fib_trie_get_next just could not handle a routing table with a single entry.
So to save myself and others further confusion, here is a simple fix for the fib proc iterator so it works even when there is only a single route in a routing table.
Signed-off-by Eric W. Biederman <[email protected]>
---
 net/ipv4/fib_trie.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)
net/ipv4/fib_trie.c +16 -5
index cfb249c..13307c0 100644
@@ -1989,6 +1989,10 @@ static struct node *fib_trie_get_next(struct fib_trie_iter *iter)
 	unsigned cindex = iter->index;
 	struct tnode *p;
 
+	/* A single entry routing table */
+	if (!tn)
+		return NULL;
+
 	pr_debug("get_next iter={node=%p index=%d depth=%d}\n",
 		 iter->tnode, iter->index, iter->depth);
 rescan:
@@ -2037,11 +2041,18 @@ static struct node *fib_trie_get_first(struct fib_trie_iter *iter,
 	if(!iter)
 		return NULL;
 
-	if (n && IS_TNODE(n)) {
-		iter->tnode = (struct tnode *) n;
-		iter->trie = t;
-		iter->index = 0;
-		iter->depth = 1;
+	if (n) {
+		if (IS_TNODE(n)) {
+			iter->tnode = (struct tnode *) n;
+			iter->trie = t;
+			iter->index = 0;
+			iter->depth = 1;
+		} else {
+			iter->tnode = NULL;
+			iter->trie  = t;
+			iter->index = 0;
+			iter->depth = 0;
+		}
 		return n;
 	}
 	return NULL;
Signature · 1 line
1.4.4.1.g278f

David Miller

maintainer

2007-01-24 22:43 UTC #2 lore raw

From: Robert Olsson <[email protected]>
Date: Wed, 24 Jan 2007 22:43:30 +0100

Quoted 3 lines

Yes the case when the trie is just a single leaf got wrong with the iterator and your patchs cures it. I think we have a similar problem with /proc/net/fib_trie

I'm happy to review a fix for that :-)

Quoted 1 line

Signed-off-by Robert Olsson <[email protected]>

Patch applied, thanks everyone.

Eric W. Biederman

author

2007-01-25 02:12 UTC #3 lore raw

Quoted 7 lines · David Miller

From: Robert Olsson <[email protected]>
Date: Wed, 24 Jan 2007 22:43:30 +0100
Quoted 3 lines
Yes the case when the trie is just a single leaf got wrong with the iterator and your patchs cures it. I think we have a similar problem with /proc/net/fib_trie

Quoted 5 lines

I'm happy to review a fix for that :-)
Quoted 1 line
Signed-off-by Robert Olsson <[email protected]>
Patch applied, thanks everyone.

Same iterator function so this fixes that as well.

Eric

Robert Olsson

2007-01-26 12:56 UTC #4 lore raw

In reply to David Miller

Quoted 5 lines · David Miller

Quoted 3 lines
Yes the case when the trie is just a single leaf got wrong with the iterator and your patchs cures it. I think we have a similar problem with /proc/net/fib_trie
I'm happy to review a fix for that :-)

When main table is just a single leaf this gets printed as belonging to the local table in /proc/net/fib_trie. A fix is below.

Cheers.

						--ro

Signed-off-by Robert Olsson <[email protected]>

net/ipv4/fib_trie.c renamed from net/ipv4/fib_trie.c.orig +7 -6

@@ -2290,16 +2290,17 @@
 	if (v == SEQ_START_TOKEN)
 		return 0;
 
+	if (!NODE_PARENT(n)) {
+		if (iter->trie == trie_local)
+			seq_puts(seq, "<local>:\n");
+		else
+			seq_puts(seq, "<main>:\n");
+	} 
+
 	if (IS_TNODE(n)) {
 		struct tnode *tn = (struct tnode *) n;
 		__be32 prf = htonl(MASK_PFX(tn->key, tn->pos));
 
-		if (!NODE_PARENT(n)) {
-			if (iter->trie == trie_local)
-				seq_puts(seq, "<local>:\n");
-			else
-				seq_puts(seq, "<main>:\n");
-		} 
 		seq_indent(seq, iter->depth-1);
 		seq_printf(seq, "  +-- %d.%d.%d.%d/%d %d %d %d\n",
 			   NIPQUAD(prf), tn->pos, tn->bits, tn->full_children,

Eric W. Biederman

author

2007-01-26 15:43 UTC #5 lore raw

Quoted 10 lines · Robert Olsson

Quoted 5 lines · David Miller
Quoted 3 lines
Yes the case when the trie is just a single leaf got wrong with the iterator and your patchs cures it. I think we have a similar problem with /proc/net/fib_trie
I'm happy to review a fix for that :-)
When main table is just a single leaf this gets printed as belonging to the local table in /proc/net/fib_trie. A fix is below.

Good spotting. I was happy the data was showing up, I missed we that we put the meta information was wrong.

Looks good to me.

Eric

David Miller

maintainer

2007-01-27 03:06 UTC #6 lore raw

From: [email protected] (Eric W. Biederman)
Date: Fri, 26 Jan 2007 08:42:25 -0700

Quoted 10 lines

Quoted 8 lines · Robert Olsson
Quoted 5 lines · David Miller
Quoted 3 lines
Yes the case when the trie is just a single leaf got wrong with the iterator and your patchs cures it. I think we have a similar problem with /proc/net/fib_trie
I'm happy to review a fix for that :-)

Quoted 7 lines

Quoted 2 lines
When main table is just a single leaf this gets printed as belonging to the local table in /proc/net/fib_trie. A fix is below.
Good spotting. I was happy the data was showing up, I missed we that we put the meta information was wrong.
Looks good to me.

To me too, applied.

Thanks!