[PATCH net-next 0/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

[PATCH net-next 0/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

[Grygorii Strashko]

hi

The intention of this series is to fully enable hard irqs deferral feature
(hrtimers based HW IRQ coalescing) from Eric Dumazet [1] for TI K3 CPSW driver
by using napi_complete_done() in TX completion path, so the combination of
parameters (/sys/class/net/ethX/):
 napi_defer_hard_irqs 
 gro_flush_timeout
can be used for hard irqs deferral.

The Patch 1 is required before enabling hard irqs deferral feature to avoid
"Unbalanced enable" issue if gro_flush_timeout is configured while
(napi_defer_hard_irqs == 0).

It's a bit sad that it can not be configured per RX/TX separately.

[1] https://lore.kernel.org/netdev/20200422161329.56026-1-edumazet@google.com/
Grygorii Strashko (1):
  net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX
    completion

Vignesh Raghavendra (1):
  net: ti: am65-cpsw-nuss: fix RX IRQ state after .ndo_stop()

 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 23 ++++++++++++++++-------
 drivers/net/ethernet/ti/am65-cpsw-nuss.h |  2 ++
 2 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.17.1

[PATCH net-next 1/2] net: ti: am65-cpsw-nuss: fix RX IRQ state after .ndo_stop()

[Grygorii Strashko]

From: Vignesh Raghavendra <vigneshr@ti.com>

On TI K3 am64x platform the issue with RX IRQ is observed - it's become
disabled forever after .ndo_stop(). The K3 CPSW driver manipulates RX IRQ
by using standard Linux enable_irq()/disable_irq_nosync() API as there is
no IRQ enable/disable options in CPSW HW itself, as result during
.ndo_stop() following sequence happens

  phy_stop()
  teardown TX/RX channels
  wait for TX tdown complete
  napi_disable(TX)
  clean up TX channels

  (a)

  napi_disable(RX)

At point (a) it's not possible to predict if RX IRQ was triggered or not.
if RX IRQ was triggered then it also not possible to definitely say if RX
NAPI was run or only scheduled and immediately canceled by
napi_disable(RX). Actually the last case causes RX IRQ to be permanently
disabled.

Another observed issue is that RX IRQ enable counter become unbalanced if
(gro_flush_timeout =! 0) while (napi_defer_hard_irqs == 0):

Unbalanced enable for IRQ 44
WARNING: CPU: 0 PID: 10 at ../kernel/irq/manage.c:776 __enable_irq+0x38/0x80
__enable_irq+0x38/0x80
enable_irq+0x54/0xb0
am65_cpsw_nuss_rx_poll+0x2f4/0x368
__napi_poll+0x34/0x1b8
net_rx_action+0xe4/0x220
_stext+0x11c/0x284
run_ksoftirqd+0x4c/0x60

To avoid above issues introduce flag indicating if RX was actually disabled
before enabling it in am65_cpsw_nuss_rx_poll() and restore RX IRQ state in
.ndo_open()

Fixes: 4f7cce272403 ("net: ethernet: ti: am65-cpsw: add support for am64x cpsw3g")
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 +++++++++++--
 drivers/net/ethernet/ti/am65-cpsw-nuss.h |  2 ++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 629c267d5254..08399f572091 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -519,6 +519,10 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common,
 	}
 
 	napi_enable(&common->napi_rx);
+	if (common->rx_irq_disabled) {
+		common->rx_irq_disabled = false;
+		enable_irq(common->rx_chns.irq);
+	}
 
 	dev_dbg(common->dev, "cpsw_nuss started\n");
 	return 0;
@@ -872,8 +876,12 @@ static int am65_cpsw_nuss_rx_poll(struct napi_struct *napi_rx, int budget)
 
 	dev_dbg(common->dev, "%s num_rx:%d %d\n", __func__, num_rx, budget);
 
-	if (num_rx < budget && napi_complete_done(napi_rx, num_rx))
-		enable_irq(common->rx_chns.irq);
+	if (num_rx < budget && napi_complete_done(napi_rx, num_rx)) {
+		if (common->rx_irq_disabled) {
+			common->rx_irq_disabled = false;
+			enable_irq(common->rx_chns.irq);
+		}
+	}
 
 	return num_rx;
 }
@@ -1091,6 +1099,7 @@ static irqreturn_t am65_cpsw_nuss_rx_irq(int irq, void *dev_id)
 {
 	struct am65_cpsw_common *common = dev_id;
 
+	common->rx_irq_disabled = true;
 	disable_irq_nosync(irq);
 	napi_schedule(&common->napi_rx);
 
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
index 5d93e346f05e..048ed10143c1 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
@@ -126,6 +126,8 @@ struct am65_cpsw_common {
 	struct am65_cpsw_rx_chn	rx_chns;
 	struct napi_struct	napi_rx;
 
+	bool			rx_irq_disabled;
+
 	u32			nuss_ver;
 	u32			cpsw_ver;
 	unsigned long		bus_freq;
-- 
2.17.1

[PATCH net-next 2/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

[Grygorii Strashko]

This patch enables support for hard irqs deferral feature from Eric Dumazet
[1] for TI K3 CPSW driver by using napi_complete_done() in TX completion
path.

Depending on gro_flush_timeout and napi_defer_hard_irqs at gives up to 30%
CPU utilization reduction:

gro_flush_timeout=50000
napi_defer_hard_irqs=2

netperf -l 10 -H 192.168.1.1  -t UDP_STREAM -c -C -- -m 1470
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.1.1 () port 0 AF_INET
Socket  Message  Elapsed      Messages                   CPU      Service
Size    Size     Time         Okay Errors   Throughput   Util     Demand
bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB

before:
212992    1470   10.00      809632      0      952.0     42.98    14.792
212992           10.00      809630             952.0     50.66    8.719

after:
212992    1470   10.00      813686      0      956.8     32.14    11.009
212992           10.00      813686             956.8     50.05    8.570

[1] https://lore.kernel.org/netdev/20200422161329.56026-1-edumazet@google.com/

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 08399f572091..b18343f9791d 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1086,13 +1086,13 @@ static int am65_cpsw_nuss_tx_poll(struct napi_struct *napi_tx, int budget)
 	else
 		num_tx = am65_cpsw_nuss_tx_compl_packets(tx_chn->common, tx_chn->id, budget);
 
-	num_tx = min(num_tx, budget);
-	if (num_tx < budget) {
-		napi_complete(napi_tx);
+	if (num_tx >= budget)
+		return budget;
+
+	if (napi_complete_done(napi_tx, num_tx))
 		enable_irq(tx_chn->irq);
-	}
 
-	return num_tx;
+	return 0;
 }
 
 static irqreturn_t am65_cpsw_nuss_rx_irq(int irq, void *dev_id)
-- 
2.17.1

Re: [PATCH net-next 0/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 6 Aug 2021 01:55:30 +0300 you wrote:
> hi
> 
> The intention of this series is to fully enable hard irqs deferral feature
> (hrtimers based HW IRQ coalescing) from Eric Dumazet [1] for TI K3 CPSW driver
> by using napi_complete_done() in TX completion path, so the combination of
> parameters (/sys/class/net/ethX/):
>  napi_defer_hard_irqs
>  gro_flush_timeout
> can be used for hard irqs deferral.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net: ti: am65-cpsw-nuss: fix RX IRQ state after .ndo_stop()
    https://git.kernel.org/netdev/net-next/c/47bfc4d128de
  - [net-next,2/2] net: ethernet: ti: am65-cpsw: use napi_complete_done() in TX completion
    https://git.kernel.org/netdev/net-next/c/3bacbe04251b

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