Message-ID: <25355.1686199455@famine>
Date: Wed, 07 Jun 2023 21:44:15 -0700
From: Jay Vosburgh <jay.vosburgh@...onical.com>
To: Hangbin Liu <liuhangbin@...il.com>
cc: netdev@...r.kernel.org
Subject: Re: [Discuss] IPv4 packets lost with macvlan over bond alb

Hangbin Liu <liuhangbin@...il.com> wrote:

>Hi Jay, any thoughts?

	Only that the more I look into this the more confused I get.  I
haven't had a chance to set it up to test and poke at it, but did look
through the code and some of the history.

>Thanks
>Hangbin
>On Fri, Jun 02, 2023 at 04:09:00PM +0800, Hangbin Liu wrote:
>> Hi Jay,
>> 
>> It looks there is an regression for commit 14af9963ba1e ("bonding: Support
>> macvlans on top of tlb/rlb mode bonds"). The author export modified ARP to
>> remote when there is macvlan over bond, which make remote add neighbor
>> with macvlan's IP and bond's mac.

	There was a report similar to yours in 2017,

https://lore.kernel.org/netdev/CAJN_NGada81u96VSRz=puy3DOXjqJ6H9vNkMjgGFBea3vgrPPQ@mail.gmail.com/

	with no responses, so I presume this hasn't worked for at least
that long (suggesting that macvlan over balance-alb is not a popular
deployment choice).  That leads me to wonder if the 14af9963ba1e patch
ever worked entirely correctly.  Perhaps whatever Vlad did to test it
missed the condition that you and the above reporter are seeing, or it
works in only a subset of macvlan modes, or maybe it did work correctly
when 14af9963ba1e was applied.

>> [...] The author expect RLB will replace all
>> inner packets to correct mac address if target is macvlan, but RLB only
>> handle ARP packets. This make all none arp packets macvlan received have
>> incorrect mac address, and dropped directly.

	Reading the 14af9963ba1e commit message, I think you're
referring to:

    To make RLB work, all we have to do is accept ARP packets
    for addresses added to the bond dev->uc list.  Since RLB
    mode will take care to update the peers directly with
    correct mac addresses, learning packets for these addresses
    do not have be send to switch.

	I don't think that means he expects RLB to fix up the MACs in
every incoming packet.  I'm reading this as that the macvlan peers will
be issued ARPs that match the IP address to the macvlan MAC address, and
that those packets should be accepted because the macvlan MAC address is
in the bond interface's unicast MAC address list (dev->uc).

>> In short, remote client learned macvlan's ip with bond's mac. So the macvlan
>> will receive packets with incorrect macs and dropped.
>> 
>> To fix this, one way is to revert the patch and only send learning packets for
>> both tlb and alb mode for macvlan. This would make all macvlan rx packets go
>> through bond's active slave.
>> 
>> Another way is to replace the bond's mac address to correct macvlan's address
>> based on the rx_hashtbl . But this may has impact to the receive performance
>> since we need to check all the upper devices and deal the mac address for
>> each packets in bond_handle_frame().
>> 
>> So which way do you prefer?

	I don't yet understand what's going on well enough have an
informed opinion; in particular, if this did work correctly when
14af9963ba1e was originally applied, then it's not clear what broke it.
I should have some time to test this in the next day or two.

	-J


>> Reproducer:
>> ```
>> #!/bin/bash
>> 
>> # Source the topo in bond selftest
>> source bond_topo_3d1c.sh
>> 
>> trap cleanup EXIT
>> 
>> setup_prepare
>> bond_reset "mode balance-alb"
>> ip -n ${s_ns} addr flush dev bond0
>> 
>> ip -n ${s_ns} link add link bond0 name macv0 type macvlan mode bridge
>> ip -n ${s_ns} link set macv0 up
>> 
>> # I just add macvlan on the server netns, you can also move it to another netns for testing
>> ip -n ${s_ns} addr add ${s_ip4}/24 dev macv0
>> ip -n ${s_ns} addr add ${s_ip6}/24 dev macv0
>> ip netns exec ${c_ns} ping ${s_ip4} -c 4
>> sleep 5
>> ip netns exec ${c_ns} ping ${s_ip4} -c 4
>> ```
>> 
>> Thanks
>> Hangbin

---
	-Jay Vosburgh, jay.vosburgh@...onical.com

Powered by blists - more mailing lists