2024-05-27 15:48:30

by Frank Wunderlich

Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

From: Frank Wunderlich <[email protected]>

The mainline MTK ethernet driver suffers long time from rarly but
annoying tx queue timeouts. We think that this is caused by fixed
dma sizes hardcoded for all SoCs.

Use the dma-size implementation from SDK in a per SoC manner.

Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623 ethernet")
Suggested-by: Daniel Golle <[email protected]>
Signed-off-by: Frank Wunderlich <[email protected]>
---
sorry for multiple posting in first version

based on SDK:

https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/fac194d6253d339e15c651c052b532a449a04d6e

v2:
- fix unused variable 'addr' in 32bit build
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 105 +++++++++++++-------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 9 +-
2 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index cae46290a7ae..f1ff1be73926 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1131,9 +1131,9 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
{
const struct mtk_soc_data *soc = eth->soc;
dma_addr_t phy_ring_tail;
- int cnt = MTK_QDMA_RING_SIZE;
+ int cnt = soc->tx.fq_dma_size;
dma_addr_t dma_addr;
- int i;
+ int i, j, len;

if (MTK_HAS_CAPS(eth->soc->caps, MTK_SRAM))
eth->scratch_ring = eth->sram_base;
@@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
cnt * soc->tx.desc_size,
&eth->phy_scratch_ring,
GFP_KERNEL);
+
if (unlikely(!eth->scratch_ring))
return -ENOMEM;

- eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
- if (unlikely(!eth->scratch_head))
- return -ENOMEM;
+ phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);

