[PATCH net-next 0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

[PATCH net-next 0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

[Paul Barker]

This series aims to improve peformance of the GbEth IP in the Renesas
RZ/G2L SoC family and the RZ/G3S SoC, which use the ravb driver. Along
the way, we do some refactoring and ensure that napi_complete_done() is
used in accordance with the NAPI documentation.

Performance improvment mainly comes from enabling SW IRQ Coalescing for
all SoCs using the GbEth IP, and NAPI Threaded mode for single core SoCs
using the GbEth IP. These can be enabled/disabled at runtime via sysfs,
but our goal is to set sensible defaults which get good performance on
the affected SoCs.

Changes are made specific to the GbEth IP, avoiding potential impact on
the other Renesas R-Car based SoCs which also use the ravb driver. This
follows the principle of only submitting patches that we can fully test.

The performance impact of this series on iperf3 testing is as follows:
  * RZ/G2L Ethernet throughput is unchanged, but CPU usage drops:
      * Bidirectional and TCP RX: 6.5% less CPU usage
      * UDP RX: 10% less CPU usage

  * RZ/G2UL and RZ/G3S Ethernet throughput is increased for all test
    cases except UDP TX, which suffers a slight loss:
      * TCP TX: 32% more throughput
      * TCP RX: 11% more throughput
      * UDP TX: 10% less throughput
      * UDP RX: 10183% more throughput - the previous throughput of
        1.06Mbps is what prompted this work.

  * RZ/G2N CPU usage and Ethernet throughput is unchanged (tested as a
    representative of the SoCs which use the R-Car based RAVB IP).

This series depends on:
  * "net: ravb: Let IP-specific receive function to interrogate descriptors" v5
    https://lore.kernel.org/all/20240131084133.1671440-2-claudiu.beznea.uj@bp.renesas.com/

To get the results shown above, you'll also need:
  * "topology: Set capacity_freq_ref in all cases"
    https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/

  * "ravb: Add Rx checksum offload support" v2
    https://lore.kernel.org/all/20240124102115.132154-2-biju.das.jz@bp.renesas.com/

  * "ravb: Add Tx checksum offload support" v2
    https://lore.kernel.org/all/20240124102115.132154-3-biju.das.jz@bp.renesas.com/

Work in this area will continue, in particular we expect to improve
TCP/UDP RX performance further with future changes to RX buffer
handling.

Paul Barker (8):
  net: ravb: Split R-Car & GbEth poll functions
  net: ravb: Simplify GbEth poll & receive functions
  net: ravb: Count packets in GbEth RX (not descriptors)
  net: ravb: Always process TX descriptor ring in GbEth poll
  net: ravb: Always update error counters
  net: ravb: Align GbEth poll function with NAPI docs
  net: ravb: Enable SW IRQ Coalescing for GbEth
  net: ravb: Use NAPI threaded mode on 1-core CPUs with GbEth IP

 drivers/net/ethernet/renesas/ravb.h      |   3 +-
 drivers/net/ethernet/renesas/ravb_main.c | 103 ++++++++++++++++-------
 2 files changed, 74 insertions(+), 32 deletions(-)

-- 
2.39.2

[PATCH net-next 1/8] net: ravb: Split R-Car & GbEth poll functions

[Paul Barker]

This allows us to optimise polling and receive processing for the GbEth
IP used in the RZ/G2L Soc family, without affecting the wider family of
R-Car based devices.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  2 +-
 drivers/net/ethernet/renesas/ravb_main.c | 74 ++++++++++++++++++------
 2 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 3cf869fb9a68..9e9cf0d36d72 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1050,7 +1050,7 @@ struct ravb_hw_info {
 	void (*rx_ring_free)(struct net_device *ndev, int q);
 	void (*rx_ring_format)(struct net_device *ndev, int q);
 	void *(*alloc_rx_desc)(struct net_device *ndev, int q);
-	bool (*receive)(struct net_device *ndev, int *quota, int q);
+	int (*poll)(struct napi_struct *napi, int budget);
 	void (*set_rate)(struct net_device *ndev);
 	int (*set_feature)(struct net_device *ndev, netdev_features_t features);
 	int (*dmac_init)(struct net_device *ndev);
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4976ecc91cde..c9464c1731b4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1069,15 +1069,6 @@ static bool ravb_rx_rcar(struct net_device *ndev, int *quota, int q)
 	return boguscnt <= 0;
 }
 
-/* Packet receive function for Ethernet AVB */
-static bool ravb_rx(struct net_device *ndev, int *quota, int q)
-{
-	struct ravb_private *priv = netdev_priv(ndev);
-	const struct ravb_hw_info *info = priv->info;
-
-	return info->receive(ndev, quota, q);
-}
-
 static void ravb_rcv_snd_disable(struct net_device *ndev)
 {
 	/* Disable TX and RX */
@@ -1345,7 +1336,56 @@ static irqreturn_t ravb_nc_interrupt(int irq, void *dev_id)
 	return ravb_dma_interrupt(irq, dev_id, RAVB_NC);
 }
 
-static int ravb_poll(struct napi_struct *napi, int budget)
+static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
+{
+	struct net_device *ndev = napi->dev;
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+	unsigned long flags;
+	int q = napi - priv->napi;
+	int mask = BIT(q);
+	int quota = budget;
+
+	/* Processing RX Descriptor Ring */
+	/* Clear RX interrupt */
+	ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
+	if (ravb_rx_gbeth(ndev, &quota, q))
+		goto out;
+
+	/* Processing TX Descriptor Ring */
+	spin_lock_irqsave(&priv->lock, flags);
+	/* Clear TX interrupt */
+	ravb_write(ndev, ~(mask | TIS_RESERVED), TIS);
+	ravb_tx_free(ndev, q, true);
+	netif_wake_subqueue(ndev, q);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	napi_complete(napi);
+
+	/* Re-enable RX/TX interrupts */
+	spin_lock_irqsave(&priv->lock, flags);
+	if (!info->irq_en_dis) {
+		ravb_modify(ndev, RIC0, mask, mask);
+		ravb_modify(ndev, TIC,  mask, mask);
+	} else {
+		ravb_write(ndev, mask, RIE0);
+		ravb_write(ndev, mask, TIE);
+	}
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	/* Receive error message handling */
+	priv->rx_over_errors =  priv->stats[RAVB_BE].rx_over_errors;
+	if (info->nc_queues)
+		priv->rx_over_errors += priv->stats[RAVB_NC].rx_over_errors;
+	if (priv->rx_over_errors != ndev->stats.rx_over_errors)
+		ndev->stats.rx_over_errors = priv->rx_over_errors;
+	if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors)
+		ndev->stats.rx_fifo_errors = priv->rx_fifo_errors;
+out:
+	return budget - quota;
+}
+
+static int ravb_poll_rcar(struct napi_struct *napi, int budget)
 {
 	struct net_device *ndev = napi->dev;
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -1358,7 +1398,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 	/* Processing RX Descriptor Ring */
 	/* Clear RX interrupt */
 	ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
-	if (ravb_rx(ndev, &quota, q))
+	if (ravb_rx_rcar(ndev, &quota, q))
 		goto out;
 
 	/* Processing TX Descriptor Ring */
@@ -2559,7 +2599,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free_rcar,
 	.rx_ring_format = ravb_rx_ring_format_rcar,
 	.alloc_rx_desc = ravb_alloc_rx_desc_rcar,
-	.receive = ravb_rx_rcar,
+	.poll = ravb_poll_rcar,
 	.set_rate = ravb_set_rate_rcar,
 	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_dmac_init_rcar,
@@ -2585,7 +2625,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free_rcar,
 	.rx_ring_format = ravb_rx_ring_format_rcar,
 	.alloc_rx_desc = ravb_alloc_rx_desc_rcar,
-	.receive = ravb_rx_rcar,
+	.poll = ravb_poll_rcar,
 	.set_rate = ravb_set_rate_rcar,
 	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_dmac_init_rcar,
@@ -2608,7 +2648,7 @@ static const struct ravb_hw_info ravb_rzv2m_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free_rcar,
 	.rx_ring_format = ravb_rx_ring_format_rcar,
 	.alloc_rx_desc = ravb_alloc_rx_desc_rcar,
-	.receive = ravb_rx_rcar,
+	.poll = ravb_poll_rcar,
 	.set_rate = ravb_set_rate_rcar,
 	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_dmac_init_rcar,
@@ -2633,7 +2673,7 @@ static const struct ravb_hw_info gbeth_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free_gbeth,
 	.rx_ring_format = ravb_rx_ring_format_gbeth,
 	.alloc_rx_desc = ravb_alloc_rx_desc_gbeth,
-	.receive = ravb_rx_gbeth,
+	.poll = ravb_poll_gbeth,
 	.set_rate = ravb_set_rate_gbeth,
 	.set_feature = ravb_set_features_gbeth,
 	.dmac_init = ravb_dmac_init_gbeth,
@@ -2990,9 +3030,9 @@ static int ravb_probe(struct platform_device *pdev)
 		goto out_dma_free;
 	}
 
-	netif_napi_add(ndev, &priv->napi[RAVB_BE], ravb_poll);
+	netif_napi_add(ndev, &priv->napi[RAVB_BE], info->poll);
 	if (info->nc_queues)
-		netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll);
+		netif_napi_add(ndev, &priv->napi[RAVB_NC], info->poll);
 
 	/* Network device register */
 	error = register_netdev(ndev);
