[PATCH next] bonding: pass link-local packets to bonding master also.

[PATCH next] bonding: pass link-local packets to bonding master also.

[Mahesh Bandewar]

From: Mahesh Bandewar <maheshb@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.

Reported-by: Michal Soltys <soltys@ziu.info>
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 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.
+	 */
+	if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) {
+		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
+
+		if (nskb) {
+			nskb->dev = bond->dev;
+			netif_rx(nskb);
+		}
 		return RX_HANDLER_PASS;
+	}
 	if (bond_should_deliver_exact_match(skb, slave, bond))
 		return RX_HANDLER_EXACT;
 
-- 
2.18.0.203.gfac676dfb9-goog

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Jay Vosburgh]

Mahesh Bandewar <mahesh@bandewar.net> wrote:

>From: Mahesh Bandewar <maheshb@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.

	Michal, can you test this?  I'm travelling this week and won't
be able to run the patch.

	Mahesh, will this confuse LLDP, et al, daemons that, e.g., bind
to every possible interface and now see the same LLDP PDU (identical
Chassis ID, Port ID, et al, TLVs) on multiple interfaces?

	Thanks,

	-J

>Reported-by: Michal Soltys <soltys@ziu.info>
>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>---
> 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.
>+	 */
>+	if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) {
>+		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
>+
>+		if (nskb) {
>+			nskb->dev = bond->dev;
>+			netif_rx(nskb);
>+		}
> 		return RX_HANDLER_PASS;
>+	}
> 	if (bond_should_deliver_exact_match(skb, slave, bond))
> 		return RX_HANDLER_EXACT;
> 
>-- 
>2.18.0.203.gfac676dfb9-goog

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Mahesh Bandewar (महेश बंडेवार)]

On Mon, Jul 16, 2018 at 2:24 PM, Jay Vosburgh
<jay.vosburgh@canonical.com> wrote:
> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
>>From: Mahesh Bandewar <maheshb@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.
>
>         Michal, can you test this?  I'm travelling this week and won't
> be able to run the patch.
>
>         Mahesh, will this confuse LLDP, et al, daemons that, e.g., bind
> to every possible interface and now see the same LLDP PDU (identical
> Chassis ID, Port ID, et al, TLVs) on multiple interfaces?
>
Well it's hard to say. In the previous world when these packets used
to appear only on bonding-master, that service had to go extra-lengths
to figure it out which link it actually came on in. With the earlier
change (SHA1: b89f04c61efe) it didn't have to but with this patch, the
best thing that they could do is just ignore those packets coming from
(any) virtual devices. The only reason why I'm OK with this change is
because L2 of a physical link is shared with a virtual link (bonding
master) and hence both links receiving the same link-local-multicast
seems acceptable. Making them appear only on bonding-master is just
wrong while correcting that behavior breaks the legacy use case and
here we are.

BTW when links are aggregated and using LACP, these packets don't
arrive the system-mac but the real mac of the sender with a dest
multicast-mac.

--mahesh..

>         Thanks,
>
>         -J
>
>>Reported-by: Michal Soltys <soltys@ziu.info>
>>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>---
>> 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.
>>+       */
>>+      if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) {
>>+              struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
>>+
>>+              if (nskb) {
>>+                      nskb->dev = bond->dev;
>>+                      netif_rx(nskb);
>>+              }
>>               return RX_HANDLER_PASS;
>>+      }
>>       if (bond_should_deliver_exact_match(skb, slave, bond))
>>               return RX_HANDLER_EXACT;
>>
>>--
>>2.18.0.203.gfac676dfb9-goog
>
> ---
>         -Jay Vosburgh, jay.vosburgh@canonical.com

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Michal Soltys]

On 07/17/2018 01:53 AM, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Mon, Jul 16, 2018 at 2:24 PM, Jay Vosburgh
> <jay.vosburgh@canonical.com> wrote:
>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>
>>> From: Mahesh Bandewar <maheshb@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.
>>
>>          Michal, can you test this?  I'm travelling this week and won't
>> be able to run the patch.


Yes, will test today and report.

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Michal Soltys]

