diff mbox series

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

Message ID 7fd76e88e2aadc03f14b040ffc762e88d05afc8c.1706451150.git.lorenzo@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series add multi-buff support for xdp running in generic mode | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5187 this patch: 5187
netdev/build_tools success Errors and warnings before: 1 this patch: 0
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1211 this patch: 1211
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 5489 this patch: 5489
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Lorenzo Bianconi Jan. 28, 2024, 2:20 p.m. UTC 
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.

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(-)

Comments

Ilias Apalodimas Jan. 29, 2024, 8:05 a.m. UTC | #1 
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?

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
>
Lorenzo Bianconi Jan. 29, 2024, 9:58 a.m. UTC | #2 
> 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
> >
diff mbox series

Patch

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) {