-- 
2.39.2

[PATCH net-next 2/8] net: ravb: Simplify GbEth poll & receive functions

[Paul Barker]

We don't need to pass the work budget to ravb_rx_gbeth() by reference,
it's cleaner to pass this by value and return the amount of work done.
This allows us to simplify the ravb_poll_gbeth() function and use the
common `work_done` variable name seen in other network drivers for
consistency and ease of understanding.

In ravb_rx_gbeth(), we can also drop the confusingly named `boguscnt`
variable and use a for loop to iterate through descriptors.

This is a pure refactor and should not affect behaviour.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 25 ++++++++++--------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index c9464c1731b4..5a6a2a7a9e7a 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -822,7 +822,7 @@ static struct sk_buff *ravb_get_skb_gbeth(struct net_device *ndev, int entry,
 }
 
 /* Packet receive function for Gigabit Ethernet */
-static bool ravb_rx_gbeth(struct net_device *ndev, int *quota, int q)
+static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
@@ -831,28 +831,24 @@ static bool ravb_rx_gbeth(struct net_device *ndev, int *quota, int q)
 	struct sk_buff *skb;
 	dma_addr_t dma_addr;
 	u8  desc_status;
-	int boguscnt;
 	u16 pkt_len;
 	u8  die_dt;
 	int entry;
 	int limit;
