2021-11-09 04:38:45

by Vladimir Oltean

[permalink] [raw]
Subject: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
base_time is in the past") allowed some base time values in the past,
but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
explicitly denied by the driver.

Remove the bogus check.

Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
Signed-off-by: Vladimir Oltean <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 8160087ee92f..1c4ea0b1b845 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -786,8 +786,6 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
goto disable;
if (qopt->num_entries >= dep)
return -EINVAL;
- if (!qopt->base_time)
- return -ERANGE;
if (!qopt->cycle_time)
return -ERANGE;

--
2.25.1


2021-11-09 17:07:37

by Kurt Kanzenbach

[permalink] [raw]
Subject: Re: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Hi Vladimir,

On Mon Nov 08 2021, Vladimir Oltean wrote:
> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
> base_time is in the past") allowed some base time values in the past,
> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
> explicitly denied by the driver.
>
> Remove the bogus check.
>
> Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
> Signed-off-by: Vladimir Oltean <[email protected]>

I've experienced the same problem and wanted to send a patch for
it. Thanks!

Reviewed-by: Kurt Kanzenbach <[email protected]>


Attachments:
signature.asc (889.00 B)

2021-11-09 20:31:28

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
> Hi Vladimir,
>
> On Mon Nov 08 2021, Vladimir Oltean wrote:
> > Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
> > base_time is in the past") allowed some base time values in the past,
> > but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
> > explicitly denied by the driver.
> >
> > Remove the bogus check.
> >
> > Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
> > Signed-off-by: Vladimir Oltean <[email protected]>
>
> I've experienced the same problem and wanted to send a patch for
> it. Thanks!
>
> Reviewed-by: Kurt Kanzenbach <[email protected]>

Cool. So you had that patch queued up? What other stmmac patches do you
have queued up? :) Do you have a fix for the driver setting the PTP time
every time when SIOCSHWTSTAMP is called? This breaks the UTC-to-TAI
offset established by phc2sys and it takes a few seconds to readjust,
which is very annoying.

2021-11-09 23:54:51

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [Linux-stm32] [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Hello Vladimir, Kurt,

On 09.11.21 11:35, Vladimir Oltean wrote:
> On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
>> Hi Vladimir,
>>
>> On Mon Nov 08 2021, Vladimir Oltean wrote:
>>> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
>>> base_time is in the past") allowed some base time values in the past,
>>> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
>>> explicitly denied by the driver.
>>>
>>> Remove the bogus check.
>>>
>>> Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
>>> Signed-off-by: Vladimir Oltean <[email protected]>
>>
>> I've experienced the same problem and wanted to send a patch for
>> it. Thanks!
>>
>> Reviewed-by: Kurt Kanzenbach <[email protected]>
>
> Cool. So you had that patch queued up? What other stmmac patches do you
> have queued up? :) Do you have a fix for the driver setting the PTP time
> every time when SIOCSHWTSTAMP is called? This breaks the UTC-to-TAI
> offset established by phc2sys and it takes a few seconds to readjust,
> which is very annoying.

Sounds like the same issue in:
https://lore.kernel.org/netdev/[email protected]/

Cheers,
Ahmad

> _______________________________________________
> Linux-stm32 mailing list
> [email protected]
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-11-09 23:55:31

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [Linux-stm32] [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

On Tue, Nov 09, 2021 at 03:19:28PM +0100, Ahmad Fatoum wrote:
> Hello Vladimir, Kurt,
>
> On 09.11.21 11:35, Vladimir Oltean wrote:
> > On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
> >> Hi Vladimir,
> >>
> >> On Mon Nov 08 2021, Vladimir Oltean wrote:
> >>> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
> >>> base_time is in the past") allowed some base time values in the past,
> >>> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
> >>> explicitly denied by the driver.
> >>>
> >>> Remove the bogus check.
> >>>
> >>> Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
> >>> Signed-off-by: Vladimir Oltean <[email protected]>
> >>
> >> I've experienced the same problem and wanted to send a patch for
> >> it. Thanks!
> >>
> >> Reviewed-by: Kurt Kanzenbach <[email protected]>
> >
> > Cool. So you had that patch queued up? What other stmmac patches do you
> > have queued up? :) Do you have a fix for the driver setting the PTP time
> > every time when SIOCSHWTSTAMP is called? This breaks the UTC-to-TAI
> > offset established by phc2sys and it takes a few seconds to readjust,
> > which is very annoying.
>
> Sounds like the same issue in:
> https://lore.kernel.org/netdev/[email protected]/
>
> Cheers,
> Ahmad

