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  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 14:35:37 +0200
From:   Jesper Dangaard Brouer <>
To:     Magnus Karlsson <>,
        Jesper Dangaard Brouer <>
Cc:, Maryam Tahhan <>,,,,,, Lorenzo Bianconi <>,
        Alexei Starovoitov <>,
        Daniel Borkmann <>,
        Andrii Nakryiko <>,, Magnus Karlsson <>,, Alexander Lobakin <>
Subject: Re: [xdp-hints] Re: [PATCH RFCv2 bpf-next 17/18] xsk: AF_XDP
 xdp-hints support in desc options

On 09/09/2022 12.14, Magnus Karlsson wrote:
> On Fri, Sep 9, 2022 at 11:42 AM Jesper Dangaard Brouer
> <> wrote:
>> 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).
> I think we should reuse the "Rx metadata valid" flag for this since
> this will not be used in the Tx case by definition. In the Tx case,
> this bit would instead mean that the user has provided a valid
> XDP-hints layout. It has a nice symmetry, on Rx it is set by the
> kernel when it has put something relevant in the metadata area. On Tx,
> it is set by user-space if it has put something relevant in the
> metadata area. 

I generally like reusing the bit, *BUT* there is the problem of 
(existing) applications ignoring the desc-options bit and forwarding 
packets.  This would cause the "Rx metadata valid" flag to be seen as 
userspace having set the "TX-hints-bit" and kernel would use what is 
provided in metadata area (leftovers from RX-hints).  IMHO that will be 
hard to debug for end-users and likely break existing applications.

> We can also reuse this bit when we get a notification
> in the completion queue to indicate if the kernel has produced some
> metadata on tx completions. This could be a Tx timestamp for example.

Big YES, reuse "Rx metadata valid" bit when we get a TX notification in 
completion queue.  This will be okay because it cannot be forgotten and 
misinterpreted as the kernel will have responsibility to update this bit.

> So hopefully we could live with only two bits :-).

I still think we need three bits ;-)
That should be enough to cover the 6 states:
  - RX hints
  - RX hints and compat
  - TX hints
  - TX hints and compat
  - TX completion
  - TX completion and compat

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

Powered by blists - more mailing lists