+	int i;
 
 	entry = priv->cur_rx[q] % priv->num_rx_ring[q];
-	boguscnt = priv->dirty_rx[q] + priv->num_rx_ring[q] - priv->cur_rx[q];
+	limit = priv->dirty_rx[q] + priv->num_rx_ring[q] - priv->cur_rx[q];
 	stats = &priv->stats[q];
 
-	boguscnt = min(boguscnt, *quota);
-	limit = boguscnt;
+	limit = min(limit, budget);
 	desc = &priv->gbeth_rx_ring[entry];
-	while (desc->die_dt != DT_FEMPTY) {
+	for (i = 0; i < limit && desc->die_dt != DT_FEMPTY; i++) {
 		/* Descriptor type must be checked before all other reads */
 		dma_rmb();
 		desc_status = desc->msc;
 		pkt_len = le16_to_cpu(desc->ds_cc) & RX_DS;
 
-		if (--boguscnt < 0)
-			break;
-
 		/* We use 0-byte descriptors to mark the DMA mapping errors */
 		if (!pkt_len)
 			continue;
@@ -949,9 +945,7 @@ static bool ravb_rx_gbeth(struct net_device *ndev, int *quota, int q)
 		desc->die_dt = DT_FEMPTY;
 	}
 
-	*quota -= limit - (++boguscnt);
-
-	return boguscnt <= 0;
+	return i;
 }
 
 /* Packet receive function for Ethernet AVB */
@@ -1344,12 +1338,13 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	unsigned long flags;
 	int q = napi - priv->napi;
 	int mask = BIT(q);
-	int quota = budget;
+	int work_done;
 
 	/* Processing RX Descriptor Ring */
 	/* Clear RX interrupt */
 	ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
-	if (ravb_rx_gbeth(ndev, &quota, q))
+	work_done = ravb_rx_gbeth(ndev, budget, q);
+	if (work_done == budget)
 		goto out;
 
 	/* Processing TX Descriptor Ring */
