public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Vincent Bernat <vincent@bernat•ch>
To: Chonggang Li <chonggangli@google•com>
Cc: Jay Vosburgh <j.vosburgh@gmail•com>,
	Andy Gospodarek <andy@greyhouse•net>,
	Veaceslav Falico <vfalico@gmail•com>,
	David Miller <davem@davemloft•net>,
	Netdev <netdev@vger•kernel.org>, Michal Soltys <soltys@ziu•info>,
	Mahesh Bandewar <maheshb@google•com>,
	Mahesh Bandewar <mahesh@bandewar•net>
Subject: Re: [next] bonding: pass link-local packets to bonding master also.
Date: Sun, 09 Dec 2018 09:30:49 +0100	[thread overview]
Message-ID: <87a7lfxft2.fsf@bernat.ch> (raw)
In-Reply-To: <m3d0qmfe0d.fsf@bernat.ch> (Vincent Bernat's message of "Fri, 30 Nov 2018 22:32:02 +0100")

 ❦ 30 novembre 2018 22:32 +0100, Vincent Bernat <vincent@bernat•ch>:

>> 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 = 263
> socket.ETH_P_ALL = 3
> socket.PACKET_ORIGDEV = 9
>
> interface = sys.argv[1] if len(sys.argv) > 1 else 'lag1'
>
> s = 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 = s.recvfrom(1500)
>     if addrinfo[2] == 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.

-- 
This night methinks is but the daylight sick.
		-- William Shakespeare, "The Merchant of Venice"

  reply	other threads:[~2018-12-09  8:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-16  1:12 [PATCH next] bonding: pass link-local packets to bonding master also Mahesh Bandewar
2018-07-16 21:24 ` Jay Vosburgh
2018-07-16 23:53   ` Mahesh Bandewar (महेश बंडेवार)
2018-07-17  9:32     ` Michal Soltys
2018-07-17 22:17       ` Michal Soltys
2018-07-16 23:33 ` Stephen Hemminger
2018-07-16 23:57   ` Mahesh Bandewar (महेश बंडेवार)
2018-07-17  0:43     ` Stephen Hemminger
2018-07-17  9:55     ` Michal Soltys
2018-11-30 21:32 ` [next] " Vincent Bernat
2018-12-09  8:30   ` Vincent Bernat [this message]
2018-12-19 22:57   ` Michal Soltys
2018-12-20  5:55     ` Vincent Bernat
2018-12-23 13:06       ` Michal Soltys
2018-12-23 13:16         ` Vincent Bernat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a7lfxft2.fsf@bernat.ch \
    --to=vincent@bernat$(echo .)ch \
    --cc=andy@greyhouse$(echo .)net \
    --cc=chonggangli@google$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=j.vosburgh@gmail$(echo .)com \
    --cc=mahesh@bandewar$(echo .)net \
    --cc=maheshb@google$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=soltys@ziu$(echo .)info \
    --cc=vfalico@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox