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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 8 Aug 2022 15:40:39 +0800
From:   "sunshouxin@...natelecom.cn" <sunshouxin@...natelecom.cn>
To:     Jay Vosburgh <jay.vosburgh@...onical.com>
Cc:     vfalico@...il.com, andy@...yhouse.net, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        ast@...nel.org, daniel@...earbox.net, hawk@...nel.org,
        john.fastabend@...il.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        huyd12@...natelecom.cn
Subject: Re: [PATCH] net:bonding:support balance-alb interface with vlan to
 bridge


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