Re: [PATCH net 1/1] bonding: fix PACKET_ORIGDEV regression on bonding masters

[Michal Soltys <soltys@ziu.info>]

On 19/01/15 03:19, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Mon, Jan 14, 2019 at 12:00 AM Vincent Bernat <vincent@bernat.ch> wrote:
>>
>>  ❦ 13 janvier 2019 18:01 -08, Maciej Żenczykowski <zenczykowski@gmail.com>:
>>
>> > But I seem to recall that the core problem we were trying to solve was
>> > that a daemon listening
>> > on an AF_PACKET ethertype 88CC [LLDP] socket not bound to any device
>> > would not receive LLDP packets
>> > arriving on inactive bond slaves (either active-backup or lag).
>>
>> Just tested and with 4.9.150, I am in fact unable to receive anything
>> on a backup link when listening to the active-backup master device or to
>> "any" device.
>>
>> > Perhaps going from:
>> >   /* don't change skb->dev for link-local packets */
>> >   if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) return RX_HANDLER_PASS;
>> >   if (bond_should_deliver_exact_match(skb, slave, bond)) return
>> > RX_HANDLER_EXACT;
>> >
>> > to something more like:
>> >   if (bond_should_deliver_exact_match(skb, slave, bond)) {
>> >     /* don't change skb->dev for link-local packets on inactive slaves */
>> >     if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) return RX_HANDLER_PASS;
>> >     return RX_HANDLER_EXACT;
>> >   }
>> >
>> > would fix both problems?
>>
> thanks for jumping in and offering a solution. This should fix the issue.
> 
> NACK for the revert-patch!
> 
> Folks, please, revert is not the solution! Last time when there was a
> problem posted I offered you a solution, so wasn't that enough to
> prove that we care about solving the problem that you are facing while
> continuing to have this functionality? No one wants to break your use
> case, it happens only because one is not aware of it. Thank you David
> for resorting to resolve it.

Mahesh, that's not it. But:

Since Vincent reported PACKET_ORIGDEV regression late november, none of 
you replied to anything posted until now. And if David hadn't called you 
guys directly, I'm not sure you would have at this point. Reverting and 
Vincent's offer to patch to update packet(7) were also clearly mentioned 
in the previous thread, none of them commented/nacked/acked either. Me 
an Vincent have been scratching our head for a while off list - but our 
guessing can only go as far as time goes on.

Maciej now was the first to ever provide the actual details about the 
issue you were facing originally. My bad I haven't added him to CC from 
the very beginning.

Anyway, I'll test Maciej's version in bridging context in coming days 
and look closer at the code overall. It probably works fine if Vincent 
is seeing packets on masters, but I'd rather be sure.