[PATCH net-next] net: stmmac: propagate feature flags to vlan

[PATCH net-next] net: stmmac: propagate feature flags to vlan

[Corinna Vinschen]

stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
like TX offloading don't correspond with the general features and it's
not possible to manipulate features via ethtool -K to affect VLANs.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e590b6fc4761..308d4ee12d41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7216,6 +7216,8 @@ int stmmac_dvr_probe(struct device *device,
 	if (priv->dma_cap.rssen && priv->plat->rss_en)
 		ndev->features |= NETIF_F_RXHASH;
 
+	ndev->vlan_features |= ndev->features;
+
 	/* MTU range: 46 - hw-specific max */
 	ndev->min_mtu = ETH_ZLEN - ETH_HLEN;
 	if (priv->plat->has_xgmac)
-- 
2.39.1

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Simon Horman]

On Tue, Apr 11, 2023 at 03:00:28PM +0200, Corinna Vinschen wrote:
> stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> like TX offloading don't correspond with the general features and it's
> not possible to manipulate features via ethtool -K to affect VLANs.
> 
> Signed-off-by: Corinna Vinschen <vinschen@redhat.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Jacob Keller]

On 4/11/2023 6:00 AM, Corinna Vinschen wrote:
> stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> like TX offloading don't correspond with the general features and it's
> not possible to manipulate features via ethtool -K to affect VLANs.
> 
> Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index e590b6fc4761..308d4ee12d41 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7216,6 +7216,8 @@ int stmmac_dvr_probe(struct device *device,
>  	if (priv->dma_cap.rssen && priv->plat->rss_en)
>  		ndev->features |= NETIF_F_RXHASH;
>  
> +	ndev->vlan_features |= ndev->features;
> +
>  	/* MTU range: 46 - hw-specific max */
>  	ndev->min_mtu = ETH_ZLEN - ETH_HLEN;
>  	if (priv->plat->has_xgmac)

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@gmail.com>

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Corinna Vinschen]

On Apr 11 15:00, Corinna Vinschen wrote:
> stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> like TX offloading don't correspond with the general features and it's
> not possible to manipulate features via ethtool -K to affect VLANs.

On second thought, I wonder if this shouldn't go into net, rather then
net-next?  Does that make sense? And, do I have to re-submit, if so?


Thanks,
Corinna

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Jakub Kicinski]

On Wed, 12 Apr 2023 15:11:44 +0200 Corinna Vinschen wrote:
> On Apr 11 15:00, Corinna Vinschen wrote:
> > stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> > like TX offloading don't correspond with the general features and it's
> > not possible to manipulate features via ethtool -K to affect VLANs.  
> 
> On second thought, I wonder if this shouldn't go into net, rather then
> net-next?  Does that make sense? And, do I have to re-submit, if so?

If it's not a regression it should go into net-next.
"never worked, doesn't crash" category.

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Corinna Vinschen]

On Apr 12 07:45, Jakub Kicinski wrote:
> On Wed, 12 Apr 2023 15:11:44 +0200 Corinna Vinschen wrote:
> > On Apr 11 15:00, Corinna Vinschen wrote:
> > > stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> > > like TX offloading don't correspond with the general features and it's
> > > not possible to manipulate features via ethtool -K to affect VLANs.  
> > 
> > On second thought, I wonder if this shouldn't go into net, rather then
> > net-next?  Does that make sense? And, do I have to re-submit, if so?
> 
> If it's not a regression it should go into net-next.
> "never worked, doesn't crash" category.

Ok, great, thanks!


Corinna

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Jakub Kicinski]

On Tue, 11 Apr 2023 15:00:28 +0200 Corinna Vinschen wrote:
> stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> like TX offloading don't correspond with the general features and it's
> not possible to manipulate features via ethtool -K to affect VLANs.

Actually, could you add to the commit message a sentence or two about
the features that you tested and/or a mention of the manual indicating
that they are supported over vlans? Especially TSO on a quick look.
I just realized now that you didn't explicitly say that those features
work, just that you can manipulate them...

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Corinna Vinschen]

