[PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[Thomas Graf]

Dave,

I would appreciate if this could go in before 2.6.12, it would
allow the upcomming netconfig pre release to run on 2.6.12 out
of the box.

Please do a

	bk pull bk://kernel.bkbits.net/tgraf/net-2.6-trunk

This will update the following files:

 net/core/rtnetlink.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

through these ChangeSets:

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2005/04/07 17:41:18+02:00 tgraf@suug.ch 
#   [RTNETLINK]: Protocol family wildcard dumping for routing rules
#   
#   Be kind to userspace and don't force them to hardcode protocol
#   families just to have it changed again once we support routing
#   rules for more than one protocol family.
#   
#   Signed-off-by: Thomas Graf <tgraf@suug.ch>
#   Signed-off-by: David S. Miller <davem@davemloft.net>
# 
# net/core/rtnetlink.c
#   2005/04/07 17:41:07+02:00 tgraf@suug.ch +2 -1
#   [RTNETLINK]: Protocol family wildcard dumping for routing rules
# 
diff -Nru a/net/core/rtnetlink.c b/net/core/rtnetlink.c
--- a/net/core/rtnetlink.c	2005-04-07 23:13:33 +02:00
+++ b/net/core/rtnetlink.c	2005-04-07 23:13:33 +02:00
@@ -647,7 +647,8 @@
 	[RTM_GETROUTE - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
 	[RTM_NEWNEIGH - RTM_BASE] = { .doit   = neigh_add	      },
 	[RTM_DELNEIGH - RTM_BASE] = { .doit   = neigh_delete	      },
-	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      }
+	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      },
+	[RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    }
 };
 
 static int rtnetlink_event(struct notifier_block *this, unsigned long event, void *ptr)

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[Herbert Xu]

Thomas Graf <tgraf@suug.ch> wrote:
>
> diff -Nru a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> --- a/net/core/rtnetlink.c      2005-04-07 23:13:33 +02:00
> +++ b/net/core/rtnetlink.c      2005-04-07 23:13:33 +02:00
> @@ -647,7 +647,8 @@
>        [RTM_GETROUTE - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
>        [RTM_NEWNEIGH - RTM_BASE] = { .doit   = neigh_add             },
>        [RTM_DELNEIGH - RTM_BASE] = { .doit   = neigh_delete          },
> -       [RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info       }
> +       [RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info       },
> +       [RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    }
> };

Just a trivial comment.  How about adding a comma at the end of the last
entry so that the next patch won't have to touch it?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[Thomas Graf]

* Herbert Xu <E1DJfBR-0007Ym-00@gondolin.me.apana.org.au> 2005-04-08 08:09
> Thomas Graf <tgraf@suug.ch> wrote:
> >
> > diff -Nru a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > --- a/net/core/rtnetlink.c      2005-04-07 23:13:33 +02:00
> > +++ b/net/core/rtnetlink.c      2005-04-07 23:13:33 +02:00
> > @@ -647,7 +647,8 @@
> >        [RTM_GETROUTE - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
> >        [RTM_NEWNEIGH - RTM_BASE] = { .doit   = neigh_add             },
> >        [RTM_DELNEIGH - RTM_BASE] = { .doit   = neigh_delete          },
> > -       [RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info       }
> > +       [RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info       },
> > +       [RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    }
> > };
> 
> Just a trivial comment.  How about adding a comma at the end of the last
> entry so that the next patch won't have to touch it?

Sure, bk fix'ed and new patch attached. However, I haven't read all
the recent happenings around bk and somewhat missed rc2. I just noticed
the freeze so not sure if this still makes sense.

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2005/04/08 00:24:50+02:00 tgraf@suug.ch 
#   [RTNETLINK]: Protocol family wildcard dumping for routing rules
#   
#   Be kind to userspace and don't force them to hardcode protocol
#   families just to have it changed again once we support routing
#   rules for more than one protocol family.
#   
#   Signed-off-by: Thomas Graf <tgraf@suug.ch>
#   Signed-off-by: David S. Miller <davem@davemloft.net>
# 
# net/core/rtnetlink.c
#   2005/04/08 00:24:40+02:00 tgraf@suug.ch +2 -1
#   [RTNETLINK]: Protocol family wildcard dumping for routing rules
# 
diff -Nru a/net/core/rtnetlink.c b/net/core/rtnetlink.c
--- a/net/core/rtnetlink.c	2005-04-08 00:35:37 +02:00
+++ b/net/core/rtnetlink.c	2005-04-08 00:35:37 +02:00
@@ -647,7 +647,8 @@
 	[RTM_GETROUTE - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
 	[RTM_NEWNEIGH - RTM_BASE] = { .doit   = neigh_add	      },
 	[RTM_DELNEIGH - RTM_BASE] = { .doit   = neigh_delete	      },
-	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      }
+	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      },
+	[RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
 };
 
 static int rtnetlink_event(struct notifier_block *this, unsigned long event, void *ptr)

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[jamal]

