Subject: Re: [Discuss] IPv4 packets lost with macvlan over bond alb

From: Jay Vosburgh <[email-masked]>
Date: Wed, 21 Jun 2023 18:42:10 -0700

Hangbin Liu <[email-masked]> wrote:

>On Sat, Jun 17, 2023 at 9:45 AM Jay Vosburgh <[email-masked]> wrote:
>>
>> Hangbin Liu <[email-masked]> wrote:
>>
>> >Hi Jay, any thoughts?
>>
>>         Just an update that I've done some poking with your reproducer;
>> as described, the ARP reply for the IP address associated with the
>> macvlan interface is coming back with the bond's MAC, not the MAC of the
>> macvlan.  If I manually create a neighbour entry on the client with the
>> corrent IP and MAC for the macvlan, then connectivity works as expected.
>>
>>         I'll have to look a bit further into the ARP MAC selection logic
>> here (i.e., where does that MAC come from when the ARP reply is
>> generated).  It also makes me wonder what's special about macvlan, e.g.,
>> why doesn't regular VLAN (or other stacked devices) fail in the same way
>> (or maybe it does and nobody has noticed).
>
>VLAN or other overlay devices use the same MAC address with Bonding.
>So they work with the alb mode. But MACVLAN uses different MAC
>addresses with Bonding.

	By default, yes, VLANs use the same MAC, but may use a different
MAC than the device the VLAN is configured above.  However, changing the
VLAN's MAC address in a similar test case (VLAN above balance-alb bond)
still works, because VLAN packets are delivered by matching the VLAN ID
(via vlan_do_receive() -> vlan_find_dev()), not via the MAC address.

	So, the RLB MAC edits done by rlb_arp_xmit() work in the sense
that traffic flows, even though peers see a MAC address from the bond
for the VLAN IP, not the VLAN's actual MAC address.

	A bridge can also use a MAC address that differs from the bond,
but rlb_arp_xmit() has a special case for bridge, and doesn't alter the
ARP if the relevant IP address is on a bridge (so, no balancing).

	Changing rlb_arp_xmit() to add macvlan to the bridge check makes
the test case pass, e.g.,

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index b9dbad3a8af8..f720c419dfb7 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -668,7 +668,7 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
 
 	dev = ip_dev_find(dev_net(bond->dev), arp->ip_src);
 	if (dev) {
-		if (netif_is_bridge_master(dev)) {
+		if (netif_is_bridge_master(dev) || netif_is_macvlan(dev)) {
 			dev_put(dev);
 			return NULL;
 		}

	but I'm not completely sure that's doing the right thing with
regard to the macvlan logic in alb_send_learning_packets() ->
alb_upper_dev_walk().

	-J

---
	-Jay Vosburgh, [email-masked]