On 2018-07-19 18:20, Michal Soltys wrote:
> On 07/19/2018 01:41 AM, Mahesh Bandewar wrote:
>> From: Mahesh Bandewar <mahe...@google.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 <sol...@ziu.info>
>> Signed-off-by: Mahesh Bandewar <mahe...@google.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).
