All of lore.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.ch \
    --cc=andy@greyhouse.net \
    --cc=chonggangli@google.com \
    --cc=davem@davemloft.net \
    --cc=j.vosburgh@gmail.com \
    --cc=mahesh@bandewar.net \
    --cc=maheshb@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=soltys@ziu.info \
    --cc=vfalico@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.