@@ -1382,7 +1377,7 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors)
 		ndev->stats.rx_fifo_errors = priv->rx_fifo_errors;
 out:
-	return budget - quota;
+	return work_done;
 }
 
 static int ravb_poll_rcar(struct napi_struct *napi, int budget)
-- 
2.39.2

[PATCH net-next 3/8] net: ravb: Count packets in GbEth RX (not descriptors)

[Paul Barker]

The units of "work done" in the RX path should be packets instead of
descriptors, as large packets can be spread over multiple descriptors.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 5a6a2a7a9e7a..077c189bcc6f 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -830,6 +830,7 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
 	struct ravb_rx_desc *desc;
 	struct sk_buff *skb;
 	dma_addr_t dma_addr;
+	int rx_packets = 0;
 	u8  desc_status;
 	u16 pkt_len;
 	u8  die_dt;
@@ -841,9 +842,8 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
 	limit = priv->dirty_rx[q] + priv->num_rx_ring[q] - priv->cur_rx[q];
 	stats = &priv->stats[q];
 
-	limit = min(limit, budget);
 	desc = &priv->gbeth_rx_ring[entry];
-	for (i = 0; i < limit && desc->die_dt != DT_FEMPTY; i++) {
+	for (i = 0; i < limit && rx_packets < budget && desc->die_dt != DT_FEMPTY; i++) {
 		/* Descriptor type must be checked before all other reads */
 		dma_rmb();
 		desc_status = desc->msc;
@@ -876,7 +876,7 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
 				if (ndev->features & NETIF_F_RXCSUM)
 					ravb_rx_csum_gbeth(skb);
 				napi_gro_receive(&priv->napi[q], skb);
-				stats->rx_packets++;
+				rx_packets++;
 				stats->rx_bytes += pkt_len;
 				break;
 			case DT_FSTART:
@@ -906,7 +906,7 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
 					ravb_rx_csum_gbeth(skb);
 				napi_gro_receive(&priv->napi[q],
 						 priv->rx_1st_skb);
-				stats->rx_packets++;
+				rx_packets++;
 				stats->rx_bytes += pkt_len;
 				break;
 			}
@@ -945,7 +945,8 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
 		desc->die_dt = DT_FEMPTY;
 	}
 
-	return i;
+	stats->rx_packets += rx_packets;
+	return rx_packets;
 }
 
 /* Packet receive function for Ethernet AVB */
-- 
2.39.2

[PATCH net-next 4/8] net: ravb: Always process TX descriptor ring in GbEth poll

[Paul Barker]

The TX queue should be serviced each time the poll function is called,
even if the full RX work budget has been consumed. This prevents
starvation of the TX queue when RX bandwidth usage is high.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 077c189bcc6f..9b0ed269e69b 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1345,8 +1345,6 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	/* Clear RX interrupt */
 	ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
 	work_done = ravb_rx_gbeth(ndev, budget, q);
-	if (work_done == budget)
-		goto out;
 
 	/* Processing TX Descriptor Ring */
 	spin_lock_irqsave(&priv->lock, flags);
@@ -1356,6 +1354,9 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	netif_wake_subqueue(ndev, q);
 	spin_unlock_irqrestore(&priv->lock, flags);
 
+	if (work_done == budget)
+		goto out;
+
 	napi_complete(napi);
 
 	/* Re-enable RX/TX interrupts */
-- 
2.39.2

[PATCH net-next 5/8] net: ravb: Always update error counters

[Paul Barker]

The error statistics should be updated each time the poll function is
called, even if the full RX work budget has been consumed. This prevents
the counts from becoming stuck when RX bandwidth usage is high.

This also ensures that error counters are not updated after we've
re-enabled interrupts as that could result in a race condition.

Also drop an unnecessary space.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9b0ed269e69b..a90e70b25875 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1354,6 +1354,15 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	netif_wake_subqueue(ndev, q);
 	spin_unlock_irqrestore(&priv->lock, flags);
 
+	/* Receive error message handling */
+	priv->rx_over_errors = priv->stats[RAVB_BE].rx_over_errors;
+	if (info->nc_queues)
+		priv->rx_over_errors += priv->stats[RAVB_NC].rx_over_errors;
+	if (priv->rx_over_errors != ndev->stats.rx_over_errors)
+		ndev->stats.rx_over_errors = priv->rx_over_errors;
+	if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors)
+		ndev->stats.rx_fifo_errors = priv->rx_fifo_errors;
+
 	if (work_done == budget)
 		goto out;
 
