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

From: Horatiu Vultur
Date: Fri Apr 21 2023 - 14:33:13 EST


The 04/21/2023 15:34, Alexander Lobakin wrote:
>
> From: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
> Date: Fri, 21 Apr 2023 15:14:22 +0200
>
> [...]
>
> > @@ -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?

Argh, no it is a mistake. For that case it should be xdpf->len +
IFH_LEN_BYTES. As I focus on the XDP_TX, I fogot to test also
XDP_REDIRECT. :(
Thanks for the good catch!

I will fix this in the next version.

>
> > + 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

--
/Horatiu