[PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[Jesper Dangaard Brouer]

XDP BPF-prog's need a way to interact with the XDP-hints. This patch
introduces a BPF-helper function, that allow XDP BPF-prog's to interact
with the XDP-hints.

BPF-prog can query if any XDP-hints have been setup and if this is
compatible with the xdp_hints_common struct. If XDP-hints are available
the BPF "origin" is returned (see enum xdp_hints_btf_origin) as BTF can
come from different sources or origins e.g. vmlinux, module or local.

Remember that XDP-hints are setup by the driver prior to calling the
XDP BPF-prog, which is useful for adjusting the HW provided XDP-hints
in-case of HW issues or missing HW features, for use-case like xdp2skb
or AF_XDP.

The BPP-prog might also prefer to use metadata area for other things,
either disabling XDP-hints or updating with another XDP-hints layout
that might still be compatible with common struct. Thus, helper have
"update" and "delete" mode flags.

RFC/TODO: Improve patch: Can verifier validate provided BTF on "update"
and detect if compatible with common struct???

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 include/net/xdp.h        |   42 ++++++++++++++++++++++++++++++++++++++----
 include/uapi/linux/bpf.h |   43 +++++++++++++++++++++++++++++++++++++++++++
 net/core/filter.c        |   45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 5b77fc8fe5ce..710d145a26f9 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -199,14 +199,22 @@ struct xdp_txq_info {
 };
 
 enum xdp_buff_flags {
-	XDP_FLAGS_HAS_FRAGS		= BIT(0), /* non-linear xdp buff */
-	XDP_FLAGS_FRAGS_PF_MEMALLOC	= BIT(1), /* xdp paged memory is under
+	XDP_FLAGS_HINTS_ORIGIN_BIT0	= BIT(0),/* enum xdp_hints_btf_origin */
+	XDP_FLAGS_HINTS_ORIGIN_BIT1	= BIT(1),
+#define	XDP_FLAGS_HINTS_COMPAT_COMMON_	  BIT(3) /* HINTS_BTF_COMPAT_COMMON */
+	XDP_FLAGS_HINTS_COMPAT_COMMON	= XDP_FLAGS_HINTS_COMPAT_COMMON_,
+
+	XDP_FLAGS_HAS_FRAGS		= BIT(4), /* non-linear xdp buff */
+	XDP_FLAGS_FRAGS_PF_MEMALLOC	= BIT(5), /* xdp paged memory is under
 						   * pressure
 						   */
-	XDP_FLAGS_HAS_HINTS		= BIT(2),
-	XDP_FLAGS_HINTS_COMPAT_COMMON	= BIT(3),
 };
 
+#define XDP_FLAGS_HINTS_ORIGIN_MASK	(XDP_FLAGS_HINTS_ORIGIN_BIT0 |	\
+					 XDP_FLAGS_HINTS_ORIGIN_BIT1)
+#define XDP_FLAGS_HINTS_RETURN_MASK	(XDP_FLAGS_HINTS_ORIGIN_MASK |	\
+					 XDP_FLAGS_HINTS_COMPAT_COMMON)
+
 struct xdp_buff {
 	void *data;
 	void *data_end;
@@ -243,6 +251,32 @@ static __always_inline void xdp_buff_set_frag_pfmemalloc(struct xdp_buff *xdp)
 	xdp->flags |= XDP_FLAGS_FRAGS_PF_MEMALLOC;
 }
 