@@ -1370,14 +1379,6 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	}
 	spin_unlock_irqrestore(&priv->lock, flags);
 
-	/* Receive error message handling */
-	priv->rx_over_errors =  priv->stats[RAVB_BE].rx_over_errors;
-	if (info->nc_queues)
-		priv->rx_over_errors += priv->stats[RAVB_NC].rx_over_errors;
-	if (priv->rx_over_errors != ndev->stats.rx_over_errors)
-		ndev->stats.rx_over_errors = priv->rx_over_errors;
-	if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors)
-		ndev->stats.rx_fifo_errors = priv->rx_fifo_errors;
 out:
 	return work_done;
 }
-- 
2.39.2

[PATCH net-next 6/8] net: ravb: Align GbEth poll function with NAPI docs

[Paul Barker]

Call napi_complete_done() in accordance with the documentation in
`Documentation/networking/napi.rst`.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 26 ++++++++++--------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index a90e70b25875..c3a5e4bb61f9 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1363,23 +1363,19 @@ static int ravb_poll_gbeth(struct napi_struct *napi, int budget)
 	if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors)
 		ndev->stats.rx_fifo_errors = priv->rx_fifo_errors;
 
-	if (work_done == budget)
-		goto out;
-
-	napi_complete(napi);
-
-	/* Re-enable RX/TX interrupts */
-	spin_lock_irqsave(&priv->lock, flags);
-	if (!info->irq_en_dis) {
-		ravb_modify(ndev, RIC0, mask, mask);
-		ravb_modify(ndev, TIC,  mask, mask);
-	} else {
-		ravb_write(ndev, mask, RIE0);
-		ravb_write(ndev, mask, TIE);
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
+		/* Re-enable RX/TX interrupts */
+		spin_lock_irqsave(&priv->lock, flags);
+		if (!info->irq_en_dis) {
+			ravb_modify(ndev, RIC0, mask, mask);
+			ravb_modify(ndev, TIC,  mask, mask);
+		} else {
+			ravb_write(ndev, mask, RIE0);
+			ravb_write(ndev, mask, TIE);
+		}
+		spin_unlock_irqrestore(&priv->lock, flags);
 	}
-	spin_unlock_irqrestore(&priv->lock, flags);
 
-out:
 	return work_done;
 }
 
-- 
2.39.2

[PATCH net-next 7/8] net: ravb: Enable SW IRQ Coalescing for GbEth

[Paul Barker]

Software IRQ Coalescing is required to improve network stack performance
in the RZ/G2L SoC family and the RZ/G3S SoC, i.e. the SoCs which use the
GbEth IP.

For the RZ/G2L, network throughput is comparable before and after this
change. CPU usage during TCP RX testing dropped by 6.5% and during UDP
RX testing dropped by 10%.

For the RZ/G2UL, network throughput is generally increased by this
change (results obtained with iperf3):
  * TCP TX: 2.9% more throughput
  * TCP RX: 1.1% more throughput
  * UDP TX: similar throughput
  * UDP RX: 41500% more throughput

For the RZ/G3S we see improvements in network throughput similar to the
RZ/G2UL.

