All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolai Buchwitz <nb@tipi-net.de>
To: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: "Andrew Lunn" <andrew+netdev@lunn.ch>,
	davem@davemloft.net, "Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Russell King" <linux@armlinux.org.uk>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Alexis Lothoré" <alexis.lothore@bootlin.com>,
	"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
	"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
	"Emil Renner Berthing" <kernel@esmil.dk>,
	"Minda Chen" <minda.chen@starfivetech.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
	"Jan Petrous" <jan.petrous@oss.nxp.com>,
	"Ovidiu Panait" <ovidiu.panait.rb@renesas.com>,
	Jose.Abreu@synopsys.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com,
	linux-arm-kernel@lists.infradead.org,
	linux-stm32@st-md-mailman.stormreply.com, PKneuper@dspace.de,
	"David Laight" <david.laight.linux@gmail.com>
Subject: Re: [PATCH net v2 2/6] net: stmmac: selftests: Validate EEE based on the actual LPI timer value
Date: Thu, 10 Sep 2026 20:50:30 +0200	[thread overview]
Message-ID: <6af83ee5b9ddeeebcc6e4cc29989b856@tipi-net.de> (raw)
In-Reply-To: <20260910151404.976753-3-maxime.chevallier@bootlin.com>

Hi Maxime,

Sorry for the noise, but after having a second look, there is something
which I think should be discussed at least.

On 10.9.2026 17:13, Maxime Chevallier wrote:
> The EEE selftest is a 2-step test :
>  - It validates that we enter in LPI mode with the
>    irq_tx_path_in_lpi_mode_n counter
>  - It then validates that we exit LPI when sending a frame, with the
>    irq_tx_path_exit_lpi_mode_n counter.
> 
> The current state of the test lacks 2 main things :
> 
>  - We don't know exactly when was the previous frame sent (it's from 
> the
>    previous selftest)
> 
>  - The timeout is hardcoded, while the LPI is entered after a
>    user-configurable delay. On top of that, the timeout loop uses a
>    pre-decrement iterator (--retries) that actually only iterate nine
>    times, so 900ms while the default LPI value is 1 second.
> 
> Let's therefore make it more deterministic :
> 
>  - Send a frame at the beginning of the test
>  - Wait for more than the lpi timer value, we timeout after about twice
>    the value,
>  - Then send another frame, and verify that we do go out of LPI, also
>    with a timeout.
> 
> Note that the test's goal isn't to validate the LPI timer value itself,
> only that we enter/leave LPI mode.
> 
> Fixes: 091810dbded9 ("net: stmmac: Introduce selftests support")
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> [...]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> [...]

> 
> +	max_duration = usecs_to_jiffies(2 * priv->tx_lpi_timer);

tx_lpi_timer comes direct from ethtool as an unvalidated u32. So if a 
(arguable privileged
user) does 'ethtool --set-eee eth0 tx-lpi-timer 1000000000', the code 
will sleep ~2000 seconds
under rtnl_lock. Tge old code capped it at 900 ms regardless of the 
timer.

Thus I think we should clamp it at some sane value. Feel free to keep my 
R-b with the clamp.

> [...]

Regards,
Nicolai


  parent reply	other threads:[~2026-09-10 18:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 15:13 [PATCH net v2 0/6] net: stmmac: More selftest-related fixes Maxime Chevallier
2026-09-10 15:13 ` [PATCH net v2 1/6] net: stmmac: selftests: Support running selftests on DSA conduits Maxime Chevallier
2026-09-10 18:12   ` Nicolai Buchwitz
2026-09-10 15:13 ` [PATCH net v2 2/6] net: stmmac: selftests: Validate EEE based on the actual LPI timer value Maxime Chevallier
2026-09-10 17:43   ` Nicolai Buchwitz
2026-09-10 18:50   ` Nicolai Buchwitz [this message]
2026-09-10 20:52     ` Maxime Chevallier
2026-09-10 15:14 ` [PATCH net v2 3/6] net: stmmac: selftests: Check the dev->features for S-TAG offload testing Maxime Chevallier
2026-09-10 18:13   ` Nicolai Buchwitz
2026-09-10 15:14 ` [PATCH net v2 4/6] net: stmmac: selftests: Capture all packets for vlan checks Maxime Chevallier
2026-09-10 18:13   ` Nicolai Buchwitz
2026-09-10 15:14 ` [PATCH net v2 5/6] net: stmmac: size the RX buffers from the frame length, not the MTU Maxime Chevallier
2026-09-10 18:42   ` Nicolai Buchwitz
2026-09-10 15:14 ` [PATCH net v2 6/6] net: stmmac: selftests: Account for alignment shift on dwmac1000 for Jumbo test Maxime Chevallier
2026-09-10 18:13   ` Nicolai Buchwitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6af83ee5b9ddeeebcc6e4cc29989b856@tipi-net.de \
    --to=nb@tipi-net.de \
    --cc=Jose.Abreu@synopsys.com \
    --cc=PKneuper@dspace.de \
    --cc=alexandre.torgue@foss.st.com \
    --cc=alexis.lothore@bootlin.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=david.laight.linux@gmail.com \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=jan.petrous@oss.nxp.com \
    --cc=jbrunet@baylibre.com \
    --cc=kernel@esmil.dk \
    --cc=khilman@baylibre.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=linux@armlinux.org.uk \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=maxime.chevallier@bootlin.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=minda.chen@starfivetech.com \
    --cc=neil.armstrong@linaro.org \
    --cc=netdev@vger.kernel.org \
    --cc=ovidiu.panait.rb@renesas.com \
    --cc=pabeni@redhat.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.