From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vincent Bernat Subject: Re: [next] bonding: pass link-local packets to bonding master also. Date: Sun, 09 Dec 2018 09:30:49 +0100 Message-ID: <87a7lfxft2.fsf@bernat.ch> References: <20180716011246.225647-1-mahesh@bandewar.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: Jay Vosburgh , Andy Gospodarek , Veaceslav Falico , David Miller , Netdev , Michal Soltys , Mahesh Bandewar , Mahesh Bandewar To: Chonggang Li Return-path: Received: from out1-smtp.messagingengine.com ([66.111.4.25]:53965 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726055AbeLIIhg (ORCPT ); Sun, 9 Dec 2018 03:37:36 -0500 In-Reply-To: (Vincent Bernat's message of "Fri, 30 Nov 2018 22:32:02 +0100") Sender: netdev-owner@vger.kernel.org List-ID: =E2=9D=A6 30 novembre 2018 22:32 +0100, Vincent Bernat : >> 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. > > Unfortunately, this doesn't completely restore the previous > functionality as PACKET_ORIGDEV is broken for the copy: the original > interface is lost through the call to netif_rx(). A LLDP daemon > listening to the master interface won't get the original interface like > it was able to before 4.12. I think I didn't get an answer. The commit introducing the regression says: Bonding driver changes the skb->dev to the bonding-master before passing the packet to stack for further processing. This, however does not make sense for the link-local packets and it loses "the link info" once its skb->dev is changed to bonding-master. This patch changes this behavior for link-local packets by not changing the skb->dev to the bonding-master and maintaining it as it is, i.e. the link on which the packet arrived. Li, do you have a test case for this? Which family are you using? When using AF_PACKET, the information was retrievable by enabling the PACKET_ORIGDEV option. For context: > I am a bit lost of what the original patch was trying to achieve. I am > using the following test program: > > #v+ > #!/usr/bin/env python3 > > import sys > import socket > import datetime > > socket.SOL_PACKET =3D 263 > socket.ETH_P_ALL =3D 3 > socket.PACKET_ORIGDEV =3D 9 > > interface =3D sys.argv[1] if len(sys.argv) > 1 else 'lag1' > > s =3D socket.socket(socket.AF_PACKET, > socket.SOCK_RAW, > socket.htons(socket.ETH_P_ALL)) > s.bind((interface, 0)) > s.setsockopt(socket.SOL_PACKET, socket.PACKET_ORIGDEV, 1) > while True: > data, addrinfo =3D s.recvfrom(1500) > if addrinfo[2] =3D=3D socket.PACKET_OUTGOING: > continue > print(f"{datetime.datetime.now().isoformat()}: " > f"Received {len(data)} bytes from {addrinfo}") > #v- > > > If I run it with a kernel compiled with the commit before b89f04c61efe > (plus a few more cherry-pick to make it work like ea8ffc0818d8 and > 72ccc471e13b), I get: > > #v+ > 2018-11-30T22:20:40.193378: Received 221 bytes from ('eth1', 35020, 2, 1,= b'RT3\x00\x00\x02') > 2018-11-30T22:20:40.194504: Received 221 bytes from ('eth0', 35020, 2, 1,= b'RT3\x00\x00\x01') > #v- > > > If I send non link-local packets, I get: > > #v+ > 2018-11-30T22:25:57.300965: Received 98 bytes from ('eth0', 2048, 0, 1, b= 'PT3\x00\x00\x02') > #v- > > I am also able to correctly receive link-local packets directly on each > interface. So, it seems everything was working as expected before > b89f04c61efe. --=20 This night methinks is but the daylight sick. -- William Shakespeare, "The Merchant of Venice"