Re: [PATCH net-next v2] lan966x: Don't use xdp_frame when action is XDP_TX

From: Alexander Lobakin
Date: Fri Apr 21 2023 - 09:36:08 EST


From: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
Date: Fri, 21 Apr 2023 15:14:22 +0200

> When the action of an xdp program was XDP_TX, lan966x was creating
> a xdp_frame and use this one to send the frame back. But it is also
> possible to send back the frame without needing a xdp_frame, because
> it is possible to send it back using the page.
> And then once the frame is transmitted is possible to use directly
> page_pool_recycle_direct as lan966x is using page pools.
> This would save some CPU usage on this path, which results in higher
> number of transmitted frames. Bellow are the statistics:
> Frame size: Improvement:
> 64 ~8%
> 256 ~11%
> 512 ~8%

Nice bump, esp. for 256 }:>

> 1000 ~0%
> 1500 ~0%

[...]

> @@ -699,15 +701,14 @@ 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,
> - bool dma_map)
> +int lan966x_fdma_xmit_xdpf(struct lan966x_port *port, void *ptr, u32 len)
> {
> struct lan966x *lan966x = port->lan966x;
> struct lan966x_tx_dcb_buf *next_dcb_buf;
> struct lan966x_tx *tx = &lan966x->tx;
> + struct xdp_frame *xdpf;
> dma_addr_t dma_addr;
> + struct page *page;
> int next_to_use;
> __be32 *ifh;
> int ret = 0;
> @@ -722,8 +723,19 @@ int lan966x_fdma_xmit_xdpf(struct lan966x_port *port,
> goto out;
> }
>
> + /* Fill up the buffer */
> + next_dcb_buf = &tx->dcbs_buf[next_to_use];
> + next_dcb_buf->use_skb = false;
> + next_dcb_buf->xdp_ndo = !len;
> + next_dcb_buf->len = len + IFH_LEN_BYTES;

Is it intended that for .ndo_xdp_xmit cases this field will equal just
%IFH_LEN_BYTES as @len is zero?

> + next_dcb_buf->used = true;
> + next_dcb_buf->ptp = false;
> + next_dcb_buf->dev = port->dev;
> +
> /* Generate new IFH */
> - if (dma_map) {
> + if (!len) {
> + xdpf = ptr;
> +
> if (xdpf->headroom < IFH_LEN_BYTES) {
> ret = NETDEV_TX_OK;
> goto out;
[...]

Thanks,
Olek