netdev - Re: [PATCH net v2] bonding: pass link-local packets to bonding master also.

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 13 Sep 2018 16:24:21 -0700
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Michal Soltys <soltys@....info>
Cc:     Mahesh Bandewar <mahesh@...dewar.net>,
        Jay Vosburgh <j.vosburgh@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        Veaceslav Falico <vfalico@...il.com>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net v2] bonding: pass link-local packets to bonding master also.

On Thu, Sep 13, 2018 at 4:00 PM, Michal Soltys <soltys@....info> wrote:
> On 2018-07-19 18:20, Michal Soltys wrote:
>> On 07/19/2018 01:41 AM, Mahesh Bandewar wrote:
>>> From: Mahesh Bandewar <maheshb@...gle.com>
>>>
>>> Commit b89f04c61efe ("bonding: deliver link-local packets with
>>> skb->dev set to link that packets arrived on") changed the behavior
>>> of how link-local-multicast packets are processed. The change in
>>> the behavior broke some legacy use cases where these packets are
>>> expected to arrive on bonding master device also.
>>>
>>> This patch passes the packet to the stack with the link it arrived
>>> on as well as passes to the bonding-master device to preserve the
>>> legacy use case.
>>>
>>> Fixes: b89f04c61efe ("bonding: deliver link-local packets with
>>> skb->dev set to link that packets arrived on")
>>> Reported-by: Michal Soltys <soltys@....info>
>>> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
>>> ---
>>> v2: Added Fixes tag.
>>> v1: Initial patch.
>>>   drivers/net/bonding/bond_main.c | 17 +++++++++++++++--
>>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_main.c
>>> b/drivers/net/bonding/bond_main.c
>>> index 9a2ea3c1f949..1d3b7d8448f2 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -1177,9 +1177,22 @@ static rx_handler_result_t
>>> bond_handle_frame(struct sk_buff **pskb)
>>>           }
>>>       }
>>>   -    /* don't change skb->dev for link-local packets */
>>> -    if (is_link_local_ether_addr(eth_hdr(skb)->h_dest))
>>> +    /* Link-local multicast packets should be passed to the
>>> +     * stack on the link they arrive as well as pass them to the
>>> +     * bond-master device. These packets are mostly usable when
>>> +     * stack receives it with the link on which they arrive
>>> +     * (e.g. LLDP) but there may be some legacy behavior that
>>> +     * expects these packets to appear on bonding master too.
>>
>> I'd really change the comment from:
>>
>> "These packets are mostly usable when stack receives it with the link on
>> which they arrive (e.g. LLDP) but there may be some legacy behavior that
>> expects these packets to appear on bonding master too."
>>
>> to something like:
>>
>> "These packets are mostly usable when stack receives it with the link on
>> which they arrive, but they also must be available on aggregations. Some
>> of the use cases include (but are not limited to): LLDP agents that must
>> be able to operate both on enslaved interfaces as well as on bonds
>> themselves; linux bridges that must be able to process/pass BPDUs from
>> attached bonds when any kind of stp version is enabled on the network."
>>
>> It's a bit longer, but clarifies the reasons more precisely (without
>> going too deep into features like group_fwd_mask).
>>
>
> Anyway, any chance for that patch to get merged ? It would be great to
> get the correct functionality back asap. As for the comment, I'll submit
> a trivial patch expanding/clarifying it later (or I can resubmit
> adjusted v3 if it's ok with Mahesh).
Hmm, didn't notice that it wasn't merged but somehow it fell through
the cracks as it needed my attention earlier. I'll resubmit.