[PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

[PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

[Russell King (Oracle)]

Hi,

With reference to
https://forum.turris.cz/t/random-kernel-exceptions-on-hbl-tos-7-0/18865/

It appears that mvneta attempts an order-6 allocation for the TSO
header memory. While this succeeds early on in the system's life time,
trying order-6 allocations later can result in failure due to memory
fragmentation.

Firstly, the reason it's so large is that we take the number of
transmit descriptors, and allocate a TSO header buffer for each, and
each TSO header is 256 bytes. The driver uses a simple mechanism to
determine the address - it uses the transmit descriptor index as an
index into the TSO header memory.

	(The first obvious question is: do there need to be this
	many? Won't each TSO header always have at least one bit
	of data to go with it? In other words, wouldn't the maximum
	number of TSO headers that a ring could accept be the number
	of ring entries divided by 2?)

There is no real need for this memory to be an order-6 allocation,
since nothing in hardware requires this buffer to be contiguous.

Therefore, this series splits this order-6 allocation up into 32
order-1 allocations (8k pages on 4k page platforms), each giving
32 TSO headers per page.

In order to do this, these patches:

1) fix a horrible transmit path error-cleanup bug - the existing
   code unmaps from the first descriptor that was allocated at
   interface bringup, not the first descriptor that the packet
   is using, resulting in the wrong descriptors being unmapped.

2) since xdp support was added, we now have buf->type which indicates
   what this transmit buffer contains. Use this to mark TSO header
   buffers.

3) get rid of IS_TSO_HEADER(), instead using buf->type to determine
   whether this transmit buffer needs to be DMA-unmapped.

4) move tso_build_hdr() into mvneta_tso_put_hdr() to keep all the
   TSO header building code together.

5) split the TSO header allocation into chunks of order-1 pages.

 drivers/net/ethernet/marvell/mvneta.c | 166 +++++++++++++++++++++++-----------
 1 file changed, 115 insertions(+), 51 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH RFC net-next 1/5] net: mvneta: fix transmit path dma-unmapping on error

[Russell King (Oracle)]

The transmit code assumes that the transmit descriptors that are used
begin with the first descriptor in the ring, but this may not be the
case. Fix this by providing a new function that dma-unmaps a range of
numbered descriptor entries, and use that to do the unmapping.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 53 +++++++++++++++++----------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 2cad76d0a50e..62400ff61e34 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2714,14 +2714,40 @@ mvneta_tso_put_data(struct net_device *dev, struct mvneta_tx_queue *txq,
 	return 0;
 }
 
+static void mvneta_release_descs(struct mvneta_port *pp,
+				 struct mvneta_tx_queue *txq,
+				 int first, int num)
+{
+	int desc_idx, i;
+
+	desc_idx = first + num;
+	if (desc_idx >= txq->size)
+		desc_idx -= txq->size;
+
+	for (i = num; i >= 0; i--) {
+		struct mvneta_tx_desc *tx_desc = txq->descs + desc_idx;
+
+		if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr))
+			dma_unmap_single(pp->dev->dev.parent,
+					 tx_desc->buf_phys_addr,
+					 tx_desc->data_size,
+					 DMA_TO_DEVICE);
+
+		mvneta_txq_desc_put(txq);
+
+		if (desc_idx == 0)
+			desc_idx = txq->size;
+		desc_idx -= 1;
+	}
+}
+
 static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
 			 struct mvneta_tx_queue *txq)
 {
 	int hdr_len, total_len, data_left;
-	int desc_count = 0;
+	int first_desc, desc_count = 0;
 	struct mvneta_port *pp = netdev_priv(dev);
 	struct tso_t tso;
-	int i;
 
 	/* Count needed descriptors */
 	if ((txq->count + tso_count_descs(skb)) >= txq->size)
@@ -2732,6 +2758,8 @@ static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
 		return 0;
 	}
 
+	first_desc = txq->txq_put_index;
+
 	/* Initialize the TSO handler, and prepare the first payload */
 	hdr_len = tso_start(skb, &tso);
 
@@ -2772,15 +2800,7 @@ static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
 	/* Release all used data descriptors; header descriptors must not
 	 * be DMA-unmapped.
 	 */
