[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zbd2r_F0ob4_dh2j@lore-desk>
Date: Mon, 29 Jan 2024 10:58:07 +0100
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc: netdev@...r.kernel.org, lorenzo.bianconi@...hat.com,
davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
pabeni@...hat.com, bpf@...r.kernel.org, toke@...hat.com,
willemdebruijn.kernel@...il.com, jasowang@...hat.com,
sdf@...gle.com, hawk@...nel.org
Subject: Re: [PATCH v6 net-next 2/5] xdp: rely on skb pointer reference in
do_xdp_generic and netif_receive_generic_xdp
> Hi Lorenzo,
>
> On Sun, 28 Jan 2024 at 16:22, Lorenzo Bianconi <lorenzo@...nel.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@...nel.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.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
> >
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists