netdev - Re: [PATCH net] net: lan78xx: Disable hardware vlan filtering in promiscuous mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Nov 2020 14:52:30 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Markus Blöchl <markus.bloechl@...tronik.com>,
        Ido Schimmel <idosch@...sch.org>, Andrew Lunn <andrew@...n.ch>,
        Vladimir Oltean <olteanv@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] net: lan78xx: Disable hardware vlan filtering in
 promiscuous mode

On Wed, Nov 11, 2020 at 7:43 AM Jakub Kicinski <kuba@...nel.org> 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.bloechl@...tronik.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?

My personal preference is for the setting of all 4096 VLANs when
promiscuous mode is enabled which is what we do with ixgbe if
virtualization is enabled. If we want to provide an option to disable
Rx VLAN filtering then just do it via the ethtool feature bit. With
that things would be at least consistent as I suspect disabling Rx
VLAN filtering may also impact Rx VLAN tag stripping so both might
need to be updated at the same time. I know in the case of
virtualization most NICs have to leave the VLAN filtering enabled for
SR-IOV to be able to handle per VF VLAN filters regardless of the PF
setting.

Anyway that is my $.02.

- Alex