- dma_addr = dma_map_single(eth->dma_dev,
- eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
- DMA_FROM_DEVICE);
- if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
- return -ENOMEM;
+ for (j = 0; j < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); j++) {
+ len = min_t(int, cnt - j * MTK_FQ_DMA_LENGTH, MTK_FQ_DMA_LENGTH);
+ eth->scratch_head[j] = kcalloc(len, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);

- phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
+ if (unlikely(!eth->scratch_head[j]))
+ return -ENOMEM;

- for (i = 0; i < cnt; i++) {
- dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
- struct mtk_tx_dma_v2 *txd;
+ dma_addr = dma_map_single(eth->dma_dev,
+ eth->scratch_head[j], len * MTK_QDMA_PAGE_SIZE,
+ DMA_FROM_DEVICE);

- txd = eth->scratch_ring + i * soc->tx.desc_size;
- txd->txd1 = addr;
- if (i < cnt - 1)
- txd->txd2 = eth->phy_scratch_ring +
- (i + 1) * soc->tx.desc_size;
+ if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
+ return -ENOMEM;

- txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
- if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
- txd->txd3 |= TX_DMA_PREP_ADDR64(addr);
- txd->txd4 = 0;
- if (mtk_is_netsys_v2_or_greater(eth)) {
- txd->txd5 = 0;
- txd->txd6 = 0;
- txd->txd7 = 0;
- txd->txd8 = 0;
+ for (i = 0; i < cnt; i++) {
+ struct mtk_tx_dma_v2 *txd;
+
+ txd = eth->scratch_ring + (j * MTK_FQ_DMA_LENGTH + i) * soc->tx.desc_size;
+ txd->txd1 = dma_addr + i * MTK_QDMA_PAGE_SIZE;
+ if (j * MTK_FQ_DMA_LENGTH + i < cnt)
+ txd->txd2 = eth->phy_scratch_ring +
+ (j * MTK_FQ_DMA_LENGTH + i + 1) * soc->tx.desc_size;
+
+ txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
+ if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
+ txd->txd3 |= TX_DMA_PREP_ADDR64(dma_addr + i * MTK_QDMA_PAGE_SIZE);
+
+ txd->txd4 = 0;
+ if (mtk_is_netsys_v2_or_greater(eth)) {
+ txd->txd5 = 0;
+ txd->txd6 = 0;
+ txd->txd7 = 0;
+ txd->txd8 = 0;
+ }
}
}

@@ -2457,7 +2463,7 @@ static int mtk_tx_alloc(struct mtk_eth *eth)
if (MTK_HAS_CAPS(soc->caps, MTK_QDMA))
ring_size = MTK_QDMA_RING_SIZE;
else
- ring_size = MTK_DMA_SIZE;
+ ring_size = soc->tx.dma_size;

ring->buf = kcalloc(ring_size, sizeof(*ring->buf),
GFP_KERNEL);
@@ -2465,8 +2471,8 @@ static int mtk_tx_alloc(struct mtk_eth *eth)
goto no_tx_mem;

if (MTK_HAS_CAPS(soc->caps, MTK_SRAM)) {
- ring->dma = eth->sram_base + ring_size * sz;
- ring->phys = eth->phy_scratch_ring + ring_size * (dma_addr_t)sz;
+ ring->dma = eth->sram_base + soc->tx.fq_dma_size * sz;
+ ring->phys = eth->phy_scratch_ring + soc->tx.fq_dma_size * (dma_addr_t)sz;
} else {
ring->dma = dma_alloc_coherent(eth->dma_dev, ring_size * sz,
&ring->phys, GFP_KERNEL);
@@ -2588,6 +2594,7 @@ static void mtk_tx_clean(struct mtk_eth *eth)
static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
{
const struct mtk_reg_map *reg_map = eth->soc->reg_map;
+ const struct mtk_soc_data *soc = eth->soc;
struct mtk_rx_ring *ring;
int rx_data_len, rx_dma_size, tx_ring_size;
int i;
@@ -2595,7 +2602,7 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
if (MTK_HAS_CAPS(eth->soc->caps, MTK_QDMA))
tx_ring_size = MTK_QDMA_RING_SIZE;
else
- tx_ring_size = MTK_DMA_SIZE;
+ tx_ring_size = soc->tx.dma_size;

if (rx_flag == MTK_RX_FLAGS_QDMA) {
if (ring_no)
@@ -2610,7 +2617,7 @@ static int mtk_rx_alloc(struct mtk_eth *eth, int ring_no, int rx_flag)
rx_dma_size = MTK_HW_LRO_DMA_SIZE;
} else {
rx_data_len = ETH_DATA_LEN;
- rx_dma_size = MTK_DMA_SIZE;
+ rx_dma_size = soc->rx.dma_size;
}

ring->frag_size = mtk_max_frag_size(rx_data_len);
@@ -3139,7 +3146,10 @@ static void mtk_dma_free(struct mtk_eth *eth)
mtk_rx_clean(eth, &eth->rx_ring[i], false);
}

- kfree(eth->scratch_head);
+ for (i = 0; i < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); i++) {
+ kfree(eth->scratch_head[i]);
+ eth->scratch_head[i] = NULL;
+ }
}

static bool mtk_hw_reset_check(struct mtk_eth *eth)
@@ -5052,11 +5062,14 @@ static const struct mtk_soc_data mt2701_data = {
.desc_size = sizeof(struct mtk_tx_dma),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
.irq_done_mask = MTK_RX_DONE_INT,
.dma_l4_valid = RX_DMA_L4_VALID,
+ .dma_size = MTK_DMA_SIZE(2K),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
},
@@ -5076,11 +5089,14 @@ static const struct mtk_soc_data mt7621_data = {
.desc_size = sizeof(struct mtk_tx_dma),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
.irq_done_mask = MTK_RX_DONE_INT,
.dma_l4_valid = RX_DMA_L4_VALID,
+ .dma_size = MTK_DMA_SIZE(2K),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
},
@@ -5102,11 +5118,14 @@ static const struct mtk_soc_data mt7622_data = {
.desc_size = sizeof(struct mtk_tx_dma),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
.irq_done_mask = MTK_RX_DONE_INT,
.dma_l4_valid = RX_DMA_L4_VALID,
+ .dma_size = MTK_DMA_SIZE(2K),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
},
@@ -5127,11 +5146,14 @@ static const struct mtk_soc_data mt7623_data = {
.desc_size = sizeof(struct mtk_tx_dma),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
.irq_done_mask = MTK_RX_DONE_INT,
.dma_l4_valid = RX_DMA_L4_VALID,
+ .dma_size = MTK_DMA_SIZE(2K),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
},
@@ -5150,11 +5172,14 @@ static const struct mtk_soc_data mt7629_data = {
.desc_size = sizeof(struct mtk_tx_dma),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
.irq_done_mask = MTK_RX_DONE_INT,
.dma_l4_valid = RX_DMA_L4_VALID,
+ .dma_size = MTK_DMA_SIZE(2K),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
},
@@ -5176,6 +5201,8 @@ static const struct mtk_soc_data mt7981_data = {
.desc_size = sizeof(struct mtk_tx_dma_v2),
.dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
.dma_len_offset = 8,
+ .dma_size = MTK_DMA_SIZE(4K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
@@ -5183,6 +5210,7 @@ static const struct mtk_soc_data mt7981_data = {
.dma_l4_valid = RX_DMA_L4_VALID_V2,
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(1K),
},
};

@@ -5202,6 +5230,8 @@ static const struct mtk_soc_data mt7986_data = {
.desc_size = sizeof(struct mtk_tx_dma_v2),
.dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
.dma_len_offset = 8,
+ .dma_size = MTK_DMA_SIZE(4K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
@@ -5209,6 +5239,7 @@ static const struct mtk_soc_data mt7986_data = {
.dma_l4_valid = RX_DMA_L4_VALID_V2,
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(1K),
},
};

@@ -5228,6 +5259,8 @@ static const struct mtk_soc_data mt7988_data = {
.desc_size = sizeof(struct mtk_tx_dma_v2),
.dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
.dma_len_offset = 8,
+ .dma_size = MTK_DMA_SIZE(4K),
+ .fq_dma_size = MTK_DMA_SIZE(4K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma_v2),
@@ -5235,6 +5268,7 @@ static const struct mtk_soc_data mt7988_data = {
.dma_l4_valid = RX_DMA_L4_VALID_V2,
.dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
.dma_len_offset = 8,
+ .dma_size = MTK_DMA_SIZE(1K),
},
};

@@ -5249,6 +5283,8 @@ static const struct mtk_soc_data rt5350_data = {
.desc_size = sizeof(struct mtk_tx_dma),
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
+ .fq_dma_size = MTK_DMA_SIZE(2K),
},
.rx = {
.desc_size = sizeof(struct mtk_rx_dma),
@@ -5256,6 +5292,7 @@ static const struct mtk_soc_data rt5350_data = {
.dma_l4_valid = RX_DMA_L4_VALID_PDMA,
.dma_max_len = MTK_TX_DMA_BUF_LEN,
.dma_len_offset = 16,
+ .dma_size = MTK_DMA_SIZE(2K),
},
};

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 4eab30b44070..f5174f6cb1bb 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -32,7 +32,9 @@
#define MTK_TX_DMA_BUF_LEN 0x3fff
#define MTK_TX_DMA_BUF_LEN_V2 0xffff
#define MTK_QDMA_RING_SIZE 2048
-#define MTK_DMA_SIZE 512
+#define MTK_DMA_SIZE(x) (SZ_##x)
+#define MTK_FQ_DMA_HEAD 32
+#define MTK_FQ_DMA_LENGTH 2048
#define MTK_RX_ETH_HLEN (ETH_HLEN + ETH_FCS_LEN)
#define MTK_RX_HLEN (NET_SKB_PAD + MTK_RX_ETH_HLEN + NET_IP_ALIGN)
#define MTK_DMA_DUMMY_DESC 0xffffffff
@@ -1176,6 +1178,8 @@ struct mtk_soc_data {
u32 desc_size;
u32 dma_max_len;
u32 dma_len_offset;
+ u32 dma_size;
+ u32 fq_dma_size;
} tx;
struct {
u32 desc_size;
@@ -1183,6 +1187,7 @@ struct mtk_soc_data {
u32 dma_l4_valid;
u32 dma_max_len;
u32 dma_len_offset;
+ u32 dma_size;
} rx;
};

@@ -1264,7 +1269,7 @@ struct mtk_eth {
struct napi_struct rx_napi;
void *scratch_ring;
dma_addr_t phy_scratch_ring;
- void *scratch_head;
+ void *scratch_head[MTK_FQ_DMA_HEAD];
struct clk *clks[MTK_CLK_MAX];

struct mii_bus *mii_bus;
--
2.34.1

2024-05-27 16:30:44

by Sunil Kovvuri Goutham

Subject: RE: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

> -----Original Message-----
> From: Frank Wunderlich <[email protected]>
> Sent: Monday, May 27, 2024 7:52 PM
> To: Felix Fietkau <[email protected]>; Sean Wang <[email protected]>;
> Mark Lee <[email protected]>; Lorenzo Bianconi
> <[email protected]>; David S. Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Matthias Brugger <[email protected]>;
> AngeloGioacchino Del Regno <[email protected]>
> Cc: Frank Wunderlich <[email protected]>; John Crispin
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected];
> Daniel Golle <[email protected]>
> Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific
>
> From: Frank Wunderlich <[email protected]>
>
> The mainline MTK ethernet driver suffers long time from rarly but annoying tx
> queue timeouts. We think that this is caused by fixed dma sizes hardcoded for
> all SoCs.
>
> Use the dma-size implementation from SDK in a per SoC manner.
>
> Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623
> ethernet")
> Suggested-by: Daniel Golle <[email protected]>
> Signed-off-by: Frank Wunderlich <[email protected]>

.............
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index cae46290a7ae..f1ff1be73926 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c

............
> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
> cnt * soc->tx.desc_size,
> &eth->phy_scratch_ring,
> GFP_KERNEL);

.............
> - for (i = 0; i < cnt; i++) {
> - dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> - struct mtk_tx_dma_v2 *txd;
> + dma_addr = dma_map_single(eth->dma_dev,
> + eth->scratch_head[j], len *
> MTK_QDMA_PAGE_SIZE,
> + DMA_FROM_DEVICE);
>

As per commit msg, the fix is for transmit queue timeouts.
But the DMA buffer changes seems for receive pkts.
Can you please elaborate the connection here.

Thanks,
Sunil.

2024-05-27 16:34:54

by Daniel Golle

Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham wrote:
>
>
> > -----Original Message-----
> > From: Frank Wunderlich <[email protected]>
> > Sent: Monday, May 27, 2024 7:52 PM
> > To: Felix Fietkau <[email protected]>; Sean Wang <[email protected]>;
> > Mark Lee <[email protected]>; Lorenzo Bianconi
> > <[email protected]>; David S. Miller <[email protected]>; Eric Dumazet
> > <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> > <[email protected]>; Matthias Brugger <[email protected]>;
> > AngeloGioacchino Del Regno <[email protected]>
> > Cc: Frank Wunderlich <[email protected]>; John Crispin
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]; [email protected];
> > Daniel Golle <[email protected]>
> > Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific
> >
> > From: Frank Wunderlich <[email protected]>
> >
> > The mainline MTK ethernet driver suffers long time from rarly but annoying tx
> > queue timeouts. We think that this is caused by fixed dma sizes hardcoded for
> > all SoCs.
> >
> > Use the dma-size implementation from SDK in a per SoC manner.
> >
> > Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623
> > ethernet")
> > Suggested-by: Daniel Golle <[email protected]>
> > Signed-off-by: Frank Wunderlich <[email protected]>
>
> ..............
> >
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index cae46290a7ae..f1ff1be73926 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>
> .............
> > @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
> > cnt * soc->tx.desc_size,
> > &eth->phy_scratch_ring,
> > GFP_KERNEL);
>
> ..............
> > - for (i = 0; i < cnt; i++) {
> > - dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> > - struct mtk_tx_dma_v2 *txd;
> > + dma_addr = dma_map_single(eth->dma_dev,
> > + eth->scratch_head[j], len *
> > MTK_QDMA_PAGE_SIZE,
> > + DMA_FROM_DEVICE);
> >
>
> As per commit msg, the fix is for transmit queue timeouts.
> But the DMA buffer changes seems for receive pkts.
> Can you please elaborate the connection here.

*I guess* the memory window used for both, TX and RX DMA descriptors
needs to be wisely split to not risk TX queue overruns, depending on the
SoC speed and without hurting RX performance...

Maybe someone inside MediaTek (I've added to Cc now) and more familiar
with the design can elaborate in more detail.

2024-05-29 04:12:15

by Bc-bocun Chen (陳柏村)

Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

On Mon, 2024-05-27 at 17:13 +0100, Daniel Golle wrote:
> > On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham
> wrote:
> > > >
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Frank Wunderlich <[email protected]>
> > > > > > Sent: Monday, May 27, 2024 7:52 PM
> > > > > > To: Felix Fietkau <[email protected]>; Sean Wang <
> > > > > > [email protected]>;
> > > > > > Mark Lee <[email protected]>; Lorenzo Bianconi
> > > > > > <[email protected]>; David S. Miller <[email protected]>
> > > ; Eric
> > > > > > Dumazet
> > > > > > <[email protected]>; Jakub Kicinski <[email protected]>;
> > > Paolo
> > > > > > Abeni
> > > > > > <[email protected]>; Matthias Brugger <
> > > [email protected]>;
> > > > > > AngeloGioacchino Del Regno <
> > > > > > [email protected]>
> > > > > > Cc: Frank Wunderlich <[email protected]>; John
> > > Crispin
> > > > > > <[email protected]>; [email protected];
> > > > > > [email protected];
> > > > > > [email protected];
> > > > > > [email protected];
> > > > > > Daniel Golle <[email protected]>
> > > > > > Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma
> > > buffer
> > > > > > size soc specific
> > > > > >
> > > > > > From: Frank Wunderlich <[email protected]>
> > > > > >
> > > > > > The mainline MTK ethernet driver suffers long time from
> > > rarly but
> > > > > > annoying tx
> > > > > > queue timeouts. We think that this is caused by fixed dma
> > > sizes
> > > > > > hardcoded for
> > > > > > all SoCs.
> > > > > >
> > > > > > Use the dma-size implementation from SDK in a per SoC
> > > manner.
> > > > > >
> > > > > > Fixes: 656e705243fd ("net-next: mediatek: add support for
> > > MT7623
> > > > > > ethernet")
> > > > > > Suggested-by: Daniel Golle <[email protected]>
> > > > > > Signed-off-by: Frank Wunderlich <[email protected]>
> >
> > > >
> > > > ..............
> > > > > >
> > > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > index cae46290a7ae..f1ff1be73926 100644
> > > > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> >
> > > >
> > > > .............
> > > > > > @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct
> > > mtk_eth
> > > > > > *eth)
> > > > > > cnt * soc-
> > > > > > >tx.desc_size,
> > > > > > &eth-
> > > > > > >phy_scratch_ring,
> > > > > > GFP_KERNEL);
> >
> > > >
> > > > ..............
> > > > > > - for (i = 0; i < cnt; i++) {
> > > > > > - dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> > > > > > - struct mtk_tx_dma_v2 *txd;
> > > > > > + dma_addr = dma_map_single(eth->dma_dev,
> > > > > > + eth->scratch_head[j], len *
> > > > > > MTK_QDMA_PAGE_SIZE,
> > > > > > + DMA_FROM_DEVICE);
> > > > > >
> >
> > > >
> > > > As per commit msg, the fix is for transmit queue timeouts.
> > > > But the DMA buffer changes seems for receive pkts.
> > > > Can you please elaborate the connection here.
>
> >
> > *I guess* the memory window used for both, TX and RX DMA
> descriptors
> > needs to be wisely split to not risk TX queue overruns, depending
> on
> > the
> > SoC speed and without hurting RX performance...
> >
> > Maybe someone inside MediaTek (I've added to Cc now) and more
> > familiar
> > with the design can elaborate in more detail.

We've encountered a transmit queue timeout issue on the MT79888 and
have identified it as being related to the RSS feature.
We suspect this problem arises from a low level of free TX DMADs, the
TX Ring alomost full.
Since RSS is enabled, there are 4 Rx Rings, with each containing 2048
DMADs, totaling 8192 for Rx. In contrast, the Tx Ring has only 2048
DMADs. Tx DMADs will be consumed rapidly during a 10G LAN to 10G WAN
forwarding test, subsequently causing the transmit queue to stop.
Therefore, we reduced the number of Rx DMADs for each ring to balance
both Tx and Rx DMADs, which resolves this issue.

> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2024-05-29 05:39:21

by Bc-bocun Chen (陳柏村)

Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

On Mon, 2024-05-27 at 16:21 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <[email protected]>
>
> The mainline MTK ethernet driver suffers long time from rarly but
> annoying tx queue timeouts. We think that this is caused by fixed
> dma sizes hardcoded for all SoCs.
>
> Use the dma-size implementation from SDK in a per SoC manner.
>
> Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623
> ethernet")
> Suggested-by: Daniel Golle <[email protected]>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> sorry for multiple posting in first version
>
> based on SDK:
>
https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/fac194d6253d339e15c651c052b532a449a04d6e
>
> v2:
> - fix unused variable 'addr' in 32bit build
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 105 +++++++++++++----
> --
> drivers/net/ethernet/mediatek/mtk_eth_soc.h | 9 +-
> 2 files changed, 78 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index cae46290a7ae..f1ff1be73926 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
..............
> @@ -5176,6 +5201,8 @@ static const struct mtk_soc_data mt7981_data =
> {
> .desc_size = sizeof(struct mtk_tx_dma_v2),
> .dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
> .dma_len_offset = 8,
> + .dma_size = MTK_DMA_SIZE(4K),
> + .fq_dma_size = MTK_DMA_SIZE(2K),
> },
> .rx = {
> .desc_size = sizeof(struct mtk_rx_dma),
> @@ -5183,6 +5210,7 @@ static const struct mtk_soc_data mt7981_data =
> {
> .dma_l4_valid = RX_DMA_L4_VALID_V2,
> .dma_max_len = MTK_TX_DMA_BUF_LEN,
> .dma_len_offset = 16,
> + .dma_size = MTK_DMA_SIZE(1K),
> },
> };
>
> @@ -5202,6 +5230,8 @@ static const struct mtk_soc_data mt7986_data =
> {
> .desc_size = sizeof(struct mtk_tx_dma_v2),
> .dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
> .dma_len_offset = 8,
> + .dma_size = MTK_DMA_SIZE(4K),
> + .fq_dma_size = MTK_DMA_SIZE(2K),
> },
> .rx = {
> .desc_size = sizeof(struct mtk_rx_dma),
> @@ -5209,6 +5239,7 @@ static const struct mtk_soc_data mt7986_data =
> {
> .dma_l4_valid = RX_DMA_L4_VALID_V2,
> .dma_max_len = MTK_TX_DMA_BUF_LEN,
> .dma_len_offset = 16,
> + .dma_size = MTK_DMA_SIZE(1K),
> },
> };
>
> @@ -5228,6 +5259,8 @@ static const struct mtk_soc_data mt7988_data =
> {
> .desc_size = sizeof(struct mtk_tx_dma_v2),
> .dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
> .dma_len_offset = 8,
> + .dma_size = MTK_DMA_SIZE(4K),
> + .fq_dma_size = MTK_DMA_SIZE(4K),
> },
> .rx = {
> .desc_size = sizeof(struct mtk_rx_dma_v2),
> @@ -5235,6 +5268,7 @@ static const struct mtk_soc_data mt7988_data =
> {
> .dma_l4_valid = RX_DMA_L4_VALID_V2,
> .dma_max_len = MTK_TX_DMA_BUF_LEN_V2,
> .dma_len_offset = 8,
> + .dma_size = MTK_DMA_SIZE(1K),
> },
> };
..............
Thank you for assisting in upstreaming this patch from the mainline MTK
driver.
Currently, the RSS feature has not been upstreamed. It is recommanded
to use 2048 DMADs for both TX and RX Rings on the MT7981/86/88.

2024-05-29 17:51:44

by Sunil Kovvuri Goutham

Subject: RE: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

On Mon, 2024-05-27 at 17:13 +0100, Daniel Golle wrote:
> > On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham
> wrote:
> > > >
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Frank Wunderlich <[email protected]>
> > > > > > Sent: Monday, May 27, 2024 7:52 PM
> > > > > > To: Felix Fietkau <[email protected]>; Sean Wang <
> > > > > > [email protected]>;
> > > > > > Mark Lee <[email protected]>; Lorenzo Bianconi
> > > > > > <[email protected]>; David S. Miller <[email protected]>
> > > ; Eric
> > > > > > Dumazet
> > > > > > <[email protected]>; Jakub Kicinski <[email protected]>;
> > > Paolo
> > > > > > Abeni
> > > > > > <[email protected]>; Matthias Brugger <
> > > [email protected]>;
> > > > > > AngeloGioacchino Del Regno <
> > > > > > [email protected]>
> > > > > > Cc: Frank Wunderlich <[email protected]>; John
> > > Crispin
> > > > > > <[email protected]>; [email protected];
> > > > > > [email protected];
> > > > > > [email protected];
> > > > > > [email protected];
> > > > > > Daniel Golle <[email protected]>
> > > > > > Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma
> > > buffer
> > > > > > size soc specific
> > > > > >
> > > > > > From: Frank Wunderlich <[email protected]>
> > > > > >
> > > > > > The mainline MTK ethernet driver suffers long time from
> > > rarly but
> > > > > > annoying tx
> > > > > > queue timeouts. We think that this is caused by fixed dma
> > > sizes
> > > > > > hardcoded for
> > > > > > all SoCs.
> > > > > >
> > > > > > Use the dma-size implementation from SDK in a per SoC
> > > manner.
> > > > > >
> > > > > > Fixes: 656e705243fd ("net-next: mediatek: add support for
> > > MT7623
> > > > > > ethernet")
> > > > > > Suggested-by: Daniel Golle <[email protected]>
> > > > > > Signed-off-by: Frank Wunderlich <[email protected]>
> >
> > > >
> > > > ..............
> > > > > >
> > > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > index cae46290a7ae..f1ff1be73926 100644
> > > > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> >
> > > >
> > > > .............
> > > > > > @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct
> > > mtk_eth
> > > > > > *eth)
> > > > > >                             cnt * soc-
> > > > > > >tx.desc_size,
> > > > > >                             &eth-
> > > > > > >phy_scratch_ring,
> > > > > >                             GFP_KERNEL);
> >
> > > >
> > > > ..............
> > > > > > -  for (i = 0; i < cnt; i++) {
> > > > > > -      dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> > > > > > -      struct mtk_tx_dma_v2 *txd;
> > > > > > +      dma_addr = dma_map_single(eth->dma_dev,
> > > > > > +                  eth->scratch_head[j], len *
> > > > > > MTK_QDMA_PAGE_SIZE,
> > > > > > +                  DMA_FROM_DEVICE);
> > > > > >
> >
> > > >
> > > > As per commit msg, the fix is for transmit queue timeouts.
> > > > But the DMA buffer changes seems for receive pkts.
> > > > Can you please elaborate the connection here.
>
> >
> > *I guess* the memory window used for both, TX and RX DMA
> descriptors
> > needs to be wisely split to not risk TX queue overruns, depending
> on
> > the
> > SoC speed and without hurting RX performance...
> >
> > Maybe someone inside MediaTek (I've added to Cc now) and more
> > familiar
> > with the design can elaborate in more detail.
 
>We've encountered a transmit queue timeout issue on the MT79888 and
>have identified it as being related to the RSS feature.
>We suspect this problem arises from a low level of free TX DMADs, the
>TX Ring alomost full.
>Since RSS is enabled, there are 4 Rx Rings, with each containing 2048
>DMADs, totaling 8192 for Rx. In contrast, the Tx Ring has only 2048
>DMADs. Tx DMADs will be consumed rapidly during a 10G LAN to 10G WAN
>forwarding test, subsequently causing the transmit queue to stop.
>Therefore, we reduced the number of Rx DMADs for each ring to balance
>both Tx and Rx DMADs, which resolves this issue.

Okay, but it’s still not clear why it’s resulting in a transmit timeout.
When transmit queue is stopped and after some Tx pkts in the pipeline are flushed out, isn’t
Tx queue wakeup not happening ?
 
 
Thanks,
Sunil.

2024-05-30 01:47:25

by Bc-bocun Chen (陳柏村)

Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

On Wed, 2024-05-29 at 17:50 +0000, Sunil Kovvuri Goutham wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Mon, 2024-05-27 at 17:13 +0100, Daniel Golle wrote:
> > > On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham
> > wrote:
> > > > >
> > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Frank Wunderlich <[email protected]>
> > > > > > > Sent: Monday, May 27, 2024 7:52 PM
> > > > > > > To: Felix Fietkau <[email protected]>; Sean Wang <
> > > > > > > [email protected]>;
> > > > > > > Mark Lee <[email protected]>; Lorenzo Bianconi
> > > > > > > <[email protected]>; David S. Miller <
> [email protected]>
> > > > ; Eric
> > > > > > > Dumazet
> > > > > > > <[email protected]>; Jakub Kicinski <[email protected]>;
> > > > Paolo
> > > > > > > Abeni
> > > > > > > <[email protected]>; Matthias Brugger <
> > > > [email protected]>;
> > > > > > > AngeloGioacchino Del Regno <
> > > > > > > [email protected]>
> > > > > > > Cc: Frank Wunderlich <[email protected]>; John
> > > > Crispin
> > > > > > > <[email protected]>; [email protected];
> > > > > > > [email protected];
> > > > > > > [email protected];
> > > > > > > [email protected];
> > > > > > > Daniel Golle <[email protected]>
> > > > > > > Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma
> > > > buffer
> > > > > > > size soc specific
> > > > > > >
> > > > > > > From: Frank Wunderlich <[email protected]>
> > > > > > >
> > > > > > > The mainline MTK ethernet driver suffers long time from
> > > > rarly but
> > > > > > > annoying tx
> > > > > > > queue timeouts. We think that this is caused by fixed dma
> > > > sizes
> > > > > > > hardcoded for
> > > > > > > all SoCs.
> > > > > > >
> > > > > > > Use the dma-size implementation from SDK in a per SoC
> > > > manner.
> > > > > > >
> > > > > > > Fixes: 656e705243fd ("net-next: mediatek: add support for
> > > > MT7623
> > > > > > > ethernet")
> > > > > > > Suggested-by: Daniel Golle <[email protected]>
> > > > > > > Signed-off-by: Frank Wunderlich <[email protected]>
> > >
> > > > >
> > > > > ..............
> > > > > > >
> > > > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > > index cae46290a7ae..f1ff1be73926 100644
> > > > > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > >
> > > > >
> > > > > .............
> > > > > > > @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct
> > > > mtk_eth
> > > > > > > *eth)
> > > > > > > cnt * soc-
> > > > > > > >tx.desc_size,
> > > > > > > &eth-
> > > > > > > >phy_scratch_ring,
> > > > > > > GFP_KERNEL);
> > >
> > > > >
> > > > > ..............
> > > > > > > - for (i = 0; i < cnt; i++) {
> > > > > > > - dma_addr_t addr = dma_addr + i *
> MTK_QDMA_PAGE_SIZE;
> > > > > > > - struct mtk_tx_dma_v2 *txd;
> > > > > > > + dma_addr = dma_map_single(eth->dma_dev,
> > > > > > > + eth->scratch_head[j], len *
> > > > > > > MTK_QDMA_PAGE_SIZE,
> > > > > > > + DMA_FROM_DEVICE);
> > > > > > >
> > >
> > > > >
> > > > > As per commit msg, the fix is for transmit queue timeouts.
> > > > > But the DMA buffer changes seems for receive pkts.
> > > > > Can you please elaborate the connection here.
> >
> > >
> > > *I guess* the memory window used for both, TX and RX DMA
> > descriptors
> > > needs to be wisely split to not risk TX queue overruns, depending
> > on
> > > the
> > > SoC speed and without hurting RX performance...
> > >
> > > Maybe someone inside MediaTek (I've added to Cc now) and more
> > > familiar
> > > with the design can elaborate in more detail.
>
> >We've encountered a transmit queue timeout issue on the MT79888 and
> >have identified it as being related to the RSS feature.
> >We suspect this problem arises from a low level of free TX DMADs,
> the
> >TX Ring alomost full.
> >Since RSS is enabled, there are 4 Rx Rings, with each containing
> 2048
> >DMADs, totaling 8192 for Rx. In contrast, the Tx Ring has only 2048
> >DMADs. Tx DMADs will be consumed rapidly during a 10G LAN to 10G WAN
> >forwarding test, subsequently causing the transmit queue to stop.
> >Therefore, we reduced the number of Rx DMADs for each ring to
> balance
> >both Tx and Rx DMADs, which resolves this issue.
>
> Okay, but it’s still not clear why it’s resulting in a transmit
> timeout.
> When transmit queue is stopped and after some Tx pkts in the pipeline
> are flushed out, isn’t
> Tx queue wakeup not happening ?
>
Yes, the transmit timeout is caused by the Tx queue not waking up.
The Tx queue stops when the free counter is less than ring->thres, and
it will wake up once the free counter is greater than ring->thres.
If the CPU is too late to wake up the Tx queues, it may cause a
transmit timeout.
Therefore, we balanced the TX and RX DMADs to improve this error
situation.
>
> Thanks,
> Sunil.
>
>

2024-05-30 04:22:12

by Florian Fainelli

Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

On 5/29/2024 6:46 PM, Bc-bocun Chen (陳柏村) wrote:
> On Wed, 2024-05-29 at 17:50 +0000, Sunil Kovvuri Goutham wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On Mon, 2024-05-27 at 17:13 +0100, Daniel Golle wrote:
>>>> On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham
>>> wrote:
>>>>>>
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Frank Wunderlich <[email protected]>
>>>>>>>> Sent: Monday, May 27, 2024 7:52 PM
>>>>>>>> To: Felix Fietkau <[email protected]>; Sean Wang <
>>>>>>>> [email protected]>;
>>>>>>>> Mark Lee <[email protected]>; Lorenzo Bianconi
>>>>>>>> <[email protected]>; David S. Miller <
>> [email protected]>
>>>>> ; Eric
>>>>>>>> Dumazet
>>>>>>>> <[email protected]>; Jakub Kicinski <[email protected]>;
>>>>> Paolo
>>>>>>>> Abeni
>>>>>>>> <[email protected]>; Matthias Brugger <
>>>>> [email protected]>;
>>>>>>>> AngeloGioacchino Del Regno <
>>>>>>>> [email protected]>
>>>>>>>> Cc: Frank Wunderlich <[email protected]>; John
>>>>> Crispin
>>>>>>>> <[email protected]>; [email protected];
>>>>>>>> [email protected];
>>>>>>>> [email protected];
>>>>>>>> [email protected];
>>>>>>>> Daniel Golle <[email protected]>
>>>>>>>> Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma
>>>>> buffer
>>>>>>>> size soc specific
>>>>>>>>
>>>>>>>> From: Frank Wunderlich <[email protected]>
>>>>>>>>
>>>>>>>> The mainline MTK ethernet driver suffers long time from
>>>>> rarly but
>>>>>>>> annoying tx
>>>>>>>> queue timeouts. We think that this is caused by fixed dma
>>>>> sizes
>>>>>>>> hardcoded for
>>>>>>>> all SoCs.
>>>>>>>>
>>>>>>>> Use the dma-size implementation from SDK in a per SoC
>>>>> manner.
>>>>>>>>
>>>>>>>> Fixes: 656e705243fd ("net-next: mediatek: add support for
>>>>> MT7623
>>>>>>>> ethernet")
>>>>>>>> Suggested-by: Daniel Golle <[email protected]>
>>>>>>>> Signed-off-by: Frank Wunderlich <[email protected]>
>>>>
>>>>>>
>>>>>> ..............
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>>>>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>>>>> index cae46290a7ae..f1ff1be73926 100644
>>>>>>>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>>>>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>
>>>>>>
>>>>>> .............
>>>>>>>> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct
>>>>> mtk_eth
>>>>>>>> *eth)
>>>>>>>> cnt * soc-
>>>>>>>>> tx.desc_size,
>>>>>>>> &eth-
>>>>>>>>> phy_scratch_ring,
>>>>>>>> GFP_KERNEL);
>>>>
>>>>>>
>>>>>> ..............
>>>>>>>> - for (i = 0; i < cnt; i++) {
>>>>>>>> - dma_addr_t addr = dma_addr + i *
>> MTK_QDMA_PAGE_SIZE;
>>>>>>>> - struct mtk_tx_dma_v2 *txd;
>>>>>>>> + dma_addr = dma_map_single(eth->dma_dev,
>>>>>>>> + eth->scratch_head[j], len *
>>>>>>>> MTK_QDMA_PAGE_SIZE,
>>>>>>>> + DMA_FROM_DEVICE);
>>>>>>>>
>>>>
>>>>>>
>>>>>> As per commit msg, the fix is for transmit queue timeouts.
>>>>>> But the DMA buffer changes seems for receive pkts.
>>>>>> Can you please elaborate the connection here.
>>>
>>>>
>>>> *I guess* the memory window used for both, TX and RX DMA
>>> descriptors
>>>> needs to be wisely split to not risk TX queue overruns, depending
>>> on
>>>> the
>>>> SoC speed and without hurting RX performance...
>>>>
>>>> Maybe someone inside MediaTek (I've added to Cc now) and more
>>>> familiar
>>>> with the design can elaborate in more detail.
>>
>>> We've encountered a transmit queue timeout issue on the MT79888 and
>>> have identified it as being related to the RSS feature.
>>> We suspect this problem arises from a low level of free TX DMADs,
>> the
>>> TX Ring alomost full.
>>> Since RSS is enabled, there are 4 Rx Rings, with each containing
>> 2048
>>> DMADs, totaling 8192 for Rx. In contrast, the Tx Ring has only 2048
>>> DMADs. Tx DMADs will be consumed rapidly during a 10G LAN to 10G WAN
>>> forwarding test, subsequently causing the transmit queue to stop.
>>> Therefore, we reduced the number of Rx DMADs for each ring to
>> balance
>>> both Tx and Rx DMADs, which resolves this issue.
>>
>> Okay, but it’s still not clear why it’s resulting in a transmit
>> timeout.
>> When transmit queue is stopped and after some Tx pkts in the pipeline
>> are flushed out, isn’t
>> Tx queue wakeup not happening ?
>>
> Yes, the transmit timeout is caused by the Tx queue not waking up.
> The Tx queue stops when the free counter is less than ring->thres, and
> it will wake up once the free counter is greater than ring->thres.
> If the CPU is too late to wake up the Tx queues, it may cause a
> transmit timeout.
> Therefore, we balanced the TX and RX DMADs to improve this error
> situation.

All of this needs to go into the commit message please, thanks!
--
Florian

2024-05-30 04:40:38

by Sunil Kovvuri Goutham

Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

> On Wed, 2024-05-29 at 17:50 +0000, Sunil Kovvuri Goutham wrote:
>  

>  On Mon, 2024-05-27 at 17:13 +0100, Daniel Golle wrote:
> > > On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham
> > wrote:
> > > > >
> > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Frank Wunderlich <[email protected]>
> > > > > > > Sent: Monday, May 27, 2024 7:52 PM
> > > > > > > To: Felix Fietkau <[email protected]>; Sean Wang <
> > > > > > > [email protected]>;
> > > > > > > Mark Lee <[email protected]>; Lorenzo Bianconi
> > > > > > > <[email protected]>; David S. Miller <
> [email protected]>
> > > > ; Eric
> > > > > > > Dumazet
> > > > > > > <[email protected]>; Jakub Kicinski <[email protected]>;
> > > > Paolo
> > > > > > > Abeni
> > > > > > > <[email protected]>; Matthias Brugger <
> > > > [email protected]>;
> > > > > > > AngeloGioacchino Del Regno <
> > > > > > > [email protected]>
> > > > > > > Cc: Frank Wunderlich <[email protected]>; John
> > > > Crispin
> > > > > > > <[email protected]>; [email protected];
> > > > > > > [email protected];
> > > > > > > [email protected];
> > > > > > > [email protected];
> > > > > > > Daniel Golle <[email protected]>
> > > > > > > Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma
> > > > buffer
> > > > > > > size soc specific
> > > > > > >
> > > > > > > From: Frank Wunderlich <[email protected]>
> > > > > > >
> > > > > > > The mainline MTK ethernet driver suffers long time from
> > > > rarly but
> > > > > > > annoying tx
> > > > > > > queue timeouts. We think that this is caused by fixed dma
> > > > sizes
> > > > > > > hardcoded for
> > > > > > > all SoCs.
> > > > > > >
> > > > > > > Use the dma-size implementation from SDK in a per SoC
> > > > manner.
> > > > > > >
> > > > > > > Fixes: 656e705243fd ("net-next: mediatek: add support for
> > > > MT7623
> > > > > > > ethernet")
> > > > > > > Suggested-by: Daniel Golle <[email protected]>
> > > > > > > Signed-off-by: Frank Wunderlich <[email protected]>
> > >
> > > > >
> > > > > ..............
> > > > > > >
> > > > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > > index cae46290a7ae..f1ff1be73926 100644
> > > > > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > >
> > > > >
> > > > > .............
> > > > > > > @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct
> > > > mtk_eth
> > > > > > > *eth)
> > > > > > >                             cnt * soc-
> > > > > > > >tx.desc_size,
> > > > > > >                             &eth-
> > > > > > > >phy_scratch_ring,
> > > > > > >                             GFP_KERNEL);
> > >
> > > > >
> > > > > ..............
> > > > > > > -  for (i = 0; i < cnt; i++) {
> > > > > > > -      dma_addr_t addr = dma_addr + i *
> MTK_QDMA_PAGE_SIZE;
> > > > > > > -      struct mtk_tx_dma_v2 *txd;
> > > > > > > +      dma_addr = dma_map_single(eth->dma_dev,
> > > > > > > +                  eth->scratch_head[j], len *
> > > > > > > MTK_QDMA_PAGE_SIZE,
> > > > > > > +                  DMA_FROM_DEVICE);
> > > > > > >
> > >
> > > > >
> > > > > As per commit msg, the fix is for transmit queue timeouts.
> > > > > But the DMA buffer changes seems for receive pkts.
> > > > > Can you please elaborate the connection here.
> >
> > >
> > > *I guess* the memory window used for both, TX and RX DMA
> > descriptors
> > > needs to be wisely split to not risk TX queue overruns, depending
> > on
> > > the
> > > SoC speed and without hurting RX performance...
> > >
> > > Maybe someone inside MediaTek (I've added to Cc now) and more
> > > familiar
> > > with the design can elaborate in more detail.
>  
> >We've encountered a transmit queue timeout issue on the MT79888 and
> >have identified it as being related to the RSS feature.
> >We suspect this problem arises from a low level of free TX DMADs,
> the
> >TX Ring alomost full.
> >Since RSS is enabled, there are 4 Rx Rings, with each containing
> 2048
> >DMADs, totaling 8192 for Rx. In contrast, the Tx Ring has only 2048
> >DMADs. Tx DMADs will be consumed rapidly during a 10G LAN to 10G WAN
> >forwarding test, subsequently causing the transmit queue to stop.
> >Therefore, we reduced the number of Rx DMADs for each ring to
> balance
> >both Tx and Rx DMADs, which resolves this issue.
>
> Okay, but it’s still not clear why it’s resulting in a transmit
> timeout.
> When transmit queue is stopped and after some Tx pkts in the pipeline
> are flushed out, isn’t
> Tx queue wakeup not happening ?
>  
> Yes, the transmit timeout is caused by the Tx queue not waking up.
> The Tx queue stops when the free counter is less than ring->thres, and
> it will wake up once the free counter is greater than ring->thres.
> If the CPU is too late to wake up the Tx queues, it may cause a transmit timeout.
> Therefore, we balanced the TX and RX DMADs to improve this error situation.
>  

Okay, please put as much of this info as possible in commit msg.

Thanks,
Sunil.