| Message ID | 20221021090711.3749009-1-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4a4b6848d1e932b977e6a00cda393adf7e839ff8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: lan966x: Stop replacing tx dcbs and dcbs_buf when changing MTU | expand |
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Fri, 21 Oct 2022 11:07:11 +0200 you wrote: > When a frame is sent using FDMA, the skb is mapped and then the mapped > address is given to an tx dcb that is different than the last used tx > dcb. Once the HW finish with this frame, it would generate an interrupt > and then the dcb can be reused and memory can be freed. For each dcb > there is an dcb buf that contains some meta-data(is used by PTP, is > it free). There is 1 to 1 relationship between dcb and dcb_buf. > The following issue was observed. That sometimes after changing the MTU > to allocate new tx dcbs and dcbs_buf, two frames were not > transmitted. The frames were not transmitted because when reloading the > tx dcbs, it was always presuming to use the first dcb but that was not > always happening. Because it could be that the last tx dcb used before > changing MTU was first dcb and then when it tried to get the next dcb it > would take dcb 1 instead of 0. Because it is supposed to take a > different dcb than the last used one. This can be fixed simply by > changing tx->last_in_use to -1 when the fdma is disabled to reload the > new dcb and dcbs_buff. > But there could be a different issue. For example, right after the frame > is sent, the MTU is changed. Now all the dcbs and dcbs_buf will be > cleared. And now get the interrupt from HW that it finished with the > frame. So when we try to clear the skb, it is not possible because we > lost all the dcbs_buf. > The solution here is to stop replacing the tx dcbs and dcbs_buf when > changing MTU because the TX doesn't care what is the MTU size, it is > only the RX that needs this information. > > [...] Here is the summary with links: - [net] net: lan966x: Stop replacing tx dcbs and dcbs_buf when changing MTU https://git.kernel.org/netdev/net/c/4a4b6848d1e9 You are awesome, thank you!
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c index 7e4061c854f0e..a42035cec611c 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c @@ -309,6 +309,7 @@ static void lan966x_fdma_tx_disable(struct lan966x_tx *tx) lan966x, FDMA_CH_DB_DISCARD); tx->activated = false; + tx->last_in_use = -1; } static void lan966x_fdma_tx_reload(struct lan966x_tx *tx) @@ -687,17 +688,14 @@ static int lan966x_qsys_sw_status(struct lan966x *lan966x) static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu) { - void *rx_dcbs, *tx_dcbs, *tx_dcbs_buf; - dma_addr_t rx_dma, tx_dma; + dma_addr_t rx_dma; + void *rx_dcbs; u32 size; int err; /* Store these for later to free them */ rx_dma = lan966x->rx.dma; - tx_dma = lan966x->tx.dma; rx_dcbs = lan966x->rx.dcbs; - tx_dcbs = lan966x->tx.dcbs; - tx_dcbs_buf = lan966x->tx.dcbs_buf; napi_synchronize(&lan966x->napi); napi_disable(&lan966x->napi); @@ -715,17 +713,6 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu) size = ALIGN(size, PAGE_SIZE); dma_free_coherent(lan966x->dev, size, rx_dcbs, rx_dma); - lan966x_fdma_tx_disable(&lan966x->tx); - err = lan966x_fdma_tx_alloc(&lan966x->tx); - if (err) - goto restore_tx; - - size = sizeof(struct lan966x_tx_dcb) * FDMA_DCB_MAX; - size = ALIGN(size, PAGE_SIZE); - dma_free_coherent(lan966x->dev, size, tx_dcbs, tx_dma); - - kfree(tx_dcbs_buf); - lan966x_fdma_wakeup_netdev(lan966x); napi_enable(&lan966x->napi); @@ -735,11 +722,6 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu) lan966x->rx.dcbs = rx_dcbs; lan966x_fdma_rx_start(&lan966x->rx); -restore_tx: - lan966x->tx.dma = tx_dma; - lan966x->tx.dcbs = tx_dcbs; - lan966x->tx.dcbs_buf = tx_dcbs_buf; - return err; }
When a frame is sent using FDMA, the skb is mapped and then the mapped address is given to an tx dcb that is different than the last used tx dcb. Once the HW finish with this frame, it would generate an interrupt and then the dcb can be reused and memory can be freed. For each dcb there is an dcb buf that contains some meta-data(is used by PTP, is it free). There is 1 to 1 relationship between dcb and dcb_buf. The following issue was observed. That sometimes after changing the MTU to allocate new tx dcbs and dcbs_buf, two frames were not transmitted. The frames were not transmitted because when reloading the tx dcbs, it was always presuming to use the first dcb but that was not always happening. Because it could be that the last tx dcb used before changing MTU was first dcb and then when it tried to get the next dcb it would take dcb 1 instead of 0. Because it is supposed to take a different dcb than the last used one. This can be fixed simply by changing tx->last_in_use to -1 when the fdma is disabled to reload the new dcb and dcbs_buff. But there could be a different issue. For example, right after the frame is sent, the MTU is changed. Now all the dcbs and dcbs_buf will be cleared. And now get the interrupt from HW that it finished with the frame. So when we try to clear the skb, it is not possible because we lost all the dcbs_buf. The solution here is to stop replacing the tx dcbs and dcbs_buf when changing MTU because the TX doesn't care what is the MTU size, it is only the RX that needs this information. Fixes: 2ea1cbac267e ("net: lan966x: Update FDMA to change MTU.") Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- .../ethernet/microchip/lan966x/lan966x_fdma.c | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-)