netdev - Re: [PATCH RFCv2 bpf-next 17/18] xsk: AF_XDP xdp-hints support in desc options

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:   Fri, 9 Sep 2022 11:42:27 +0200
From:   Jesper Dangaard Brouer <jbrouer@...hat.com>
To:     Maryam Tahhan <mtahhan@...hat.com>,
        Magnus Karlsson <magnus.karlsson@...il.com>,
        Jesper Dangaard Brouer <jbrouer@...hat.com>
Cc:     brouer@...hat.com, bpf@...r.kernel.org, netdev@...r.kernel.org,
        xdp-hints@...-project.net, larysa.zaremba@...el.com,
        memxor@...il.com, Lorenzo Bianconi <lorenzo@...nel.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        dave@...cker.co.uk, Magnus Karlsson <magnus.karlsson@...el.com>,
        bjorn@...nel.org
Subject: Re: [PATCH RFCv2 bpf-next 17/18] xsk: AF_XDP xdp-hints support in
 desc options

On 09/09/2022 10.12, Maryam Tahhan wrote:
> <snip>
>>>>>
>>>>> * Instead encode this information into each metadata entry in the
>>>>> metadata area, in some way so that a flags field is not needed (-1
>>>>> signifies not valid, or whatever happens to make sense). This has the
>>>>> drawback that the user might have to look at a large number of entries
>>>>> just to find out there is nothing valid to read. To alleviate this, it
>>>>> could be combined with the next suggestion.
>>>>>
>>>>> * Dedicate one bit in the options field to indicate that there is at
>>>>> least one valid metadata entry in the metadata area. This could be
>>>>> combined with the two approaches above. However, depending on what
>>>>> metadata you have enabled, this bit might be pointless. If some
>>>>> metadata is always valid, then it serves no purpose. But it might if
>>>>> all enabled metadata is rarely valid, e.g., if you get an Rx timestamp
>>>>> on one packet out of one thousand.
>>>>>
>>>
>>> I like this option better! Except that I have hoped to get 2 bits ;-)
>>
>> I will give you two if you need it Jesper, no problem :-).
>>
> 
> Ok I will look at implementing and testing this and post an update.

Perfect if you Maryam have cycles to work on this.

Let me explain what I wanted the 2nd bit for.  I simply wanted to also
transfer the XDP_FLAGS_HINTS_COMPAT_COMMON flag.  One could argue that
is it redundant information as userspace AF_XDP will have to BTF decode
all the know XDP-hints. Thus, it could know if a BTF type ID is
compatible with the common struct.   This problem is performance as my
userspace AF_XDP code will have to do more code (switch/jump-table or
table lookup) to map IDs to common compat (to e.g. extract the RX-csum
indication).  Getting this extra "common-compat" bit is actually a
micro-optimization.  It is up to AF_XDP maintainers if they can spare
this bit.

> Thanks folks
> 
>>> The performance advantage is that the AF_XDP descriptor bits will
>>> already be cache-hot, and if it indicates no-metadata-hints the AF_XDP
>>> application can avoid reading the metadata cache-line :-).
>>
>> Agreed. I prefer if we can keep it simple and fast like this.
>>

Great, lets proceed this way then.

> <snip>
> 

Thinking ahead: We will likely need 3 bits.

The idea is that for TX-side, we set a bit indicating that AF_XDP have
provided a valid XDP-hints layout (incl corresponding BTF ID). (I would
overload and reuse "common-compat" bit if TX gets a common struct).

But lets land RX-side first, but make sure we can easily extend for the 
TX-side.

--Jesper