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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230421183248.n7a2c67umthlm3fg@soft-dev3-1>
Date:   Fri, 21 Apr 2023 20:32:48 +0200
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Alexander Lobakin <aleksander.lobakin@...el.com>
CC:     <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <bpf@...r.kernel.org>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <ast@...nel.org>, <daniel@...earbox.net>, <hawk@...nel.org>,
        <john.fastabend@...il.com>, <richardcochran@...il.com>,
        <UNGLinuxDriver@...rochip.com>, <alexandr.lobakin@...el.com>,
        <maciej.fijalkowski@...el.com>
Subject: Re: [PATCH net-next v2] lan966x: Don't use xdp_frame when action is
 XDP_TX

The 04/21/2023 15:34, Alexander Lobakin wrote:
> 
> From: Horatiu Vultur <horatiu.vultur@...rochip.com>
> 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