On Thu, 2005-04-07 at 18:38, Thomas Graf wrote:
> -	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      }
> +	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      },
> +	[RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
>  };
>  

Shouldnt this just work (without this change) if you have
CONFIG_IP_MULTIPLE_TABLES?

If you want to be funky and have some default dumper always,
why not make the change to set dumpit to rtnetlink_dump_all in the code
when dumpit is found to be NULL as a last resort?

cheers,
jamal

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[Thomas Graf]

* jamal <1112915616.1089.27.camel@jzny.localdomain> 2005-04-07 19:13
> On Thu, 2005-04-07 at 18:38, Thomas Graf wrote:
> > -	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      }
> > +	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      },
> > +	[RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
> >  };
> >  
> 
> Shouldnt this just work (without this change) if you have
> CONFIG_IP_MULTIPLE_TABLES?

Not sure what you mean, it doesn't even get into the routing
code but already fails in rtnetlink_rcv_msg when it doesn't
find a dumpit implementation for family == AF_UNSPEC.

> If you want to be funky and have some default dumper always,
> why not make the change to set dumpit to rtnetlink_dump_all in the code
> when dumpit is found to be NULL as a last resort?

That is exactly what happens, it will first look up the address
family dumpit and if none exist falls back to the rtnetlink_dump_all
introduced above.

	if (link->dumpit == NULL)
		link = &(rtnetlink_links[PF_UNSPEC][type]);

	if (link->dumpit == NULL)
		goto err_inval; /* <-- we used to fail here for PF_UNSPEC */

	if ((*errp = netlink_dump_start(rtnl, skb, nlh, ...

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[jamal]

On Thu, 2005-04-07 at 19:24, Thomas Graf wrote:
> * jamal <1112915616.1089.27.camel@jzny.localdomain> 2005-04-07 19:13
> > On Thu, 2005-04-07 at 18:38, Thomas Graf wrote:
> > > -	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      }
> > > +	[RTM_GETNEIGH - RTM_BASE] = { .dumpit = neigh_dump_info	      },
> > > +	[RTM_GETRULE  - RTM_BASE] = { .dumpit = rtnetlink_dump_all    },
> > >  };
> > >  
> > 
> > Shouldnt this just work (without this change) if you have
> > CONFIG_IP_MULTIPLE_TABLES?
> 
> Not sure what you mean, it doesn't even get into the routing
> code but already fails in rtnetlink_rcv_msg when it doesn't
> find a dumpit implementation for family == AF_UNSPEC.
> 

Right but if CONFIG_IP_MULTIPLE_TABLES is compiled it will find it in 
family = PF_INET, no?
In the minimal shouldnt you have #ifdef CONFIG_IP_MULTIPLE_TABLES around
that defined?

> That is exactly what happens, it will first look up the address
> family dumpit and if none exist falls back to the rtnetlink_dump_all
> introduced above.
> 
> 	if (link->dumpit == NULL)
> 		link = &(rtnetlink_links[PF_UNSPEC][type]);
> 
> 	if (link->dumpit == NULL)
> 		goto err_inval; /* <-- we used to fail here for PF_UNSPEC */
> 
> 	if ((*errp = netlink_dump_start(rtnl, skb, nlh, ...

I mean why dont you just set it there in that last part where it
currently fails? i.e set link->dumpit to rtnetlink_dump_all

cheers,
jamal

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[Thomas Graf]

* jamal <1112921300.1088.54.camel@jzny.localdomain> 2005-04-07 20:48
> Right but if CONFIG_IP_MULTIPLE_TABLES is compiled it will find it in 
> family = PF_INET, no?

Yes if rtgenmsg->rtgen_family is set to PF_IPV4. What I'm trying to
achieve is that if the user specifies PF_UNSPEC that it will look
through all families looking for possible rules to dump, ...

> In the minimal shouldnt you have #ifdef CONFIG_IP_MULTIPLE_TABLES around
> that defined?

...  this may include rule systems that do not depend on
CONFIG_IP_MULTIPLE_TABLES, e.g. IPv6 is likely to get an own config option
for this. Or someday we might have DECnet rules? I don't know.

> I mean why dont you just set it there in that last part where it
> currently fails? i.e set link->dumpit to rtnetlink_dump_all

Because that would interefer with the RTM_GETLINK and RTM_GETNEIGH.
We need that link_rtnetlink_table which is then mapped to PF_UNSPEC
and PF_PACKET protocol family for the wildcard support.

Re: [PATCH] RTNETLINK: Protocol family wildcard dumping for routing rules

[jamal]

On Thu, 2005-04-07 at 21:04, Thomas Graf wrote:

> ...  this may include rule systems that do not depend on
> CONFIG_IP_MULTIPLE_TABLES, e.g. IPv6 is likely to get an own config option
> for this. Or someday we might have DECnet rules? I don't know.
> 

Ok, fair enough.

BTW Decnet already has it for family PF_DECnet ;-> 
[I dont think there has existed a decnet as sophisticated as the one we
have - too bad its not used that much].

cheers,
jamal