On 10/4/22 11:26 AM, Stanislav Fomichev wrote:
On Tue, Oct 4, 2022 at 2:29 AM Jesper Dangaard Brouer
<jbrouer@xxxxxxxxxx> wrote:
On 04/10/2022 01.55, sdf@xxxxxxxxxx wrote:
On 09/07, Jesper Dangaard Brouer wrote:
This patchset expose the traditional hardware offload hints to XDP and
rely on BTF to expose the layout to users.
Main idea is that the kernel and NIC drivers simply defines the struct
layouts they choose to use for XDP-hints. These XDP-hints structs gets
naturally and automatically described via BTF and implicitly exported to
users. NIC drivers populate and records their own BTF ID as the last
member in XDP metadata area (making it easily accessible by AF_XDP
userspace at a known negative offset from packet data start).
Naming conventions for the structs (xdp_hints_*) is used such that
userspace can find and decode the BTF layout and match against the
provided BTF IDs. Thus, no new UAPI interfaces are needed for exporting
what XDP-hints a driver supports.
The patch "i40e: Add xdp_hints_union" introduce the idea of creating a
union named "xdp_hints_union" in every driver, which contains all
xdp_hints_* struct this driver can support. This makes it easier/quicker
to find and parse the relevant BTF types. (Seeking input before fixing
up all drivers in patchset).
The main different from RFC-v1:
- Drop idea of BTF "origin" (vmlinux, module or local)
- Instead to use full 64-bit BTF ID that combine object+type ID
I've taken some of Alexandr/Larysa's libbpf patches and integrated
those.
Patchset exceeds netdev usually max 15 patches rule. My excuse is three
NIC drivers (i40e, ixgbe and mvneta) gets XDP-hints support and which
required some refactoring to remove the SKB dependencies.
Hey Jesper,
I took a quick look at the series.
Appreciate that! :-)
Do we really need the enum with the flags?
The primary reason for using enum is that these gets exposed as BTF.
The proposal is that userspace/BTF need to obtain the flags via BTF,
such that they don't become UAPI, but something we can change later.
We might eventually hit that "first 16 bits are reserved" issue?
Instead of exposing enum with the flags, why not solve it as follows:
a. We define UAPI struct xdp_rx_hints with _all_ possible hints
How can we know _all_ possible hints from the beginning(?).
UAPI + central struct dictating all possible hints, will limit innovation.
We don't need to know them all in advance. The same way we don't know
them all for flags enum. That UAPI xdp_rx_hints can be extended any
time some driver needs some new hint offload. The benefit here is that
we have a "common registry" of all offloads and different drivers have
an opportunity to share.
Think of it like current __sk_buff vs sk_buff. xdp_rx_hints is a fake
uapi struct (__sk_buff) and the access to it gets translated into
<device>_xdp_rx_hints offsets (sk_buff).
b. Each device defines much denser <device>_xdp_rx_hints struct with the
metadata that it supports
Thus, the NIC device is limited to what is defined in UAPI struct
xdp_rx_hints. Again this limits innovation.
I guess what I'm missing from your series is the bpf/userspace side.
Do you have an example on the bpf side that will work for, say,
xdp_hints_ixgbe_timestamp?
+1. A selftest is useful.
Suppose, you pass this custom hints btf_id via xdp_md as proposed,
what's the action on the bpf side to consume this?
If (ctx_hints_btf_id == xdp_hints_ixgbe_timestamp_btf_id /* supposedly
populated at runtime by libbpf? */) {
// do something with rx_timestamp
// also, handle xdp_hints_ixgbe and then xdp_hints_common ?
} else if (ctx_hints_btf_id == xdp_hints_ixgbe) {
// do something else
// plus explicitly handle xdp_hints_common here?
} else {
// handle xdp_hints_common
}
What I'd like to avoid is an xdp program targeting specific drivers.
Where possible, we should aim towards something like "if this device
has rx_timestamp offload -> use it without depending too much on
specific btf_ids.
It would be my preference also if it can avoid btf_id comparison of a specific
driver like the above and let the libbpf CO-RE to handle the
matching/relocation. For rx hwtimestamp, the value could be just 0 if a
specific hw/driver cannot provide it for all packets while some other hw can.
A intentionally wild question, what does it take for the driver to return the
hints. Is the rx_desc and rx_queue enough? When the xdp prog is calling a
kfunc/bpf-helper, like 'hwtstamp = bpf_xdp_get_hwtstamp()', can the driver
replace it with some inline bpf code (like how the inline code is generated for
the map_lookup helper). The xdp prog can then store the hwstamp in the meta
area in any layout it wants.
