netdev - Re: [PATCH v12 bpf-next 06/18] net: marvell: rely on xdp_update_skb_shared

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ0CqmX3JtmZk9fKr9tH-SF4=1G8FxuoBLdFQ=3JQ1mOZAuiOg@mail.gmail.com>
Date:   Fri, 3 Sep 2021 19:27:00 +0200
From:   Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>,
        BPF-dev-list <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "Agroskin, Shay" <shayagr@...zon.com>,
        David Ahern <dsahern@...nel.org>,
        Jesper Brouer <brouer@...hat.com>,
        Eelco Chaudron <echaudro@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Saeed Mahameed <saeed@...nel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        "Sarkar, Tirthendu" <tirthendu.sarkar@...el.com>,
        Toke Hoiland Jorgensen <toke@...hat.com>
Subject: Re: [PATCH v12 bpf-next 06/18] net: marvell: rely on
 xdp_update_skb_shared_info utility routine

>
> Lorenzo Bianconi wrote:
> > Rely on xdp_update_skb_shared_info routine in order to avoid
> > resetting frags array in skb_shared_info structure building
> > the skb in mvneta_swbm_build_skb(). Frags array is expected to
> > be initialized by the receiving driver building the xdp_buff
> > and here we just need to update memory metadata.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > ---
>
> Acked-by: John Fastabend <john.fastabend@...il.com>
>
> >  drivers/net/ethernet/marvell/mvneta.c | 35 +++++++++++++++------------
> >  1 file changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index cbf614d6b993..b996eb49d813 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -2304,11 +2304,19 @@ mvneta_swbm_add_rx_fragment(struct mvneta_port *pp,
> >               skb_frag_size_set(frag, data_len);
> >               __skb_frag_set_page(frag, page);
> >
> > -             if (!xdp_buff_is_mb(xdp))
> > +             if (!xdp_buff_is_mb(xdp)) {
> > +                     sinfo->xdp_frags_size = *size;
> >                       xdp_buff_set_mb(xdp);
> > +             }
> > +             if (page_is_pfmemalloc(page))
> > +                     xdp_buff_set_frag_pfmemalloc(xdp);
> >       } else {
> >               page_pool_put_full_page(rxq->page_pool, page, true);
> >       }
> > +
> > +     /* last fragment */
> > +     if (len == *size)
> > +             sinfo->xdp_frags_tsize = sinfo->nr_frags * PAGE_SIZE;
> >       *size -= len;
> >  }
> >
> > @@ -2316,13 +2324,18 @@ static struct sk_buff *
> >  mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool,
> >                     struct xdp_buff *xdp, u32 desc_status)
> >  {
> > -     struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> > +     unsigned int size, truesize;
> >       struct sk_buff *skb;
> >       u8 num_frags;
> > -     int i;
> >
> > -     if (unlikely(xdp_buff_is_mb(xdp)))
> > +     if (unlikely(xdp_buff_is_mb(xdp))) {
>
> Just curious does the mvneta hardware support header split? If we
> get to that point then we can drop the unlikely.

nope, it does not :(

Regards,
Lorenzo

>
> > +             struct skb_shared_info *sinfo;
> > +
> > +             sinfo = xdp_get_shared_info_from_buff(xdp);
> > +             truesize = sinfo->xdp_frags_tsize;
> > +             size = sinfo->xdp_frags_size;
> >               num_frags = sinfo->nr_frags;
> > +     }
> >
> >       skb = build_skb(xdp->data_hard_start, PAGE_SIZE);
> >       if (!skb)
> > @@ -2334,18 +2347,10 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct page_pool *pool,
> >       skb_put(skb, xdp->data_end - xdp->data);
> >       skb->ip_summed = mvneta_rx_csum(pp, desc_status);
> >
> > -     if (likely(!xdp_buff_is_mb(xdp)))
> > -             goto out;
> > -
> > -     for (i = 0; i < num_frags; i++) {
> > -             skb_frag_t *frag = &sinfo->frags[i];
> > -
> > -             skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> > -                             skb_frag_page(frag), skb_frag_off(frag),
> > -                             skb_frag_size(frag), PAGE_SIZE);
> > -     }
> > +     if (unlikely(xdp_buff_is_mb(xdp)))
> > +             xdp_update_skb_shared_info(skb, num_frags, size, truesize,
> > +                                        xdp_buff_is_frag_pfmemalloc(xdp));
> >
> > -out:
> >       return skb;
> >  }
> >
> > --
> > 2.31.1
> >
>
>