Re: [PATCH] net:bonding:support balance-alb interface with vlan to bridge

[sunshouxin@xxxxxxxxxxxxxxx]

在 2022/8/6 3:18, Jay Vosburgh 写道:
> Sun Shouxin <sunshouxin@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > Signed-off-by: Sun Shouxin <sunshouxin@xxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxx