netdev - Re: [PATCH] libbpf: Don't error out if getsockopt() fails for XDP

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, 16 Sep 2019 07:09:06 +0200
From:   Björn Töpel <bjorn.topel@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "maximmi@...lanox.com" <maximmi@...lanox.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>
Subject: Re: [PATCH] libbpf: Don't error out if getsockopt() fails for XDP_OPTIONS

On Fri, 13 Sep 2019 at 21:46, Yonghong Song <yhs@...com> wrote:
>
>
>
> On 9/10/19 12:06 AM, Toke Høiland-Jørgensen wrote:
> > Yonghong Song <yhs@...com> writes:
> >
> >> On 9/9/19 10:46 AM, Toke Høiland-Jørgensen wrote:
> >>> The xsk_socket__create() function fails and returns an error if it cannot
> >>> get the XDP_OPTIONS through getsockopt(). However, support for XDP_OPTIONS
> >>> was not added until kernel 5.3, so this means that creating XSK sockets
> >>> always fails on older kernels.
> >>>
> >>> Since the option is just used to set the zero-copy flag in the xsk struct,
> >>> there really is no need to error out if the getsockopt() call fails.
> >>>
> >>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> >>> ---
> >>>    tools/lib/bpf/xsk.c | 8 ++------
> >>>    1 file changed, 2 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> >>> index 680e63066cf3..598e487d9ce8 100644
> >>> --- a/tools/lib/bpf/xsk.c
> >>> +++ b/tools/lib/bpf/xsk.c
> >>> @@ -603,12 +603,8 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
> >>>
> >>>     optlen = sizeof(opts);
> >>>     err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen);
> >>> -   if (err) {
> >>> -           err = -errno;
> >>> -           goto out_mmap_tx;
> >>> -   }
> >>> -
> >>> -   xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
> >>> +   if (!err)
> >>> +           xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
> >>>
> >>>     if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
> >>>             err = xsk_setup_xdp_prog(xsk);
> >>
> >> Since 'zc' is not used by anybody, maybe all codes 'zc' related can be
> >> removed? It can be added back back once there is an interface to use
> >> 'zc'?
> >
> > Fine with me; up to the maintainers what they prefer, I guess? :)
>
> Maxim,
>
> Your originally introduced `'zc' and getting XDP_OPTIONS.
> What is your opinion of how to deal with the unused xsk->zc?
>

This was previously discussed here [1]. The TL;DR version is "stay
tuned for a proper interface". :-)


Björn

[1] https://lore.kernel.org/bpf/CAJ8uoz1qhaHwebmjOOS9xfJe93Eq0v=SXhQUnjHv7imVL3ONsQ@mail.gmail.com/#t



> >
> > -Toke
> >