From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Soltys Subject: Re: [PATCH net v2] bonding: pass link-local packets to bonding master also. Date: Fri, 14 Sep 2018 01:00:10 +0200 Message-ID: <9da916a6-73c0-75b5-a7cc-dac70d549ddd@ziu.info> References: <20180718234106.243196-1-mahesh@bandewar.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Netdev , Mahesh Bandewar To: Mahesh Bandewar , Jay Vosburgh , Andy Gospodarek , Veaceslav Falico , David Miller Return-path: Received: from drutsystem.com ([84.10.39.251]:37226 "EHLO drutsystem.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725757AbeINEUA (ORCPT ); Fri, 14 Sep 2018 00:20:00 -0400 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018-07-19 18:20, Michal Soltys wrote: > On 07/19/2018 01:41 AM, Mahesh Bandewar wrote: >> From: Mahesh Bandewar >> >> 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 >> Signed-off-by: Mahesh Bandewar >> --- >> 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).