| Message ID | 20211108202854.1740995-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [net] net: stmmac: allow a tc-taprio base-time of zero | expand |
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 <vladimir.oltean@nxp.com> I've experienced the same problem and wanted to send a patch for it. Thanks! Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
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 <vladimir.oltean@nxp.com> > > I've experienced the same problem and wanted to send a patch for > it. Thanks! > > Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> 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.
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 <vladimir.oltean@nxp.com> >> >> I've experienced the same problem and wanted to send a patch for >> it. Thanks! >> >> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> > > 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/20201216113239.2980816-1-h.assmann@pengutronix.de/ Cheers, Ahmad > _______________________________________________ > Linux-stm32 mailing list > Linux-stm32@st-md-mailman.stormreply.com > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32 >
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 <vladimir.oltean@nxp.com> > >> > >> I've experienced the same problem and wanted to send a patch for > >> it. Thanks! > >> > >> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> > > > > 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/20201216113239.2980816-1-h.assmann@pengutronix.de/ > > Cheers, > Ahmad Indeed. Was there a v2 to that?
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 <vladimir.oltean@nxp.com> >> >> I've experienced the same problem and wanted to send a patch for >> it. Thanks! >> >> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> > > 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
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 <vladimir.oltean@nxp.com> >>> >>> I've experienced the same problem and wanted to send a patch for >>> it. Thanks! >>> >>> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> >> >> 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
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 <vladimir.oltean@nxp.com> > > >> > > >> I've experienced the same problem and wanted to send a patch for > > >> it. Thanks! > > >> > > >> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> > > > > > > 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/20201216113239.2980816-1-h.assmann@pengutronix.de/ > > > > 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.
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: 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!
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
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 <vladimir.oltean@nxp.com> >>>> >>>> I've experienced the same problem and wanted to send a patch for >>>> it. Thanks! >>>> >>>> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> >>> >>> 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
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;
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 <vladimir.oltean@nxp.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 -- 1 file changed, 2 deletions(-)