+static __always_inline bool xdp_buff_has_hints(struct xdp_buff *xdp)
+{
+	return !!(xdp->flags & XDP_FLAGS_HINTS_ORIGIN_MASK);
+}
+
+static __always_inline bool xdp_buff_has_hints_compat(struct xdp_buff *xdp)
+{
+	u32 flags = xdp->flags;
+
+	if (!(flags & XDP_FLAGS_HINTS_COMPAT_COMMON))
+		return false;
+
+	return !!(flags & XDP_FLAGS_HINTS_ORIGIN_MASK);
+}
+
+static __always_inline void xdp_buff_set_hints(struct xdp_buff *xdp,
+					       u32 btf_origin,
+					       bool is_compat_common)
+{
+	u32 common = is_compat_common ? XDP_FLAGS_HINTS_COMPAT_COMMON : 0;
+
+	/* enum xdp_hints_btf_origin */
+	btf_origin &= XDP_FLAGS_HINTS_ORIGIN_MASK;
+	xdp->flags |= btf_origin | common;
+}
+
 static __always_inline void
 xdp_init_buff(struct xdp_buff *xdp, u32 frame_sz, struct xdp_rxq_info *rxq)
 {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e81362891596..1c3780c02239 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5325,6 +5325,31 @@ union bpf_attr {
  *		**-EACCES** if the SYN cookie is not valid.
  *
  *		**-EPROTONOSUPPORT** if CONFIG_IPV6 is not builtin.
+ *
+ * long xdp_hints_btf(struct xdp_buff *xdp_md, u32 btf_origin, u64 flags)
+ *     Description
+ *             Update and get info on XDP hints BTF type and origin.
+ *
+ *             Drivers can provide XDP-hints information via the metadata area,
+ *             which defines the layout of this area via BTF. The BTF ID is
+ *             available as the last member. The BTF ID can originate from
+ *             different sources, e.g. vmlinux, module or local BTF-object.
+ *
+ *             In-case a BPF-prog want to redefine the layout of this area it
+ *             should update the BTF ID (last-member) and MUST call this helper
+ *             to specify the origin for the BTF ID.
+ *
+ *             If updating the BTF ID then caller can request that the layout
+ *             is compatible with kernels xdp_hints_common. This is then
+ *             validated (TODO HOW?!?) before kernel side trust this can be
+ *             used for e.g. populating SKB fields.
+ *
+ *             The **flags** are used to control the mode of the helper.
+ *
+ *     Return
+ *             Returns indications on whether XDP-hints were populated by
+ *             driver via an 'origin' value and whether this layout is
+ *             compatible with kernels xdp_hints_common.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5535,6 +5560,7 @@ union bpf_attr {
 	FN(tcp_raw_gen_syncookie_ipv6),	\
 	FN(tcp_raw_check_syncookie_ipv4),	\
 	FN(tcp_raw_check_syncookie_ipv6),	\
+	FN(xdp_hints_btf),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
@@ -5946,6 +5972,23 @@ struct xdp_md {
 	__u32 egress_ifindex;  /* txq->dev->ifindex */
 };
 
+/* Mode flags for BPF_FUNC_xdp_hints_btf helper. */
+enum xdp_hints_btf_mode_flags {
+	HINTS_BTF_QUERY_ONLY    = (1U << 0),
+	HINTS_BTF_UPDATE        = (1U << 1),
+	HINTS_BTF_DISABLE       = (1U << 2),
+	HINTS_BTF_COMPAT_COMMON = (1U << 3),
+};
+
+/* BTF can come from different sources e.g. vmlinux, module or local */
+enum xdp_hints_btf_origin {
+	BTF_ORIGIN_NONE    = 0,
+	BTF_ORIGIN_VMLINUX = 1,
+	BTF_ORIGIN_MODULE  = 2,
+	BTF_ORIGIN_LOCAL   = 3,
+	BTF_ORIGIN_MASK    = 0x3,
+};
+
 /* DEVMAP map-value layout
  *
  * The struct data-layout of map-value is a configuration interface.
diff --git a/net/core/filter.c b/net/core/filter.c
index 151aa4756bd6..614054f89fdc 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6129,6 +6129,49 @@ static const struct bpf_func_proto bpf_xdp_check_mtu_proto = {
 	.arg5_type      = ARG_ANYTHING,
 };
 
+/* btf_origin type &enum xdp_hints_btf_origin
+ * flags      type &enum xdp_hints_btf_mode_flags
+ */
+BPF_CALL_3(bpf_xdp_hints_btf, struct xdp_buff *, xdp, u32, btf_origin, u64, flags)
+{
+	s64 ret = BTF_ORIGIN_NONE;
+	bool is_compat_common;
+
+	if (flags & HINTS_BTF_QUERY_ONLY) {
+		BUILD_BUG_ON(HINTS_BTF_COMPAT_COMMON != XDP_FLAGS_HINTS_COMPAT_COMMON_);
+		ret = xdp->flags & XDP_FLAGS_HINTS_RETURN_MASK;
+		goto out;
+	}
+	if (flags & HINTS_BTF_DISABLE) {
+		xdp_buff_set_hints(xdp, BTF_ORIGIN_NONE, false);
+		goto out;
+	}
+	if (flags & HINTS_BTF_UPDATE) {
+		is_compat_common = !!(flags & HINTS_BTF_COMPAT_COMMON);
+	/* TODO: Can kernel validate if hints are BTF compat with common? */
+	/* TODO: Could BPF prog provide BTF as ARG_PTR_TO_BTF_ID to prove compat_common ? */
+	/* TODO: Validate if metadata size is >= sizeof(xdp_hints_common) */
+		btf_origin &= BTF_ORIGIN_MASK;
+	/* TODO: Validate if module BTF_ID is large than vmlinux base */
+		xdp_buff_set_hints(xdp, btf_origin, is_compat_common);
+		BUILD_BUG_ON(BTF_ORIGIN_MASK != XDP_FLAGS_HINTS_ORIGIN_MASK);
+		ret = xdp->flags & XDP_FLAGS_HINTS_RETURN_MASK;
+		goto out;
+	}
+
+ out:
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_xdp_hints_btf_proto = {
+	.func		= bpf_xdp_hints_btf,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_ANYTHING,
+};
+
 #if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
 static int bpf_push_seg6_encap(struct sk_buff *skb, u32 type, void *hdr, u32 len)
 {
@@ -7959,6 +8002,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_xdp_fib_lookup_proto;
 	case BPF_FUNC_check_mtu:
 		return &bpf_xdp_check_mtu_proto;
+	case BPF_FUNC_xdp_hints_btf:
+		return &bpf_xdp_hints_btf_proto;
 #ifdef CONFIG_INET
 	case BPF_FUNC_sk_lookup_udp:
 		return &bpf_xdp_sk_lookup_udp_proto;

Re: [PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[Zaremba, Larysa]

Toke Høiland-Jørgensen <toke@redhat.com> writes:
> 
> Jesper Dangaard Brouer <jbrouer@redhat.com> writes:
> 
> > On 29/06/2022 16.20, Toke Høiland-Jørgensen wrote:
> >> Jesper Dangaard Brouer <brouer@redhat.com> writes:
> >>
> >>> XDP BPF-prog's need a way to interact with the XDP-hints. This 
> >>> patch introduces a BPF-helper function, that allow XDP BPF-prog's 
> >>> to interact with the XDP-hints.
> >>>
> >>> BPF-prog can query if any XDP-hints have been setup and if this is 
> >>> compatible with the xdp_hints_common struct. If XDP-hints are 
> >>> available the BPF "origin" is returned (see enum
> >>> xdp_hints_btf_origin) as BTF can come from different sources or 
> >>> origins
> e.g. vmlinux, module or local.
> >>
> >> I'm not sure I quite understand what this origin is supposed to be 
> >> good for?
> >
> > Some background info on BTF is needed here: BTF_ID numbers are not 
> > globally unique identifiers, thus we need to know where it originate 
> > from, to make it unique (as we store this BTF_ID in XDP-hints).
> >
> > There is a connection between origin "vmlinux" and "module", which 
> > is that vmlinux will start at ID=1 and end at a max ID number.  
> > Modules refer to ID's in "vmlinux", and for this to work, they will 
> > shift their own numbering to start after ID=max-vmlinux-id.
> >
> > Origin "local" is for BTF information stored in the BPF-ELF object file.
> > Their numbering starts at ID=1.  The use-case is that a BPF-prog 
> > want to extend the kernel drivers BTF-layout, and e.g. add a 
> > RX-timestamp like [1].  Then BPF-prog can check if it knows module's 
> > BTF_ID and then extend via bpf_xdp_adjust_meta, and update BTF_ID in 
> > XDP-hints and call the helper (I introduced) marking this as origin 
> > "local" for kernel to know this is no-longer origin "module".
> 
> Right, I realise that :)
> 
> My point was that just knowing "this is a BTF ID coming from a module"
> is not terribly useful; you could already figure that out by just 
> looking at the ID and seeing if it's larger than the maximum ID in vmlinux BTF.
> 
> Rather, what we need is a way to identify *which* module the BTF ID 
> comes from; and luckily, the kernel assigns a unique ID to every BTF
> *object* as well as to each type ID within that object. These can be 
> dumped by bpftool:
> 
> # bpftool btf
> bpftool btf
> [sudo] password for alrua:
> 1: name [vmlinux]  size 4800187B
> 2: name [serio]  size 2588B
> 3: name [i8042]  size 11786B
> 4: name [rng_core]  size 8184B
> [...]
> 2062: name <anon>  size 36965B
> 	pids bpftool(547298)
> 
> IDs 2-4 are module BTF objects, and that last one is the ID of a BTF 
> object loaded along with a BPF program by bpftool itself... So we *do* 
> in fact have a unique ID, by combining the BTF object ID with the type 
> ID; this is what Alexander is proposing to put into the xdp-hints 
> struct as well (combining the two IDs into a single u64).

That's correct, concept was previously discussed [1]. The ID of BTF object wasn't
exposed in CO-RE allocations though, we've changed it in the first 4 patches.
The main logic is in "libbpf: factor out BTF loading from load_module_btfs()"
and "libbpf: patch module BTF ID into BPF insns".

We have a sample that wasn't included eventually, but can possibly
give a general understanding of our approach [2].

[1] https://lore.kernel.org/all/CAEf4BzZO=7MKWfx2OCwEc+sKkfPZYzaELuobi4q5p1bOKk4AQQ@mail.gmail.com/
[2] https://github.com/alobakin/linux/pull/16/files#diff-c5983904cbe0c280453d59e8a1eefb56c67018c38d5da0c1122abc86225fc7c9

> >> What is a BPF (or AF_XDP) program supposed to do with the 
> >> information "this XDP hints struct came from a module?" without 
> >> knowing which module that was?
> >
> > For AF_XDP my claim is the userspace program will already know that 
> > driver it is are talking to because it need to "bind" to a specific 
> > interface (and attach to XDP-prog to ifindex). See sample code[2] 
> > for get_driver_name from ifindex.
> > Thus, part of using XDP-hints already involves (resolving and) 
> > opening /sys/kernel/btf/driver_name.  So the origin "module" is 
> > enough for the API end-user to make the BTF_ID unique.
> 
> This will probably work in the most common cases, but offers no way to 
> verify that this "offline" resolution of module ID is actually correct.
> Explicitly encoding the full unique ID will be more robust.
> 
> > Runtime the BPF-prog and kernel can find out what net_device the 
> > origin "module" refers to via xdp_buff->rxq->dev.  When an 
> > end-user/program attach XDP they also need to know the ifindex, 
> > again giving them knowledge that make origin "module" BTF_ID's 
> > unique for them,
> 
> Right, but then the BPF program needs to keep its own lookup table 
> from ifindex to BTF ID? If we just encode the full ID in the packet, 
> it's a simple check, and we can likely create a "magic" CO-RE macro 
> that turns a struct definition into the right ID check at load time...
> 
> >> Ultimately, the origin is useful for a consumer to check that the 
> >> metadata is in the format that it's expecting it to be in (so it 
> >> can just load the data from the appropriate offsets). But to answer 
> >> this, we really need a unique identifier; so I think the approach 
> >> in Alexander's series of encoding the ID of the BTF structure 
> >> itself into the next 32 bits is better? That way we'll have a unique "pointer"
> >> to the actual struct that's in the metadata area and can act on this.
> >
> > I would really like an explanation from Alexander, how his approach 
> > creates unique identifier across all kernel modules.  I don't get it 
> > from reading the code.  To me it looks like some extra BTF "type"
> > information about the BTF_ID.
> >
> > E.g. how do BTF "local" BPF-ELF object's get a unique identifier, 
> > that doesn't overlap with e.g. kernel modules?
> 
> See above: the kernel generates a unique (until the next reboot) ID 
> for every BTF object when it's loaded into the kernel.
> 
> >>> RFC/TODO: Improve patch: Can verifier validate provided BTF on
> "update"
> >>> and detect if compatible with common struct???
> >>
> >> If we have the unique ID as mentioned above, I think the kernel 
> >> probably could resolve this automatically: whenever a module is 
> >> loaded, the kernel could walk the BTF information from that module 
> >> an simply inspect all the metadata structs and see if they contain 
> >> the embedded xdp_hints_common struct. The IDs of any metadata 
> >> structs that do contain the common struct can then be kept in a 
> >> central lookup table and the consumption code can then simply 
> >> compare the BTF ID to this table when building an SKB?
> >
> > I'm not against the idea for the kernel to keep track of these structs.
> > I just don't like the idea of checking this runtime, especially as 
> > this approach for walking all other modules BTF struct's doesn't scale.
> 
> Yeah, we should optimise this. See below...
> 
> >> As for the validation on the BPF side:n
> >>
> >>> +	if (flags & HINTS_BTF_UPDATE) {
> >>> +		is_compat_common = !!(flags &
> HINTS_BTF_COMPAT_COMMON);
> >>> +	/* TODO: Can kernel validate if hints are BTF compat with common?
> */
> >>> +	/* TODO: Could BPF prog provide BTF as ARG_PTR_TO_BTF_ID to
> prove
> >>> +compat_common ? */
> >>
> >> If we use the "global ID + lookup table" approach above, we don't 
> >> really need to validate anything here: if the program says it's 
> >> writing metadata with a format given by a specific ID, that implies 
> >> compatibility (or not) as given by the ID. We could sanity-check 
> >> the metadata area size, but the consumption code has to do that 
> >> anyway, so I'm not sure it's worth the runtime overhead to have an 
> >> additional check here?
> >
> > As you know I hate "runtime checks", and try hard to push checks to 
> > "setup time".  Maybe we could have verifier (or libbpf) do the check 
> > at setup/load time, by identifying the helper call and check if 
> > provided BTF do match COMPAT_COMMON claim.
> >
> > For this to work, the verifier need to be able to resolve origin 
> > "module", which happens at BPF load-time, so we would need to set 
> > the ifindex (curr used for XDP-hardware-offload) at BPF load-time.
> 
> If we make the UAPI on the BPF side just accept a full BTF object+type 
> ID, and also require that the value being passed to the helper is a 
> compile-time constant (so it is visible to the verifier at 
> verification time), it is straight- forward for the verifier to just 
> lookup the BTF type, check if it contains the "hints_common" struct 
> and if it does, rewrite the helper call to set the right value of the "compat_common"
> flag without exposing the flag itself as UAPI.
> 
> The driver code would probably still have to set this flag "manually", 
> but that's internal kernel API, so that's probably fine...
> 
> >> As for safety of the metadata content itself, I don't really think 
> >> we can do anything to guarantee this: in any case the BPF program 
> >> can pass a valid BTF ID and still write garbage values into the 
> >> actual fields, so the consumption code has to do enough validation 
> >> that this won't crash the kernel anyway. But this is no different 
> >> from the packet
> data itself:
> >> XDP is basically in a position to be a MITM attacker of the network 
> >> stack itself, which is why loading XDP programs is a privileged 
> >> operation...
> >
> > I agree, that we cannot stop the end-user from screwing up their 
> > BPF-prog to provide garbage in the fields, as long as it doesn't 
> > crash the kernel.  I do think it would improve usability for 
> > end-users if we can detect and report that their BPF-prog have 
> > gotten out of sync with the running kernel and their claim that 
> > their BTF layout are COMPAT_COMMON isn't actually true.  But I guess 
> > it is shouldn't block the code, as it's only an extra usability help.
> 
> Yeah, I agree this could be error prone; which is why I think not 
> exposing the flag itself as UAPI is a better solution ;)
> 
> -Toke

Please CC me in the hints discussions.
- Larysa

Re: [PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[Jesper Dangaard Brouer]

On 04/07/2022 13.00, Zaremba, Larysa wrote:
> Toke Høiland-Jørgensen <toke@redhat.com> writes:
>>
>> Jesper Dangaard Brouer <jbrouer@redhat.com> writes:
>>
>>> On 29/06/2022 16.20, Toke Høiland-Jørgensen wrote:
>>>> Jesper Dangaard Brouer <brouer@redhat.com> writes:
>>>>
>>>>> XDP BPF-prog's need a way to interact with the XDP-hints. This
>>>>> patch introduces a BPF-helper function, that allow XDP BPF-prog's
>>>>> to interact with the XDP-hints.
>>>>>
>>>>> BPF-prog can query if any XDP-hints have been setup and if this is
>>>>> compatible with the xdp_hints_common struct. If XDP-hints are
>>>>> available the BPF "origin" is returned (see enum
>>>>> xdp_hints_btf_origin) as BTF can come from different sources or
>>>>> origins e.g. vmlinux, module or local.
>>>>
>>>> I'm not sure I quite understand what this origin is supposed to be
>>>> good for?
>>>
>>> Some background info on BTF is needed here: BTF_ID numbers are not
>>> globally unique identifiers, thus we need to know where it originate
>>> from, to make it unique (as we store this BTF_ID in XDP-hints).
>>>
>>> There is a connection between origin "vmlinux" and "module", which
>>> is that vmlinux will start at ID=1 and end at a max ID number.
>>> Modules refer to ID's in "vmlinux", and for this to work, they will
>>> shift their own numbering to start after ID=max-vmlinux-id.
>>>
>>> Origin "local" is for BTF information stored in the BPF-ELF object file.
>>> Their numbering starts at ID=1.  The use-case is that a BPF-prog
>>> want to extend the kernel drivers BTF-layout, and e.g. add a
>>> RX-timestamp like [1].  Then BPF-prog can check if it knows module's
>>> BTF_ID and then extend via bpf_xdp_adjust_meta, and update BTF_ID in
>>> XDP-hints and call the helper (I introduced) marking this as origin
>>> "local" for kernel to know this is no-longer origin "module".
>>
>> Right, I realise that :)
>>
>> My point was that just knowing "this is a BTF ID coming from a module"
>> is not terribly useful; you could already figure that out by just
>> looking at the ID and seeing if it's larger than the maximum ID in vmlinux BTF.
>>
>> Rather, what we need is a way to identify *which* module the BTF ID
>> comes from; and luckily, the kernel assigns a unique ID to every BTF
>> *object* as well as to each type ID within that object. These can be
>> dumped by bpftool:
>>
>> # bpftool btf
>> bpftool btf
>> [sudo] password for alrua:
>> 1: name [vmlinux]  size 4800187B
>> 2: name [serio]  size 2588B
>> 3: name [i8042]  size 11786B
>> 4: name [rng_core]  size 8184B
>> [...]
>> 2062: name <anon>  size 36965B
>> 	pids bpftool(547298)
>>
>> IDs 2-4 are module BTF objects, and that last one is the ID of a BTF
>> object loaded along with a BPF program by bpftool itself... So we *do*
>> in fact have a unique ID, by combining the BTF object ID with the type
>> ID; this is what Alexander is proposing to put into the xdp-hints
>> struct as well (combining the two IDs into a single u64).

Thanks for the explanation. I think I understand it now, and I agree
that we should extend/combining the two IDs into a single u64.

To Andrii, what is the right terminology when talking about these two
different BTF-ID's:

- BTF object ID and BTF type ID?

- Where BTF *object* ID are the IDs we see above from 'bpftool btf',
   where vmlinux=1 and module's IDs will start after 1.

- Where BTF *type* ID are the IDs the individual data "types" within a
   BTF "object" (e.g. struct xdp_hints_common that BPF-prog's can get
   via calling bpf_core_type_id_kernel()).


> That's correct, concept was previously discussed [1]. The ID of BTF object wasn't
> exposed in CO-RE allocations though, we've changed it in the first 4 patches.
> The main logic is in "libbpf: factor out BTF loading from load_module_btfs()"
> and "libbpf: patch module BTF ID into BPF insns".
> 
> We have a sample that wasn't included eventually, but can possibly
> give a general understanding of our approach [2].
> 
> [1] https://lore.kernel.org/all/CAEf4BzZO=7MKWfx2OCwEc+sKkfPZYzaELuobi4q5p1bOKk4AQQ@mail.gmail.com/
> [2] https://github.com/alobakin/linux/pull/16/files#diff-c5983904cbe0c280453d59e8a1eefb56c67018c38d5da0c1122abc86225fc7c9
> 
(appreciate the links)

I wonder how these BTF object IDs gets resolved for my "local" category?
(Origin "local" is for BTF information stored in the BPF-ELF object file)

Note: For "local" BTF type IDs BPF-prog resolve these via
bpf_core_type_id_local() (why I choose the term "local").

--Jesper

p.s. For unknown reasons lore.kernel.org did match Larysa's reply with 
the patchset thread here[3].

  [3] 
https://lore.kernel.org/bpf/165643378969.449467.13237011812569188299.stgit@firesoul/#r

Re: [PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[Larysa Zaremba]

On Mon, Jul 04, 2022 at 08:26:15PM +0200, Jesper Dangaard Brouer wrote:
> 
> 
> On 04/07/2022 13.00, Zaremba, Larysa wrote:
> > Toke Høiland-Jørgensen <toke@redhat.com> writes:
> > > 
> > > Jesper Dangaard Brouer <jbrouer@redhat.com> writes:
> > > 
> > > > On 29/06/2022 16.20, Toke Høiland-Jørgensen wrote:
> > > > > Jesper Dangaard Brouer <brouer@redhat.com> writes:
> > > > > 
> > > > > > XDP BPF-prog's need a way to interact with the XDP-hints. This
> > > > > > patch introduces a BPF-helper function, that allow XDP BPF-prog's
> > > > > > to interact with the XDP-hints.
> > > > > > 
> > > > > > BPF-prog can query if any XDP-hints have been setup and if this is
> > > > > > compatible with the xdp_hints_common struct. If XDP-hints are
> > > > > > available the BPF "origin" is returned (see enum
> > > > > > xdp_hints_btf_origin) as BTF can come from different sources or
> > > > > > origins e.g. vmlinux, module or local.
> > > > > 
> > > > > I'm not sure I quite understand what this origin is supposed to be
> > > > > good for?
> > > > 
> > > > Some background info on BTF is needed here: BTF_ID numbers are not
> > > > globally unique identifiers, thus we need to know where it originate
> > > > from, to make it unique (as we store this BTF_ID in XDP-hints).
> > > > 
> > > > There is a connection between origin "vmlinux" and "module", which
> > > > is that vmlinux will start at ID=1 and end at a max ID number.
> > > > Modules refer to ID's in "vmlinux", and for this to work, they will
> > > > shift their own numbering to start after ID=max-vmlinux-id.
> > > > 
> > > > Origin "local" is for BTF information stored in the BPF-ELF object file.
> > > > Their numbering starts at ID=1.  The use-case is that a BPF-prog
> > > > want to extend the kernel drivers BTF-layout, and e.g. add a
> > > > RX-timestamp like [1].  Then BPF-prog can check if it knows module's
> > > > BTF_ID and then extend via bpf_xdp_adjust_meta, and update BTF_ID in
> > > > XDP-hints and call the helper (I introduced) marking this as origin
> > > > "local" for kernel to know this is no-longer origin "module".
> > > 
> > > Right, I realise that :)
> > > 
> > > My point was that just knowing "this is a BTF ID coming from a module"
> > > is not terribly useful; you could already figure that out by just
> > > looking at the ID and seeing if it's larger than the maximum ID in vmlinux BTF.
> > > 
> > > Rather, what we need is a way to identify *which* module the BTF ID
> > > comes from; and luckily, the kernel assigns a unique ID to every BTF
> > > *object* as well as to each type ID within that object. These can be
> > > dumped by bpftool:
> > > 
> > > # bpftool btf
> > > bpftool btf
> > > [sudo] password for alrua:
> > > 1: name [vmlinux]  size 4800187B
> > > 2: name [serio]  size 2588B
> > > 3: name [i8042]  size 11786B
> > > 4: name [rng_core]  size 8184B
> > > [...]
> > > 2062: name <anon>  size 36965B
> > > 	pids bpftool(547298)
> > > 
> > > IDs 2-4 are module BTF objects, and that last one is the ID of a BTF
> > > object loaded along with a BPF program by bpftool itself... So we *do*
> > > in fact have a unique ID, by combining the BTF object ID with the type
> > > ID; this is what Alexander is proposing to put into the xdp-hints
> > > struct as well (combining the two IDs into a single u64).
> 
> Thanks for the explanation. I think I understand it now, and I agree
> that we should extend/combining the two IDs into a single u64.
> 
> To Andrii, what is the right terminology when talking about these two
> different BTF-ID's:
> 
> - BTF object ID and BTF type ID?
> 
> - Where BTF *object* ID are the IDs we see above from 'bpftool btf',
>   where vmlinux=1 and module's IDs will start after 1.
> 
> - Where BTF *type* ID are the IDs the individual data "types" within a
>   BTF "object" (e.g. struct xdp_hints_common that BPF-prog's can get
>   via calling bpf_core_type_id_kernel()).
> 

AFAIK, that's the most correct way of distinguish one from another in 
conversation.

Would be still great, if Andrii could confirm that.

I should mention that out patch makes bpf_core_type_id_kernel() return 
u64 (BTF obj ID + BTF type ID), but your statement is true for current 
libbpf version.

> 
> > That's correct, concept was previously discussed [1]. The ID of BTF object wasn't
> > exposed in CO-RE allocations though, we've changed it in the first 4 patches.
> > The main logic is in "libbpf: factor out BTF loading from load_module_btfs()"
> > and "libbpf: patch module BTF ID into BPF insns".
> > 
> > We have a sample that wasn't included eventually, but can possibly
> > give a general understanding of our approach [2].
> > 
> > [1] https://lore.kernel.org/all/CAEf4BzZO=7MKWfx2OCwEc+sKkfPZYzaELuobi4q5p1bOKk4AQQ@mail.gmail.com/
> > [2] https://github.com/alobakin/linux/pull/16/files#diff-c5983904cbe0c280453d59e8a1eefb56c67018c38d5da0c1122abc86225fc7c9
> > 
> (appreciate the links)
> 
> I wonder how these BTF object IDs gets resolved for my "local" category?
> (Origin "local" is for BTF information stored in the BPF-ELF object file)
> 
> Note: For "local" BTF type IDs BPF-prog resolve these via
> bpf_core_type_id_local() (why I choose the term "local").
> 

Every program during CO-RE relocs sees a single local BTF obj, in which 
BTF type IDs start from 1 and correspond to all data types used in 
program. So local BTF obj and type IDs inside are valid only in single 
program, therefore u32 type ID returned by bpf_core_type_id_local() is 
enough.

Local IDs are not resolved, they are just assigned during compilation. 
After program load with CO-RE each local type gets a resolved
vmlinux/module BTF obj pointer and an ID of a type inside this BTF obj 
that is similar enough.

Both local and target type IDs are mainly needed just for comfortable 
iteration inside libbpf, so they are just a side product that is only 
patched in, if we use bpf_core_type_id_local/target() inside a program 
for testing purposes.

> --Jesper
> 
> p.s. For unknown reasons lore.kernel.org did match Larysa's reply with the
> patchset thread here[3].
> 
>  [3] https://lore.kernel.org/bpf/165643378969.449467.13237011812569188299.stgit@firesoul/#r
> 
> 

- Larysa

Re: [PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[Jesper Dangaard Brouer]

On 05/07/2022 19.07, Larysa Zaremba wrote:
> On Mon, Jul 04, 2022 at 08:26:15PM +0200, Jesper Dangaard Brouer wrote:
>>
>>
>> On 04/07/2022 13.00, Zaremba, Larysa wrote:
>>> Toke Høiland-Jørgensen <toke@redhat.com> writes:
>>>>
>>>> Jesper Dangaard Brouer <jbrouer@redhat.com> writes:
>>>>
>>>>> On 29/06/2022 16.20, Toke Høiland-Jørgensen wrote:
>>>>>> Jesper Dangaard Brouer <brouer@redhat.com> writes:
>>>>>>
>>>>>>> XDP BPF-prog's need a way to interact with the XDP-hints. This
>>>>>>> patch introduces a BPF-helper function, that allow XDP BPF-prog's
>>>>>>> to interact with the XDP-hints.
>>>>>>>
>>>>>>> BPF-prog can query if any XDP-hints have been setup and if this is
>>>>>>> compatible with the xdp_hints_common struct. If XDP-hints are
>>>>>>> available the BPF "origin" is returned (see enum
>>>>>>> xdp_hints_btf_origin) as BTF can come from different sources or
>>>>>>> origins e.g. vmlinux, module or local.
>>>>>>
>>>>>> I'm not sure I quite understand what this origin is supposed to be
>>>>>> good for?
>>>>>
>>>>> Some background info on BTF is needed here: BTF_ID numbers are not
>>>>> globally unique identifiers, thus we need to know where it originate
>>>>> from, to make it unique (as we store this BTF_ID in XDP-hints).
>>>>>
>>>>> There is a connection between origin "vmlinux" and "module", which
>>>>> is that vmlinux will start at ID=1 and end at a max ID number.
>>>>> Modules refer to ID's in "vmlinux", and for this to work, they will
>>>>> shift their own numbering to start after ID=max-vmlinux-id.
>>>>>
>>>>> Origin "local" is for BTF information stored in the BPF-ELF object file.
>>>>> Their numbering starts at ID=1.  The use-case is that a BPF-prog
>>>>> want to extend the kernel drivers BTF-layout, and e.g. add a
>>>>> RX-timestamp like [1].  Then BPF-prog can check if it knows module's
>>>>> BTF_ID and then extend via bpf_xdp_adjust_meta, and update BTF_ID in
>>>>> XDP-hints and call the helper (I introduced) marking this as origin
>>>>> "local" for kernel to know this is no-longer origin "module".
>>>>
>>>> Right, I realise that :)
>>>>
>>>> My point was that just knowing "this is a BTF ID coming from a module"
>>>> is not terribly useful; you could already figure that out by just
>>>> looking at the ID and seeing if it's larger than the maximum ID in vmlinux BTF.
>>>>
>>>> Rather, what we need is a way to identify *which* module the BTF ID
>>>> comes from; and luckily, the kernel assigns a unique ID to every BTF
>>>> *object* as well as to each type ID within that object. These can be
>>>> dumped by bpftool:
>>>>
>>>> # bpftool btf
>>>> bpftool btf
>>>> [sudo] password for alrua:
>>>> 1: name [vmlinux]  size 4800187B
>>>> 2: name [serio]  size 2588B
>>>> 3: name [i8042]  size 11786B
>>>> 4: name [rng_core]  size 8184B
>>>> [...]
>>>> 2062: name <anon>  size 36965B
>>>> 	pids bpftool(547298)
>>>>
>>>> IDs 2-4 are module BTF objects, and that last one is the ID of a BTF
>>>> object loaded along with a BPF program by bpftool itself... So we *do*
>>>> in fact have a unique ID, by combining the BTF object ID with the type
>>>> ID; this is what Alexander is proposing to put into the xdp-hints
>>>> struct as well (combining the two IDs into a single u64).
>>
>> Thanks for the explanation. I think I understand it now, and I agree
>> that we should extend/combining the two IDs into a single u64.
>>
>> To Andrii, what is the right terminology when talking about these two
>> different BTF-ID's:
>>
>> - BTF object ID and BTF type ID?
>>
>> - Where BTF *object* ID are the IDs we see above from 'bpftool btf',
>>    where vmlinux=1 and module's IDs will start after 1.
>>
>> - Where BTF *type* ID are the IDs the individual data "types" within a
>>    BTF "object" (e.g. struct xdp_hints_common that BPF-prog's can get
>>    via calling bpf_core_type_id_kernel()).
>>
> 
> AFAIK, that's the most correct way of distinguish one from another in
> conversation.

Good to get confirmed that you agree with these terms.

> 
> Would be still great, if Andrii could confirm that.

Yes, it would :-)

> I should mention that out patch makes bpf_core_type_id_kernel() return
> u64 (BTF obj ID + BTF type ID), but your statement is true for current
> libbpf version.

It sounds useful that your patched bpf_core_type_id_kernel() returns u64
(BTF obj ID + BTF type ID).

I wonder if/how we need to deal with libbpf versions that only returns
the u32 BTF type ID ?

> 
>>
>>> That's correct, concept was previously discussed [1]. The ID of BTF object wasn't
>>> exposed in CO-RE allocations though, we've changed it in the first 4 patches.
>>> The main logic is in "libbpf: factor out BTF loading from load_module_btfs()"
>>> and "libbpf: patch module BTF ID into BPF insns".
>>>
>>> We have a sample that wasn't included eventually, but can possibly
>>> give a general understanding of our approach [2].
>>>
>>> [1] https://lore.kernel.org/all/CAEf4BzZO=7MKWfx2OCwEc+sKkfPZYzaELuobi4q5p1bOKk4AQQ@mail.gmail.com/
>>> [2] https://github.com/alobakin/linux/pull/16/files#diff-c5983904cbe0c280453d59e8a1eefb56c67018c38d5da0c1122abc86225fc7c9
>>>
>> (appreciate the links)
>>
>> I wonder how these BTF object IDs gets resolved for my "local" category?
>> (Origin "local" is for BTF information stored in the BPF-ELF object file)
>>
>> Note: For "local" BTF type IDs BPF-prog resolve these via
>> bpf_core_type_id_local() (why I choose the term "local").
>>
> 
> Every program during CO-RE relocs sees a single local BTF obj, in which
> BTF type IDs start from 1 and correspond to all data types used in
> program. So local BTF obj and type IDs inside are valid only in single
> program, therefore u32 type ID returned by bpf_core_type_id_local() is
> enough.

Sure it makes sense if only a single XDP-prog is running.

For the use-case of multiple XDP-progs (e.g. via libxdp) are running,
where they send info to each-other via metadata area. There it would be
valuable to get a BTF *object* ID associated with these "local" types.

Note that I believe that a TC ingress BPF-prog can also read the
metadata area.

> Local IDs are not resolved, they are just assigned during compilation.
> After program load with CO-RE each local type gets a resolved
> vmlinux/module BTF obj pointer and an ID of a type inside this BTF obj
> that is similar enough.

Yes, but only if __attribute__((preserve_access_index)) is defined on
the "local" BPF-prog struct will libbpf do this matching to kernel
structs, see[1].

  [1] 
https://github.com/xdp-project/bpf-examples/blob/18908873a7f48483ed8bab2d949e8760cff30810/AF_XDP-interaction/af_xdp_kern.c#L37-L40
  [2] 
https://github.com/xdp-project/bpf-examples/tree/master/AF_XDP-interaction


> 
> Both local and target type IDs are mainly needed just for comfortable
> iteration inside libbpf, so they are just a side product that is only
> patched in, if we use bpf_core_type_id_local/target() inside a program
> for testing purposes.

In my use-case[2] I want to extract the "local" BTF ID and update the
BTF ID in metadata area, such that my AF_XDP program can see it.

The use-case is that I have a BPF-prog that want to extend the
kernel-module provided XDP-hints, with an XDP-software RX timestamp, but
only for packets containing HW timestamps. It will (load/setup time)
know BTF obj+type ID via
bpf_core_type_id_kernel(xdp_hints_i40e_timestamp) to match on, and then
extend metadata area, type-cast to "local" struct and record
bpf_ktime_get_ns().  It now need to update BTF ID in XDP-hints metadata
area, to tell AF_XDP userspace prog (or chained XDP/TC BPF-prog) that
layout format have changed.

- My question is: What BTF *object* ID should I use for my "local" u32
BTF type ID ? (returned by bpf_core_type_id_local())

--Jesper

>>
>> p.s. For unknown reasons lore.kernel.org did match Larysa's reply with the
>> patchset thread here[3].
>>
>>   [3] https://lore.kernel.org/bpf/165643378969.449467.13237011812569188299.stgit@firesoul/#r