Re: [PATCH v6 net-next 2/5] xdp: rely on skb pointer reference in do_xdp_generic and netif_receive_generic_xdp

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5734 bytes --]

> Hi Lorenzo,
> 
> On Sun, 28 Jan 2024 at 16:22, Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > Rely on skb pointer reference instead of the skb pointer in do_xdp_generic and
> > netif_receive_generic_xdp routine signatures. This is a preliminary patch to add
> > multi-buff support for xdp running in generic mode.
> 
> The patch looks fine, but can we tweak the commit message explaining
> in more detail  why this is needed?

ack, I will update commit log in the next iteration.

Regards,
Lorenzo

> 
> Thanks
> /Ilias
> >
> > Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/net/tun.c         |  4 ++--
> >  include/linux/netdevice.h |  2 +-
> >  net/core/dev.c            | 16 +++++++++-------
> >  3 files changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 4a4f8c8e79fa..5bd98bdaddf2 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1927,7 +1927,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >                 rcu_read_lock();
> >                 xdp_prog = rcu_dereference(tun->xdp_prog);
> >                 if (xdp_prog) {
> > -                       ret = do_xdp_generic(xdp_prog, skb);
> > +                       ret = do_xdp_generic(xdp_prog, &skb);
> >                         if (ret != XDP_PASS) {
> >                                 rcu_read_unlock();
> >                                 local_bh_enable();
> > @@ -2517,7 +2517,7 @@ static int tun_xdp_one(struct tun_struct *tun,
> >         skb_record_rx_queue(skb, tfile->queue_index);
> >
> >         if (skb_xdp) {
> > -               ret = do_xdp_generic(xdp_prog, skb);
> > +               ret = do_xdp_generic(xdp_prog, &skb);
> >                 if (ret != XDP_PASS) {
> >                         ret = 0;
> >                         goto out;
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 118c40258d07..7eee99a58200 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -3958,7 +3958,7 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
> >  u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
> >                              struct bpf_prog *xdp_prog);
> >  void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
> > -int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb);
> > +int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb);
> >  int netif_rx(struct sk_buff *skb);
> >  int __netif_rx(struct sk_buff *skb);
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index bf9ec740b09a..960f39ac5e33 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4924,10 +4924,11 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
> >         return act;
> >  }
> >
> > -static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> > +static u32 netif_receive_generic_xdp(struct sk_buff **pskb,
> >                                      struct xdp_buff *xdp,
> >                                      struct bpf_prog *xdp_prog)
> >  {
> > +       struct sk_buff *skb = *pskb;
> >         u32 act = XDP_DROP;
> >
> >         /* Reinjected packets coming from act_mirred or similar should
> > @@ -5008,24 +5009,24 @@ void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog)
> >
> >  static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
> >
> > -int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
> > +int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb)
> >  {
> >         if (xdp_prog) {
> >                 struct xdp_buff xdp;
> >                 u32 act;
> >                 int err;
> >
> > -               act = netif_receive_generic_xdp(skb, &xdp, xdp_prog);
> > +               act = netif_receive_generic_xdp(pskb, &xdp, xdp_prog);
> >                 if (act != XDP_PASS) {
> >                         switch (act) {
> >                         case XDP_REDIRECT:
> > -                               err = xdp_do_generic_redirect(skb->dev, skb,
> > +                               err = xdp_do_generic_redirect((*pskb)->dev, *pskb,
> >                                                               &xdp, xdp_prog);
> >                                 if (err)
> >                                         goto out_redir;
> >                                 break;
> >                         case XDP_TX:
> > -                               generic_xdp_tx(skb, xdp_prog);
> > +                               generic_xdp_tx(*pskb, xdp_prog);
> >                                 break;
> >                         }
> >                         return XDP_DROP;
> > @@ -5033,7 +5034,7 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
> >         }
> >         return XDP_PASS;
> >  out_redir:
> > -       kfree_skb_reason(skb, SKB_DROP_REASON_XDP);
> > +       kfree_skb_reason(*pskb, SKB_DROP_REASON_XDP);
> >         return XDP_DROP;
> >  }
> >  EXPORT_SYMBOL_GPL(do_xdp_generic);
> > @@ -5356,7 +5357,8 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
> >                 int ret2;
> >
> >                 migrate_disable();
> > -               ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb);
> > +               ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog),
> > +                                     &skb);
> >                 migrate_enable();
> >
> >                 if (ret2 != XDP_PASS) {
> > --
> > 2.43.0
> >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]