netdev - Re: [PATCH bpf-next] libbpf: remove zc variable as it is not used

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]

Date:   Mon, 19 Aug 2019 08:35:13 +0200
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Jonathan Lemon <jonathan.lemon@...il.com>,
        Maxim Mikityanskiy <maximmi@...lanox.com>
Cc:     Yonghong Song <yhs@...com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn.topel@...el.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next] libbpf: remove zc variable as it is not used

On Sat, Aug 17, 2019 at 12:02 AM Jonathan Lemon
<jonathan.lemon@...il.com> wrote:
>
>
>
> On 16 Aug 2019, at 8:37, Yonghong Song wrote:
>
> > On 8/16/19 3:26 AM, Magnus Karlsson wrote:
> >> The zc is not used in the xsk part of libbpf, so let us remove it.
> >> Not
> >> good to have dead code lying around.
> >>
> >> Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
> >> Reported-by: Yonghong Song <yhs@...com> > ---
> >>   tools/lib/bpf/xsk.c | 3 ---
> >>   1 file changed, 3 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> >> index 680e630..9687da9 100644
> >> --- a/tools/lib/bpf/xsk.c
> >> +++ b/tools/lib/bpf/xsk.c
> >> @@ -65,7 +65,6 @@ struct xsk_socket {
> >>      int xsks_map_fd;
> >>      __u32 queue_id;
> >>      char ifname[IFNAMSIZ];
> >> -    bool zc;
> >>   };
> >>
> >>   struct xsk_nl_info {
> >> @@ -608,8 +607,6 @@ int xsk_socket__create(struct xsk_socket
> >> **xsk_ptr, const char *ifname,
> >>              goto out_mmap_tx;
> >>      }
> >>
> >> -    xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
> >
> > Since opts.flags usage is removed. Do you think it makes sense to
> > remove
> >          optlen = sizeof(opts);
> >          err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts,
> > &optlen);
> >          if (err) {
> >                  err = -errno;
> >                  goto out_mmap_tx;
> >          }
> > as well since nobody then uses opts?
>
> IIRC, this was added specifically in
> 2761ed4b6e192820760d5ba913834b2ba05fd08c
> so that userland code could know whether the socket was operating in
> zero-copy
> mode or not.

Thanks for reminding me Jonathan.

Roping in Maxim here since he wrote the patch. Was this something you
planned on using but the functionality that needed it was removed? The
patch set did go through a number of changes in the libbpf area, if I
remember correctly.

There are two options: either we remove it, or we add an interface in
xsk.h so that people can use it. I vote for the latter since I think
it could be useful. The sample app could use it at least :-).

/Magnus

> --
> Jonathan