Message-ID: <CAF2d9jhmaZZN7U1JcECr6eYNwMuusTwOqk04QN=Gvr9suqpuNw@mail.gmail.com>
Date:   Mon, 14 Jan 2019 18:19:57 -0800
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Vincent Bernat <vincent@...nat.ch>
Cc:     Maciej Żenczykowski <zenczykowski@...il.com>,
        David Miller <davem@...emloft.net>, soltys@....info,
        Linux NetDev <netdev@...r.kernel.org>,
        Jay Vosburgh <jay.vosburgh@...onical.com>,
        Chonggang Li <chonggangli@...gle.com>
Subject: Re: [PATCH net 1/1] bonding: fix PACKET_ORIGDEV regression on bonding masters

On Mon, Jan 14, 2019 at 12:00 AM Vincent Bernat <vincent@...nat.ch> wrote:
>
>  ❦ 13 janvier 2019 18:01 -08, Maciej Żenczykowski <zenczykowski@...il.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?
>
thanks for jumping in and offering a solution. This should fix the issue.

NACK for the revert-patch!

Folks, please, revert is not the solution! Last time when there was a
problem posted I offered you a solution, so wasn't that enough to
prove that we care about solving the problem that you are facing while
continuing to have this functionality? No one wants to break your use
case, it happens only because one is not aware of it. Thank you David
for resorting to resolve it.


> 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)

Powered by blists - more mailing lists