Daniel Borkmann <dan...@iogearbox.net> writes:

> 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 <y...@fb.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 <t...@redhat.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!

Sure, can do :)

-Toke

Reply via email to