Hi,
The earlier patch to implement the prefix list has been redone to use fib.
Following are the implementation details and a couple of issues :
1. I change the netlink_dump_start to pass another parameter, <type>, which is
stored in a new field in the cb, <type>. All users of this function have been
changed to pass a -1 since they don't care about the type, except the
generic routine rtnetlink_rcv_msg() which calculates the type and stores it.
So the same routine which is used to dump route table can be used to dump
the prefix list by checking the type. It might be possible to derive the
type from the table offset, but that is more complicated (probably doable).
2. Added yoshifuji's patch to store the M/O flags (now it is needed).
3. Added user interface for retrieving M/O flags. This is a separate interface
from the one for getting the prefix list since the flags are per interface
while the prefix list is per route. However these two can be merged into one
if needed.
4. Changed the usage of RTF_ADDRCONF to be used only when the action is being
performed due to receipt of a RA.
5. Though this patch is modified to use only routing table for updating and
accessing the prefix list, I did a performace analysis for this approach vs
storing the plist on the idev. Following is the result :
System : 1 CPU. 866 MHz, 256MB memory
For 1000 VLAN devices (4036 route entries gets created automatically as part
of address assignment), retrieve prefix list for (system times only) :
#devices #iteration for each dev plist on IDEV plist in RTTABLE %
200 100 3.95 secs 40.14 secs 916%
1000 10 2.60 secs 20.98 secs 706%
200 1000 38.44 secs 400.76 secs 942%
6. I have kept #ifdef PREFIXLIST in a few places, I can modify the patch to
remove that if required.
7. I removed the /proc interface since I was not able to cleanly use seq_file
with fib6_walk(). If needed, I can work on this later (but will need some
input on how to proceed). So currently, the only user interface is using
rtnetlink.
8. The patch can be extended to issue events on new prefix addition and on
prefix deletion. I can do that if required.
9. I have tested using rtnetlink for both interfaces (prefix list and get O/M
flags), no issues found.
Please let me know if this looks acceptable, in which case I can also send the patch for 2.4 kernel.
Thanks,
- KK
diff -ruN linux-2.5.70.org/include/linux/ipv6_route.h linux-2.5.70.new/include/linux/ipv6_route.h
@@ -44,4 +44,16 @@#define RTMSG_NEWROUTE 0x21#define RTMSG_DELROUTE 0x22+#ifdef CONFIG_IPV6_PREFIXLIST++/* Structure to return prefix and prefix length for all devices */++structin6_prefix_msg+{+intifindex;+intprefix_len;+structin6_addrprefix;+};+#endif+#endif
Grr, I've almost lost this...
In article [off-list ref] (at Fri, 20 Jun 2003 13:53:44 -0700), Krishna Kumar [off-list ref] says:
1. I change the netlink_dump_start to pass another parameter, <type>, which is
stored in a new field in the cb, <type>. All users of this function have been
changed to pass a -1 since they don't care about the type, except the
generic routine rtnetlink_rcv_msg() which calculates the type and stores it.
So the same routine which is used to dump route table can be used to dump
the prefix list by checking the type. It might be possible to derive the
type from the table offset, but that is more complicated (probably doable).
I think this is not required.
Rename inet6_dump_fib() to __inet6_dump_fib() and introduce
extra argument. and call it like
inet6_dump_fib() { __inet6_dump_fib(...,0); }
and
inet6_dump_prefix() { __inet6_dump_fib(...,1); }
etc.
3. Added user interface for retrieving M/O flags. This is a separate interface
from the one for getting the prefix list since the flags are per interface
while the prefix list is per route. However these two can be merged into one
if needed.
Hmm, what I expected is to get information via RTA_NEWLINK message.
This is because, this is per-interface thing.
5. Though this patch is modified to use only routing table for updating and
accessing the prefix list, I did a performace analysis for this approach vs
storing the plist on the idev. Following is the result :
System : 1 CPU. 866 MHz, 256MB memory
For 1000 VLAN devices (4036 route entries gets created automatically as part
of address assignment), retrieve prefix list for (system times only) :
#devices #iteration for each dev plist on IDEV plist in RTTABLE %
200 100 3.95 secs 40.14 secs 916%
1000 10 2.60 secs 20.98 secs 706%
200 1000 38.44 secs 400.76 secs 942%
Hmm...
Well, what should we do...
--
Hideaki YOSHIFUJI @ USAGI Project [off-list ref]
GPG FP: 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
Rename inet6_dump_fib() to __inet6_dump_fib() and introduce
OK, done.
Hmm, what I expected is to get information via RTA_NEWLINK message.
I have made changes to return per interface flags, however I am not very
familiar with netlink and it's different interfaces. I wanted to clarify whether
the following code is what you are trying to get done. Otherwise please let me
know what changes need to be done. What I want to happen is :
1. Return entire prefix list on request from user.
2. Return flags for a particular interface on request from user.
What I have not yet done is to broadcast events when a new prefix arrives or
an existing prefix gets expired, and to broadcast flags when an RA is received.
Currently there is no need for these from DHCP, but it could be added later on.
quoted
#devices #iteration for each dev plist on IDEV plist in RTTABLE %
200 100 3.95 secs 40.14 secs 916%
Well, what should we do...
The original code, though faster, you have more code as dave said in his initial
mail. I think this operation is not done too often to be concerned about
performance. Besides it is taking 40 secs to iterate 20,000 times over a 4K
routing table, effectively about 2ms for getting the prefix list for one device.
And for most systems, this is not an issue since this code will not run at all.
Thanks,
- KK
diff -ruN linux-2.5.70.org/include/linux/ipv6_route.h
linux-2.5.70.new/include/linux/ipv6_route.h
@@ -2389,6 +2388,43 @@netlink_broadcast(rtnl,skb,0,RTMGRP_IPV6_IFADDR,GFP_ATOMIC);}+intinet6_dump_linkflags(structsk_buff*skb,structnetlink_callback*cb)+{+intifindex,flags=0;+structnet_device*dev;+structinet6_dev*idev;+structnlmsghdr*nlh;+structifp_if6info*ifp=NLMSG_DATA(cb->nlh);+unsignedchar*org_tail=skb->tail;++/* ifindex = cb->args[0]; ? */+ifindex=ifp->ifindex;++if((dev=dev_get_by_index(ifindex))==NULL)+gotoout;+if((idev=in6_dev_get(dev))!=NULL){+flags=idev->if_flags;+in6_dev_put(idev);+}+dev_put(dev);++nlh=NLMSG_PUT(skb,NETLINK_CB(cb->skb).pid,cb->nlh->nlmsg_seq,+RTA_LINKFLAGS,sizeof(*ifp));+ifp=NLMSG_DATA(nlh);+ifp->flags=flags;+ifp->ifindex=ifindex;/* duplicate information for user to verify */++nlh->nlmsg_len=skb->tail-org_tail;+returnskb->len;++nlmsg_failure:+printk(KERN_INFO"inet6_dump_linkflags:skb size not enough\n");+skb_trim(skb,org_tail-skb->data);++out:+return-1;+}+staticstructrtnetlink_linkinet6_rtnetlink_table[RTM_MAX-RTM_BASE+1]={[RTM_NEWADDR-RTM_BASE]={.doit=inet6_rtm_newaddr,},[RTM_DELADDR-RTM_BASE]={.doit=inet6_rtm_deladdr,},
From: David S. Miller <hidden> Date: 2003-06-26 06:42:51
I don't think it's wise to make RTNETLINK facilities
dependant upon ifdef values.
Please kill CONFIG_IPV6_PREFIXLIST. People don't need to
to enable funny options to get a fully function dhcp on
ipv4, they should therefore not have to for ipv6 either.
I don't think it's wise to make RTNETLINK facilities dependant upon ifdef
Following is the patch without the PREFIXLIST config option. I have remade it
against 2.5.73.
We would like to have this functionality in 2.4 kernel, so can I go ahead and
send this patch against 2.4.21 too ?
Thanks,
- KK
--------------------------------------------------------------------------------
diff -ruN linux-2.5.73.org/include/linux/ipv6_route.h linux-2.5.73.new/include/linux/ipv6_route.h
@@ -2451,6 +2450,42 @@netlink_broadcast(rtnl,skb,0,RTMGRP_IPV6_IFADDR,GFP_ATOMIC);}+intinet6_dump_linkflags(structsk_buff*skb,structnetlink_callback*cb)+{+intifindex,flags=0;+structnet_device*dev;+structinet6_dev*idev;+structnlmsghdr*nlh;+structifp_if6info*ifp=NLMSG_DATA(cb->nlh);+unsignedchar*org_tail=skb->tail;++ifindex=ifp->ifindex;++if((dev=dev_get_by_index(ifindex))==NULL)+gotoout;+if((idev=in6_dev_get(dev))!=NULL){+flags=idev->if_flags;+in6_dev_put(idev);+}+dev_put(dev);++nlh=NLMSG_PUT(skb,NETLINK_CB(cb->skb).pid,cb->nlh->nlmsg_seq,+RTA_LINKFLAGS,sizeof(*ifp));+ifp=NLMSG_DATA(nlh);+ifp->flags=flags;+ifp->ifindex=ifindex;/* duplicate information for user to verify */++nlh->nlmsg_len=skb->tail-org_tail;+returnskb->len;++nlmsg_failure:+printk(KERN_INFO"inet6_dump_linkflags:skb size not enough\n");+skb_trim(skb,org_tail-skb->data);++out:+return-1;+}+staticstructrtnetlink_linkinet6_rtnetlink_table[RTM_MAX-RTM_BASE+1]={[RTM_NEWADDR-RTM_BASE]={.doit=inet6_rtm_newaddr,},[RTM_DELADDR-RTM_BASE]={.doit=inet6_rtm_deladdr,},
Hi dave,
This is the same patch against 2.4.21. Minor changes in initialization of
inet6_rtnetlink_table, otherwise the rest is the same. I have tested it, and it
works fine.
thanks,
- KK
--------------------------------------------------------------------------------
diff -ruN linux-2.4.21.org/include/linux/ipv6_route.h linux-2.4.21/include/linux/ipv6_route.h
@@ -1971,6 +1970,42 @@netlink_broadcast(rtnl,skb,0,RTMGRP_IPV6_IFADDR,GFP_ATOMIC);}+intinet6_dump_linkflags(structsk_buff*skb,structnetlink_callback*cb)+{+intifindex,flags=0;+structnet_device*dev;+structinet6_dev*idev;+structnlmsghdr*nlh;+structifp_if6info*ifp=NLMSG_DATA(cb->nlh);+unsignedchar*org_tail=skb->tail;++ifindex=ifp->ifindex;++if((dev=dev_get_by_index(ifindex))==NULL)+gotoout;+if((idev=in6_dev_get(dev))!=NULL){+flags=idev->if_flags;+in6_dev_put(idev);+}+dev_put(dev);++nlh=NLMSG_PUT(skb,NETLINK_CB(cb->skb).pid,cb->nlh->nlmsg_seq,+RTA_LINKFLAGS,sizeof(*ifp));+ifp=NLMSG_DATA(nlh);+ifp->flags=flags;+ifp->ifindex=ifindex;/* duplicate information for user to verify */++nlh->nlmsg_len=skb->tail-org_tail;+returnskb->len;++nlmsg_failure:+printk(KERN_INFO"inet6_dump_linkflags:skb size not enough\n");+skb_trim(skb,org_tail-skb->data);++out:+return-1;+}+staticstructrtnetlink_linkinet6_rtnetlink_table[RTM_MAX-RTM_BASE+1]={{NULL,NULL,},
From: David S. Miller <hidden> Date: 2003-06-27 06:07:27
From: Krishna Kumar [off-list ref]
Date: Thu, 26 Jun 2003 09:32:23 -0700
I still have problems with this patch.
-#define RTM_MAX (RTM_BASE+31)
+#define RTM_GETLNKFLAGS (RTM_BASE+34)
+
+#define RTM_GETPLIST (RTM_BASE+38)
Please allocate contiguous numbers to the new messages, don't skip
around like this.
Thanks. (this of course means you have to redo your 2.4.x patch
as well)
rtnetlink_rcv_msg() calls dumpit() (via netlink_dump_start) only for those
messages for which the last two bits are binary '10'. So I had to use these
values. All the other *GET* macros use the same semantics.
thanks,
- KK
David S. Miller wrote:
From: Krishna Kumar [off-list ref]
Date: Thu, 26 Jun 2003 09:32:23 -0700
I still have problems with this patch.
-#define RTM_MAX (RTM_BASE+31)
+#define RTM_GETLNKFLAGS (RTM_BASE+34)
+
+#define RTM_GETPLIST (RTM_BASE+38)
Please allocate contiguous numbers to the new messages, don't skip
around like this.
Thanks. (this of course means you have to redo your 2.4.x patch
as well)
From: David S. Miller <hidden> Date: 2003-06-27 21:47:52
From: Krishna Kumar [off-list ref]
Date: Fri, 27 Jun 2003 08:45:19 -0700
rtnetlink_rcv_msg() calls dumpit() (via netlink_dump_start) only
for those messages for which the last two bits are binary '10'. So
I had to use these values. All the other *GET* macros use the same
semantics.
Ok, please retransmit your two patches (2.4.x and 2.5.x) to me
under seperate cover. I don't keep a copy around of patches
I've decided not to apply.
In article [off-list ref] (at Fri, 27 Jun 2003 14:47:52 -0700 (PDT)), "David S. Miller" [off-list ref] says:
From: Krishna Kumar [off-list ref]
Date: Fri, 27 Jun 2003 08:45:19 -0700
rtnetlink_rcv_msg() calls dumpit() (via netlink_dump_start) only
for those messages for which the last two bits are binary '10'. So
I had to use these values. All the other *GET* macros use the same
semantics.
Ok, please retransmit your two patches (2.4.x and 2.5.x) to me
under seperate cover. I don't keep a copy around of patches
I've decided not to apply.
Well...
1. is it okay to have another hook for garbbig prefix list?
Userspace application can get such information via
- routing table
- interface flag
2. is the "managed" flags etc, which is per interface variable,
really NEWROUTE information?
It is NOT L2 thing, but it is per-link information.
I think it is NEWLINK thing.
What I'm thinking is:
- fix "ADDRCONF" flag in route information
- manage / other flags via NEWLINK message
(- No new interface to get prefix itself.)
--
Hideaki YOSHIFUJI @ USAGI Project [off-list ref]
GPG FP: 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
1. is it okay to have another hook for garbbig prefix list?
Userspace application can get such information via
- routing table
- interface flag
2. is the "managed" flags etc, which is per interface variable, really NEWROUTE information?
It is NOT L2 thing, but it is per-link information.
I think it is NEWLINK thing.
What I'm thinking is:
- fix "ADDRCONF" flag in route information
- manage / other flags via NEWLINK message
(- No new interface to get prefix itself.)
Well, there are two reason that I can see to not do so (ADDRCONF flag is already
fixed in earlier patch) :
- With the latest submission, the actual code to get the prefix list itself
is very small, the top level inet6_dump_fib uses either the dump_node or
the dump_prefix, the latter being the new function added. This is the whole
user interface, 50 odd lines of code with comments.
- If I understood your point about using interface flag and routing table,
you are suggesting that the user can make look at rttable and get the prefix
entries by make checks (it is non-trivial, eg the address should not LL or MC,
there should be no nexthop and it should be added via an RA, etc). However,
having a user interface makes it easier to get the prefix list without
significant bloat to the kernel, and the user doesn't have to make a lot of
checks to get the system prefixes. I don't see much gain from this approach.
About your point about the managed flag, I think it is a per interface flag
that gets returned when a request for getting flags on that interface is made.
That's why I have made it per interface as part of a GETLNKFLAGS operation.
I don't understand why you think it is NEWLINK thing (not sure what you mean by
that), since it is a flag information on your existing device that a RA is
advertising. I want to get this information not on receipt of an RA, but when
a request is made.
Thanks,
- KK
In article [off-list ref] (at Mon, 30 Jun 2003 11:54:41 -0700), Krishna Kumar [off-list ref] says:
Well, there are two reason that I can see to not do so (ADDRCONF flag is already
fixed in earlier patch) :
:
You do not explain why we (or kernel) NEED(s) this.
It is not so important how SMALL it is
though it may cause problems how LARGE it is.
About your point about the managed flag, I think it is a per interface flag
that gets returned when a request for getting flags on that interface is made.
That's why I have made it per interface as part of a GETLNKFLAGS operation.
I don't understand why you think it is NEWLINK thing (not sure what you mean by
that), since it is a flag information on your existing device that a RA is
advertising. I want to get this information not on receipt of an RA, but when
a request is made.
This is design issue; how we should provide L3 per-interface
information to userspace; eg. in_device and/or inet6_dev things
including per-interface statistics.
Since I think it is not appropriate to provide per-interface
statistics via RTM_xxxROUTE, so I don't agree to provide
the RA infomation (i.e. Manage/Otherconf Flags) via
RTM_xxxROUTE.
Options:
- use RTM_xxxLINK for L3 operation
- introduce RTM_xxxIFACE for L3 per-interface operations
I really want to hear from other maintainers here...
David? Alexey?
Well, on moving forward; you can split your patch up to 3 things:
1. fix routing flags
2. provide Managed/Otherconf flags API
(3. provide the prefix list API (if it IS required))
I'm not against the first item.
We need to discuss on the design related to the 2nd item.
I don't think that we really need 3rd item.
Thank you.
--
Hideaki YOSHIFUJI @ USAGI Project [off-list ref]
GPG FP: 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
You do not explain why we (or kernel) NEED(s) this.
It is not so important how SMALL it is
though it may cause problems how LARGE it is.
I had explained the reasons for having prefix list i/f in my previous mail. To recap :
- User don't need to know what the definition of a prefix is, all he has to do is ask
the kernel and get the list. Otherwise different user apps will have to know the
definition of a prefix and parse the entry themselves. The parsing is non-trivial (eg
the address should not LL or MC, there should be no nexthop and it should be added via
an RA, etc).
- The kernel code to get the prefix list is small, the top level inet6_dump_fib uses
either the dump_node or the dump_prefix, the latter being the new user interface. Having
a user interface makes it easier to get the prefix list without significant bloat to the
kernel.
This is design issue; how we should provide L3 per-interface information to userspace; eg. in_device and/or inet6_dev things including per-interface statistics.
Since I think it is not appropriate to provide per-interface statistics via RTM_xxxROUTE, so I don't agree to provide the RA infomation (i.e. Manage/Otherconf Flags) via RTM_xxxROUTE.
Options:
- use RTM_xxxLINK for L3 operation
- introduce RTM_xxxIFACE for L3 per-interface operations
Yes, there are a couple of different ways to do this. One is as you have suggested, but there
is a problem with it. The existing RTM_GETLINK interface returns very generic elements of the
dev (mtu, hardware address, dev statistics), while the change you suggested is specific to
ipv6. I am not sure if this is a good design to implement. Either we could use the current
(submitted) way or use a different RTM_GETADDR interface in inet6_fill_ifaddr (and introduce
RTM_IFACEFLAGS). This will be specific to IPv6. Are you agreeable to this ?
Well, on moving forward; you can split your patch up to 3 things:
1. fix routing flags
2. provide Managed/Otherconf flags API
(3. provide the prefix list API (if it IS required))
I'm not against the first item.
We need to discuss on the design related to the 2nd item.
I don't think that we really need 3rd item.
- I am ok with 1 :-)
- I have suggested changes for 2, please let me know what you think, whether we can go with the
old way or make the change suggested above.
- I believe we need #3 for the reasons given above.
Thanks,
- KK