Indeed. Was there a v2 to that?

2021-11-09 23:58:02

by Kurt Kanzenbach

[permalink] [raw]
Subject: Re: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

On Tue Nov 09 2021, Vladimir Oltean wrote:
> On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
>> Hi Vladimir,
>>
>> On Mon Nov 08 2021, Vladimir Oltean wrote:
>> > Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
>> > base_time is in the past") allowed some base time values in the past,
>> > but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
>> > explicitly denied by the driver.
>> >
>> > Remove the bogus check.
>> >
>> > Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
>> > Signed-off-by: Vladimir Oltean <[email protected]>
>>
>> I've experienced the same problem and wanted to send a patch for
>> it. Thanks!
>>
>> Reviewed-by: Kurt Kanzenbach <[email protected]>
>
> Cool. So you had that patch queued up? What other stmmac patches do you
> have queued up? :).

I'm experiencing some problems with XDP using this driver. We're
currently investigating.

Thanks,
Kurt


Attachments:
signature.asc (889.00 B)

2021-11-09 23:59:54

by Yannick Vignon

[permalink] [raw]
Subject: Re: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Hi Kurt,

On 11/9/2021 3:47 PM, Kurt Kanzenbach wrote:
> On Tue Nov 09 2021, Vladimir Oltean wrote:
>> On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
>>> Hi Vladimir,
>>>
>>> On Mon Nov 08 2021, Vladimir Oltean wrote:
>>>> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
>>>> base_time is in the past") allowed some base time values in the past,
>>>> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
>>>> explicitly denied by the driver.
>>>>
>>>> Remove the bogus check.
>>>>
>>>> Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
>>>> Signed-off-by: Vladimir Oltean <[email protected]>
>>>
>>> I've experienced the same problem and wanted to send a patch for
>>> it. Thanks!
>>>
>>> Reviewed-by: Kurt Kanzenbach <[email protected]>
>>
>> Cool. So you had that patch queued up? What other stmmac patches do you
>> have queued up? :).
>
> I'm experiencing some problems with XDP using this driver. We're
> currently investigating.