On 2018-07-17 11:32, Michal Soltys wrote:
> On 07/17/2018 01:53 AM, Mahesh Bandewar (महेश बंडेवार) wrote:
>> On Mon, Jul 16, 2018 at 2:24 PM, Jay Vosburgh
>> <jay.vosburgh@canonical.com> wrote:
>>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>>
>>>> From: Mahesh Bandewar <maheshb@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.
>>>
>>>          Michal, can you test this?  I'm travelling this week and won't
>>> be able to run the patch.
> 
> 
> Yes, will test today and report.
> 

The patch looks to be working fine - tested both passive bridge
(stp_state == 0) and with in-kernel implementation active (stp_state ==
1). No loops, no issues so far.

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Stephen Hemminger]

On Sun, 15 Jul 2018 18:12:46 -0700
Mahesh Bandewar <mahesh@bandewar.net> wrote:

> From: Mahesh Bandewar <maheshb@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.
> 
> Reported-by: Michal Soltys <soltys@ziu.info>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>

Thanks for fixing this.

Why not add a Fixes: tag instead of just talking about the commit?
That helps the stable maintainers know which versions of the kernel
need the patch.

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Mahesh Bandewar (महेश बंडेवार)]

On Mon, Jul 16, 2018 at 4:33 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Sun, 15 Jul 2018 18:12:46 -0700
> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
>> From: Mahesh Bandewar <maheshb@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.
>>
>> Reported-by: Michal Soltys <soltys@ziu.info>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>
> Thanks for fixing this.
>
> Why not add a Fixes: tag instead of just talking about the commit?
> That helps the stable maintainers know which versions of the kernel
> need the patch.
Well, I thought about it. It's definitely 'related' but not sure it
'fixes' in true sense. It definitely fixes the broken legacy case
though. Is that sufficient to add 'fixes' tag?

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Stephen Hemminger]

On Mon, 16 Jul 2018 16:57:22 -0700
Mahesh Bandewar (महेश बंडेवार) <maheshb@google.com> wrote:

> On Mon, Jul 16, 2018 at 4:33 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > On Sun, 15 Jul 2018 18:12:46 -0700
> > Mahesh Bandewar <mahesh@bandewar.net> wrote:
> >  
> >> From: Mahesh Bandewar <maheshb@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.
> >>
> >> Reported-by: Michal Soltys <soltys@ziu.info>
> >> Signed-off-by: Mahesh Bandewar <maheshb@google.com>  
> >
> > Thanks for fixing this.
> >
> > Why not add a Fixes: tag instead of just talking about the commit?
> > That helps the stable maintainers know which versions of the kernel
> > need the patch.  
> Well, I thought about it. It's definitely 'related' but not sure it
> 'fixes' in true sense. It definitely fixes the broken legacy case
> though. Is that sufficient to add 'fixes' tag?

The previous commit caused a regression. your change fixes the regression

Re: [PATCH next] bonding: pass link-local packets to bonding master also.

[Michal Soltys]

On 07/17/2018 01:57 AM, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Mon, Jul 16, 2018 at 4:33 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>> On Sun, 15 Jul 2018 18:12:46 -0700
>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>
>>> From: Mahesh Bandewar <maheshb@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.
>>>
>>> Reported-by: Michal Soltys <soltys@ziu.info>
>>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>
>> Thanks for fixing this.
>>
>> Why not add a Fixes: tag instead of just talking about the commit?
>> That helps the stable maintainers know which versions of the kernel
>> need the patch.

> Well, I thought about it. It's definitely 'related' but not sure it
> 'fixes' in true sense. It definitely fixes the broken legacy case
> though. Is that sufficient to add 'fixes' tag?
> 

