| Message ID | 20220805074556.70297-1-sunshouxin@chinatelecom.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net:bonding:support balance-alb interface with vlan to bridge | expand |
Sun Shouxin <sunshouxin@chinatelecom.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 cannot process 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 In principle, since 567b871e5033, the bond alb mode shouldn't be load balancing incoming traffic for an IP address arriving via a bridge configured above the bond. Looking at it, there's logic in rlb_arp_xmit to exclude the bridge-over-bond case, but it relies on the MAC of traffic arriving via the bridge being different from the bond's MAC. I suspect this is because 567b871e5033 was intended to manage traffic originating from other bridge ports, and didn't consider the case of the bridge itself when the bridge MAC equals the bond MAC. The bridge MAC will equal the bond MAC if the bond is the first port added to the bridge, because the bridge will normally adopt the MAC of the first port added (unless manually set to something else). I think the correct fix here is to update the test in rlb_arp_xmit to properly exclude all bridge traffic (i.e., handle the bridge MAC == bond MAC case), not to alter the destination MAC address in incoming traffic. -J >Suggested-by: Hu Yadi <huyd12@chinatelecom.cn> >Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn> >--- > drivers/net/bonding/bond_main.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index e75acb14d066..6210a9c7ca76 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -1537,9 +1537,11 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) > struct sk_buff *skb = *pskb; > struct slave *slave; > struct bonding *bond; >+ struct net_device *vlan; > int (*recv_probe)(const struct sk_buff *, struct bonding *, > struct slave *); > int ret = RX_HANDLER_ANOTHER; >+ unsigned int headroom; > > skb = skb_share_check(skb, GFP_ATOMIC); > if (unlikely(!skb)) >@@ -1591,6 +1593,24 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) > bond->dev->addr_len); > } > >+ if (skb_vlan_tag_present(skb)) { >+ if (BOND_MODE(bond) == BOND_MODE_ALB && skb->pkt_type == PACKET_HOST) { >+ vlan = __vlan_find_dev_deep_rcu(bond->dev, skb->vlan_proto, >+ skb_vlan_tag_get(skb) & VLAN_VID_MASK); >+ if (vlan) { >+ if (vlan->priv_flags & IFF_BRIDGE_PORT) { >+ headroom = skb->data - skb_mac_header(skb); >+ if (unlikely(skb_cow_head(skb, headroom))) { >+ kfree_skb(skb); >+ return RX_HANDLER_CONSUMED; >+ } >+ bond_hw_addr_copy(eth_hdr(skb)->h_dest, vlan->dev_addr, >+ vlan->addr_len); >+ } >+ } >+ } >+ } >+ > return ret; > } > >-- >2.27.0 > --- -Jay Vosburgh, jay.vosburgh@canonical.com
在 2022/8/6 3:18, Jay Vosburgh 写道: > Sun Shouxin <sunshouxin@chinatelecom.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 cannot process 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 > In principle, since 567b871e5033, the bond alb mode shouldn't be > load balancing incoming traffic for an IP address arriving via a bridge > configured above the bond. > > Looking at it, there's logic in rlb_arp_xmit to exclude the > bridge-over-bond case, but it relies on the MAC of traffic arriving via > the bridge being different from the bond's MAC. I suspect this is > because 567b871e5033 was intended to manage traffic originating from > other bridge ports, and didn't consider the case of the bridge itself > when the bridge MAC equals the bond MAC. > > The bridge MAC will equal the bond MAC if the bond is the first > port added to the bridge, because the bridge will normally adopt the MAC > of the first port added (unless manually set to something else). > > I think the correct fix here is to update the test in > rlb_arp_xmit to properly exclude all bridge traffic (i.e., handle the > bridge MAC == bond MAC case), not to alter the destination MAC address > in incoming traffic. > > -J Thanks your warm instruction, I'll resend patch as your suggestion. -Sun > >> Suggested-by: Hu Yadi <huyd12@chinatelecom.cn> >> Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn> >> --- >> drivers/net/bonding/bond_main.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >> index e75acb14d066..6210a9c7ca76 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -1537,9 +1537,11 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) >> struct sk_buff *skb = *pskb; >> struct slave *slave; >> struct bonding *bond; >> + struct net_device *vlan; >> int (*recv_probe)(const struct sk_buff *, struct bonding *, >> struct slave *); >> int ret = RX_HANDLER_ANOTHER; >> + unsigned int headroom; >> >> skb = skb_share_check(skb, GFP_ATOMIC); >> if (unlikely(!skb)) >> @@ -1591,6 +1593,24 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) >> bond->dev->addr_len); >> } >> >> + if (skb_vlan_tag_present(skb)) { >> + if (BOND_MODE(bond) == BOND_MODE_ALB && skb->pkt_type == PACKET_HOST) { >> + vlan = __vlan_find_dev_deep_rcu(bond->dev, skb->vlan_proto, >> + skb_vlan_tag_get(skb) & VLAN_VID_MASK); >> + if (vlan) { >> + if (vlan->priv_flags & IFF_BRIDGE_PORT) { >> + headroom = skb->data - skb_mac_header(skb); >> + if (unlikely(skb_cow_head(skb, headroom))) { >> + kfree_skb(skb); >> + return RX_HANDLER_CONSUMED; >> + } >> + bond_hw_addr_copy(eth_hdr(skb)->h_dest, vlan->dev_addr, >> + vlan->addr_len); >> + } >> + } >> + } >> + } >> + >> return ret; >> } >> >> -- >> 2.27.0 >> > --- > -Jay Vosburgh, jay.vosburgh@canonical.com
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index e75acb14d066..6210a9c7ca76 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1537,9 +1537,11 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) struct sk_buff *skb = *pskb; struct slave *slave; struct bonding *bond; + struct net_device *vlan; int (*recv_probe)(const struct sk_buff *, struct bonding *, struct slave *); int ret = RX_HANDLER_ANOTHER; + unsigned int headroom; skb = skb_share_check(skb, GFP_ATOMIC); if (unlikely(!skb)) @@ -1591,6 +1593,24 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) bond->dev->addr_len); } + if (skb_vlan_tag_present(skb)) { + if (BOND_MODE(bond) == BOND_MODE_ALB && skb->pkt_type == PACKET_HOST) { + vlan = __vlan_find_dev_deep_rcu(bond->dev, skb->vlan_proto, + skb_vlan_tag_get(skb) & VLAN_VID_MASK); + if (vlan) { + if (vlan->priv_flags & IFF_BRIDGE_PORT) { + headroom = skb->data - skb_mac_header(skb); + if (unlikely(skb_cow_head(skb, headroom))) { + kfree_skb(skb); + return RX_HANDLER_CONSUMED; + } + bond_hw_addr_copy(eth_hdr(skb)->h_dest, vlan->dev_addr, + vlan->addr_len); + } + } + } + } + return ret; }
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 cannot process 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@chinatelecom.cn> Signed-off-by: Sun Shouxin <sunshouxin@chinatelecom.cn> --- drivers/net/bonding/bond_main.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)