On Wed, Nov 11, 2020 at 07:43:41AM -0800, Jakub Kicinski wrote:
> On Tue, 10 Nov 2020 16:39:58 +0100 Markus Blöchl wrote:
> > The rx-vlan-filter feature flag prevents unexpected tagged frames on
> > the wire from reaching the kernel in promiscuous mode.
> > Disable this offloading feature in the lan7800 controller whenever
> > IFF_PROMISC is set and make sure that the hardware features
> > are updated when IFF_PROMISC changes.
> >
> > Signed-off-by: Markus Blöchl <markus.bloe...@ipetronik.com>
> > ---
> >
> > Notes:
> > When sniffing ethernet using a LAN7800 ethernet controller, vlan-tagged
> > frames are silently dropped by the controller due to the
> > RFE_CTL_VLAN_FILTER flag being set by default since commit
> > 4a27327b156e("net: lan78xx: Add support for VLAN filtering.").
> >
> > In order to receive those tagged frames it is necessary to manually
> > disable
> > rx vlan filtering using ethtool ( `ethtool -K ethX rx-vlan-filter off`
> > or
> > corresponding ioctls ). Setting all bits in the vlan filter table to 1
> > is
> > an even worse approach, imho.
> >
> > As a user I would probably expect that setting IFF_PROMISC should be
> > enough
> > in all cases to receive all valid traffic.
> > Therefore I think this behaviour is a bug in the driver, since other
> > drivers (notably ixgbe) automatically disable rx-vlan-filter when
> > IFF_PROMISC is set. Please correct me if I am wrong here.
>
> I've been mulling over this, I'm not 100% sure that disabling VLAN
> filters on promisc is indeed the right thing to do. The ixgbe doing
> this is somewhat convincing. OTOH users would not expect flow filters
> to get disabled when promisc is on, so why disable vlan filters?
>
> Either way we should either document this as an expected behavior or
> make the core clear the features automatically rather than force
> drivers to worry about it.
>
> Does anyone else have an opinion, please?
I agree it is weird, but it seems to be justified by the bridge code.
Note that the code was added around 2013, so it is possible it was
influenced by the behavior of existing drivers.
1. vi net/bridge/br_vlan.c +245
/* Add VLAN to the device filter if it is supported.
* This ensures tagged traffic enters the bridge when
* promiscuous mode is disabled by br_manage_promisc().
*/
2. The bridge device itself will not filter packets based on their VID
when it is in promiscuous mode:
vi net/bridge/br_input.c +45
vg = br_vlan_group_rcu(br);
/* Bridge is just like any other port. Make sure the
* packet is allowed except in promisc modue when someone
* may be running packet capture.
*/
if (!(brdev->flags & IFF_PROMISC) &&
!br_allowed_egress(vg, skb)) {
kfree_skb(skb);
return NET_RX_DROP;
}