Hi Jakub,

On Apr 12 21:15, Jakub Kicinski wrote:
> On Tue, 11 Apr 2023 15:00:28 +0200 Corinna Vinschen wrote:
> > stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> > like TX offloading don't correspond with the general features and it's
> > not possible to manipulate features via ethtool -K to affect VLANs.
> 
> Actually, could you add to the commit message a sentence or two about
> the features that you tested and/or a mention of the manual indicating
> that they are supported over vlans? Especially TSO on a quick look.
> I just realized now that you didn't explicitly say that those features
> work, just that you can manipulate them...

I tested that I can, for instance, set and reset the tx-checksumming
flag with ethtool -K.  As for TSO, I checked the source code, and the
function stmmac_tso_xmit handles VLANs just fine.  While different
NICs supported by stmmac have different offload features, there's no
indication in the driver source that VLANs have less offloading features
than a non-VLAN connection on the same HW.  Admittedly, I never saw
documentation explicitely claiming this.

If that's not sufficient, testing will take another day or two, because
I have to ask for a remote test setup first.


Corinna

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Jakub Kicinski]

On Thu, 13 Apr 2023 17:25:56 +0200 Corinna Vinschen wrote:
> I tested that I can, for instance, set and reset the tx-checksumming
> flag with ethtool -K.  As for TSO, I checked the source code, and the
> function stmmac_tso_xmit handles VLANs just fine.  While different
> NICs supported by stmmac have different offload features, there's no
> indication in the driver source that VLANs have less offloading features
> than a non-VLAN connection on the same HW.  Admittedly, I never saw
> documentation explicitely claiming this.
> 
> If that's not sufficient, testing will take another day or two, because
> I have to ask for a remote test setup first.

Testing would be great, I think it's worth waiting for that.

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Corinna Vinschen]

Hi Jakub,

On Apr 13 09:00, Jakub Kicinski wrote:
> On Thu, 13 Apr 2023 17:25:56 +0200 Corinna Vinschen wrote:
> > I tested that I can, for instance, set and reset the tx-checksumming
> > flag with ethtool -K.  As for TSO, I checked the source code, and the
> > function stmmac_tso_xmit handles VLANs just fine.  While different
> > NICs supported by stmmac have different offload features, there's no
> > indication in the driver source that VLANs have less offloading features
> > than a non-VLAN connection on the same HW.  Admittedly, I never saw
> > documentation explicitely claiming this.
> > 
> > If that's not sufficient, testing will take another day or two, because
> > I have to ask for a remote test setup first.
> 
> Testing would be great, I think it's worth waiting for that.

Yes, that was a good idea.  Turns out, TSO doesn't really work well
with VLANs.  The speed is... suboptimal.  Here are some results
with iperf, showing only the summary lines.

Base interface with TSO off:

$ ethtool -K enp0s29f1 tso off
$ iperf3 -c 192.168.1.2
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  1015 MBytes   852 Mbits/sec    0             sender
[  5]   0.00-10.04  sec  1012 MBytes   845 Mbits/sec                  receiver

Base interface with TSO on:

$ ethtool -K enp0s29f1 tso on
$ iperf3 -c 192.168.1.2
[...]
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  1.08 GBytes   928 Mbits/sec    0             sender
[  5]   0.00-10.04  sec  1.08 GBytes   921 Mbits/sec                  receiver

VLAN interface with TSO off:

$ ethtool -K enp0s29f1 tso off
$ iperf3 -c 192.168.3.2
[...]
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec   975 MBytes   818 Mbits/sec    0             sender
[  5]   0.00-10.04  sec   973 MBytes   813 Mbits/sec                  receiver

VLAN interface with TSO on:

$ ethtool -K enp0s29f1 tso on
$ iperf3 -c 192.168.3.2
[...]
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  16.0 MBytes  13.4 Mbits/sec  899             sender
[  5]   0.00-10.04  sec  15.9 MBytes  13.3 Mbits/sec                  receiver

