netdev - Re: [PATCH v3 1/1] xdp: remove the function xsk_map

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] [day] [month] [year] [list]

Message-ID: <CAJ8uoz1sQ35p8o+iWt6PErUJHu-G=eOicsN-68vKajcUfgx_xw@mail.gmail.com>
Date:   Mon, 23 Nov 2020 15:40:11 +0100
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Zhu Yanjun <zyjzyj2000@...il.com>
Cc:     "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn.topel@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 1/1] xdp: remove the function xsk_map_inc

On Mon, Nov 23, 2020 at 3:30 PM Zhu Yanjun <zyjzyj2000@...il.com> wrote:
>
> On Mon, Nov 23, 2020 at 10:27 PM <zyjzyj2000@...il.com> wrote:
> >
> > From: Zhu Yanjun <zyjzyj2000@...il.com>
> >
> > The function xsk_map_inc is a simple wrapper of bpf_map_inc and
> > always returns zero. As such, replacing this function with bpf_map_inc
> > and removing the test code.
> >
> > Signed-off-by: Zhu Yanjun <zyjzyj2000@...il.com>
> > ---
> >  net/xdp/xsk.c    |  2 +-
> >  net/xdp/xsk.h    |  1 -
> >  net/xdp/xskmap.c | 13 +------------
> >  3 files changed, 2 insertions(+), 14 deletions(-)
> >
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index cfbec3989a76..a3c1f07d77d8 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -548,7 +548,7 @@ static struct xsk_map *xsk_get_map_list_entry(struct xdp_sock *xs,
> >         node = list_first_entry_or_null(&xs->map_list, struct xsk_map_node,
> >                                         node);
> >         if (node) {
> > -               WARN_ON(xsk_map_inc(node->map));
> > +               bpf_map_inc(&node->map->map);
>
> Thanks. This is the latest version.
>
> Zhu Yanjun

Thank you. The code now looks good, but could you please resend this
without any of your replies in it (like the comments above). Something
that can be applied as a patch to source code. Use git format-patch
followed by git send-email (without any editing in between). It will
produce a clean patch.

Have you run "checkpatch.pl -strict" on it?

> >                 map = node->map;
> >                 *map_entry = node->map_entry;
> >         }
> > diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
> > index b9e896cee5bb..0aad25c0e223 100644
> > --- a/net/xdp/xsk.h
> > +++ b/net/xdp/xsk.h
> > @@ -41,7 +41,6 @@ static inline struct xdp_sock *xdp_sk(struct sock *sk)
> >
> >  void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,
> >                              struct xdp_sock **map_entry);
> > -int xsk_map_inc(struct xsk_map *map);
> >  void xsk_map_put(struct xsk_map *map);
> >  void xsk_clear_pool_at_qid(struct net_device *dev, u16 queue_id);
> >  int xsk_reg_pool_at_qid(struct net_device *dev, struct xsk_buff_pool *pool,
> > diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
> > index 49da2b8ace8b..6b7e9a72b101 100644
> > --- a/net/xdp/xskmap.c
> > +++ b/net/xdp/xskmap.c
> > @@ -11,12 +11,6 @@
> >
> >  #include "xsk.h"
> >
> > -int xsk_map_inc(struct xsk_map *map)
> > -{
> > -       bpf_map_inc(&map->map);
> > -       return 0;
> > -}
> > -
> >  void xsk_map_put(struct xsk_map *map)
> >  {
> >         bpf_map_put(&map->map);
> > @@ -26,17 +20,12 @@ static struct xsk_map_node *xsk_map_node_alloc(struct xsk_map *map,
> >                                                struct xdp_sock **map_entry)
> >  {
> >         struct xsk_map_node *node;
> > -       int err;
> >
> >         node = kzalloc(sizeof(*node), GFP_ATOMIC | __GFP_NOWARN);
> >         if (!node)
> >                 return ERR_PTR(-ENOMEM);
> >
> > -       err = xsk_map_inc(map);
> > -       if (err) {
> > -               kfree(node);
> > -               return ERR_PTR(err);
> > -       }
> > +       bpf_map_inc(&map->map);
> >
> >         node->map = map;
> >         node->map_entry = map_entry;
> > --
> > 2.25.1
> >