Message ID | 20090526151717.GB11147@psychotron.englab.brq.redhat.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, May 26, 2009 at 05:17:17PM +0200, Jiri Pirko wrote: > [PATCH net-next] bonding: allow bond in mode balance-alb to work properly in bridge -try4.1 > > Hi all. > > The problem is described in following bugzilla: > https://bugzilla.redhat.com/show_bug.cgi?id=487763 > > Basically here's what's going on. In every mode, bonding interface uses the same > mac address for all enslaved devices (except fail_over_mac). Only balance-alb > will simultaneously use multiple MAC addresses across different slaves. When you > put this kind of bond device into a bridge it will only add one of mac adresses > into a hash list of mac addresses, say X. This mac address is marked as local. > But this bonding interface also has mac address Y. Now then packet arrives with > destination address Y, this address is not marked as local and the packed looks > like it needs to be forwarded. This packet is then lost which is wrong. > > Notice that interfaces can be added and removed from bond while it is in bridge. > > *** > When the multiple addresses for bridge port approach failed to solve this issue > due to STP I started to think other way to solve this. I returned to previous > solution but tweaked one. > > This patch solves the situation in the bonding without touching bridge code. > For every incoming frame to bonding the destination address is compared to > current address of the slave device from which tha packet came. If these two > match destination address is replaced by mac address of the master. This address > is known by bridge so it is delivered properly. Did you test this with a bond with more than 2 ports? I ask because I might also expect a check against all the members of the bond (rather than simply the receiving device). That check would be quite expensive for every frame and I think the scenario is quite unlikely based on the frequency of 'learning frames' sent by the alb code (so the switch connected to the host should have it's forwarding database correct), but it might be something to think about in the future. > I experimentally tried that this works as good as searching through the slave > list (v4 of this patch). > > I was forced to create a new header because I need to use > compare_ether_addr_64bits() (defined in linux/etherdevice.h) in > linux/netdevice.h. I've hit some cross include issues. I think that it's good > to have skb_bond_should_drop() in a separate file anyway. > > Jirka > > > Signed-off-by: Jiri Pirko <jpirko@redhat.com> > This certainly won't cure all of the problems that arise with bonding and bridging interactions, but it's a step in the right direction. Acked-by: Andy Gospodarek <andy@greyhouse.net> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jiri Pirko a écrit : > [PATCH net-next] bonding: allow bond in mode balance-alb to work properly in bridge -try4.1 > > Hi all. > > The problem is described in following bugzilla: > https://bugzilla.redhat.com/show_bug.cgi?id=487763 > > Basically here's what's going on. In every mode, bonding interface uses the same > mac address for all enslaved devices (except fail_over_mac). Only balance-alb > will simultaneously use multiple MAC addresses across different slaves. When you > put this kind of bond device into a bridge it will only add one of mac adresses > into a hash list of mac addresses, say X. This mac address is marked as local. > But this bonding interface also has mac address Y. Now then packet arrives with > destination address Y, this address is not marked as local and the packed looks > like it needs to be forwarded. This packet is then lost which is wrong. > > Notice that interfaces can be added and removed from bond while it is in bridge. > > *** > When the multiple addresses for bridge port approach failed to solve this issue > due to STP I started to think other way to solve this. I returned to previous > solution but tweaked one. > > This patch solves the situation in the bonding without touching bridge code. > For every incoming frame to bonding the destination address is compared to > current address of the slave device from which tha packet came. If these two > match destination address is replaced by mac address of the master. This address > is known by bridge so it is delivered properly. > > I experimentally tried that this works as good as searching through the slave > list (v4 of this patch). > > I was forced to create a new header because I need to use > compare_ether_addr_64bits() (defined in linux/etherdevice.h) in > linux/netdevice.h. I've hit some cross include issues. I think that it's good > to have skb_bond_should_drop() in a separate file anyway. > > Jirka > > > Signed-off-by: Jiri Pirko <jpirko@redhat.com> > > diff --git a/include/linux/bonding.h b/include/linux/bonding.h > new file mode 100644 > index 0000000..3081ddb > --- /dev/null > +++ b/include/linux/bonding.h > @@ -0,0 +1,78 @@ > +/* > + * include/linux/bonding.h > + * > + * Copyright (C) 2009 Jiri Pirko <jpirko@redhat.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 > + * as published by the Free Software Foundation. > + * > + * Bonding device helpers. > + */ > + > +#ifndef _LINUX_BONDING_H > +#define _LINUX_BONDING_H > + > +#ifdef __KERNEL__ > + > +#include <linux/skbuff.h> > +#include <linux/netdevice.h> > +#include <linux/if.h> > +#include <linux/etherdevice.h> > +#include <linux/if_ether.h> > + > +static inline void skb_bond_set_mac_by_master(struct sk_buff *skb, > + struct net_device *dev, > + struct net_device *master) > +{ > + unsigned char *dest = eth_hdr(skb)->h_dest; > + > + if (compare_ether_addr_64bits(dest, master->dev_addr) && > + !compare_ether_addr_64bits(dest, dev->dev_addr)) > + memcpy(dest, master->dev_addr, ETH_ALEN); But couldnt we test skb->pkt_type == PACKET_HOST instead, Or eth_type_trans() not yet called at this point ? I would suggest : if (skb->pkt_type == PACKET_HOST) memcpy(dest, master->dev_addr, ETH_ALEN); > +} > + > +/* On bonding slaves other than the currently active slave, suppress > + * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and > + * ARP on active-backup slaves with arp_validate enabled. > + */ > +static inline int skb_bond_should_drop(struct sk_buff *skb) > +{ > + struct net_device *dev = skb->dev; > + struct net_device *master = dev->master; > + > + if (master) { > + if (master->priv_flags & IFF_MASTER_ARPMON) > + dev->last_rx = jiffies; > + > + if ((master->priv_flags & IFF_MASTER_ALB) && master->br_port) { > + /* Do address unmangle. The local destination address > + * will be always the one master has. Provides the right > + * functionality in a bridge. > + */ > + skb_bond_set_mac_by_master(skb, dev, master); > + } > + > + if (dev->priv_flags & IFF_SLAVE_INACTIVE) { > + if ((dev->priv_flags & IFF_SLAVE_NEEDARP) && > + skb->protocol == __cpu_to_be16(ETH_P_ARP)) > + return 0; > + > + if (master->priv_flags & IFF_MASTER_ALB) { > + if (skb->pkt_type != PACKET_BROADCAST && > + skb->pkt_type != PACKET_MULTICAST) > + return 0; > + } > + if (master->priv_flags & IFF_MASTER_8023AD && > + skb->protocol == __cpu_to_be16(ETH_P_SLOW)) > + return 0; > + > + return 1; > + } > + } > + return 0; > +} > + > +#endif /* __KERNEL__ */ > + > +#endif /* _LINUX_BONDING_H */ > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index ae3c209..06e24ae 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1897,39 +1897,6 @@ static inline void netif_set_gso_max_size(struct net_device *dev, > dev->gso_max_size = size; > } > > -/* On bonding slaves other than the currently active slave, suppress > - * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and > - * ARP on active-backup slaves with arp_validate enabled. > - */ > -static inline int skb_bond_should_drop(struct sk_buff *skb) > -{ > - struct net_device *dev = skb->dev; > - struct net_device *master = dev->master; > - > - if (master) { > - if (master->priv_flags & IFF_MASTER_ARPMON) > - dev->last_rx = jiffies; > - > - if (dev->priv_flags & IFF_SLAVE_INACTIVE) { > - if ((dev->priv_flags & IFF_SLAVE_NEEDARP) && > - skb->protocol == __cpu_to_be16(ETH_P_ARP)) > - return 0; > - > - if (master->priv_flags & IFF_MASTER_ALB) { > - if (skb->pkt_type != PACKET_BROADCAST && > - skb->pkt_type != PACKET_MULTICAST) > - return 0; > - } > - if (master->priv_flags & IFF_MASTER_8023AD && > - skb->protocol == __cpu_to_be16(ETH_P_SLOW)) > - return 0; > - > - return 1; > - } > - } > - return 0; > -} > - > extern struct pernet_operations __net_initdata loopback_net_ops; > > static inline int dev_ethtool_get_settings(struct net_device *dev, > diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c > index 7f7de1a..c6eae40 100644 > --- a/net/8021q/vlan_core.c > +++ b/net/8021q/vlan_core.c > @@ -2,6 +2,7 @@ > #include <linux/netdevice.h> > #include <linux/if_vlan.h> > #include <linux/netpoll.h> > +#include <linux/bonding.h> > #include "vlan.h" > > /* VLAN rx hw acceleration helper. This acts like netif_{rx,receive_skb}(). */ > diff --git a/net/core/dev.c b/net/core/dev.c > index 241613f..221b43f 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -127,6 +127,7 @@ > #include <linux/jhash.h> > #include <linux/random.h> > #include <trace/napi.h> > +#include <linux/bonding.h> > > #include "net-sysfs.h" > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tue, May 26, 2009 at 06:32:42PM CEST, andy@greyhouse.net wrote: >On Tue, May 26, 2009 at 05:17:17PM +0200, Jiri Pirko wrote: >> [PATCH net-next] bonding: allow bond in mode balance-alb to work properly in bridge -try4.1 >> >> Hi all. >> >> The problem is described in following bugzilla: >> https://bugzilla.redhat.com/show_bug.cgi?id=487763 >> >> Basically here's what's going on. In every mode, bonding interface uses the same >> mac address for all enslaved devices (except fail_over_mac). Only balance-alb >> will simultaneously use multiple MAC addresses across different slaves. When you >> put this kind of bond device into a bridge it will only add one of mac adresses >> into a hash list of mac addresses, say X. This mac address is marked as local. >> But this bonding interface also has mac address Y. Now then packet arrives with >> destination address Y, this address is not marked as local and the packed looks >> like it needs to be forwarded. This packet is then lost which is wrong. >> >> Notice that interfaces can be added and removed from bond while it is in bridge. >> >> *** >> When the multiple addresses for bridge port approach failed to solve this issue >> due to STP I started to think other way to solve this. I returned to previous >> solution but tweaked one. >> >> This patch solves the situation in the bonding without touching bridge code. >> For every incoming frame to bonding the destination address is compared to >> current address of the slave device from which tha packet came. If these two >> match destination address is replaced by mac address of the master. This address >> is known by bridge so it is delivered properly. > >Did you test this with a bond with more than 2 ports? I ask because I >might also expect a check against all the members of the bond (rather >than simply the receiving device). Yes, my testing machine has 3 interfaces for bond. Works fine. > >That check would be quite expensive for every frame and I think the >scenario is quite unlikely based on the frequency of 'learning frames' >sent by the alb code (so the switch connected to the host should have >it's forwarding database correct), but it might be something to think >about in the future. As you can see, my previous patch did the checking vs all slaves. I tried this experimentally and searched address from the list and dev->dev_addr differs only when I unplug cable and mac swap occurs. Then one packet is lost. But there are many lost packet during the unplug anyway so.... > >> I experimentally tried that this works as good as searching through the slave >> list (v4 of this patch). >> >> I was forced to create a new header because I need to use >> compare_ether_addr_64bits() (defined in linux/etherdevice.h) in >> linux/netdevice.h. I've hit some cross include issues. I think that it's good >> to have skb_bond_should_drop() in a separate file anyway. >> >> Jirka >> >> >> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >> > >This certainly won't cure all of the problems that arise with bonding >and bridging interactions, but it's a step in the right direction. > >Acked-by: Andy Gospodarek <andy@greyhouse.net> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tue, May 26, 2009 at 06:59:53PM CEST, dada1@cosmosbay.com wrote: >Jiri Pirko a écrit : >> [PATCH net-next] bonding: allow bond in mode balance-alb to work properly in bridge -try4.1 >> >> Hi all. >> >> The problem is described in following bugzilla: >> https://bugzilla.redhat.com/show_bug.cgi?id=487763 >> >> Basically here's what's going on. In every mode, bonding interface uses the same >> mac address for all enslaved devices (except fail_over_mac). Only balance-alb >> will simultaneously use multiple MAC addresses across different slaves. When you >> put this kind of bond device into a bridge it will only add one of mac adresses >> into a hash list of mac addresses, say X. This mac address is marked as local. >> But this bonding interface also has mac address Y. Now then packet arrives with >> destination address Y, this address is not marked as local and the packed looks >> like it needs to be forwarded. This packet is then lost which is wrong. >> >> Notice that interfaces can be added and removed from bond while it is in bridge. >> >> *** >> When the multiple addresses for bridge port approach failed to solve this issue >> due to STP I started to think other way to solve this. I returned to previous >> solution but tweaked one. >> >> This patch solves the situation in the bonding without touching bridge code. >> For every incoming frame to bonding the destination address is compared to >> current address of the slave device from which tha packet came. If these two >> match destination address is replaced by mac address of the master. This address >> is known by bridge so it is delivered properly. >> >> I experimentally tried that this works as good as searching through the slave >> list (v4 of this patch). >> >> I was forced to create a new header because I need to use >> compare_ether_addr_64bits() (defined in linux/etherdevice.h) in >> linux/netdevice.h. I've hit some cross include issues. I think that it's good >> to have skb_bond_should_drop() in a separate file anyway. >> >> Jirka >> >> >> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >> >> diff --git a/include/linux/bonding.h b/include/linux/bonding.h >> new file mode 100644 >> index 0000000..3081ddb >> --- /dev/null >> +++ b/include/linux/bonding.h >> @@ -0,0 +1,78 @@ >> +/* >> + * include/linux/bonding.h >> + * >> + * Copyright (C) 2009 Jiri Pirko <jpirko@redhat.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 >> + * as published by the Free Software Foundation. >> + * >> + * Bonding device helpers. >> + */ >> + >> +#ifndef _LINUX_BONDING_H >> +#define _LINUX_BONDING_H >> + >> +#ifdef __KERNEL__ >> + >> +#include <linux/skbuff.h> >> +#include <linux/netdevice.h> >> +#include <linux/if.h> >> +#include <linux/etherdevice.h> >> +#include <linux/if_ether.h> >> + >> +static inline void skb_bond_set_mac_by_master(struct sk_buff *skb, >> + struct net_device *dev, >> + struct net_device *master) >> +{ >> + unsigned char *dest = eth_hdr(skb)->h_dest; >> + >> + if (compare_ether_addr_64bits(dest, master->dev_addr) && >> + !compare_ether_addr_64bits(dest, dev->dev_addr)) >> + memcpy(dest, master->dev_addr, ETH_ALEN); > >But couldnt we test skb->pkt_type == PACKET_HOST instead, >Or eth_type_trans() not yet called at this point ? > >I would suggest : > >if (skb->pkt_type == PACKET_HOST) > memcpy(dest, master->dev_addr, ETH_ALEN); Yes Eric, you are right, good point. eth_type_trans() is called in any driver before and compare_ether_addr_64bits(dest, dev->dev_addr) is done there. So it's safe to use this here (and save some ticks). I'm going to make a new patch and test it. Jirka > >> +} >> + >> +/* On bonding slaves other than the currently active slave, suppress >> + * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and >> + * ARP on active-backup slaves with arp_validate enabled. >> + */ >> +static inline int skb_bond_should_drop(struct sk_buff *skb) >> +{ >> + struct net_device *dev = skb->dev; >> + struct net_device *master = dev->master; >> + >> + if (master) { >> + if (master->priv_flags & IFF_MASTER_ARPMON) >> + dev->last_rx = jiffies; >> + >> + if ((master->priv_flags & IFF_MASTER_ALB) && master->br_port) { >> + /* Do address unmangle. The local destination address >> + * will be always the one master has. Provides the right >> + * functionality in a bridge. >> + */ >> + skb_bond_set_mac_by_master(skb, dev, master); >> + } >> + >> + if (dev->priv_flags & IFF_SLAVE_INACTIVE) { >> + if ((dev->priv_flags & IFF_SLAVE_NEEDARP) && >> + skb->protocol == __cpu_to_be16(ETH_P_ARP)) >> + return 0; >> + >> + if (master->priv_flags & IFF_MASTER_ALB) { >> + if (skb->pkt_type != PACKET_BROADCAST && >> + skb->pkt_type != PACKET_MULTICAST) >> + return 0; >> + } >> + if (master->priv_flags & IFF_MASTER_8023AD && >> + skb->protocol == __cpu_to_be16(ETH_P_SLOW)) >> + return 0; >> + >> + return 1; >> + } >> + } >> + return 0; >> +} >> + >> +#endif /* __KERNEL__ */ >> + >> +#endif /* _LINUX_BONDING_H */ >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index ae3c209..06e24ae 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -1897,39 +1897,6 @@ static inline void netif_set_gso_max_size(struct net_device *dev, >> dev->gso_max_size = size; >> } >> >> -/* On bonding slaves other than the currently active slave, suppress >> - * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and >> - * ARP on active-backup slaves with arp_validate enabled. >> - */ >> -static inline int skb_bond_should_drop(struct sk_buff *skb) >> -{ >> - struct net_device *dev = skb->dev; >> - struct net_device *master = dev->master; >> - >> - if (master) { >> - if (master->priv_flags & IFF_MASTER_ARPMON) >> - dev->last_rx = jiffies; >> - >> - if (dev->priv_flags & IFF_SLAVE_INACTIVE) { >> - if ((dev->priv_flags & IFF_SLAVE_NEEDARP) && >> - skb->protocol == __cpu_to_be16(ETH_P_ARP)) >> - return 0; >> - >> - if (master->priv_flags & IFF_MASTER_ALB) { >> - if (skb->pkt_type != PACKET_BROADCAST && >> - skb->pkt_type != PACKET_MULTICAST) >> - return 0; >> - } >> - if (master->priv_flags & IFF_MASTER_8023AD && >> - skb->protocol == __cpu_to_be16(ETH_P_SLOW)) >> - return 0; >> - >> - return 1; >> - } >> - } >> - return 0; >> -} >> - >> extern struct pernet_operations __net_initdata loopback_net_ops; >> >> static inline int dev_ethtool_get_settings(struct net_device *dev, >> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c >> index 7f7de1a..c6eae40 100644 >> --- a/net/8021q/vlan_core.c >> +++ b/net/8021q/vlan_core.c >> @@ -2,6 +2,7 @@ >> #include <linux/netdevice.h> >> #include <linux/if_vlan.h> >> #include <linux/netpoll.h> >> +#include <linux/bonding.h> >> #include "vlan.h" >> >> /* VLAN rx hw acceleration helper. This acts like netif_{rx,receive_skb}(). */ >> diff --git a/net/core/dev.c b/net/core/dev.c >> index 241613f..221b43f 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -127,6 +127,7 @@ >> #include <linux/jhash.h> >> #include <linux/random.h> >> #include <trace/napi.h> >> +#include <linux/bonding.h> >> >> #include "net-sysfs.h" >> >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/bonding.h b/include/linux/bonding.h new file mode 100644 index 0000000..3081ddb --- /dev/null +++ b/include/linux/bonding.h @@ -0,0 +1,78 @@ +/* + * include/linux/bonding.h + * + * Copyright (C) 2009 Jiri Pirko <jpirko@redhat.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation. + * + * Bonding device helpers. + */ + +#ifndef _LINUX_BONDING_H +#define _LINUX_BONDING_H + +#ifdef __KERNEL__ + +#include <linux/skbuff.h> +#include <linux/netdevice.h> +#include <linux/if.h> +#include <linux/etherdevice.h> +#include <linux/if_ether.h> + +static inline void skb_bond_set_mac_by_master(struct sk_buff *skb, + struct net_device *dev, + struct net_device *master) +{ + unsigned char *dest = eth_hdr(skb)->h_dest; + + if (compare_ether_addr_64bits(dest, master->dev_addr) && + !compare_ether_addr_64bits(dest, dev->dev_addr)) + memcpy(dest, master->dev_addr, ETH_ALEN); +} + +/* On bonding slaves other than the currently active slave, suppress + * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and + * ARP on active-backup slaves with arp_validate enabled. + */ +static inline int skb_bond_should_drop(struct sk_buff *skb) +{ + struct net_device *dev = skb->dev; + struct net_device *master = dev->master; + + if (master) { + if (master->priv_flags & IFF_MASTER_ARPMON) + dev->last_rx = jiffies; + + if ((master->priv_flags & IFF_MASTER_ALB) && master->br_port) { + /* Do address unmangle. The local destination address + * will be always the one master has. Provides the right + * functionality in a bridge. + */ + skb_bond_set_mac_by_master(skb, dev, master); + } + + if (dev->priv_flags & IFF_SLAVE_INACTIVE) { + if ((dev->priv_flags & IFF_SLAVE_NEEDARP) && + skb->protocol == __cpu_to_be16(ETH_P_ARP)) + return 0; + + if (master->priv_flags & IFF_MASTER_ALB) { + if (skb->pkt_type != PACKET_BROADCAST && + skb->pkt_type != PACKET_MULTICAST) + return 0; + } + if (master->priv_flags & IFF_MASTER_8023AD && + skb->protocol == __cpu_to_be16(ETH_P_SLOW)) + return 0; + + return 1; + } + } + return 0; +} + +#endif /* __KERNEL__ */ + +#endif /* _LINUX_BONDING_H */ diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index ae3c209..06e24ae 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1897,39 +1897,6 @@ static inline void netif_set_gso_max_size(struct net_device *dev, dev->gso_max_size = size; } -/* On bonding slaves other than the currently active slave, suppress - * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and - * ARP on active-backup slaves with arp_validate enabled. - */ -static inline int skb_bond_should_drop(struct sk_buff *skb) -{ - struct net_device *dev = skb->dev; - struct net_device *master = dev->master; - - if (master) { - if (master->priv_flags & IFF_MASTER_ARPMON) - dev->last_rx = jiffies; - - if (dev->priv_flags & IFF_SLAVE_INACTIVE) { - if ((dev->priv_flags & IFF_SLAVE_NEEDARP) && - skb->protocol == __cpu_to_be16(ETH_P_ARP)) - return 0; - - if (master->priv_flags & IFF_MASTER_ALB) { - if (skb->pkt_type != PACKET_BROADCAST && - skb->pkt_type != PACKET_MULTICAST) - return 0; - } - if (master->priv_flags & IFF_MASTER_8023AD && - skb->protocol == __cpu_to_be16(ETH_P_SLOW)) - return 0; - - return 1; - } - } - return 0; -} - extern struct pernet_operations __net_initdata loopback_net_ops; static inline int dev_ethtool_get_settings(struct net_device *dev, diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c index 7f7de1a..c6eae40 100644 --- a/net/8021q/vlan_core.c +++ b/net/8021q/vlan_core.c @@ -2,6 +2,7 @@ #include <linux/netdevice.h> #include <linux/if_vlan.h> #include <linux/netpoll.h> +#include <linux/bonding.h> #include "vlan.h" /* VLAN rx hw acceleration helper. This acts like netif_{rx,receive_skb}(). */ diff --git a/net/core/dev.c b/net/core/dev.c index 241613f..221b43f 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -127,6 +127,7 @@ #include <linux/jhash.h> #include <linux/random.h> #include <trace/napi.h> +#include <linux/bonding.h> #include "net-sysfs.h"
[PATCH net-next] bonding: allow bond in mode balance-alb to work properly in bridge -try4.1 Hi all. The problem is described in following bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=487763 Basically here's what's going on. In every mode, bonding interface uses the same mac address for all enslaved devices (except fail_over_mac). Only balance-alb will simultaneously use multiple MAC addresses across different slaves. When you put this kind of bond device into a bridge it will only add one of mac adresses into a hash list of mac addresses, say X. This mac address is marked as local. But this bonding interface also has mac address Y. Now then packet arrives with destination address Y, this address is not marked as local and the packed looks like it needs to be forwarded. This packet is then lost which is wrong. Notice that interfaces can be added and removed from bond while it is in bridge. *** When the multiple addresses for bridge port approach failed to solve this issue due to STP I started to think other way to solve this. I returned to previous solution but tweaked one. This patch solves the situation in the bonding without touching bridge code. For every incoming frame to bonding the destination address is compared to current address of the slave device from which tha packet came. If these two match destination address is replaced by mac address of the master. This address is known by bridge so it is delivered properly. I experimentally tried that this works as good as searching through the slave list (v4 of this patch). I was forced to create a new header because I need to use compare_ether_addr_64bits() (defined in linux/etherdevice.h) in linux/netdevice.h. I've hit some cross include issues. I think that it's good to have skb_bond_should_drop() in a separate file anyway. Jirka Signed-off-by: Jiri Pirko <jpirko@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html