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@xxxxxxxxxx> > > 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@xxxxxxxxxx> > + * > + * 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > _______________________________________________ Bridge mailing list Bridge@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/bridge
