netdev - RE: [PATCH v2] net: fec: allow disable coalescing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <PAXPR04MB8510ACEEA236D77E6E59A4068845A@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Fri, 27 Jun 2025 01:30:30 +0000
From: Wei Fang <wei.fang@....com>
To: Jonas Rebmann <jre@...gutronix.de>
CC: "imx@...ts.linux.dev" <imx@...ts.linux.dev>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "kernel@...gutronix.de"
	<kernel@...gutronix.de>, Shenwei Wang <shenwei.wang@....com>, Clark Wang
	<xiaoning.wang@....com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Subject: RE: [PATCH v2] net: fec: allow disable coalescing

> In the current implementation, IP coalescing is always enabled and
> cannot be disabled.
>
> As setting maximum frames to 0 or 1, or setting delay to zero implies
> immediate delivery of single packets/IRQs, disable coalescing in
> hardware in these cases.
>
> This also guarantees that coalescing is never enabled with ICFT or ICTT
> set to zero, a configuration that could lead to unpredictable behaviour
> according to i.MX8MP reference manual.
>
> Signed-off-by: Jonas Rebmann <jre@...gutronix.de>
> ---
> Changes in v2:
> - Adjust type of rx_itr, tx_itr (Thanks, Wei)
> - Set multiple FEC_ITR_ flags in one line for more compact code (Thanks, Wei)
> - Commit Message: mention ICFT/CTT fields constraints (Thanks, Andrew)
> - Link to v1:
> https://lore.kern/
> el.org%2Fr%2F20250625-fec_deactivate_coalescing-v1-1-57a1e41a45d3%40pe
> ngutronix.de&data=05%7C02%7Cwei.fang%40nxp.com%7C49dd91b2cd334451
> 91a608ddb4b78819%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 38865422543116485%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRy
> dWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D
> %3D%7C0%7C%7C%7C&sdata=t0upws9HvxuIzBe5INmHspMnBpZ4%2B61hZAG
> pfikHb74%3D&reserved=0
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++++----------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index
> 63dac42720453a8b8a847bdd1eec76ac072030bf..d4eed252ad4098a7962f615b
> ce98338bc3d12f5c 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3121,27 +3121,25 @@ static int fec_enet_us_to_itr_clock(struct
> net_device *ndev, int us)
>  static void fec_enet_itr_coal_set(struct net_device *ndev)
>  {
>       struct fec_enet_private *fep = netdev_priv(ndev);
> -     int rx_itr, tx_itr;
> +     u32 rx_itr = 0, tx_itr = 0;
> +     int rx_ictt, tx_ictt;
>
> -     /* Must be greater than zero to avoid unpredictable behavior */
> -     if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
> -         !fep->tx_time_itr || !fep->tx_pkts_itr)
> -             return;
> -
> -     /* Select enet system clock as Interrupt Coalescing
> -      * timer Clock Source
> -      */
> -     rx_itr = FEC_ITR_CLK_SEL;
> -     tx_itr = FEC_ITR_CLK_SEL;
> +     rx_ictt = fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr);
> +     tx_ictt = fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr);
>
> -     /* set ICFT and ICTT */
> -     rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
> -     rx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr));
> -     tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
> -     tx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr));
> +     if (rx_ictt > 0 && fep->rx_pkts_itr > 1) {
> +             /* Enable with enet system clock as Interrupt Coalescing timer Clock
> Source */
> +             rx_itr = FEC_ITR_EN | FEC_ITR_CLK_SEL;
> +             rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
> +             rx_itr |= FEC_ITR_ICTT(rx_ictt);
> +     }
>
> -     rx_itr |= FEC_ITR_EN;
> -     tx_itr |= FEC_ITR_EN;
> +     if (tx_ictt > 0 && fep->tx_pkts_itr > 1) {
> +             /* Enable with enet system clock as Interrupt Coalescing timer Clock
> Source */
> +             tx_itr = FEC_ITR_EN | FEC_ITR_CLK_SEL;
> +             tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
> +             tx_itr |= FEC_ITR_ICTT(tx_ictt);
> +     }
>
>       writel(tx_itr, fep->hwp + FEC_TXIC0);
>       writel(rx_itr, fep->hwp + FEC_RXIC0);
>
> ---

Reviewed-by: Wei Fang <wei.fang@....com>