lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <14780.1660141945@famine> Date: Wed, 10 Aug 2022 07:32:25 -0700 From: Jay Vosburgh <jay.vosburgh@...onical.com> To: Sun Shouxin <sunshouxin@...natelecom.cn> cc: vfalico@...il.com, andy@...yhouse.net, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, razor@...ckwall.org, huyd12@...natelecom.cn Subject: Re: [PATCH v2] net:bonding:support balance-alb interface with vlan to bridge Sun Shouxin <sunshouxin@...natelecom.cn> wrote: >In my test, balance-alb bonding with two slaves eth0 and eth1, >and then Bond0.150 is created with vlan id attached bond0. >After adding bond0.150 into one linux bridge, I noted that Bond0, >bond0.150 and bridge were assigned to the same MAC as eth0. >Once bond0.150 receives a packet whose dest IP is bridge's >and dest MAC is eth1's, the linux bridge will not match >eth1's MAC entry in FDB, and not handle it as expected. >The patch fix the issue, and diagram as below: > >eth1(mac:eth1_mac)--bond0(balance-alb,mac:eth0_mac)--eth0(mac:eth0_mac) > | > bond0.150(mac:eth0_mac) > | > bridge(ip:br_ip, mac:eth0_mac)--other port > >Suggested-by: Hu Yadi <huyd12@...natelecom.cn> >Signed-off-by: Sun Shouxin <sunshouxin@...natelecom.cn> As Nik suggested, please add some additional explanation here. You can cut and paste my description from the original discussion if you'd like. >--- > >changelog: >v1->v2: > -declare variabls in reverse xmas tree order > -delete {} > -add explanation in commit message >--- > drivers/net/bonding/bond_alb.c | 7 +++++++ > 1 file changed, 7 insertions(+) > >diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c >index 007d43e46dcb..60cb9a0225aa 100644 >--- a/drivers/net/bonding/bond_alb.c >+++ b/drivers/net/bonding/bond_alb.c >@@ -653,6 +653,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, > static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond) > { > struct slave *tx_slave = NULL; >+ struct net_device *dev; > struct arp_pkt *arp; > > if (!pskb_network_may_pull(skb, sizeof(*arp))) >@@ -665,6 +666,12 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond) > if (!bond_slave_has_mac_rx(bond, arp->mac_src)) > return NULL; > >+ dev = ip_dev_find(dev_net(bond->dev), arp->ip_src); >+ if (dev) { >+ if (netif_is_bridge_master(dev)) >+ return NULL; Stylistically, the "if dev" and "if netif_is_bridge_master" could be one line, e.g., "if dev && netif_is_bridge_master". Functionally, ip_dev_find acquires a reference to dev, and this code will need to release (dev_put) that reference. I'm also wondering if testing bond->dev for netif_if_bridge_port before ip_dev_find would help here (as an optimization); I think so, for the case where the bond is directly in the bridge without a VLAN in the middle. -J >+ } >+ > if (arp->op_code == htons(ARPOP_REPLY)) { > /* the arp must be sent on the selected rx channel */ > tx_slave = rlb_choose_channel(skb, bond, arp); >-- >2.27.0 > --- -Jay Vosburgh, jay.vosburgh@...onical.com
Powered by blists - more mailing lists