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: <CAC_iWj+qTUmzD6Du-FRf7yhQj-euG3cFHcT5hZccdeP6tB=jGg@mail.gmail.com>
Date: Mon, 29 Jan 2024 10:05:45 +0200
From: Ilias Apalodimas <ilias.apalodimas@...aro.org>
To: Lorenzo Bianconi <lorenzo@...nel.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?

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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