Could you elaborate a bit?
I've been using XDP a lot with the stmmac driver recently, and while I
did see issues initially, most of them got fixed by using a recent
enough kernel, thanks to the following commits:
. a6451192da2691dcf39507bd ("net: stmmac: fix kernel panic due to NULL
pointer dereference of xsk_pool")
. 2b9fff64f03219d78044d1ab ("net: stmmac: fix kernel panic due to NULL
pointer dereference of buf->xdp")
. 81d0885d68ec427e62044cf4 ("net: stmmac: Fix overall budget calculation
for rxtx_napi")

There was one remaining issue for which I need to push a fix: if you
remove an XDP program from an interface while transmitting traffic, you
are likely to trigger a kernel panic. I'll try to push a patch for that
soon.

> Thanks,
> Kurt
>

Regards,
Yannick

2021-11-10 12:43:20

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [Linux-stm32] [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

On Tue, Nov 09, 2021 at 04:22:55PM +0200, Vladimir Oltean wrote:
> On Tue, Nov 09, 2021 at 03:19:28PM +0100, Ahmad Fatoum wrote:
> > Hello Vladimir, Kurt,
> >
> > On 09.11.21 11:35, Vladimir Oltean wrote:
> > > On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
> > >> Hi Vladimir,
> > >>
> > >> On Mon Nov 08 2021, Vladimir Oltean wrote:
> > >>> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
> > >>> base_time is in the past") allowed some base time values in the past,
> > >>> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
> > >>> explicitly denied by the driver.
> > >>>
> > >>> Remove the bogus check.
> > >>>
> > >>> Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
> > >>> Signed-off-by: Vladimir Oltean <[email protected]>
> > >>
> > >> I've experienced the same problem and wanted to send a patch for
> > >> it. Thanks!
> > >>
> > >> Reviewed-by: Kurt Kanzenbach <[email protected]>
> > >
> > > Cool. So you had that patch queued up? What other stmmac patches do you
> > > have queued up? :) Do you have a fix for the driver setting the PTP time
> > > every time when SIOCSHWTSTAMP is called? This breaks the UTC-to-TAI
> > > offset established by phc2sys and it takes a few seconds to readjust,
> > > which is very annoying.
> >
> > Sounds like the same issue in:
> > https://lore.kernel.org/netdev/[email protected]/
> >
> > Cheers,
> > Ahmad
>
> Indeed. Was there a v2 to that?

FWIW I've applied that patch and made a few fixups according to my
liking, and it works fine. I can resend it myself if there aren't any
volunteers from Pengutronix.

2021-11-10 14:42:58

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:

On Mon, 8 Nov 2021 22:28:54 +0200 you wrote:
> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
> base_time is in the past") allowed some base time values in the past,
> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
> explicitly denied by the driver.
>
> Remove the bogus check.
>
> [...]

Here is the summary with links:
- [net] net: stmmac: allow a tc-taprio base-time of zero
https://git.kernel.org/netdev/net/c/f64ab8e4f368

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


2021-11-11 10:48:57

by Holger Assmann

[permalink] [raw]
Subject: Re: [Linux-stm32] [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Hi,

Am 10.11.21 um 13:38 schrieb Vladimir Oltean:
>>
>> Indeed. Was there a v2 to that?
>
> FWIW I've applied that patch and made a few fixups according to my
> liking, and it works fine. I can resend it myself if there aren't any
> volunteers from Pengutronix.
>

Feel free to do so!

Greetings,
Holger

--
Pengutronix e.K. | Holger Assmann |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-11-16 12:19:25

by Kurt Kanzenbach

[permalink] [raw]
Subject: Re: [PATCH net] net: stmmac: allow a tc-taprio base-time of zero

Hi Yannick,

On Tue Nov 09 2021, Yannick Vignon wrote:
> Hi Kurt,
>
> On 11/9/2021 3:47 PM, Kurt Kanzenbach wrote:
>> On Tue Nov 09 2021, Vladimir Oltean wrote:
>>> On Tue, Nov 09, 2021 at 09:20:53AM +0100, Kurt Kanzenbach wrote:
>>>> Hi Vladimir,
>>>>
>>>> On Mon Nov 08 2021, Vladimir Oltean wrote:
>>>>> Commit fe28c53ed71d ("net: stmmac: fix taprio configuration when
>>>>> base_time is in the past") allowed some base time values in the past,
>>>>> but apparently not all, the base-time value of 0 (Jan 1st 1970) is still
>>>>> explicitly denied by the driver.
>>>>>
>>>>> Remove the bogus check.
>>>>>
>>>>> Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
>>>>> Signed-off-by: Vladimir Oltean <[email protected]>
>>>>
>>>> I've experienced the same problem and wanted to send a patch for
>>>> it. Thanks!
>>>>
>>>> Reviewed-by: Kurt Kanzenbach <[email protected]>
>>>
>>> Cool. So you had that patch queued up? What other stmmac patches do you
>>> have queued up? :).
>>
>> I'm experiencing some problems with XDP using this driver. We're
>> currently investigating.
>
> Could you elaborate a bit?

It was a combination of oddities within the PCP based VLAN steering and
bugs in my application. No driver issues.

The last issue I have is packet loss from time to time. Still debugging.

> I've been using XDP a lot with the stmmac driver recently, and while I
> did see issues initially, most of them got fixed by using a recent
> enough kernel, thanks to the following commits:
> . a6451192da2691dcf39507bd ("net: stmmac: fix kernel panic due to NULL
> pointer dereference of xsk_pool")
> . 2b9fff64f03219d78044d1ab ("net: stmmac: fix kernel panic due to NULL
> pointer dereference of buf->xdp")
> . 81d0885d68ec427e62044cf4 ("net: stmmac: Fix overall budget calculation
> for rxtx_napi")
>
> There was one remaining issue for which I need to push a fix: if you
> remove an XDP program from an interface while transmitting traffic, you
> are likely to trigger a kernel panic. I'll try to push a patch for that
> soon.

OK, great.

Thanks,
Kurt


Attachments:
signature.asc (873.00 B)