netdev - Re: [RFC bpf-next 2/6] xsk: refactor xdp_umem_assign

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Jul 2018 13:41:27 +0200
From:   Björn Töpel <bjorn.topel@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Björn Töpel <bjorn.topel@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        oss-drivers@...ronome.com, Netdev <netdev@...r.kernel.org>
Subject: Re: [RFC bpf-next 2/6] xsk: refactor xdp_umem_assign_dev()

Den tors 26 juli 2018 kl 23:44 skrev Jakub Kicinski
<jakub.kicinski@...ronome.com>:
>
> Return early and only take the ref on dev once there is no possibility
> of failing.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@...ronome.com>
> ---
>  net/xdp/xdp_umem.c | 49 ++++++++++++++++++++--------------------------
>  1 file changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index f47abb46c587..c199d66b5f3f 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -56,41 +56,34 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
>         if (force_copy)
>                 return 0;
>
> -       dev_hold(dev);
> +       if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_async_xmit)
> +               return force_zc ? -ENOTSUPP : 0; /* fail or fallback */
>
> -       if (dev->netdev_ops->ndo_bpf && dev->netdev_ops->ndo_xsk_async_xmit) {
> -               bpf.command = XDP_QUERY_XSK_UMEM;
> +       bpf.command = XDP_QUERY_XSK_UMEM;
>
> -               rtnl_lock();
> -               err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> -               rtnl_unlock();
> +       rtnl_lock();
> +       err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> +       rtnl_unlock();
>
> -               if (err) {
> -                       dev_put(dev);
> -                       return force_zc ? -ENOTSUPP : 0;
> -               }
> +       if (err)
> +               return force_zc ? -ENOTSUPP : 0;
>
> -               bpf.command = XDP_SETUP_XSK_UMEM;
> -               bpf.xsk.umem = umem;
> -               bpf.xsk.queue_id = queue_id;
> +       bpf.command = XDP_SETUP_XSK_UMEM;
> +       bpf.xsk.umem = umem;
> +       bpf.xsk.queue_id = queue_id;
>
> -               rtnl_lock();
> -               err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> -               rtnl_unlock();
> +       rtnl_lock();
> +       err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> +       rtnl_unlock();
>
> -               if (err) {
> -                       dev_put(dev);
> -                       return force_zc ? err : 0; /* fail or fallback */
> -               }
> -
> -               umem->dev = dev;
> -               umem->queue_id = queue_id;
> -               umem->zc = true;
> -               return 0;
> -       }
> +       if (err)
> +               return force_zc ? err : 0; /* fail or fallback */
>
> -       dev_put(dev);
> -       return force_zc ? -ENOTSUPP : 0; /* fail or fallback */
> +       dev_hold(dev);
> +       umem->dev = dev;
> +       umem->queue_id = queue_id;
> +       umem->zc = true;
> +       return 0;
>  }
>
>  static void xdp_umem_clear_dev(struct xdp_umem *umem)
> --
> 2.17.1
>

Much cleaner! Please spin this w/o the RFC tag.

Acked-by: Björn Töpel <bjorn.topel@...il.com>


Björn