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]

Message-ID: <9271a44f-1bbf-1305-bff9-8cbb8bae9098@iogearbox.net>
Date:   Mon, 16 Sep 2019 10:08:14 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Yonghong Song <yhs@...com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "maximmi@...lanox.com" <maximmi@...lanox.com>
Subject: Re: [PATCH] libbpf: Don't error out if getsockopt() fails for
 XDP_OPTIONS

On 9/13/19 8:53 PM, Yonghong Song 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? :)

Given this is not exposed to applications at this point and we don't do anything
useful with it, lets just remove the zc cruft until there is a proper interface
added to libbpf. Toke, please respin with the suggested removal, thanks!