From: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx> Date: Tue, 15 Nov 2022 22:44:55 +0100 Extend lan966x XDP support with the action XDP_TX. In this case when the received buffer needs to execute XDP_TX, the buffer will be moved to the TX buffers. So a new RX buffer will be allocated. When the TX finish with the frame, it would release completely this buffer. Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx> --- .../ethernet/microchip/lan966x/lan966x_fdma.c | 78 +++++++++++++++++-- .../ethernet/microchip/lan966x/lan966x_main.c | 4 +- .../ethernet/microchip/lan966x/lan966x_main.h | 8 ++ .../ethernet/microchip/lan966x/lan966x_xdp.c | 8 ++ 4 files changed, 91 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c index 384ed34197d58..c2e56233a8da5 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c @@ -394,13 +394,21 @@ static void lan966x_fdma_tx_clear_buf(struct lan966x *lan966x, int weight) dcb_buf->dev->stats.tx_bytes += dcb_buf->len; dcb_buf->used = false; - dma_unmap_single(lan966x->dev, - dcb_buf->dma_addr, - dcb_buf->len, - DMA_TO_DEVICE); - if (!dcb_buf->ptp) + if (dcb_buf->skb) + dma_unmap_single(lan966x->dev, + dcb_buf->dma_addr, + dcb_buf->len, + DMA_TO_DEVICE); + + if (dcb_buf->skb && !dcb_buf->ptp) dev_kfree_skb_any(dcb_buf->skb); + if (dcb_buf->page) { + page_pool_release_page(lan966x->rx.page_pool, + dcb_buf->page); + put_page(dcb_buf->page); + } Hmm, that's not really correct. For skb, you need to unmap + free, true (BPW, just use napi_consume_skb()). For %XDP_TX, as you use Page Pool, you don't need to unmap, but you need to do xdp_return_frame{,_bulk}. Plus, as Tx is being done here directly from an Rx NAPI polling cycle, xdp_return_frame_rx_napi() is usually used. Anyway, each of xdp_return_frame()'s variants will call page_pool_put_full_page() for you. For %XDP_REDIRECT, as you don't know the source of the XDP frame, you need to unmap it (as it was previously mapped in ::ndo_xdp_xmit()), plus call xdp_return_frame{,_bulk} to free the XDP frame. Note that _rx_napi() variant is not applicable here. That description might be confusing, so you can take a look at the already existing code[0] to get the idea. I think this piece shows the expected logics rather well. + clear = true; } @@ -532,6 +540,9 @@ static int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight) lan966x_fdma_rx_free_page(rx); lan966x_fdma_rx_advance_dcb(rx); goto allocate_new; + case FDMA_TX: + lan966x_fdma_rx_advance_dcb(rx); + continue; case FDMA_DROP: lan966x_fdma_rx_free_page(rx); lan966x_fdma_rx_advance_dcb(rx); @@ -653,6 +664,62 @@ static void lan966x_fdma_tx_start(struct lan966x_tx *tx, int next_to_use) tx->last_in_use = next_to_use; } +int lan966x_fdma_xmit_xdpf(struct lan966x_port *port, + struct xdp_frame *xdpf, + struct page *page) +{ + struct lan966x *lan966x = port->lan966x; + struct lan966x_tx_dcb_buf *next_dcb_buf; + struct lan966x_tx *tx = &lan966x->tx; + dma_addr_t dma_addr; + int next_to_use; + __be32 *ifh; + int ret = 0; + + spin_lock(&lan966x->tx_lock); + + /* Get next index */ + next_to_use = lan966x_fdma_get_next_dcb(tx); + if (next_to_use < 0) { + netif_stop_queue(port->dev); + ret = NETDEV_TX_BUSY; + goto out; + } + + /* Generate new IFH */ + ifh = page_address(page) + XDP_PACKET_HEADROOM; + memset(ifh, 0x0, sizeof(__be32) * IFH_LEN); + lan966x_ifh_set_bypass(ifh, 1); + lan966x_ifh_set_port(ifh, BIT_ULL(port->chip_port)); + + dma_addr = page_pool_get_dma_addr(page); + dma_sync_single_for_device(lan966x->dev, dma_addr + XDP_PACKET_HEADROOM, + xdpf->len + IFH_LEN_BYTES, + DMA_TO_DEVICE); Also not correct. This page was mapped with %DMA_FROM_DEVICE in the Rx code, now you sync it for the opposite. Most drivers in case of XDP enabled create Page Pools with ::dma_dir set to %DMA_BIDIRECTIONAL. Now you would need only to sync it here with the same direction (bidir) and that's it. + + /* Setup next dcb */ + lan966x_fdma_tx_setup_dcb(tx, next_to_use, xdpf->len + IFH_LEN_BYTES, + dma_addr + XDP_PACKET_HEADROOM); + + /* Fill up the buffer */ + next_dcb_buf = &tx->dcbs_buf[next_to_use]; + next_dcb_buf->skb = NULL; + next_dcb_buf->page = page; + next_dcb_buf->len = xdpf->len + IFH_LEN_BYTES; + next_dcb_buf->dma_addr = dma_addr; + next_dcb_buf->used = true; + next_dcb_buf->ptp = false; + next_dcb_buf->dev = port->dev; + + /* Start the transmission */ + lan966x_fdma_tx_start(tx, next_to_use); + +out: + spin_unlock(&lan966x->tx_lock); + + return ret; +} + int lan966x_fdma_xmit(struct sk_buff *skb, __be32 *ifh, struct net_device *dev) { struct lan966x_port *port = netdev_priv(dev); @@ -709,6 +776,7 @@ int lan966x_fdma_xmit(struct sk_buff *skb, __be32 *ifh, struct net_device *dev) /* Fill up the buffer */ next_dcb_buf = &tx->dcbs_buf[next_to_use]; next_dcb_buf->skb = skb; + next_dcb_buf->page = NULL; next_dcb_buf->len = skb->len; next_dcb_buf->dma_addr = dma_addr; next_dcb_buf->used = true; [...] -- 2.38.0 Thanks, Olek