> -----Original Message-----
> From: Florian Fainelli <[email protected]>
> Sent: Friday, July 21, 2023 11:59 PM
> To: Ng, Boon Khai <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected]; [email protected];
> Ng, Boon Khai <[email protected]>; Giuseppe Cavallaro
> <[email protected]>; Alexandre Torgue <[email protected]>;
> Jose Abreu <[email protected]>; David S . Miller <[email protected]>;
> Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Maxime Coquelin
> <[email protected]>; [email protected]; linux-stm32@st-md-
> mailman.stormreply.com; [email protected]; linux-
> [email protected]
> Cc: Shevchenko, Andriy <[email protected]>; Tham, Mun Yew
> <[email protected]>; Swee, Leong Ching
> <[email protected]>; G Thomas, Rohan
> <[email protected]>; Shevchenko Andriy
> <[email protected]>
> 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:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <[email protected]>
> >> Sent: Friday, July 21, 2023 6:11 PM
> >> To: [email protected]; [email protected]; "Ng
> >> <boon.khai.ng"@intel.com; Giuseppe Cavallaro
> >> <[email protected]>; Alexandre Torgue
> >> <[email protected]>; Jose Abreu <[email protected]>;
> >> David S . Miller <[email protected]>; Eric Dumazet
> >> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> >> <[email protected]>; Maxime Coquelin <[email protected]>;
> >> [email protected]; linux-stm32@st- md-mailman.stormreply.com;
> >> [email protected]; linux- [email protected]
> >> Cc: Ng, Boon Khai <[email protected]>; Shevchenko, Andriy
> >> <[email protected]>; Tham, Mun Yew
> >> <[email protected]>; Swee, Leong Ching
> >> <[email protected]>; G Thomas, Rohan
> >> <[email protected]>; Shevchenko Andriy
> >> <[email protected]>
> >> 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, [email protected] wrote:
> >>> From: Boon Khai Ng <[email protected]>
> >>>
> >>> 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.
> >> ...
> >>
> >>> if (priv->dma_cap.vlhash) {
> >>> ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> >>> ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; diff --
> >> git
> >>> 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)
> >>> 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?
I don’t think ethool can "remember" the setting once the device get rebooted?
Any other suggestion of doing it other than using the dts method?
> What you are doing here is encode a policy as a Device Tree property rather
> than describe whether the hardware supports a given feature and this is frowned
> upon.
> --
> Florian
On 7/21/2023 9:12 AM, Ng, Boon Khai wrote:
>> -----Original Message-----
>> From: Florian Fainelli <[email protected]>
>> Sent: Friday, July 21, 2023 11:59 PM
>> To: Ng, Boon Khai <[email protected]>; Krzysztof Kozlowski
>> <[email protected]>; [email protected]; [email protected];
>> Ng, Boon Khai <[email protected]>; Giuseppe Cavallaro
>> <[email protected]>; Alexandre Torgue <[email protected]>;
>> Jose Abreu <[email protected]>; David S . Miller <[email protected]>;
>> Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
>> Paolo Abeni <[email protected]>; Maxime Coquelin
>> <[email protected]>; [email protected]; linux-stm32@st-md-
>> mailman.stormreply.com; [email protected]; linux-
>> [email protected]
>> Cc: Shevchenko, Andriy <[email protected]>; Tham, Mun Yew
>> <[email protected]>; Swee, Leong Ching
>> <[email protected]>; G Thomas, Rohan
>> <[email protected]>; Shevchenko Andriy
>> <[email protected]>
>> 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:
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <[email protected]>
>>>> Sent: Friday, July 21, 2023 6:11 PM
>>>> To: [email protected]; [email protected]; "Ng
>>>> <boon.khai.ng"@intel.com; Giuseppe Cavallaro
>>>> <[email protected]>; Alexandre Torgue
>>>> <[email protected]>; Jose Abreu <[email protected]>;
>>>> David S . Miller <[email protected]>; Eric Dumazet
>>>> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
>>>> <[email protected]>; Maxime Coquelin <[email protected]>;
>>>> [email protected]; linux-stm32@st- md-mailman.stormreply.com;
>>>> [email protected]; linux- [email protected]
>>>> Cc: Ng, Boon Khai <[email protected]>; Shevchenko, Andriy
>>>> <[email protected]>; Tham, Mun Yew
>>>> <[email protected]>; Swee, Leong Ching
>>>> <[email protected]>; G Thomas, Rohan
>>>> <[email protected]>; Shevchenko Andriy
>>>> <[email protected]>
>>>> 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, [email protected] wrote:
>>>>> From: Boon Khai Ng <[email protected]>
>>>>>
>>>>> 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.
>>>> ...
>>>>
>>>>> if (priv->dma_cap.vlhash) {
>>>>> ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>>>>> ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; diff --
>>>> git
>>>>> 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)
>>>>> 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
> -----Original Message-----
> From: Florian Fainelli <[email protected]>
> Sent: Saturday, July 22, 2023 12:30 AM
> To: Ng, Boon Khai <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected];
> [email protected]; Giuseppe Cavallaro <[email protected]>;
> Alexandre Torgue <[email protected]>; Jose Abreu
> <[email protected]>; David S . Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Maxime Coquelin
> <[email protected]>; [email protected]; linux-stm32@st-
> md-mailman.stormreply.com; [email protected]; linux-
> [email protected]
> Cc: Shevchenko, Andriy <[email protected]>; Tham, Mun Yew
> <[email protected]>; Swee, Leong Ching
> <[email protected]>; G Thomas, Rohan
> <[email protected]>; Shevchenko Andriy
> <[email protected]>
> Subject: Re: [Enable Designware XGMAC VLAN Stripping Feature 2/2] net:
> stmmac: dwxgmac2: Add support for HW-accelerated VLAN Stripping
>
>
>
> On 7/21/2023 9:12 AM, Ng, Boon Khai wrote:
> >> -----Original Message-----
> >> From: Florian Fainelli <[email protected]>
> >> Sent: Friday, July 21, 2023 11:59 PM
> >> To: Ng, Boon Khai <[email protected]>; Krzysztof Kozlowski
> >> <[email protected]>; [email protected];
> >> [email protected]; Ng, Boon Khai <[email protected]>;
> >> Giuseppe Cavallaro <[email protected]>; Alexandre Torgue
> >> <[email protected]>; Jose Abreu <[email protected]>;
> >> David S . Miller <[email protected]>; Eric Dumazet
> >> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> Abeni
> >> <[email protected]>; Maxime Coquelin
> <[email protected]>;
> >> [email protected]; linux-stm32@st-md- mailman.stormreply.com;
> >> [email protected]; linux- [email protected]
> >> Cc: Shevchenko, Andriy <[email protected]>; Tham, Mun
> Yew
> >> <[email protected]>; Swee, Leong Ching
> >> <[email protected]>; G Thomas, Rohan
> >> <[email protected]>; Shevchenko Andriy
> >> <[email protected]>
> >> 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:
> >>>> -----Original Message-----
> >>>> From: Krzysztof Kozlowski <[email protected]>
> >>>> Sent: Friday, July 21, 2023 6:11 PM
> >>>> To: [email protected]; [email protected]; "Ng
> >>>> <boon.khai.ng"@intel.com; Giuseppe Cavallaro
> >>>> <[email protected]>; Alexandre Torgue
> >>>> <[email protected]>; Jose Abreu
> <[email protected]>;
> >>>> David S . Miller <[email protected]>; Eric Dumazet
> >>>> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> >>>> Abeni <[email protected]>; Maxime Coquelin
> >>>> <[email protected]>; [email protected];
> >>>> linux-stm32@st- md-mailman.stormreply.com;
> >>>> [email protected]; linux- [email protected]
> >>>> Cc: Ng, Boon Khai <[email protected]>; Shevchenko, Andriy
> >>>> <[email protected]>; Tham, Mun Yew
> >>>> <[email protected]>; Swee, Leong Ching
> >>>> <[email protected]>; G Thomas, Rohan
> >>>> <[email protected]>; Shevchenko Andriy
> >>>> <[email protected]>
> >>>> 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, [email protected] wrote:
> >>>>> From: Boon Khai Ng <[email protected]>
> >>>>>
> >>>>> 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.
> >>>> ...
> >>>>
> >>>>> if (priv->dma_cap.vlhash) {
> >>>>> ndev->features |=
> NETIF_F_HW_VLAN_CTAG_FILTER;
> >>>>> ndev->features |=
> NETIF_F_HW_VLAN_STAG_FILTER; diff --
> >>>> git
> >>>>> 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)
> >>>>> 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.
>
Okay got it.
> >
> > 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.
>
Okay, will try to search that, is adding the ethool command turning the
Hw vlan striping at the startup script consider one way of doing it?
> > 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.
Okay, so seem like it is solely depends on my side whether or not to turn it on by default,
Either way, if it go against the user will to have it on/off by default, they will need to write
A startup script to turn the ethool on/off?
> --
> Florian