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

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

[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 <ebiederm@xmission.com>
---
 net/ipv4/fib_trie.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index cfb249c..13307c0 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -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;
-- 
1.4.4.1.g278f

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

[Robert Olsson]

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 <robert.olsson@its.uu.se>



Eric W. Biederman writes:
 > 
 > 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 <ebiederm@xmission.com>
 > ---
 >  net/ipv4/fib_trie.c |   21 ++++++++++++++++-----
 >  1 files changed, 16 insertions(+), 5 deletions(-)
 > 
 > diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
 > index cfb249c..13307c0 100644
 > --- a/net/ipv4/fib_trie.c
 > +++ b/net/ipv4/fib_trie.c
 > @@ -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;
 > -- 
 > 1.4.4.1.g278f

Re: [PATCH] net: Fix the fib trie iterator to work with a single entry routing tables

[David Miller]

From: Robert Olsson <Robert.Olsson@data.slu.se>
Date: Wed, 24 Jan 2007 22:43:30 +0100

> 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 :-)

> Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>

Patch applied, thanks everyone.

Re: [PATCH] net: Fix the fib trie iterator to work with a single entry routing tables

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> From: Robert Olsson <Robert.Olsson@data.slu.se>
> Date: Wed, 24 Jan 2007 22:43:30 +0100
>
>> 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 :-)
>
>> Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>
>
> Patch applied, thanks everyone.

Same iterator function so this fixes that as well.

Eric

Re: [PATCH] net: Fix the fib trie iterator to work with a single entry routing tables

[Robert Olsson]

David Miller writes:

 > > 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 <robert.olsson@its.uu.se>


--- net-2.6.20/net/ipv4/fib_trie.c.orig	2007-01-26 13:18:13.000000000 +0100
+++ net-2.6.20/net/ipv4/fib_trie.c	2007-01-26 13:19:36.000000000 +0100
@@ -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, 
 

 

Re: [PATCH] net: Fix the fib trie iterator to work with a single entry routing tables

[Eric W. Biederman]

Robert Olsson <Robert.Olsson@data.slu.se> writes:

> David Miller writes:
>
>  > > 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

Re: [PATCH] net: Fix the fib trie iterator to work with a single entry routing tables

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 26 Jan 2007 08:42:25 -0700

> Robert Olsson <Robert.Olsson@data.slu.se> writes:
> 
> > David Miller writes:
> >
> >  > > 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.

To me too, applied.

Thanks!