On 2019-08-19 09:35, Magnus Karlsson wrote:
> On Sat, Aug 17, 2019 at 12:02 AM Jonathan Lemon
> <jonathan.lemon@xxxxxxxxx> 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@xxxxxxxxx>
>>>> Reported-by: Yonghong Song <yhs@xxxxxx> > ---
>>>> 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 :-).
+1, let's expose it and make xdpsock read and print it. I added this
flag to libbpf when I added the ability to query it from the kernel, but
I didn't pay attention that struct xsk_socket was private to libbpf, and
xsk->zc couldn't be accessed externally.
> /Magnus
>
>> --
>> Jonathan