-	for (i = desc_count - 1; i >= 0; i--) {
-		struct mvneta_tx_desc *tx_desc = txq->descs + i;
-		if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr))
-			dma_unmap_single(pp->dev->dev.parent,
-					 tx_desc->buf_phys_addr,
-					 tx_desc->data_size,
-					 DMA_TO_DEVICE);
-		mvneta_txq_desc_put(txq);
-	}
+	mvneta_release_descs(pp, txq, first_desc, desc_count - 1);
 	return 0;
 }
 
@@ -2790,6 +2810,7 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
 {
 	struct mvneta_tx_desc *tx_desc;
 	int i, nr_frags = skb_shinfo(skb)->nr_frags;
+	int first_desc = txq->txq_put_index;
 
 	for (i = 0; i < nr_frags; i++) {
 		struct mvneta_tx_buf *buf = &txq->buf[txq->txq_put_index];
@@ -2828,15 +2849,7 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
 	/* Release all descriptors that were used to map fragments of
 	 * this packet, as well as the corresponding DMA mappings
 	 */
-	for (i = i - 1; i >= 0; i--) {
-		tx_desc = txq->descs + i;
-		dma_unmap_single(pp->dev->dev.parent,
-				 tx_desc->buf_phys_addr,
-				 tx_desc->data_size,
-				 DMA_TO_DEVICE);
-		mvneta_txq_desc_put(txq);
-	}
-
+	mvneta_release_descs(pp, txq, first_desc, i - 1);
 	return -ENOMEM;
 }
 
-- 
2.30.2

[PATCH RFC net-next 2/5] net: mvneta: mark mapped and tso buffers separately

[Russell King (Oracle)]

Mark dma-mapped skbs and TSO buffers separately, so we can use
buf->type to identify their differences.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 62400ff61e34..c05649f33d18 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -638,6 +638,7 @@ struct mvneta_rx_desc {
 #endif
 
 enum mvneta_tx_buf_type {
+	MVNETA_TYPE_TSO,
 	MVNETA_TYPE_SKB,
 	MVNETA_TYPE_XDP_TX,
 	MVNETA_TYPE_XDP_NDO,
@@ -1883,7 +1884,8 @@ static void mvneta_txq_bufs_free(struct mvneta_port *pp,
 			dma_unmap_single(pp->dev->dev.parent,
 					 tx_desc->buf_phys_addr,
 					 tx_desc->data_size, DMA_TO_DEVICE);
-		if (buf->type == MVNETA_TYPE_SKB && buf->skb) {
+		if ((buf->type == MVNETA_TYPE_TSO ||
+		     buf->type == MVNETA_TYPE_SKB) && buf->skb) {
 			bytes_compl += buf->skb->len;
 			pkts_compl++;
 			dev_kfree_skb_any(buf->skb);
@@ -2674,7 +2676,7 @@ mvneta_tso_put_hdr(struct sk_buff *skb, struct mvneta_tx_queue *txq)
 	tx_desc->command |= MVNETA_TXD_F_DESC;
 	tx_desc->buf_phys_addr = txq->tso_hdrs_phys +
 				 txq->txq_put_index * TSO_HEADER_SIZE;
-	buf->type = MVNETA_TYPE_SKB;
+	buf->type = MVNETA_TYPE_TSO;
 	buf->skb = NULL;
 
 	mvneta_txq_inc_put(txq);
-- 
2.30.2

[PATCH RFC net-next 3/5] net: mvneta: use buf->type to determine whether to dma-unmap

[Russell King (Oracle)]

Now that we use a different buffer type for TSO headers, we can use
buf->type to determine whether the original buffer was DMA-mapped or
not. The rules are:

	MVNETA_TYPE_XDP_TX - from a DMA pool, no unmap is required
	MVNETA_TYPE_XDP_NDO - dma_map_single()'d
	MVNETA_TYPE_SKB - normal skbuff, dma_map_single()'d
	MVNETA_TYPE_TSO - from the TSO buffer area

This means we only need to call dma_unmap_single() on the XDP_NDO and
SKB types of buffer, and we no longer need the private IS_TSO_HEADER()
which relies on the TSO region being contiguously allocated.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index c05649f33d18..c23d75af65ee 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -364,10 +364,6 @@
 			 MVNETA_SKB_HEADROOM))
 #define MVNETA_MAX_RX_BUF_SIZE	(PAGE_SIZE - MVNETA_SKB_PAD)
 
-#define IS_TSO_HEADER(txq, addr) \
-	((addr >= txq->tso_hdrs_phys) && \
-	 (addr < txq->tso_hdrs_phys + txq->size * TSO_HEADER_SIZE))
-
 #define MVNETA_RX_GET_BM_POOL_ID(rxd) \
 	(((rxd)->status & MVNETA_RXD_BM_POOL_MASK) >> MVNETA_RXD_BM_POOL_SHIFT)
 
@@ -1879,8 +1875,8 @@ static void mvneta_txq_bufs_free(struct mvneta_port *pp,
 
 		mvneta_txq_inc_get(txq);
 
-		if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr) &&
-		    buf->type != MVNETA_TYPE_XDP_TX)
+		if (buf->type == MVNETA_TYPE_XDP_NDO ||
+		    buf->type == MVNETA_TYPE_SKB)
 			dma_unmap_single(pp->dev->dev.parent,
 					 tx_desc->buf_phys_addr,
 					 tx_desc->data_size, DMA_TO_DEVICE);
@@ -2728,8 +2724,9 @@ static void mvneta_release_descs(struct mvneta_port *pp,
 
 	for (i = num; i >= 0; i--) {
 		struct mvneta_tx_desc *tx_desc = txq->descs + desc_idx;
+		struct mvneta_tx_buf *buf = &txq->buf[desc_idx];
 
-		if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr))
+		if (buf->type == MVNETA_TYPE_SKB)
 			dma_unmap_single(pp->dev->dev.parent,
 					 tx_desc->buf_phys_addr,
 					 tx_desc->data_size,
-- 
2.30.2

[PATCH RFC net-next 4/5] net: mvneta: move tso_build_hdr() into mvneta_tso_put_hdr()

[Russell King (Oracle)]

Move tso_build_hdr() into mvneta_tso_put_hdr() so that all the TSO
header building code is in one place.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index c23d75af65ee..bea84e86cf99 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2659,19 +2659,24 @@ static int mvneta_rx_hwbm(struct napi_struct *napi,
 	return rx_done;
 }
 
-static inline void
-mvneta_tso_put_hdr(struct sk_buff *skb, struct mvneta_tx_queue *txq)
+static void mvneta_tso_put_hdr(struct sk_buff *skb, struct mvneta_tx_queue *txq,
+			       struct tso_t *tso, int size, bool is_last)
 {
 	struct mvneta_tx_buf *buf = &txq->buf[txq->txq_put_index];
-	int hdr_len = skb_tcp_all_headers(skb);
+	int tso_offset, hdr_len = skb_tcp_all_headers(skb);
 	struct mvneta_tx_desc *tx_desc;
+	char *hdr;
+
+	tso_offset = txq->txq_put_index * TSO_HEADER_SIZE;
+
+	hdr = txq->tso_hdrs + tso_offset;
+	tso_build_hdr(skb, hdr, tso, size, is_last);
 
 	tx_desc = mvneta_txq_next_desc_get(txq);
 	tx_desc->data_size = hdr_len;
 	tx_desc->command = mvneta_skb_tx_csum(skb);
 	tx_desc->command |= MVNETA_TXD_F_DESC;
-	tx_desc->buf_phys_addr = txq->tso_hdrs_phys +
-				 txq->txq_put_index * TSO_HEADER_SIZE;
+	tx_desc->buf_phys_addr = txq->tso_hdrs_phys + tso_offset;
 	buf->type = MVNETA_TYPE_TSO;
 	buf->skb = NULL;
 
@@ -2764,17 +2769,12 @@ static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
 
 	total_len = skb->len - hdr_len;
 	while (total_len > 0) {
-		char *hdr;
-
 		data_left = min_t(int, skb_shinfo(skb)->gso_size, total_len);
 		total_len -= data_left;
 		desc_count++;
 
 		/* prepare packet headers: MAC + IP + TCP */
-		hdr = txq->tso_hdrs + txq->txq_put_index * TSO_HEADER_SIZE;
-		tso_build_hdr(skb, hdr, &tso, data_left, total_len == 0);
-
-		mvneta_tso_put_hdr(skb, txq);
+		mvneta_tso_put_hdr(skb, txq, &tso, data_left, total_len == 0);
 
 		while (data_left > 0) {
 			int size;
-- 
2.30.2

[PATCH RFC net-next 5/5] net: mvneta: allocate TSO header DMA memory in chunks

[Russell King (Oracle)]

Now that we no longer need to check whether the DMA address is within
the TSO header DMA memory range for the queue, we can allocate the TSO
header DMA memory in chunks rather than one contiguous order-6 chunk,
which can stress the kernel's memory subsystems to allocate.

Instead, use order-1 (8k) allocations, which will result in 32 order-1
pages containing 32 TSO headers.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 88 +++++++++++++++++++++------
 1 file changed, 70 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index bea84e86cf99..6c6b66d3ea6e 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -344,6 +344,15 @@
 
 #define MVNETA_MAX_SKB_DESCS (MVNETA_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
 
+/* The size of a TSO header page */
+#define MVNETA_TSO_PAGE_SIZE (2 * PAGE_SIZE)
+
+/* Number of TSO headers per page. This should be a power of 2 */
+#define MVNETA_TSO_PER_PAGE (MVNETA_TSO_PAGE_SIZE / TSO_HEADER_SIZE)
+
+/* Maximum number of TSO header pages */
+#define MVNETA_MAX_TSO_PAGES (MVNETA_MAX_TXD / MVNETA_TSO_PER_PAGE)
+
 /* descriptor aligned size */
 #define MVNETA_DESC_ALIGNED_SIZE	32
 
@@ -687,10 +696,10 @@ struct mvneta_tx_queue {
 	int next_desc_to_proc;
 
 	/* DMA buffers for TSO headers */
-	char *tso_hdrs;
+	char *tso_hdrs[MVNETA_MAX_TSO_PAGES];
 
 	/* DMA address of TSO headers */
-	dma_addr_t tso_hdrs_phys;
+	dma_addr_t tso_hdrs_phys[MVNETA_MAX_TSO_PAGES];
 
 	/* Affinity mask for CPUs*/
 	cpumask_t affinity_mask;
@@ -2659,24 +2668,71 @@ static int mvneta_rx_hwbm(struct napi_struct *napi,
 	return rx_done;
 }
 
+static void mvneta_free_tso_hdrs(struct mvneta_port *pp,
+				 struct mvneta_tx_queue *txq)
+{
+	struct device *dev = pp->dev->dev.parent;
+	int i;
+
+	for (i = 0; i < MVNETA_MAX_TSO_PAGES; i++) {
+		if (txq->tso_hdrs[i]) {
+			dma_free_coherent(dev, MVNETA_TSO_PAGE_SIZE,
+					  txq->tso_hdrs[i],
+					  txq->tso_hdrs_phys[i]);
+			txq->tso_hdrs[i] = NULL;
+		}
+	}
+}
+
+static int mvneta_alloc_tso_hdrs(struct mvneta_port *pp,
+				 struct mvneta_tx_queue *txq)
+{
+	struct device *dev = pp->dev->dev.parent;
+	int i, num;
+
+	num = DIV_ROUND_UP(txq->size, MVNETA_TSO_PER_PAGE);
+	for (i = 0; i < num; i++) {
+		txq->tso_hdrs[i] = dma_alloc_coherent(dev, MVNETA_TSO_PAGE_SIZE,
+						      &txq->tso_hdrs_phys[i],
+						      GFP_KERNEL);
+		if (!txq->tso_hdrs[i]) {
+			mvneta_free_tso_hdrs(pp, txq);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
+static char *mvneta_get_tso_hdr(struct mvneta_tx_queue *txq, dma_addr_t *dma)
+{
+	int index, offset;
+
+	index = txq->txq_put_index / MVNETA_TSO_PER_PAGE;
+	offset = (txq->txq_put_index % MVNETA_TSO_PER_PAGE) * TSO_HEADER_SIZE;
+
+	*dma = txq->tso_hdrs_phys[index] + offset;
+
+	return txq->tso_hdrs[index] + offset;
+}
+
 static void mvneta_tso_put_hdr(struct sk_buff *skb, struct mvneta_tx_queue *txq,
 			       struct tso_t *tso, int size, bool is_last)
 {
 	struct mvneta_tx_buf *buf = &txq->buf[txq->txq_put_index];
-	int tso_offset, hdr_len = skb_tcp_all_headers(skb);
+	int hdr_len = skb_tcp_all_headers(skb);
 	struct mvneta_tx_desc *tx_desc;
+	dma_addr_t hdr_phys;
 	char *hdr;
 
-	tso_offset = txq->txq_put_index * TSO_HEADER_SIZE;
-
-	hdr = txq->tso_hdrs + tso_offset;
+	hdr = mvneta_get_tso_hdr(txq, &hdr_phys);
 	tso_build_hdr(skb, hdr, tso, size, is_last);
 
 	tx_desc = mvneta_txq_next_desc_get(txq);
 	tx_desc->data_size = hdr_len;
 	tx_desc->command = mvneta_skb_tx_csum(skb);
 	tx_desc->command |= MVNETA_TXD_F_DESC;
-	tx_desc->buf_phys_addr = txq->tso_hdrs_phys + tso_offset;
+	tx_desc->buf_phys_addr = hdr_phys;
 	buf->type = MVNETA_TYPE_TSO;
 	buf->skb = NULL;
 
@@ -3469,7 +3525,7 @@ static void mvneta_rxq_deinit(struct mvneta_port *pp,
 static int mvneta_txq_sw_init(struct mvneta_port *pp,
 			      struct mvneta_tx_queue *txq)
 {
-	int cpu;
+	int cpu, err;
 
 	txq->size = pp->tx_ring_size;
 
@@ -3494,11 +3550,9 @@ static int mvneta_txq_sw_init(struct mvneta_port *pp,
 		return -ENOMEM;
 
 	/* Allocate DMA buffers for TSO MAC/IP/TCP headers */
-	txq->tso_hdrs = dma_alloc_coherent(pp->dev->dev.parent,
-					   txq->size * TSO_HEADER_SIZE,
-					   &txq->tso_hdrs_phys, GFP_KERNEL);
-	if (!txq->tso_hdrs)
-		return -ENOMEM;
+	err = mvneta_alloc_tso_hdrs(pp, txq);
+	if (err)
+		return err;
 
 	/* Setup XPS mapping */
 	if (pp->neta_armada3700)
@@ -3550,10 +3604,7 @@ static void mvneta_txq_sw_deinit(struct mvneta_port *pp,
 
 	kfree(txq->buf);
 
-	if (txq->tso_hdrs)
-		dma_free_coherent(pp->dev->dev.parent,
-				  txq->size * TSO_HEADER_SIZE,
-				  txq->tso_hdrs, txq->tso_hdrs_phys);
+	mvneta_free_tso_hdrs(pp, txq);
 	if (txq->descs)
 		dma_free_coherent(pp->dev->dev.parent,
 				  txq->size * MVNETA_DESC_ALIGNED_SIZE,
@@ -3562,7 +3613,6 @@ static void mvneta_txq_sw_deinit(struct mvneta_port *pp,
 	netdev_tx_reset_queue(nq);
 
 	txq->buf               = NULL;
-	txq->tso_hdrs          = NULL;
 	txq->descs             = NULL;
 	txq->last_desc         = 0;
 	txq->next_desc_to_proc = 0;
@@ -5833,6 +5883,8 @@ static int __init mvneta_driver_init(void)
 {
 	int ret;
 
+	BUILD_BUG_ON_NOT_POWER_OF_2(MVNETA_TSO_PER_PAGE);
+
 	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "net/mvneta:online",
 				      mvneta_cpu_online,
 				      mvneta_cpu_down_prepare);
-- 
2.30.2

Re: [PATCH RFC net-next 1/5] net: mvneta: fix transmit path dma-unmapping on error

[Eric Dumazet]

On Mon, Apr 3, 2023 at 8:30 PM Russell King (Oracle)
<rmk+kernel@armlinux.org.uk> wrote:
>
> The transmit code assumes that the transmit descriptors that are used
> begin with the first descriptor in the ring, but this may not be the
> case. Fix this by providing a new function that dma-unmaps a range of
> numbered descriptor entries, and use that to do the unmapping.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Nice patch series !

I guess this one will need to be backported to stable versions. It
would be nice adding:

Fixes: 2adb719d74f6 ("net: mvneta: Implement software TSO")

Thanks.

> ---
>  drivers/net/ethernet/marvell/mvneta.c | 53 +++++++++++++++++----------
>  1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 2cad76d0a50e..62400ff61e34 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -2714,14 +2714,40 @@ mvneta_tso_put_data(struct net_device *dev, struct mvneta_tx_queue *txq,
>         return 0;
>  }
>
> +static void mvneta_release_descs(struct mvneta_port *pp,
> +                                struct mvneta_tx_queue *txq,
> +                                int first, int num)
> +{
> +       int desc_idx, i;
> +
> +       desc_idx = first + num;
> +       if (desc_idx >= txq->size)
> +               desc_idx -= txq->size;
> +
> +       for (i = num; i >= 0; i--) {
> +               struct mvneta_tx_desc *tx_desc = txq->descs + desc_idx;
> +
> +               if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr))
> +                       dma_unmap_single(pp->dev->dev.parent,
> +                                        tx_desc->buf_phys_addr,
> +                                        tx_desc->data_size,
> +                                        DMA_TO_DEVICE);
> +
> +               mvneta_txq_desc_put(txq);
> +
> +               if (desc_idx == 0)
> +                       desc_idx = txq->size;
> +               desc_idx -= 1;
> +       }
> +}
> +
>  static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
>                          struct mvneta_tx_queue *txq)
>  {
>         int hdr_len, total_len, data_left;
> -       int desc_count = 0;
> +       int first_desc, desc_count = 0;
>         struct mvneta_port *pp = netdev_priv(dev);
>         struct tso_t tso;
> -       int i;
>
>         /* Count needed descriptors */
>         if ((txq->count + tso_count_descs(skb)) >= txq->size)
> @@ -2732,6 +2758,8 @@ static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
>                 return 0;
>         }
>
> +       first_desc = txq->txq_put_index;
> +
>         /* Initialize the TSO handler, and prepare the first payload */
>         hdr_len = tso_start(skb, &tso);
>
> @@ -2772,15 +2800,7 @@ static int mvneta_tx_tso(struct sk_buff *skb, struct net_device *dev,
>         /* Release all used data descriptors; header descriptors must not
>          * be DMA-unmapped.
>          */
> -       for (i = desc_count - 1; i >= 0; i--) {
> -               struct mvneta_tx_desc *tx_desc = txq->descs + i;
> -               if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr))
> -                       dma_unmap_single(pp->dev->dev.parent,
> -                                        tx_desc->buf_phys_addr,
> -                                        tx_desc->data_size,
> -                                        DMA_TO_DEVICE);
> -               mvneta_txq_desc_put(txq);
> -       }
> +       mvneta_release_descs(pp, txq, first_desc, desc_count - 1);
>         return 0;
>  }
>
> @@ -2790,6 +2810,7 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
>  {
>         struct mvneta_tx_desc *tx_desc;
>         int i, nr_frags = skb_shinfo(skb)->nr_frags;
> +       int first_desc = txq->txq_put_index;
>
>         for (i = 0; i < nr_frags; i++) {
>                 struct mvneta_tx_buf *buf = &txq->buf[txq->txq_put_index];
> @@ -2828,15 +2849,7 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
>         /* Release all descriptors that were used to map fragments of
>          * this packet, as well as the corresponding DMA mappings
>          */
> -       for (i = i - 1; i >= 0; i--) {
> -               tx_desc = txq->descs + i;
> -               dma_unmap_single(pp->dev->dev.parent,
> -                                tx_desc->buf_phys_addr,
> -                                tx_desc->data_size,
> -                                DMA_TO_DEVICE);
> -               mvneta_txq_desc_put(txq);
> -       }
> -
> +       mvneta_release_descs(pp, txq, first_desc, i - 1);
>         return -ENOMEM;
>  }
>
> --
> 2.30.2
>

Re: [PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

[Eric Dumazet]

On Mon, Apr 3, 2023 at 8:30 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> Hi,
>
> With reference to
> https://forum.turris.cz/t/random-kernel-exceptions-on-hbl-tos-7-0/18865/
>
> It appears that mvneta attempts an order-6 allocation for the TSO
> header memory. While this succeeds early on in the system's life time,
> trying order-6 allocations later can result in failure due to memory
> fragmentation.
>
> Firstly, the reason it's so large is that we take the number of
> transmit descriptors, and allocate a TSO header buffer for each, and
> each TSO header is 256 bytes. The driver uses a simple mechanism to
> determine the address - it uses the transmit descriptor index as an
> index into the TSO header memory.
>
>         (The first obvious question is: do there need to be this
>         many? Won't each TSO header always have at least one bit
>         of data to go with it? In other words, wouldn't the maximum
>         number of TSO headers that a ring could accept be the number
>         of ring entries divided by 2?)
>
> There is no real need for this memory to be an order-6 allocation,
> since nothing in hardware requires this buffer to be contiguous.
>
> Therefore, this series splits this order-6 allocation up into 32
> order-1 allocations (8k pages on 4k page platforms), each giving
> 32 TSO headers per page.
>
> In order to do this, these patches:
>
> 1) fix a horrible transmit path error-cleanup bug - the existing
>    code unmaps from the first descriptor that was allocated at
>    interface bringup, not the first descriptor that the packet
>    is using, resulting in the wrong descriptors being unmapped.
>
> 2) since xdp support was added, we now have buf->type which indicates
>    what this transmit buffer contains. Use this to mark TSO header
>    buffers.
>
> 3) get rid of IS_TSO_HEADER(), instead using buf->type to determine
>    whether this transmit buffer needs to be DMA-unmapped.
>
> 4) move tso_build_hdr() into mvneta_tso_put_hdr() to keep all the
>    TSO header building code together.
>
> 5) split the TSO header allocation into chunks of order-1 pages.
>

Series looks very good to me, thanks.

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

[Russell King (Oracle)]

Hi,

I think the Turris folk are waiting for me to get this into the kernel
and backported to stable before they merge it into their tree and we
therefore end up with it being tested.

We are now at -rc7, and this series is in danger of missing the
upcoming merge window.

So, I think it's time that I posted a wake-up call here to say that no,
that's not going to happen until such time that we know whether these
patches solve the problem that they identified. I'm not bunging patches
into the kernel for problems people have without those people testing
the proposed changes.

I think if the Turris folk want to engage with mainline for assistance
in resolving issues, they need to do their part and find a way to
provide kernels to test out proposed fixes for their problems.

On Mon, Apr 03, 2023 at 07:29:59PM +0100, Russell King (Oracle) wrote:
> Hi,
> 
> With reference to
> https://forum.turris.cz/t/random-kernel-exceptions-on-hbl-tos-7-0/18865/
> 
> It appears that mvneta attempts an order-6 allocation for the TSO
> header memory. While this succeeds early on in the system's life time,
> trying order-6 allocations later can result in failure due to memory
> fragmentation.
> 
> Firstly, the reason it's so large is that we take the number of
> transmit descriptors, and allocate a TSO header buffer for each, and
> each TSO header is 256 bytes. The driver uses a simple mechanism to
> determine the address - it uses the transmit descriptor index as an
> index into the TSO header memory.
> 
> 	(The first obvious question is: do there need to be this
> 	many? Won't each TSO header always have at least one bit
> 	of data to go with it? In other words, wouldn't the maximum
> 	number of TSO headers that a ring could accept be the number
> 	of ring entries divided by 2?)
> 
> There is no real need for this memory to be an order-6 allocation,
> since nothing in hardware requires this buffer to be contiguous.
> 
> Therefore, this series splits this order-6 allocation up into 32
> order-1 allocations (8k pages on 4k page platforms), each giving
> 32 TSO headers per page.
> 
> In order to do this, these patches:
> 
> 1) fix a horrible transmit path error-cleanup bug - the existing
>    code unmaps from the first descriptor that was allocated at
>    interface bringup, not the first descriptor that the packet
>    is using, resulting in the wrong descriptors being unmapped.
> 
> 2) since xdp support was added, we now have buf->type which indicates
>    what this transmit buffer contains. Use this to mark TSO header
>    buffers.
> 
> 3) get rid of IS_TSO_HEADER(), instead using buf->type to determine
>    whether this transmit buffer needs to be DMA-unmapped.
> 
> 4) move tso_build_hdr() into mvneta_tso_put_hdr() to keep all the
>    TSO header building code together.
> 
> 5) split the TSO header allocation into chunks of order-1 pages.
> 
>  drivers/net/ethernet/marvell/mvneta.c | 166 +++++++++++++++++++++++-----------
>  1 file changed, 115 insertions(+), 51 deletions(-)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

[Eric Dumazet]

On Tue, Apr 11, 2023 at 5:50 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> Hi,
>
> I think the Turris folk are waiting for me to get this into the kernel
> and backported to stable before they merge it into their tree and we
> therefore end up with it being tested.
>
> We are now at -rc7, and this series is in danger of missing the
> upcoming merge window.
>
> So, I think it's time that I posted a wake-up call here to say that no,
> that's not going to happen until such time that we know whether these
> patches solve the problem that they identified. I'm not bunging patches
> into the kernel for problems people have without those people testing
> the proposed changes.
>
> I think if the Turris folk want to engage with mainline for assistance
> in resolving issues, they need to do their part and find a way to
> provide kernels to test out proposed fixes for their problems.
>

Just make sure to repost the series without the RFC tag :)

Thanks.

Re: [PATCH RFC net-next 0/6] net: mvneta: reduce size of TSO header allocation

[Marek Behún]

On Tue, 11 Apr 2023 16:50:47 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> Hi,
> 
> I think the Turris folk are waiting for me to get this into the kernel
> and backported to stable before they merge it into their tree and we
> therefore end up with it being tested.
> 
> We are now at -rc7, and this series is in danger of missing the
> upcoming merge window.
> 
> So, I think it's time that I posted a wake-up call here to say that no,
> that's not going to happen until such time that we know whether these
> patches solve the problem that they identified. I'm not bunging patches
> into the kernel for problems people have without those people testing
> the proposed changes.
> 
> I think if the Turris folk want to engage with mainline for assistance
> in resolving issues, they need to do their part and find a way to
> provide kernels to test out proposed fixes for their problems.

Hello Russell,

sorry about this, our kernel team was halved a few months ago and things
are a little hectic here. Tomorrow I will try to apply these patches
and build a testing release of TurrisOS for users who have this issue.
Hopefully they will report positively to it.

Marek


> On Mon, Apr 03, 2023 at 07:29:59PM +0100, Russell King (Oracle) wrote:
> > Hi,
> > 
> > With reference to
> > https://forum.turris.cz/t/random-kernel-exceptions-on-hbl-tos-7-0/18865/
> > 
> > It appears that mvneta attempts an order-6 allocation for the TSO
> > header memory. While this succeeds early on in the system's life time,
> > trying order-6 allocations later can result in failure due to memory
> > fragmentation.
> > 
> > Firstly, the reason it's so large is that we take the number of
> > transmit descriptors, and allocate a TSO header buffer for each, and
> > each TSO header is 256 bytes. The driver uses a simple mechanism to
> > determine the address - it uses the transmit descriptor index as an
> > index into the TSO header memory.
> > 
> > 	(The first obvious question is: do there need to be this
> > 	many? Won't each TSO header always have at least one bit
> > 	of data to go with it? In other words, wouldn't the maximum
> > 	number of TSO headers that a ring could accept be the number
> > 	of ring entries divided by 2?)
> > 
> > There is no real need for this memory to be an order-6 allocation,
> > since nothing in hardware requires this buffer to be contiguous.
> > 
> > Therefore, this series splits this order-6 allocation up into 32
> > order-1 allocations (8k pages on 4k page platforms), each giving
> > 32 TSO headers per page.
> > 
> > In order to do this, these patches:
> > 
> > 1) fix a horrible transmit path error-cleanup bug - the existing
> >    code unmaps from the first descriptor that was allocated at
> >    interface bringup, not the first descriptor that the packet
> >    is using, resulting in the wrong descriptors being unmapped.
> > 
> > 2) since xdp support was added, we now have buf->type which indicates
> >    what this transmit buffer contains. Use this to mark TSO header
> >    buffers.
> > 
> > 3) get rid of IS_TSO_HEADER(), instead using buf->type to determine
> >    whether this transmit buffer needs to be DMA-unmapped.
> > 
> > 4) move tso_build_hdr() into mvneta_tso_put_hdr() to keep all the
> >    TSO header building code together.
> > 
> > 5) split the TSO header allocation into chunks of order-1 pages.
> > 
> >  drivers/net/ethernet/marvell/mvneta.c | 166 +++++++++++++++++++++++-----------
> >  1 file changed, 115 insertions(+), 51 deletions(-)
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> >   
>