All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Bernat <vincent@bernat.ch>
To: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	soltys@ziu.info, Linux NetDev <netdev@vger.kernel.org>,
	Jay Vosburgh <jay.vosburgh@canonical.com>,
	Mahesh Bandewar <maheshb@google.com>,
	Chonggang Li <chonggangli@google.com>
Subject: Re: [PATCH net 1/1] bonding: fix PACKET_ORIGDEV regression on bonding masters
Date: Mon, 14 Jan 2019 08:00:24 +0000	[thread overview]
Message-ID: <87imyr3bzb.fsf@bernat.ch> (raw)
In-Reply-To: <CAHo-Ooya1gz-a=3bdZNDc3pDcETU+V7MGvdXq+kA0ALADrLn7g@mail.gmail.com> ("Maciej Żenczykowski"'s message of "Sun, 13 Jan 2019 18:01:39 -0800")

 ❦ 13 janvier 2019 18:01 -08, Maciej Żenczykowski <zenczykowski@gmail.com>:

> But I seem to recall that the core problem we were trying to solve was
> that a daemon listening
> on an AF_PACKET ethertype 88CC [LLDP] socket not bound to any device
> would not receive LLDP packets
> arriving on inactive bond slaves (either active-backup or lag).

Just tested and with 4.9.150, I am in fact unable to receive anything
on a backup link when listening to the active-backup master device or to
"any" device.

> Perhaps going from:
>   /* don't change skb->dev for link-local packets */
>   if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) return RX_HANDLER_PASS;
>   if (bond_should_deliver_exact_match(skb, slave, bond)) return
> RX_HANDLER_EXACT;
>
> to something more like:
>   if (bond_should_deliver_exact_match(skb, slave, bond)) {
>     /* don't change skb->dev for link-local packets on inactive slaves */
>     if (is_link_local_ether_addr(eth_hdr(skb)->h_dest)) return RX_HANDLER_PASS;
>     return RX_HANDLER_EXACT;
>   }
>
> would fix both problems?

It makes PACKET_ORIGDEV works again. Moreover, when not binding to any
interface, we receive packets on both active and backup links. But when
binding to the master device, I only receive packets from the active
devices (which is the same behaviour than pre-4.12). When not binding to
any device and not using PACKET_ORIGDEV, one packet is said to be from
the master device and one packet is said to be from the backup device.
Previously, I had one packet from active device, one packet from backup
device and two packets from master device.

For me, this is a better situation than previously as we return to the
situation before 4.12 but you can get what you want by not binding to
any device _and_ using PACKET_ORIGDEV (otherwise, you are don't get the
right interface in all cases).

If it's unclear, I can provide more extensive results. I am using this
test program (comment the s.bind/s.setsockopt line if needed):

#v+
#!/usr/bin/env python3

import sys
import socket
import datetime

socket.SOL_PACKET = 263
socket.PACKET_ORIGDEV = 9

s = socket.socket(socket.AF_PACKET,
                  socket.SOCK_RAW,
                  socket.htons(0x88cc))
s.bind(("bond0", 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-
-- 
Localise input and output in subroutines.
            - The Elements of Programming Style (Kernighan & Plauger)

  reply	other threads:[~2019-01-14  8:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-07 16:29 [PATCH net 0/1] bonding: fix PACKET_ORIGDEV regression Michal Soltys
2019-01-07 16:29 ` [PATCH net 1/1] bonding: fix PACKET_ORIGDEV regression on bonding masters Michal Soltys
2019-01-07 17:12   ` David Miller
2019-01-08 13:46     ` Vincent Bernat
2019-01-13 23:03   ` David Miller
2019-01-14  2:01     ` Maciej Żenczykowski
2019-01-14  8:00       ` Vincent Bernat [this message]
2019-01-15  2:19         ` Mahesh Bandewar (महेश बंडेवार)
2019-01-16  2:58           ` Michal Soltys
2019-01-16  2:01       ` Michal Soltys
2019-01-18  0:27       ` Michal Soltys
2019-01-18  6:58         ` Maciej Żenczykowski
2019-01-29  1:47           ` Michal Soltys
2019-01-29  9:39             ` Maciej Żenczykowski
2019-02-18 16:55               ` [PATCH v2] bonding: fix PACKET_ORIGDEV regression Michal Soltys
2019-02-19  1:51                 ` David Ahern
2019-02-19  9:14                 ` Maciej Żenczykowski
2019-02-21 21:21                 ` David Miller

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=87imyr3bzb.fsf@bernat.ch \
    --to=vincent@bernat.ch \
    --cc=chonggangli@google.com \
    --cc=davem@davemloft.net \
    --cc=jay.vosburgh@canonical.com \
    --cc=maheshb@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=soltys@ziu.info \
    --cc=zenczykowski@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.