Date:   Sat, 14 Nov 2020 20:11:03 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Markus Blöchl <Markus.Bloechl@...tronik.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Ido Schimmel <idosch@...sch.org>, Andrew Lunn <andrew@...n.ch>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net] net: lan78xx: Disable hardware vlan filtering in
 promiscuous mode

On Thu, Nov 12, 2020 at 11:53:15AM +0100, Markus Blöchl wrote:
> From what I can see, most other drivers use a special hardware register
> flag to enable promiscuous mode, which overrules all other filters.

Yes, but it may not mean what you think.

> e.g. from the ASIX AX88178 datasheet:
>
>   PRO:  PACKET_TYPE_PROMISCUOUS.
>     1: All frames received by the ASIC are forwarded up toward the host.
>     0: Disabled (default).

See, that's one definition of promiscuity that is really strange, and
not backed by any standards body that I know of (if you know otherwise,
please speak up).

> It is just so that the lan78xx controllers don't have this explicit flag.

Which is not surprising, at least under that description. Others may be
inclined to call that behavior "packet trapping" when applying it to
e.g. an Ethernet switch.

There might be an entire discussion about how promiscuity does _not_
mean "deliver all packets to the CPU" that might be of interest to you:
https://lkml.org/lkml/2019/8/29/255

> But since my change is more controversial than I anticipated, I would like
> to take a step back and ask some general questions first:
>
> We used to connect something akin to a port mirror to a lan7800 NIC
> (and a few others) in order to record and debug some VLAN heavy traffic.
> On older kernel versions putting the interface into promiscuous mode
> and opening and binding an AF_PACKET socket (or just throwing tcpdump
> or libpcap at it) was sufficient.
> After a kernel upgrade the same setup missed most of the traffic.
> Does this qualify as a regression? Why not?

If something used to work but no longer does, that's what a regression
is. But fixing it depends on whether it should have worked like that in
the first place or not. That we don't know.

> Should there be a documented and future proof way to receive *all*
> valid ethernet frames from an interface?

Yes, there should.

> This could be something like:
>
> a) - Bring up the interface
>    - Put the interface into promiscuous mode

This one would be necessary in order to not drop packets with unknown
addresses, not more.

>    - Open, bind and read a raw AF_PACKET socket with ETH_P_ALL
>    - Patch up the 801.1Q headers if required.

What do you mean by "patching up"? Are you talking about the fact that
packets are untagged by the stack in the receive path anyway, and the
VLAN header would need to be reconstructed?
https://patchwork.ozlabs.org/project/netdev/patch/e06dbb47-2d1c-03ca-4cd7-cc958b6a939e@gmail.com/

>
> b) - The same as a)
>    - Additionally enumerate and disable all available offloading features

If said offloading features have to do with the CPU not receiving some
frames any longer, and you _do_ want the CPU to see them, then obviously
said offloading features should be disabled. This includes not only VLAN
filtering, but also bridging offload, IP forwarding offload, etc.

I'd say that (b) should be sufficient, but not future-proof in the sense
that new offloading features might come every day, and they would need
to be disabled on a case-by-case basis.

For the forwarding offload, there would be no question whatsoever that
you'd need to turn it off, or add a mirroring rule towards the CPU, or
do _something_ user-visible, to get that traffic. But as for VLAN
filtering offload in particular, there's the (not negligible at all)
precedent created by the bridge driver, that Ido pointed out. That would
be a decision for the maintainers to make, if the Linux network stack
creates its own definition of promiscuity which openly contradicts IEEE's.
One might perhaps try to argue that the VLAN ID is an integral part of
the station's address (which is true if you look at it from the
perspective of an IEEE 802.1Q bridge), and therefore promiscuity should
apply on the VLAN ID too, not just the MAC address. Either way, that
should be made more clear than it currently is, once a decision is
taken.

> c) - Use libpcap / Do whatever libpcap does (like with TPACKET)
>    In this case you need to help me convince the tcpdump folks that this
>    is a bug on their side... ;-)

Well, that assumes that today, tcpdump can always capture all traffic on
all types of interfaces, something which is already false (see bridging
offload / IP forwarding offload). There, it was even argued that you'd
better be 100% sure that you want to see all received traffic, as the
interfaces can be very high-speed, and not even a mirroring rule might
guarantee reception of 100% of the traffic. That's why things like sFlow
/ tc-sample exist.

> d) - Read the controller datasheet
>    - Read the kernel documentation
>    - Read your kernels and drivers sources
>    - Do whatever might be necessary

Yes, in general reading is helpful, but I'm not quite sure where you're
going with this?

> e) - No, there is no guaranteed way to to this

No, there should be a way to achieve that through some sort of
configuration.

> Any opinions on these questions?

My 2 cents have just been shared.

> After those are answered, I am open to suggestions on how to fix this
> differently (if still needed).

Turn off VLAN filtering, or get a commonly accepted definition of
promiscuity?

Powered by blists - more mailing lists