From: Joakim Zhang <qiangqing.zhang@xxxxxxx> [ Upstream commit 471ff4455d61c9929ae912328859921708e1eafc ] Frieder Schrempf reported a TX throuthput issue [1], it happens quite often that the measured bandwidth in TX direction drops from its expected/nominal value to something like ~50% (for 100M) or ~67% (for 1G) connections. [1] https://lore.kernel.org/linux-arm-kernel/421cc86c-b66f-b372-32f7-21e59f9a98bc@xxxxxxxxxx/ The issue becomes clear after digging into it, Net core would select queues when transmitting packets. Since FEC have not impletemented ndo_select_queue callback yet, so it will call netdev_pick_tx to select queues randomly. For i.MX6SX ENET IP with AVB support, driver default enables this feature. According to the setting of QOS/RCMRn/DMAnCFG registers, AVB configured to Credit-based scheme, 50% bandwidth of each queue 1&2. With below tests let me think more: 1) With FEC_QUIRK_HAS_AVB quirk, can reproduce TX bandwidth fluctuations issue. 2) Without FEC_QUIRK_HAS_AVB quirk, can't reproduce TX bandwidth fluctuations issue. The related difference with or w/o FEC_QUIRK_HAS_AVB quirk is that, whether we program FTYPE field of TxBD or not. As I describe above, AVB feature is enabled by default. With FEC_QUIRK_HAS_AVB quirk, frames in queue 0 marked as non-AVB, and frames in queue 1&2 marked as AVB Class A&B. It's unreasonable if frames in queue 1&2 are not required to be time-sensitive. So when Net core select tx queues ramdomly, Credit-based scheme would work and lead to TX bandwidth fluctuated. On the other hand, w/o FEC_QUIRK_HAS_AVB quirk, frames in queue 1&2 are all marked as non-AVB, so Credit-based scheme would not work. Till now, how can we fix this TX throughput issue? Yes, please remove FEC_QUIRK_HAS_AVB quirk if you suffer it from time-nonsensitive networking. However, this quirk is used to indicate i.MX6SX, other setting depends on it. So this patch adds a new quirk FEC_QUIRK_HAS_MULTI_QUEUES to represent i.MX6SX, it is safe for us remove FEC_QUIRK_HAS_AVB quirk now. FEC_QUIRK_HAS_AVB quirk is set by default in the driver, and users may not know much about driver details, they would waste effort to find the root cause, that is not we want. The following patch is a implementation to fix it and users don't need to modify the driver. Tested-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> Reported-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> Signed-off-by: Joakim Zhang <qiangqing.zhang@xxxxxxx> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> --- drivers/net/ethernet/freescale/fec.h | 5 +++++ drivers/net/ethernet/freescale/fec_main.c | 11 ++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 0602d5d5d2ee..2e002e4b4b4a 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -467,6 +467,11 @@ struct bufdesc_ex { */ #define FEC_QUIRK_NO_HARD_RESET (1 << 18) +/* i.MX6SX ENET IP supports multiple queues (3 queues), use this quirk to + * represents this ENET IP. + */ +#define FEC_QUIRK_HAS_MULTI_QUEUES (1 << 19) + struct bufdesc_prop { int qid; /* Address of Rx and Tx buffers */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index ad82cffc6f3f..98cd38379275 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -122,7 +122,7 @@ static const struct fec_devinfo fec_imx6x_info = { FEC_QUIRK_HAS_VLAN | FEC_QUIRK_HAS_AVB | FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE | FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE | - FEC_QUIRK_CLEAR_SETUP_MII, + FEC_QUIRK_CLEAR_SETUP_MII | FEC_QUIRK_HAS_MULTI_QUEUES, }; static const struct fec_devinfo fec_imx6ul_info = { @@ -421,6 +421,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; + ebdp->cbd_bdu = 0; ebdp->cbd_esc = cpu_to_fec32(estatus); } @@ -954,7 +955,7 @@ fec_restart(struct net_device *ndev) * For i.MX6SX SOC, enet use AXI bus, we use disable MAC * instead of reset MAC itself. */ - if (fep->quirks & FEC_QUIRK_HAS_AVB || + if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES || ((fep->quirks & FEC_QUIRK_NO_HARD_RESET) && fep->link)) { writel(0, fep->hwp + FEC_ECNTRL); } else { @@ -1165,7 +1166,7 @@ fec_stop(struct net_device *ndev) * instead of reset MAC itself. */ if (!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) { - if (fep->quirks & FEC_QUIRK_HAS_AVB) { + if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES) { writel(0, fep->hwp + FEC_ECNTRL); } else { writel(1, fep->hwp + FEC_ECNTRL); @@ -2570,7 +2571,7 @@ static void fec_enet_itr_coal_set(struct net_device *ndev) writel(tx_itr, fep->hwp + FEC_TXIC0); writel(rx_itr, fep->hwp + FEC_RXIC0); - if (fep->quirks & FEC_QUIRK_HAS_AVB) { + if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES) { writel(tx_itr, fep->hwp + FEC_TXIC1); writel(rx_itr, fep->hwp + FEC_RXIC1); writel(tx_itr, fep->hwp + FEC_TXIC2); @@ -3371,7 +3372,7 @@ static int fec_enet_init(struct net_device *ndev) fep->csum_flags |= FLAG_RX_CSUM_ENABLED; } - if (fep->quirks & FEC_QUIRK_HAS_AVB) { + if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES) { fep->tx_align = 0; fep->rx_align = 0x3f; } -- 2.30.2