The improvement of UDP RX bandwidth for the single core SoCs (RZ/G2UL &
RZ/G3S) is particularly critical.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      | 1 +
 drivers/net/ethernet/renesas/ravb_main.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 9e9cf0d36d72..da2c09b14389 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1078,6 +1078,7 @@ struct ravb_hw_info {
 	unsigned nc_queues:1;		/* AVB-DMAC has RX and TX NC queues */
 	unsigned magic_pkt:1;		/* E-MAC supports magic packet detection */
 	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
+	unsigned needs_irq_coalesce:1;	/* Requires SW IRQ Coalescing to achieve best performance */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index c3a5e4bb61f9..483993ec25ba 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2684,6 +2684,7 @@ static const struct ravb_hw_info gbeth_hw_info = {
 	.tx_counters = 1,
 	.carrier_counters = 1,
 	.half_duplex = 1,
+	.needs_irq_coalesce = 1,
 };
 
 static const struct of_device_id ravb_match_table[] = {
@@ -3028,6 +3029,9 @@ static int ravb_probe(struct platform_device *pdev)
 	if (info->nc_queues)
 		netif_napi_add(ndev, &priv->napi[RAVB_NC], info->poll);
 
+	if (info->needs_irq_coalesce)
+		netdev_sw_irq_coalesce_default_on(ndev);
+
 	/* Network device register */
 	error = register_netdev(ndev);
 	if (error)
-- 
2.39.2

[PATCH net-next 8/8] net: ravb: Use NAPI threaded mode on 1-core CPUs with GbEth IP

[Paul Barker]

NAPI Threaded mode (along with the previously enabled SW IRQ Coalescing)
is required to improve network stack performance for single core SoCs
using the GbEth IP (currently the RZ/G2L SoC family and the RZ/G3S SoC).

For the RZ/G2UL, network throughput is increased by this change for all
test cases except UDP TX (results obtained with iperf3):
  * TCP TX: 30% more throughput
  * TCP RX: 9.8% more throughput
  * UDP TX: 9.7% less throughput
  * UDP RX: 89% more throughput

For the RZ/G3S we see improvements in network throughput similar to the
RZ/G2UL.

The improvement of UDP RX bandwidth for the single core SoCs (RZ/G2UL &
RZ/G3S) is particularly critical. NAPI Threaded mode can be disabled at
runtime via sysfs for applications where UDP TX performance is a
priority.

Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 483993ec25ba..202a3229d436 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -3029,8 +3029,11 @@ static int ravb_probe(struct platform_device *pdev)
 	if (info->nc_queues)
 		netif_napi_add(ndev, &priv->napi[RAVB_NC], info->poll);
 
-	if (info->needs_irq_coalesce)
+	if (info->needs_irq_coalesce) {
 		netdev_sw_irq_coalesce_default_on(ndev);
+		if (num_present_cpus() == 1)
+			dev_set_threaded(ndev, true);
+	}
 
 	/* Network device register */
 	error = register_netdev(ndev);
-- 
2.39.2

Re: [PATCH net-next 0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

[Andrew Lunn]

> Changes are made specific to the GbEth IP, avoiding potential impact on
> the other Renesas R-Car based SoCs which also use the ravb driver. This
> follows the principle of only submitting patches that we can fully test.
 
Are you saying that Renesas does not have access to all Renesas RDKs?

I don't particularly like the way your first patch makes a copy of
shared functions. Is it not likely that R-Car would also benefit from
this?

	Andrew

Re: [PATCH net-next 0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

[Sergey Shtylyov]

On 1/31/24 8:05 PM, Paul Barker wrote:

> This series aims to improve peformance of the GbEth IP in the Renesas

   Performance.

> RZ/G2L SoC family and the RZ/G3S SoC, which use the ravb driver. Along
> the way, we do some refactoring and ensure that napi_complete_done() is
> used in accordance with the NAPI documentation.
> 
> Performance improvment mainly comes from enabling SW IRQ Coalescing for

   Improvement.

> all SoCs using the GbEth IP, and NAPI Threaded mode for single core SoCs
> using the GbEth IP. These can be enabled/disabled at runtime via sysfs,
> but our goal is to set sensible defaults which get good performance on
> the affected SoCs.
> 
> Changes are made specific to the GbEth IP, avoiding potential impact on
> the other Renesas R-Car based SoCs which also use the ravb driver. This
> follows the principle of only submitting patches that we can fully test.
> 
> The performance impact of this series on iperf3 testing is as follows:
>   * RZ/G2L Ethernet throughput is unchanged, but CPU usage drops:
>       * Bidirectional and TCP RX: 6.5% less CPU usage
>       * UDP RX: 10% less CPU usage
> 
>   * RZ/G2UL and RZ/G3S Ethernet throughput is increased for all test
>     cases except UDP TX, which suffers a slight loss:
>       * TCP TX: 32% more throughput
>       * TCP RX: 11% more throughput
>       * UDP TX: 10% less throughput
>       * UDP RX: 10183% more throughput - the previous throughput of

   10183%, really? 8-)

>         1.06Mbps is what prompted this work.
> 
>   * RZ/G2N CPU usage and Ethernet throughput is unchanged (tested as a
>     representative of the SoCs which use the R-Car based RAVB IP).
> 
> This series depends on:
>   * "net: ravb: Let IP-specific receive function to interrogate descriptors" v5
>     https://lore.kernel.org/all/20240131084133.1671440-2-claudiu.beznea.uj@bp.renesas.com/

   So this is based on yet unmerged patch?
   Dave, Jakub, et al., the series should be considered RFC then.

> To get the results shown above, you'll also need:
>   * "topology: Set capacity_freq_ref in all cases"
>     https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/
> 
>   * "ravb: Add Rx checksum offload support" v2
>     https://lore.kernel.org/all/20240124102115.132154-2-biju.das.jz@bp.renesas.com/
> 
>   * "ravb: Add Tx checksum offload support" v2
>     https://lore.kernel.org/all/20240124102115.132154-3-biju.das.jz@bp.renesas.com/

   Those 2 are not finalized yet...

> Work in this area will continue, in particular we expect to improve
> TCP/UDP RX performance further with future changes to RX buffer
> handling.
> 
> Paul Barker (8):
>   net: ravb: Split R-Car & GbEth poll functions
>   net: ravb: Simplify GbEth poll & receive functions
>   net: ravb: Count packets in GbEth RX (not descriptors)
>   net: ravb: Always process TX descriptor ring in GbEth poll
>   net: ravb: Always update error counters
>   net: ravb: Align GbEth poll function with NAPI docs
>   net: ravb: Enable SW IRQ Coalescing for GbEth
>   net: ravb: Use NAPI threaded mode on 1-core CPUs with GbEth IP
> 
>  drivers/net/ethernet/renesas/ravb.h      |   3 +-
>  drivers/net/ethernet/renesas/ravb_main.c | 103 ++++++++++++++++-------
>  2 files changed, 74 insertions(+), 32 deletions(-)

MBR, Sergey

Re: [PATCH net-next 0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

[Paul Barker]

[-- Attachment #1.1.1: Type: text/plain, Size: 1229 bytes --]

On 31/01/2024 18:26, Andrew Lunn wrote:
>> Changes are made specific to the GbEth IP, avoiding potential impact on
>> the other Renesas R-Car based SoCs which also use the ravb driver. This
>> follows the principle of only submitting patches that we can fully test.
>  
> Are you saying that Renesas does not have access to all Renesas RDKs?
> 
> I don't particularly like the way your first patch makes a copy of
> shared functions. Is it not likely that R-Car would also benefit from
> this?

We have the required RDKs. For the R-Car based SoCs, we need to confirm
that gPTP still works if we change the poll/receive code paths - this
will require an AVB-capable network switch and additional time to test.
So our plan was to handle the GbEth code paths first without affecting
R-Car, then follow up with another patch set for the R-Car code paths
when we've done the required tests.

I discussed this with our team, and we're happy to do this in one go for
both R-Car and GbEth code paths if that's preferred. I'll send the
patches as an RFC (as Sergey has commented it should be an RFC anyway as
it depends on an unmerged patch) and we'll do the gPTP test with a
couple of R-Car boards.

Thanks,
Paul

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3577 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net-next 0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

[Andrew Lunn]

On Fri, Feb 02, 2024 at 09:39:42AM +0000, Paul Barker wrote:
> On 31/01/2024 18:26, Andrew Lunn wrote:
> >> Changes are made specific to the GbEth IP, avoiding potential impact on
> >> the other Renesas R-Car based SoCs which also use the ravb driver. This
> >> follows the principle of only submitting patches that we can fully test.
> >  
> > Are you saying that Renesas does not have access to all Renesas RDKs?
> > 
> > I don't particularly like the way your first patch makes a copy of
> > shared functions. Is it not likely that R-Car would also benefit from
> > this?
> 
> We have the required RDKs. For the R-Car based SoCs, we need to confirm
> that gPTP still works if we change the poll/receive code paths - this
> will require an AVB-capable network switch and additional time to test.
> So our plan was to handle the GbEth code paths first without affecting
> R-Car, then follow up with another patch set for the R-Car code paths
> when we've done the required tests.
> 
> I discussed this with our team, and we're happy to do this in one go for
> both R-Car and GbEth code paths if that's preferred.

Hi Paul

I think it would be simpler, since you would then need to recombine
the code paths you have just split. Its better to not split them in
the first place if possible.

    Andrew