Oops.

I'll send a v2 patch which disables TSO on VLANs for the time being.


Corinna

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Jakub Kicinski]

On Mon, 17 Apr 2023 21:03:12 +0200 Corinna Vinschen wrote:
> Yes, that was a good idea.  Turns out, TSO doesn't really work well
> with VLANs.  The speed is... suboptimal.  Here are some results
> with iperf, showing only the summary lines.

Hah, good to find out before users did :)

IIRC there is something in the TCP stack which retries sending 
as non-TSO if TSO keeps failing so "TSO is very slow" may in fact 
mean TSO is completely broken but TCP is managing to deliver a little
bit with just retransmissions or super tiny cwnd or some such.

[PATCH v2 net-next] net: stmmac: propagate feature flags to vlan

[Corinna Vinschen]

stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
like offloading don't correspond with the general features and it's not
possible to manipulate features via ethtool -K to affect VLANs.

Propagate feature flags to vlan features.  Drop TSO feature because
it does not work on VLANs yet.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d7fcab057032..8ab67c020a08 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7253,6 +7253,10 @@ int stmmac_dvr_probe(struct device *device,
 	if (priv->dma_cap.rssen && priv->plat->rss_en)
 		ndev->features |= NETIF_F_RXHASH;
 
+	ndev->vlan_features |= ndev->features;
+	/* TSO doesn't work on VLANs yet */
+	ndev->vlan_features &= ~NETIF_F_TSO;
+
 	/* MTU range: 46 - hw-specific max */
 	ndev->min_mtu = ETH_ZLEN - ETH_HLEN;
 	if (priv->plat->has_xgmac)
-- 
2.31.1

Re: [PATCH v2 net-next] net: stmmac: propagate feature flags to vlan

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 17 Apr 2023 21:28:45 +0200 you wrote:
> stmmac_dev_probe doesn't propagate feature flags to VLANs.  So features
> like offloading don't correspond with the general features and it's not
> possible to manipulate features via ethtool -K to affect VLANs.
> 
> Propagate feature flags to vlan features.  Drop TSO feature because
> it does not work on VLANs yet.
> 
> [...]

Here is the summary with links:
  - [v2,net-next] net: stmmac: propagate feature flags to vlan
    https://git.kernel.org/netdev/net-next/c/6b2c6e4a938f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next] net: stmmac: propagate feature flags to vlan

[Oleksij Rempel]

Hi Corinna,

this patch breaks netboot on PRTT1C. Reverting this patch make it works again.
Here is a boot log with reverted patch:
root@prtt1c:~ dmesg | grep -e "\(ether\|sja\)"
[    2.523153] sja1105 spi0.0: Probed switch chip: SJA1105Q
[    2.534541] stm32-dwmac 5800a000.ethernet: IRQ eth_wake_irq not found
[    2.539678] stm32-dwmac 5800a000.ethernet: IRQ eth_lpi not found
[    2.564079] stm32-dwmac 5800a000.ethernet: User ID: 0x40, Synopsys ID: 0x42
[    2.569726] stm32-dwmac 5800a000.ethernet:   DWMAC4/5
[    2.582496] stm32-dwmac 5800a000.ethernet: DMA HW capability register supported
[    2.588415] stm32-dwmac 5800a000.ethernet: RX Checksum Offload Engine supported
[    2.612482] stm32-dwmac 5800a000.ethernet: TX Checksum insertion supported
[    2.617992] stm32-dwmac 5800a000.ethernet: Wake-Up On Lan supported
[    2.632481] stm32-dwmac 5800a000.ethernet: TSO supported
[    2.636369] stm32-dwmac 5800a000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[    2.662493] stm32-dwmac 5800a000.ethernet: Enabled L3L4 Flow TC (entries=2)
[    2.668109] stm32-dwmac 5800a000.ethernet: Enabled RFS Flow TC (entries=10)
[    2.682494] stm32-dwmac 5800a000.ethernet: TSO feature enabled
[    2.686993] stm32-dwmac 5800a000.ethernet: Using 32/32 bits DMA host/device width
[    3.513207] sja1105 spi0.0: Probed switch chip: SJA1105Q
[    3.536541] stm32-dwmac 5800a000.ethernet eth0: Only single VLAN ID supported
[    3.544291] stm32-dwmac 5800a000.ethernet eth0: Only single VLAN ID supported
[    3.551823] stm32-dwmac 5800a000.ethernet eth0: Only single VLAN ID supported
[    3.559610] sja1105 spi0.0: configuring for fixed/rmii link mode
[    3.565608] sja1105 spi0.0: Link is Up - 100Mbps/Full - flow control off
[    3.575253] sja1105 spi0.0 t1l0 (uninitialized): PHY [gpio-0:06] driver [TI DP83TD510E] (irq=31)
[    3.594168] sja1105 spi0.0 t1l1 (uninitialized): PHY [gpio-0:07] driver [TI DP83TD510E] (irq=32)
[    3.612244] sja1105 spi0.0 t1l2 (uninitialized): PHY [gpio-0:0a] driver [TI DP83TD510E] (irq=33)
[    3.715610] sja1105 spi0.0 rj45 (uninitialized): PHY [gpio-0:02] driver [Micrel KSZ9031 Gigabit PHY] (irq=34)
[    3.729040] stm32-dwmac 5800a000.ethernet eth0: entered promiscuous mode
[    3.860011] stm32-dwmac 5800a000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
[    3.876544] stm32-dwmac 5800a000.ethernet eth0: No Safety Features support found
[    4.084884] stm32-dwmac 5800a000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[    4.092530] stm32-dwmac 5800a000.ethernet eth0: registered PTP clock
[    4.099997] stm32-dwmac 5800a000.ethernet eth0: configuring for fixed/rmii link mode
[    4.106645] stm32-dwmac 5800a000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
[    4.120533] stm32-dwmac 5800a000.ethernet eth0: Adding VLAN ID 0 is not supported
[    4.129557] sja1105 spi0.0 t1l0: configuring for phy/rmii link mode
[    4.143546] sja1105 spi0.0 t1l1: configuring for phy/rmii link mode
[    4.161097] sja1105 spi0.0 t1l2: configuring for phy/rmii link mode
[    4.167342] sja1105 spi0.0 rj45: configuring for phy/rgmii-id link mode
[    9.809396] sja1105 spi0.0 rj45: Link is Up - 1Gbps/Full - flow control off
[   66.587255] sja1105 spi0.0 t1l0: entered allmulticast mode
[   66.605179] stm32-dwmac 5800a000.ethernet eth0: entered allmulticast mode
[   66.624612] sja1105 spi0.0 t1l0: entered promiscuous mode
[   66.676943] sja1105 spi0.0 t1l1: entered allmulticast mode
[   66.700590] sja1105 spi0.0 t1l1: entered promiscuous mode
[   66.762670] sja1105 spi0.0 t1l2: entered allmulticast mode
[   66.782927] sja1105 spi0.0 t1l2: entered promiscuous mode
[   66.921089] sja1105 spi0.0 t1l0: configuring for phy/rmii link mode
[   66.951074] sja1105 spi0.0 t1l1: configuring for phy/rmii link mode
[   67.008160] sja1105 spi0.0 t1l2: configuring for phy/rmii link mode
[   68.557994] sja1105 spi0.0 t1l1: Link is Up - 10Mbps/Full - flow control off
[   93.820549] sja1105 spi0.0 t1l1: Link is Down
[   94.412697] sja1105 spi0.0 t1l1: Link is Up - 10Mbps/Full - flow control off
[   94.445096] stm32-dwmac 5800a000.ethernet eth0: Couldn't decode source port

Please tell me if you'll need help to debug this issue.

Regards,
Oleksij