netdev - Re: VLAN "issue" on STMMAC

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]

Message-ID: <2308719d-1994-4cd4-a6b1-8ee3ec291caf@gmail.com>
Date: Fri, 16 Feb 2024 09:23:03 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Christophe ROULLIER <christophe.roullier@...s.st.com>,
 Jakub Kicinski <kuba@...nel.org>, kim.tatt.chuah@...el.com,
 "Ong, Boon Leong" <boon.leong.ong@...el.com>, vee.khee.wong@...el.com
Cc: davem@...emloft.net, Alexandre Torgue <alexandre.torgue@...s.st.com>,
 netdev@...r.kernel.org
Subject: Re: VLAN "issue" on STMMAC

Adding original authors,

On 2/16/24 06:35, Christophe ROULLIER wrote:
> Hello,
> 
> I've a question concerning following commit:
> 
> ed64639bc1e0899d89120b82af52e74fcbeebf6a :
> 
> net: stmmac: Add support for VLAN Rx filtering
> 
> Add support for VLAN ID-based filtering by the MAC controller for MAC
> 
> drivers that support it. Only the 12-bit VID field is used.
> 
> Signed-off-by: Chuah Kim Tatt kim.tatt.chuah@...el.com
> Signed-off-by: Ong Boon Leong boon.leong.ong@...el.com
> Signed-off-by: Wong Vee Khee vee.khee.wong@...el.com
> Signed-off-by: David S. Miller davem@...emloft.net
> 
> So now with this commit is no more possible to create some VLAN than 
> previously (it depends of number of HW Tx queue) (one VLAN max)
> 
> root@...32mp1:~# ip link add link end0 name end0.200 type vlan id 200
> [   61.207767] 8021q: 802.1Q VLAN Support v1.8
> [   61.210629] 8021q: adding VLAN 0 to HW filter on device end0
> [   61.230515] stm32-dwmac 5800a000.ethernet end0: Adding VLAN ID 0 is 
> not supported

OK, so here the VLAN module was not yet loaded, so as part of the 
operation, we get the module, load it, which triggers the 802.1q driver 
to install a VLAN ID filter for VLAN ID #0, that fails because there is 
a single VLAN supported, and this apparently means VLAN promiscuous... 
not sure why that is an error, if this means accepting all VLANs, then 
this means we need to filter in software, so it really should not be an 
error IMHO.

> root@...32mp1:~# ip link add link end0 name end0.300 type vlan id 300
> [   71.403195] stm32-dwmac 5800a000.ethernet end0: Only single VLAN ID 
> supported
> RTNETLINK answers: Operation not permitted
> root@...32mp1:~#
> 
> I've tried to deactivate VLAN filtering with ethtool, but not possible 
> ("fixed" value)
> 
> root@...32mp1:~# ethtool -k end0 | grep -i vlan
> rx-vlan-offload: on [fixed]
> tx-vlan-offload: on [fixed]
> rx-vlan-filter: on [fixed]
> vlan-challenged: off [fixed]
> tx-vlan-stag-hw-insert: on [fixed]
> rx-vlan-stag-hw-parse: on [fixed]
> rx-vlan-stag-filter: on [fixed]
> root@...32mp1:~#
> root@...32mp1:~# ethtool -K end0 rxvlan off
> Actual changes:
> rx-vlan-hw-parse: on [requested off]
> Could not change any device features
> 
> Do you know if there are possibility to force creation of VLAN ID (may 
> be in full SW ?) and keep the rest of Ethernet Frame processing to GMAC HW.

It is not clear to me how -EPERM ended up being chosen as a return code 
here rather than -EOPNOTSUPP which would be allow for the upper layers 
to decide how to play through, not that it would matter much here,
because we sort of expect the operation not to fail.

Can you confirm that dwmac4_get_num_vlan() does return 1 single VLAN 
filter? Also, given the comment about single VLAN and VID #0 meaning 
VLAN promiscuous does this work:

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 6b6d0de09619..e2134a5e6d7e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -492,10 +492,8 @@ static int dwmac4_add_hw_vlan_rx_fltr(struct 
net_device *dev,
         /* Single Rx VLAN Filter */
         if (hw->num_vlan == 1) {
                 /* For single VLAN filter, VID 0 means VLAN promiscuous */
-               if (vid == 0) {
-                       netdev_warn(dev, "Adding VLAN ID 0 is not 
supported\n");
-                       return -EPERM;
-               }
+               if (vid == 0)
+                       return 0;

                 if (hw->vlan_filter[0] & GMAC_VLAN_TAG_VID) {
                         netdev_err(dev, "Only single VLAN ID supported\n");

-- 
Florian