On 7/21/2023 9:12 AM, Ng, Boon Khai wrote:

quoted

-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: Friday, July 21, 2023 11:59 PM
To: Ng, Boon Khai <redacted>; Krzysztof Kozlowski
[off-list ref]; Boon@ecsmtp.png.intel.com; Khai@ecsmtp.png.intel.com;
Ng, Boon Khai [off-list ref]; Giuseppe Cavallaro
[off-list ref]; Alexandre Torgue [off-list ref];
Jose Abreu [off-list ref]; David S . Miller [off-list ref];
Eric Dumazet [off-list ref]; Jakub Kicinski [off-list ref];
Paolo Abeni [off-list ref]; Maxime Coquelin
[off-list ref]; netdev@vger.kernel.org; linux-stm32@st-md-
mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org
Cc: Shevchenko, Andriy <redacted>; Tham, Mun Yew
[off-list ref]; Swee, Leong Ching
[off-list ref]; G Thomas, Rohan
[off-list ref]; Shevchenko Andriy
[off-list ref]
Subject: Re: [Enable Designware XGMAC VLAN Stripping Feature 2/2] net:
stmmac: dwxgmac2: Add support for HW-accelerated VLAN Stripping



On 7/21/2023 8:30 AM, Ng, Boon Khai wrote:

quoted

quoted

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org>
Sent: Friday, July 21, 2023 6:11 PM
To: Boon@ecsmtp.png.intel.com; Khai@ecsmtp.png.intel.com; "Ng
<boon.khai.ng"@intel.com; Giuseppe Cavallaro
[off-list ref]; Alexandre Torgue
[off-list ref]; Jose Abreu [off-list ref];
David S . Miller [off-list ref]; Eric Dumazet
[off-list ref]; Jakub Kicinski [off-list ref]; Paolo Abeni
[off-list ref]; Maxime Coquelin [off-list ref];
netdev@vger.kernel.org; linux-stm32@st- md-mailman.stormreply.com;
linux-arm-kernel@lists.infradead.org; linux- kernel@vger.kernel.org
Cc: Ng, Boon Khai <redacted>; Shevchenko, Andriy
[off-list ref]; Tham, Mun Yew
[off-list ref]; Swee, Leong Ching
[off-list ref]; G Thomas, Rohan
[off-list ref]; Shevchenko Andriy
[off-list ref]
Subject: Re: [Enable Designware XGMAC VLAN Stripping Feature 2/2] net:
stmmac: dwxgmac2: Add support for HW-accelerated VLAN Stripping

On 21/07/2023 08:26, Boon@ecsmtp.png.intel.com wrote:

quoted

From: Boon Khai Ng <redacted>

Currently, VLAN tag stripping is done by software driver in
stmmac_rx_vlan(). This patch is to Add support for VLAN tag
stripping by the MAC hardware and MAC drivers to support it.
This is done by adding rx_hw_vlan() and set_hw_vlan_mode() callbacks
at stmmac_ops struct which are called from upper software layer.

...

quoted

   	if (priv->dma_cap.vlhash) {
   		ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
   		ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; diff --

git

quoted

a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 23d53ea04b24..bd7f3326a44c 100644

--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c

@@ -543,6 +543,12 @@ stmmac_probe_config_dt(struct platform_device

*pdev, u8 *mac)

quoted

   			plat->flags |= STMMAC_FLAG_TSO_EN;
   	}

+	/* Rx VLAN HW Stripping */
+	if (of_property_read_bool(np, "snps,rx-vlan-offload")) {
+		dev_info(&pdev->dev, "RX VLAN HW Stripping\n");

Why? Drop.

This is an dts option export to dts for user to choose whether or not
they Want a Hardware stripping or a software stripping.

May I know what is the reason to drop this?

Because the networking stack already exposes knobs for drivers to advertise and
control VLAN stripping/insertion on RX/TX using ethtool and feature bits
(NETIF_F_HW_VLAN_CTAG_RX, NETIF_F_HW_VLAN_CTAG_TX).

Hi Florian,

Understood, but how does user choose to have the default option
either hardware strip or software strip, when the device just boot up?

You need the hardware to advertise it and decide as a maintainer of that 
driver whether it makes sense to have one or the other behavior by default.

I don’t think ethool can "remember" the setting once the device get rebooted?

If by "device" you mean a system that incorporates a XGMAC core, then I 
suppose that is true, though you could have some user-space logic that 
does remember the various ethtool options and re-applies them as soon as 
the device is made available to user-space, this would not be too far 
fetched.

Any other suggestion of doing it other than using the dts method?

Let me ask you this question: what are you trying to solve by making 
this configurable? HW stripping should always be more efficient, should 
not it, if so, what would be the reasons for not enabling that by 
default? If not, then leave it off and let users enable it if they feel 
like they want it.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel