VLAN "issue" on STMMAC

VLAN "issue" on STMMAC

[Christophe ROULLIER]

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@intel.com
Signed-off-by: Ong Boon Leong boon.leong.ong@intel.com
Signed-off-by: Wong Vee Khee vee.khee.wong@intel.com
Signed-off-by: David S. Miller davem@davemloft.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@stm32mp1:~# 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
root@stm32mp1:~# 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@stm32mp1:~#

I've tried to deactivate VLAN filtering with ethtool, but not possible 
("fixed" value)

root@stm32mp1:~# 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@stm32mp1:~#
root@stm32mp1:~# 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.

Thanks for your help/feedback

Regards,

Christophe

Re: VLAN "issue" on STMMAC

[Florian Fainelli]

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@intel.com
> Signed-off-by: Ong Boon Leong boon.leong.ong@intel.com
> Signed-off-by: Wong Vee Khee vee.khee.wong@intel.com
> Signed-off-by: David S. Miller davem@davemloft.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@stm32mp1:~# 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@stm32mp1:~# 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@stm32mp1:~#
> 
> I've tried to deactivate VLAN filtering with ethtool, but not possible 
> ("fixed" value)
> 
> root@stm32mp1:~# 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@stm32mp1:~#
> root@stm32mp1:~# 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

Re: VLAN "issue" on STMMAC

[Christophe ROULLIER]

On 2/16/24 18:23, Florian Fainelli wrote:
> Adding original authors,
Good, you are right
>
> 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@intel.com
>> Signed-off-by: Ong Boon Leong boon.leong.ong@intel.com
>> Signed-off-by: Wong Vee Khee vee.khee.wong@intel.com
>> Signed-off-by: David S. Miller davem@davemloft.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@stm32mp1:~# 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.

Module VLAN (Config VLAN_8021Q) is well loaded

root@stm32mp1:~# lsmod
Module                  Size  Used by
8021q                  28672  0
garp                   16384  1 8021q
mrp                    20480  1 8021q
...

>
>> root@stm32mp1:~# 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@stm32mp1:~#
>>
>> I've tried to deactivate VLAN filtering with ethtool, but not 
>> possible ("fixed" value)
>>
>> root@stm32mp1:~# 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@stm32mp1:~#
>> root@stm32mp1:~# 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");
>
I confirm that dwmac4_get_num_vlan() return 1.

I've tested your patch in dwmac4_add_hw_vlan_rx_fltr, for sure no more 
warning but same issue :-(

root@stm32mp1:~# ip link add link end0 name end0.200 type vlan id 200
[   73.536876] 8021q: 802.1Q VLAN Support v1.8
[   73.539853] 8021q: adding VLAN 0 to HW filter on device end0

root@stm32mp1:~# ip link add link end0 name end0.300 type vlan id 300
[  121.560317] stm32-dwmac 5800a000.ethernet end0: Only single VLAN ID 
supported
RTNETLINK answers: Operation not permitted

root@stm32mp1:~# 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]

I've also try to remove second "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");
-            return -EPERM;
+//            return -EPERM;
          }

And now it ok to create some VLANs, but it is clearly not clean.

Is it possible/consistent to have a DT property to bypass HW VLAN 
capabilies (hw->num_vlan and hw->vlan_filter ..) and manage VLAN in 
software ?

Thanks for your feedback