It's __not__ broken legacy case. It's normal behavior, starting with 
specification covering LLDP itself (IEEE Std 802.1AB-2016, page 18, '6.8 LLDP 
and Link Aggregation') and ending with a linux bridge actively doing stp via 
in-kernel implementation or with userspace helper (or inactively passing) and 
being blind to bpdus. Not mentioning a very recent kernel feature like 
per-port group_fwd_mask rendered useless in this case.

Unless you also consider attaching a bond to a linux bridge as a broken legacy 
use case. Among other things mentioned in the other thread.

In this context, the comment in code/log message IMHO (of the attached patch), 
should be changed - as it will be just confusing for anyone reading it in the 
future.

(and I'd very much like the fix to hit relevant stable kernels as well)

Re: [next] bonding: pass link-local packets to bonding master also.

[Vincent Bernat]

 ❦ 15 juillet 2018 19:12 -0700, Mahesh Bandewar <mahesh@bandewar.net>:

> 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 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.
-- 
AWAKE! FEAR! FIRE! FOES! AWAKE!
	FEAR! FIRE! FOES!
		AWAKE! AWAKE!
		-- J. R. R. Tolkien

Re: [next] bonding: pass link-local packets to bonding master also.

[Vincent Bernat]

 ❦ 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"

Re: [next] bonding: pass link-local packets to bonding master also.

[Michal Soltys]

On 18/11/30 22:32, Vincent Bernat wrote:
>  ❦ 15 juillet 2018 19:12 -0700, Mahesh Bandewar <mahesh@bandewar.net>:
> 
>> 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 am a bit lost of what the original patch was trying to achieve. I am
> using the following test program:
> 

Basing on what you wrote below, it seems that everything was already
possible before the Chonggang Li's commit ?

If I understand correctly:

- if listening on master, you could get both slave interface
(PACKET_ORIGDEV) as well as bonding master
- if listening on slave, you would just get the slave interface

Right ?

> <snip>
> #!/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.
> 

Re: [next] bonding: pass link-local packets to bonding master also.

[Vincent Bernat]

 ❦ 19 décembre 2018 23:57 +01, Michal Soltys <soltys@ziu.info>:

>>> 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 am a bit lost of what the original patch was trying to achieve. I am
>> using the following test program:
>> 
>
> Basing on what you wrote below, it seems that everything was already
> possible before the Chonggang Li's commit ?
>
> If I understand correctly:
>
> - if listening on master, you could get both slave interface
> (PACKET_ORIGDEV) as well as bonding master
> - if listening on slave, you would just get the slave interface
>
> Right ?

Yes. This has been this way since 2.6.27.
-- 
Test programs at their boundary values.
            - The Elements of Programming Style (Kernighan & Plauger)

Re: [next] bonding: pass link-local packets to bonding master also.

[Michal Soltys]

On 18/12/20 06:55, Vincent Bernat wrote:
>  ❦ 19 décembre 2018 23:57 +01, Michal Soltys <soltys@ziu.info>:
> 
>>>> 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 am a bit lost of what the original patch was trying to achieve. I am
>>> using the following test program:
>>> 
>>
>> Basing on what you wrote below, it seems that everything was already
>> possible before the Chonggang Li's commit ?
>>
>> If I understand correctly:
>>
>> - if listening on master, you could get both slave interface
>> (PACKET_ORIGDEV) as well as bonding master
>> - if listening on slave, you would just get the slave interface
>>
>> Right ?
> 
> Yes. This has been this way since 2.6.27.
> 

In the earlier thread, the author of the original patch
said that:

> The exact problem that was fixed is, when a LLDP/CDP packet is received, we want to know which exact interface did it come from, instead of the bonding master.
> 
> Thanks,
> Chonggang

But if PACKET_ORIGDEV provided it (and this functionally is now broken,
so counts as another regression - earlier bridging issues aside), maybe
it should be just reverted ?

Re: [next] bonding: pass link-local packets to bonding master also.

[Vincent Bernat]

 ❦ 23 décembre 2018 14:06 +01, Michal Soltys <soltys@ziu.info>:

>>> Basing on what you wrote below, it seems that everything was already
>>> possible before the Chonggang Li's commit ?
>>>
>>> If I understand correctly:
>>>
>>> - if listening on master, you could get both slave interface
>>> (PACKET_ORIGDEV) as well as bonding master
>>> - if listening on slave, you would just get the slave interface
>>>
>>> Right ?
>> 
>> Yes. This has been this way since 2.6.27.
>
> In the earlier thread, the author of the original patch
> said that:
>
>> The exact problem that was fixed is, when a LLDP/CDP packet is
>> received, we want to know which exact interface did it come from,
>> instead of the bonding master.
>> 
>> Thanks,
>> Chonggang
>
> But if PACKET_ORIGDEV provided it (and this functionally is now broken,
> so counts as another regression - earlier bridging issues aside), maybe
> it should be just reverted ?

I think this would be better. Once fixed, I can push a change for
packet(7) to document this option as it is currently not present.
-- 
The last thing one knows in constructing a work is what to put first.
		-- Blaise Pascal