Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
On Mon, Dec 24, 2007 at 09:50:59AM -0500, jamal wrote: > > Herbert, i agree with you that userspace is the best spot for this[1]; > we unfortunately have precedence already on the kernel sending arps with > bonding when link status changes (that was added recently). > So it sounds reasonable to have this patch in the kernel as well. Well just because we've made a mistake in the past does not mean that we should repeat it over and over again. If we keep doing this the kernel will soon end up as a whole heap of junk. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
On Mon, 2007-24-12 at 15:38 +0200, David Shwatrz wrote: > Hello, > > First, it indeed can be handled by user space. (even though it should > be done twice, once for ifconig of net-tools and once for ip of > iproute2) it needs to be done once only: reacting to netlink events when MAC address changes. > / However, we have already > methods which deal with bringing down an interface - neigh_ifdown(), > and changing MAC address of an interface (neigh_changeaddr). So why > not do it from the kernel ? Herbert, i agree with you that userspace is the best spot for this[1]; we unfortunately have precedence already on the kernel sending arps with bonding when link status changes (that was added recently). So it sounds reasonable to have this patch in the kernel as well. cheers, jamal [1] Things like these tend to be very policy rich and thats why user space is the best spot for them. I have infact implemented this feature in user space in some random box i have where i failover MACs for HA reasons. Depending on how much traffic there is on the wire, arps do get dropped. One of the hardest things to decide on was how many times to retry the grat arp sending and what the timeout would be between each sent gratarp. The earlier patch posted didnt consider this but would be nice to have a couple of sysctls to add the two parameters if this makes it in. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
On Sun, 2007-23-12 at 08:17 -0500, jamal wrote: > On Sun, 2007-23-12 at 22:04 +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote: > > > If the secondary MACs are used with ARP/NDP, we should take care of > > that, but I think we use the primary MAC for ARP/NDP, no? > > (In other words, we always use primary MAC for ARP reply / NA, no?) > > I think it maybe a policy decision; Never mind, that was my body being in a different time zone. I went back and looked at a little chat i had with Patrick when he posted with the macvlan driver. At the moment we are still maintaining the model that a NIC can _only_ appear to have a single MAC to the upper layers. IOW, you have to instantiate a macvlan device for each of the 16 MAC addresses on the e1000 if you need to expose them. This means that the ip layer - even with multiple ip addresses will only ever see one MAC address. Note also: The name macvlan is a little misleading since it allows for the above without need for vlans. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
Hello, First, it indeed can be handled by user space. (even though it should be done twice, once for ifconig of net-tools and once for ip of iproute2) / However, we have already methods which deal with bringing down an interface - neigh_ifdown(), and changing MAC address of an interface (neigh_changeaddr). So why not do it from the kernel ? DS On Dec 23, 2007 4:02 PM, Herbert Xu <[EMAIL PROTECTED]> wrote: > David Shwatrz <[EMAIL PROTECTED]> wrote: > > > > Hi, > > Oop, I am TWICE sorry ! I wrongly attached a wrong, empty file. > > Attached here is the patch. > > > > Regarding your answer; I accept it and I will soon send a revised > > version of this patch (making changes to > > arp_netdev_event() and ndisc_netdev_event().) > > I had IPv4 in mind, there is no reason that it will no be also in IPv6. > > Hmm, why can't you do this from user-space? > > Cheers, > -- > Visit Openswan at http://www.openswan.org/ > Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
David Shwatrz <[EMAIL PROTECTED]> wrote: > > Hi, > Oop, I am TWICE sorry ! I wrongly attached a wrong, empty file. > Attached here is the patch. > > Regarding your answer; I accept it and I will soon send a revised > version of this patch (making changes to > arp_netdev_event() and ndisc_netdev_event().) > I had IPv4 in mind, there is no reason that it will no be also in IPv6. Hmm, why can't you do this from user-space? Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
Yoshfuji, Thanks, you are right ! soon I will send the patches. Thanks ! DS On Dec 23, 2007 3:13 PM, YOSHIFUJI Hideaki / 吉藤英明 <[EMAIL PROTECTED]> wrote: > In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 15:04:37 +0200), "David > Shwatrz" <[EMAIL PROTECTED]> says: > > > Hello, > > > > > > >You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). > > > For IPv6, we also have anycast (maintained by ac_list) as well. > > > > I am not sure that we need to iterate all of ifa_list in IPv4. > > The reason is that we end with arp_send, and it initiates a broadcast. > > So all neighbours will receive it and update their arp tables > > accordingly. > > The dest hw in the arp_send is NULL according to this patch ; this means > > that > > we will assign dev->broadcast to dest_hw in apr_create(). > > > > It seems to me there's no reason to send more than one broadcast. > > Urgh? what is happend if you have multiple IPv4 addresses on the device? > > > > In IPv6, I need to check, since it is multicast. > > Please read RFC2461 Section 7.2.6. In short we should send a few > unsolicited NA, but I think you can start from sending once per an > address. > > --yoshfuji > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
On Sun, 2007-23-12 at 22:04 +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote: > If the secondary MACs are used with ARP/NDP, we should take care of > that, but I think we use the primary MAC for ARP/NDP, no? > (In other words, we always use primary MAC for ARP reply / NA, no?) I think it maybe a policy decision; In the IPV4 case, where the system owns the IP addresses for the classical scenario where there is a single MAC address per ethx then we always respond with MAC address of ethx wherever the arp request was received from. I think it is different in the case of IPV6 where the eth device owns the IP address, no? i.e is that where you are drawing the concept of primary MAC? The case of multiple MACs per interface requires further policy resolution IMO. It would be nice to be able to tell the kernel which MAC to use when responding for which ip address it owns. This can be then easily mapped to the routing table src address selection. Patrick? cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 15:04:37 +0200), "David Shwatrz" <[EMAIL PROTECTED]> says: > Hello, > > > >You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). > > For IPv6, we also have anycast (maintained by ac_list) as well. > > I am not sure that we need to iterate all of ifa_list in IPv4. > The reason is that we end with arp_send, and it initiates a broadcast. > So all neighbours will receive it and update their arp tables > accordingly. > The dest hw in the arp_send is NULL according to this patch ; this means that > we will assign dev->broadcast to dest_hw in apr_create(). > > It seems to me there's no reason to send more than one broadcast. Urgh? what is happend if you have multiple IPv4 addresses on the device? > In IPv6, I need to check, since it is multicast. Please read RFC2461 Section 7.2.6. In short we should send a few unsolicited NA, but I think you can start from sending once per an address. --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 07:46:15 -0500), jamal <[EMAIL PROTECTED]> says: > On Sun, 2007-23-12 at 21:38 +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote: > > In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 14:24:00 +0200), "David > > Shwatrz" <[EMAIL PROTECTED]> says: > > > > > Regarding your answer; I accept it and I will soon send a revised > > > version of this patch (making changes to > > > arp_netdev_event() and ndisc_netdev_event().) > > > I had IPv4 in mind, there is no reason that it will no be also in IPv6. > > > > You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). > > For IPv6, we also have anycast (maintained by ac_list) as well. > > > > Hrm, how is this going to work for the case of multiple MACs on a > device? > Changing one MAC address doesnt equate to issuing a grat arp with _the > new MAC_ for all ifa (given each MAC may be map to a different ifa) If the secondary MACs are used with ARP/NDP, we should take care of that, but I think we use the primary MAC for ARP/NDP, no? (In other words, we always use primary MAC for ARP reply / NA, no?) --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
Hello, >You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). > For IPv6, we also have anycast (maintained by ac_list) as well. I am not sure that we need to iterate all of ifa_list in IPv4. The reason is that we end with arp_send, and it initiates a broadcast. So all neighbours will receive it and update their arp tables accordingly. The dest hw in the arp_send is NULL according to this patch ; this means that we will assign dev->broadcast to dest_hw in apr_create(). It seems to me there's no reason to send more than one broadcast. In IPv6, I need to check, since it is multicast. Thoughts ? Regards, DS On Dec 23, 2007 2:38 PM, YOSHIFUJI Hideaki / 吉藤英明 <[EMAIL PROTECTED]> wrote: > In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 14:24:00 +0200), "David > Shwatrz" <[EMAIL PROTECTED]> says: > > > Regarding your answer; I accept it and I will soon send a revised > > version of this patch (making changes to > > arp_netdev_event() and ndisc_netdev_event().) > > I had IPv4 in mind, there is no reason that it will no be also in IPv6. > > You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). > For IPv6, we also have anycast (maintained by ac_list) as well. > > --yoshfuji > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
On Sun, 2007-23-12 at 21:38 +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote: > In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 14:24:00 +0200), "David > Shwatrz" <[EMAIL PROTECTED]> says: > > > Regarding your answer; I accept it and I will soon send a revised > > version of this patch (making changes to > > arp_netdev_event() and ndisc_netdev_event().) > > I had IPv4 in mind, there is no reason that it will no be also in IPv6. > > You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). > For IPv6, we also have anycast (maintained by ac_list) as well. > Hrm, how is this going to work for the case of multiple MACs on a device? Changing one MAC address doesnt equate to issuing a grat arp with _the new MAC_ for all ifa (given each MAC may be map to a different ifa) cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 14:24:00 +0200), "David Shwatrz" <[EMAIL PROTECTED]> says: > Regarding your answer; I accept it and I will soon send a revised > version of this patch (making changes to > arp_netdev_event() and ndisc_netdev_event().) > I had IPv4 in mind, there is no reason that it will no be also in IPv6. You should iterate all of ifa_list (for IPv4) / addr_list (for IPv6). For IPv6, we also have anycast (maintained by ac_list) as well. --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
Hi, Oop, I am TWICE sorry ! I wrongly attached a wrong, empty file. Attached here is the patch. Regarding your answer; I accept it and I will soon send a revised version of this patch (making changes to arp_netdev_event() and ndisc_netdev_event().) I had IPv4 in mind, there is no reason that it will no be also in IPv6. Regads, David Shwatrz On Dec 23, 2007 2:11 PM, YOSHIFUJI Hideaki / 吉藤英明 <[EMAIL PROTECTED]> wrote: > In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 13:41:36 +0200), "David > Shwatrz" <[EMAIL PROTECTED]> says: > > > I had written a small patch to neigh_changeaddr() in net/core/neighbour.c > > against the 2.6 git net tree, which sends a gratuitous ARP to update > > the list of > > all the involved neighbours with the change of MAC address. > > The patch is for neigh_changeaddr() only. > > Though I can see no patch, but I disagree. ;-) > I do think you should change arp_netdev_event() and ndisc_netdev_event(). > > --yoshfuji > diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 29b8ee4..ddeae82 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -36,6 +36,9 @@ #include #include +#include +#include + #define NEIGH_DEBUG 1 #define NEIGH_PRINTK(x...) printk(x) @@ -228,9 +231,26 @@ static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev) void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev) { + struct in_device *in_dev; write_lock_bh(&tbl->lock); neigh_flush_dev(tbl, dev); write_unlock_bh(&tbl->lock); + + /* Send a gratuitous ARP to the neighbours to update their arp tables */ + + rcu_read_lock(); + in_dev = __in_dev_get_rcu(dev); + if (in_dev == NULL) + goto out; + if (in_dev->ifa_list) + + arp_send(ARPOP_REQUEST, ETH_P_ARP, +in_dev->ifa_list->ifa_address, +dev, +in_dev->ifa_list->ifa_address, +NULL, dev->dev_addr, NULL); +out: + rcu_read_unlock(); } int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
Re: [PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
In article <[EMAIL PROTECTED]> (at Sun, 23 Dec 2007 13:41:36 +0200), "David Shwatrz" <[EMAIL PROTECTED]> says: > I had written a small patch to neigh_changeaddr() in net/core/neighbour.c > against the 2.6 git net tree, which sends a gratuitous ARP to update > the list of > all the involved neighbours with the change of MAC address. > The patch is for neigh_changeaddr() only. Though I can see no patch, but I disagree. ;-) I do think you should change arp_netdev_event() and ndisc_netdev_event(). --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-2.6][NEIGH] Updating affected neighbours when about MAC address change
Hello, Attached here is the patch - please read the following text here below: We know that changes in MAC addresses are not frequent but we are working on a special, highly advanced networking Linux based project in our LABs, where we do change MAC addresses of an interface quite frequently. (We do not go totally wild; the MAC addresses we are changing into are from a set of given MAC addresses). Normally, when we change a MAC address of some interface, the relevant neighbours in the LAN which have entries with the previous MAC address are not sent any update notification; instead, it is there regular timers mechanisms which update the MAC address to the new one in their ARP tables. I had written a small patch to neigh_changeaddr() in net/core/neighbour.c against the 2.6 git net tree, which sends a gratuitous ARP to update the list of all the involved neighbours with the change of MAC address. The patch is for neigh_changeaddr() only. This patch was tested and it does work in my LAB; if such a patch is not needed, I wonder why ? It seems to me that it could not cause any troubles. BTW, I had noticed that in irlan driver, there is such a mechanism of sending a gratuitous ARP to update all the neighbours when a MAC address is changed. Signed-off-by: David Shwatrz <[EMAIL PROTECTED]> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html