[PATCH bpf-next 00/15] xdp: add SKB core extensions to keep RX metadata until skb creation

[PATCH bpf-next 00/15] xdp: add SKB core extensions to keep RX metadata until skb creation

[Tianyi Gao]

An skb gets its RX hash, VLAN tag and checksum status from the RX
descriptor, which describes the packet before the XDP program ran.

Without this series:

- On XDP_PASS, the values are stale. After the program strips a UDP
  tunnel header, the stack applies the outer CHECKSUM_UNNECESSARY to the
  inner packet [1]. If the outer UDP checksum is 0, a packet with a bad
  inner checksum reaches the socket.
- On XDP_REDIRECT, the values are lost. The skb that cpumap or veth
  builds from the xdp_frame has no RX hash and no VLAN tag, and its
  checksum status is CHECKSUM_NONE.

In one use case, an XDP program redirects packets into a veth device.
The checksum status is lost, so GRO verifies every checksum in software.
With one TCP flow, this costs 34% of the receiving CPU.

Another use case [2] is load balancing IPsec tunnel traffic with cpumap.
Without the RX hash, GRO on cpumap puts all flows into the same bucket.
In a cpumap test with 16 TCP flows and no IPsec, the throughput is
17.6 Gbit/s instead of 23.4.

This series introduces SKB core extensions for XDP. It reserves an skb
core extension area in the headroom, right after the space for struct
xdp_frame. An XDP program stores RX metadata in the area with kfuncs,
and the kernel sets the skb fields from it. This design was initially
proposed in [3].

The layout of the area follows skb extensions (struct skb_ext). A small
header gives the offset of each entry. An entry takes headroom only when
the program stores it, so the area does not grow with each new type of
RX metadata, unlike [4]. Unlike skb_ext, the area needs no allocation.

This series supports three types of RX metadata. Each type has a store
kfunc, and bpf_xdp_skb_ext_del() deletes an entry:

- The RX hash and its type: bpf_xdp_skb_ext_store_rx_hash().
- The VLAN tag and its protocol: bpf_xdp_skb_ext_store_rx_vlan().
- The checksum status: bpf_xdp_skb_ext_store_rx_csum(). Only
  XDP_CHECKSUM_NONE, or XDP_CHECKSUM_UNNECESSARY with a csum_level up to
  SKB_MAX_CSUM_LEVEL, are allowed [5][6].

The kernel sets the skb fields from the area:

- After XDP_REDIRECT, in __xdp_build_skb_from_frame().
- On XDP_PASS, in the NIC driver. A driver that does this sets the new
  XDP feature skb-ext-pass. On other native XDP drivers, the kernel
  warns once when the program returns XDP_PASS with stored values, and
  ignores the area. This series adds the support to ice.

The existing XDP RX metadata kfuncs are bpf_xdp_metadata_rx_*(). Only
a device-bound program can call them. Programs on cpumap and devmap are
not device-bound, so they cannot call them. Relaxing this check would
change their semantics. So this series adds three load kfuncs,
bpf_xdp_skb_ext_load_rx_*(). They read the stored values.

bpf_xdp_adjust_head(), bpf_xdp_adjust_meta() and bpf_xdp_pull_data() do
not move the packet or the metadata into the area. In this series, the
area takes at most 20 bytes of headroom. With the hardware RX timestamp,
the RX queue index and CHECKSUM_COMPLETE, it would take up to 40 bytes.

Lorenzo's RX checksum kfunc series v5 [7] also defines enum
xdp_checksum. The series that is merged second drops its copy.

Performance:

We measured on ice (E810, 25 Gbit/s) with bpf-next 0fb18ce8a2e9, patches
1-3 of [7] and this series. In the RX metadata column, the XDP program
reads a value with bpf_xdp_metadata_rx_*() and stores it. The veth test
stores the checksum status, and the cpumap test stores the RX hash. The
kernel then sets the value on the skb. Baseline does not store it. Each
value is the median of 5 rounds:

test                  metric          baseline     RX metadata
veth, 1 TCP flow      RX CPU busy     100%         66% (-34%)
veth, 1 TCP flow      RX cycles/pkt   2027         1343 (-34%)
cpumap, 16 TCP flows  throughput      17.6 Gbit/s  23.4 (+33%)
cpumap, 16 TCP flows  pkts/GRO skb    3.4          7.5 (+122%)

Both veth columns reach line rate, so the gain is in CPU time. A store
kfunc takes 2.9 to 4.9 ns. Details are in [8].

Not in this series:

- XDP_PASS support in mlx5 and bnxt. To align with Lorenzo's series [7],
  we plan to add xmo_rx_checksum and XDP_PASS support to them together,
  one driver at a time.
- AF_XDP zero-copy. User space can write the headroom of zero-copy
  buffers, so the store kfuncs return -EOPNOTSUPP there.
- Packets that enter XDP as an skb and leave it as an skb. This happens
  in generic XDP, and on XDP_PASS in veth when its peer sends it an skb.
  The series only sets the stored values on an skb that the kernel
  builds from an xdp_buff or an xdp_frame.
- Stored entries on frames that dpaa and dpaa2 redirect. These drivers
  move data_hard_start before XDP_REDIRECT, so patch 2 clears the flag
  of the area.

[1] https://lore.kernel.org/all/20260227153231.78d16b69@kernel.org/
[2] https://lore.kernel.org/all/ca38f2ed-999f-4ce1-8035-8ee9247f27f2@kernel.org/
[3] https://lore.kernel.org/all/1d5521eb-84fc-4417-a3f5-6366694d160a@kernel.org/
[4] https://lore.kernel.org/all/175146824674.1421237.18351246421763677468.stgit@firesoul/
[5] https://lore.kernel.org/all/20260722113525.34092660@kernel.org/
[6] https://lore.kernel.org/all/20260721183256.6f49d6e1@kernel.org/
[7] https://lore.kernel.org/all/20260715-bpf-xdp-meta-rxcksum-v5-0-623d5c0d0ab7@kernel.org/
[8] https://github.com/tgao-cf/skb-core-extensions-bench

Tianyi Gao (15):
  xdp: reserve an skb core extension area after the xdp_frame
  net: dpaa, dpaa2: clear the skb core extension flag
  xdp: add kfuncs to store RX hash and VLAN tag
  xdp: add a kfunc to delete skb core extension entries
  xdp: add a kfunc to store the RX checksum status
  selftests/bpf: Test the skb core extension kfuncs
  xdp: set skb RX metadata from the skb core extension area of xdp_frame
  selftests/bpf: Test RX metadata on skbs built from xdp_frames
  xdp: add the skb-ext-pass XDP feature and warn without it
  xdp: warn on XDP_PASS with stored values when XDP runs on an skb
  ice: set skb RX metadata from the skb core extension area of xdp_buff
  selftests: drv-net: xdp: test the stored RX metadata on XDP_PASS
  xdp: add kfuncs to load the stored RX metadata
  selftests/bpf: Test the load kfuncs on cpumap and veth
  docs: net: document the skb core extension area

 Documentation/netlink/specs/netdev.yaml       |   5 +
 Documentation/networking/xdp-rx-metadata.rst  | 127 +++++-
 .../net/ethernet/freescale/dpaa/dpaa_eth.c    |   2 +
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |   2 +
 drivers/net/ethernet/intel/ice/ice_main.c     |   3 +-
 drivers/net/ethernet/intel/ice/ice_sf_eth.c   |   3 +-
 drivers/net/ethernet/intel/ice/ice_txrx.c     |   4 +-
 drivers/net/ethernet/intel/ice/ice_txrx_lib.c |   7 +-
 drivers/net/ethernet/intel/ice/ice_txrx_lib.h |   3 +-
 drivers/net/ethernet/intel/ice/ice_xsk.c      |   2 +-
 drivers/net/veth.c                            |   5 +-
 include/net/xdp.h                             |  74 +++
 include/uapi/linux/netdev.h                   |   6 +-
 net/bpf/test_run.c                            |   4 +-
 net/core/dev.c                                |   2 +
 net/core/filter.c                             |   8 +-
 net/core/xdp.c                                | 428 +++++++++++++++++-
 tools/include/uapi/linux/netdev.h             |   6 +-
 .../selftests/bpf/prog_tests/test_xsk.c       |  25 +
 .../selftests/bpf/prog_tests/test_xsk.h       |   2 +
 .../selftests/bpf/prog_tests/xdp_bonding.c    |   8 +-
 .../bpf/prog_tests/xdp_do_redirect.c          |   8 +-
 .../selftests/bpf/prog_tests/xdp_pull_data.c  |  12 +-
 .../selftests/bpf/prog_tests/xdp_skb_ext.c    | 375 +++++++++++++++
 .../selftests/bpf/progs/test_xdp_pull_data.c  |   7 +
 .../selftests/bpf/progs/test_xdp_skb_ext.c    | 173 +++++++
 .../selftests/bpf/progs/xdp_skb_ext_failure.c |  16 +
 .../selftests/bpf/progs/xdp_skb_ext_success.c | 245 ++++++++++
 .../selftests/bpf/progs/xsk_xdp_progs.c       |  13 +
 tools/testing/selftests/bpf/xdp_skb_ext.h     |  53 +++
 .../drivers/net/hw/lib/py/__init__.py         |   3 +-
 .../selftests/drivers/net/hw/xdp_metadata.py  | 118 ++++-
 .../drivers/net/hw/xdp_skb_ext.bpf.c          | 166 +++++++
 33 files changed, 1873 insertions(+), 42 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_skb_ext_failure.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_skb_ext_success.c
 create mode 100644 tools/testing/selftests/bpf/xdp_skb_ext.h
 create mode 100644 tools/testing/selftests/drivers/net/hw/xdp_skb_ext.bpf.c


base-commit: 0fb18ce8a2e9de830433923cf65838933d615359
-- 
2.56.0

[PATCH bpf-next 01/15] xdp: reserve an skb core extension area after the xdp_frame

[Tianyi Gao]

Reserve the skb core extension area in the headroom, right after the
xdp_frame. An XDP program stores RX metadata there for the skb that is
built later.

The area begins with a header, struct xdp_skb_ext. It records the offset
of each entry and the end of the last entry, in 4-byte units.
bpf_xdp_adjust_head(), bpf_xdp_adjust_meta() and bpf_xdp_pull_data() use
xdp_skb_ext_end() as their lower bound, so packet data and metadata
cannot overwrite the entries.

The XDP_FLAGS_HAS_SKB_EXT flag indicates that the header is valid. Most
drivers reuse the xdp_buff between packets, so xdp_prepare_buff() clears
it. With BPF_F_TEST_XDP_LIVE_FRAMES, reset_ctx() restores the flags
before each run, and ctx_was_changed() compares them.

Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
Link: https://lore.kernel.org/all/1d5521eb-84fc-4417-a3f5-6366694d160a@kernel.org/
Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 include/net/xdp.h  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 net/bpf/test_run.c |  4 +++-
 net/core/filter.c  |  8 +++-----
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 07231adfb5f8..c888353425d4 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -81,6 +81,9 @@ enum xdp_buff_flags {
 	 * XDP program is not attached.
 	 */
 	XDP_FLAGS_FRAGS_UNREADABLE	= BIT(2),
+	XDP_FLAGS_HAS_SKB_EXT		= BIT(3), /* skb core extension header
+						   * is valid
+						   */
 };
 
 struct xdp_buff {
@@ -146,6 +149,16 @@ static __always_inline void xdp_buff_clear_frag_pfmemalloc(struct xdp_buff *xdp)
 	xdp->flags &= ~XDP_FLAGS_FRAGS_PF_MEMALLOC;
 }
 
+static __always_inline bool xdp_buff_has_skb_ext(const struct xdp_buff *xdp)
+{
+	return !!(xdp->flags & XDP_FLAGS_HAS_SKB_EXT);
+}
+
+static __always_inline void xdp_buff_clear_skb_ext_flag(struct xdp_buff *xdp)
+{
+	xdp->flags &= ~XDP_FLAGS_HAS_SKB_EXT;
+}
+
 static __always_inline void
 xdp_init_buff(struct xdp_buff *xdp, u32 frame_sz, struct xdp_rxq_info *rxq)
 {
@@ -174,6 +187,7 @@ xdp_prepare_buff(struct xdp_buff *xdp, unsigned char *hard_start,
 	xdp->data = data;
 	xdp->data_end = data + data_len;
 	xdp->data_meta = meta_valid ? data : data + 1;
+	xdp_buff_clear_skb_ext_flag(xdp);
 }
 
 /* Reserve memory area at end-of data area.
@@ -450,6 +464,37 @@ struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
 	return xdp_frame;
 }
 
+/* skb core extensions: RX metadata that an XDP program stores in the
+ * headroom right after the xdp_frame, for the skb that is built later.
+ */
+enum xdp_skb_ext_id {
+	XDP_SKB_EXT_HASH,
+	XDP_SKB_EXT_VLAN,
+	XDP_SKB_EXT_CSUM,
+	XDP_SKB_EXT_NUM,
+};
+
+#define XDP_SKB_EXT_ALIGN	4
+
+struct xdp_skb_ext {
+	u8 offset[XDP_SKB_EXT_NUM];	/* in chunks of 4 bytes */
+	u8 chunks;			/* same */
+};
+
+static __always_inline struct xdp_skb_ext *xdp_skb_ext(struct xdp_frame *frame)
+{
+	return (void *)(frame + 1);
+}
+
+static __always_inline void *xdp_skb_ext_end(const struct xdp_buff *xdp)
+{
+	struct xdp_skb_ext *ext = xdp_skb_ext(xdp->data_hard_start);
+
+	if (!xdp_buff_has_skb_ext(xdp))
+		return ext;
+	return (void *)ext + ext->chunks * XDP_SKB_EXT_ALIGN;
+}
+
 void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
 		  bool napi_direct, struct xdp_buff *xdp);
 void xdp_return_frame(struct xdp_frame *xdpf);
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index d6c9dc62182e..f25581851339 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -223,7 +223,8 @@ static bool ctx_was_changed(struct xdp_page_head *head)
 {
 	return head->orig_ctx.data != head->ctx.data ||
 		head->orig_ctx.data_meta != head->ctx.data_meta ||
-		head->orig_ctx.data_end != head->ctx.data_end;
+		head->orig_ctx.data_end != head->ctx.data_end ||
+		head->orig_ctx.flags != head->ctx.flags;
 }
 
 static void reset_ctx(struct xdp_page_head *head)
@@ -234,6 +235,7 @@ static void reset_ctx(struct xdp_page_head *head)
 	head->ctx.data = head->orig_ctx.data;
 	head->ctx.data_meta = head->orig_ctx.data_meta;
 	head->ctx.data_end = head->orig_ctx.data_end;
+	head->ctx.flags = head->orig_ctx.flags;
 	xdp_update_frame_from_buff(&head->ctx, head->frame);
 	head->frame->mem_type = head->orig_ctx.rxq->mem.type;
 }
diff --git a/net/core/filter.c b/net/core/filter.c
index 9245009d9f1e..4f6698f75d90 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4243,9 +4243,8 @@ static unsigned long xdp_get_metalen(const struct xdp_buff *xdp)
 
 BPF_CALL_2(bpf_xdp_adjust_head, struct xdp_buff *, xdp, int, offset)
 {
-	void *xdp_frame_end = xdp->data_hard_start + sizeof(struct xdp_frame);
 	unsigned long metalen = xdp_get_metalen(xdp);
-	void *data_start = xdp_frame_end + metalen;
+	void *data_start = xdp_skb_ext_end(xdp) + metalen;
 	void *data = xdp->data + offset;
 
 	if (unlikely(data < data_start ||
@@ -4547,13 +4546,12 @@ static const struct bpf_func_proto bpf_xdp_adjust_tail_proto = {
 
 BPF_CALL_2(bpf_xdp_adjust_meta, struct xdp_buff *, xdp, int, offset)
 {
-	void *xdp_frame_end = xdp->data_hard_start + sizeof(struct xdp_frame);
 	void *meta = xdp->data_meta + offset;
 	unsigned long metalen = xdp->data - meta;
 
 	if (xdp_data_meta_unsupported(xdp))
 		return -ENOTSUPP;
-	if (unlikely(meta < xdp_frame_end ||
+	if (unlikely(meta < xdp_skb_ext_end(xdp) ||
 		     meta > xdp->data))
 		return -EINVAL;
 	if (unlikely(xdp_metalen_invalid(metalen)))
@@ -12794,7 +12792,7 @@ __bpf_kfunc int bpf_xdp_pull_data(struct xdp_md *x, u32 len)
 
 	start = xdp_data_meta_unsupported(xdp) ? xdp->data : xdp->data_meta;
 
-	headroom = start - xdp->data_hard_start - sizeof(struct xdp_frame);
+	headroom = start - xdp_skb_ext_end(xdp);
 	tailroom = data_hard_end - xdp->data_end;
 
 	delta = len - data_len;
-- 
2.56.0

[PATCH bpf-next 02/15] net: dpaa, dpaa2: clear the skb core extension flag

[Tianyi Gao]

dpaa_run_xdp() and dpaa2_eth_run_xdp() move data_hard_start to the start
of the buffer after the XDP program runs and before XDP_REDIRECT.
xdp_skb_ext() then returns a different address from the one used while
storing the entries. These drivers have not implemented the XDP RX
metadata kfuncs. Hence they now clear the XDP_FLAGS_HAS_SKB_EXT flag
before XDP_REDIRECT, and their redirected frames carry no stored entries.

With the A050385 erratum workaround, dpaa already moves data_hard_start
before the XDP program runs, so the entries remain valid. The flag is
cleared on that path too, to keep the change simple. This can change
when dpaa implements the XDP RX metadata kfuncs.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c   | 2 ++
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index ad2d8256eb8d..4cafc0b84dc9 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2654,6 +2654,8 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr,
 		/* Allow redirect to use the full headroom */
 		xdp.data_hard_start = vaddr;
 		xdp.frame_sz = DPAA_BP_RAW_SIZE;
+		/* data_hard_start moves, so the stored entries are lost */
+		xdp_buff_clear_skb_ext_flag(&xdp);
 
 		err = xdp_do_redirect(priv->net_dev, &xdp, xdp_prog);
 		if (err) {
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 7227a35dfaf2..fcefdc9daf2b 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -472,6 +472,8 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv,
 		/* Allow redirect use of full headroom */
 		xdp.data_hard_start = vaddr;
 		xdp.frame_sz = DPAA2_ETH_RX_BUF_RAW_SIZE;
+		/* data_hard_start moves, so the stored entries are lost */
+		xdp_buff_clear_skb_ext_flag(&xdp);
 
 		err = xdp_do_redirect(priv->net_dev, &xdp, xdp_prog);
 		if (unlikely(err)) {
-- 
2.56.0

[PATCH bpf-next 03/15] xdp: add kfuncs to store RX hash and VLAN tag

[Tianyi Gao]

Add bpf_xdp_skb_ext_store_rx_hash() and bpf_xdp_skb_ext_store_rx_vlan().
They store the RX hash and the VLAN tag in the skb core extension area.
A later patch sets the stored values on the skb built from the
xdp_frame.

Both kfuncs call xdp_skb_ext_add(), which reserves the entry for an id.
On the first store for a packet, it clears the header and sets
XDP_FLAGS_HAS_SKB_EXT, so stale headroom is never read as a header. A
second store of the same id, also after a delete, reuses its offset.

xdp_skb_ext_add() returns -EOPNOTSUPP for AF_XDP zero-copy buffers. User
space can write their headroom while the program runs, and could change
the header between two stores.

The store kfuncs are in their own kfunc set. They do not need a
device-bound program, so programs on cpumap and veth can call them.

bpf_xdp_skb_ext_store_rx_vlan() accepts only ETH_P_8021Q and
ETH_P_8021AD, because vlan_proto_idx() warns on any other TPID.

A BUILD_BUG_ON() in xdp_skb_ext_struct_check() limits the area to 64
bytes. As in skb_extensions_init(), the function is noinline
__no_profile so that it builds with CONFIG_GCOV_PROFILE_ALL.

The two kfuncs are based on bpf_xdp_store_rx_hash() and
bpf_xdp_store_rx_vlan() by Lorenzo Bianconi in Jesper's XDP_REDIRECT RX
hints series V2 [1].

Link: https://lore.kernel.org/all/175146831297.1421237.17665319427079757435.stgit@firesoul/ [1]
Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 net/core/xdp.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 151 insertions(+), 3 deletions(-)

diff --git a/net/core/xdp.c b/net/core/xdp.c
index 1d679e8fd649..97dc0e403a78 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -990,11 +990,159 @@ bool bpf_dev_bound_kfunc_id(u32 btf_id)
 	return btf_id_set8_contains(&xdp_metadata_kfunc_ids, btf_id);
 }
 
-static int __init xdp_metadata_init(void)
+#define XDP_SKB_EXT_CHUNKSIZEOF(x) DIV_ROUND_UP(sizeof(x), XDP_SKB_EXT_ALIGN)
+#define XDP_SKB_EXT_ENTRY_ACTIVE BIT(7)
+
+struct xdp_skb_ext_hash {
+	u32 val;
+	u32 type; /* enum xdp_rss_hash_type */
+};
+
+struct xdp_skb_ext_vlan {
+	__be16 proto;
+	u16 tci;
+};
+
+static const u8 xdp_skb_ext_type_len[] = {
+	[XDP_SKB_EXT_HASH] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_hash),
+	[XDP_SKB_EXT_VLAN] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_vlan),
+};
+
+/* Limit the header and all entries to 64 bytes, so that a new entry
+ * cannot grow the area without notice.
+ */
+static noinline void __init __no_profile xdp_skb_ext_struct_check(void)
+{
+	unsigned int chunks = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(xdp_skb_ext_type_len); i++)
+		chunks += xdp_skb_ext_type_len[i];
+
+	BUILD_BUG_ON(chunks * XDP_SKB_EXT_ALIGN > 64);
+}
+
+static void *xdp_skb_ext_add(struct xdp_buff *xdp, enum xdp_skb_ext_id id)
+{
+	struct xdp_skb_ext *ext = xdp_skb_ext(xdp->data_hard_start);
+	bool valid = xdp_buff_has_skb_ext(xdp);
+	unsigned int newlen, newoff;
+	void *start;
+
+	/* User space can write the headroom of AF_XDP zero-copy buffers */
+	if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
+		return ERR_PTR(-EOPNOTSUPP);
+
+	if (!valid) {
+		newoff = XDP_SKB_EXT_CHUNKSIZEOF(*ext);
+		newlen = newoff + xdp_skb_ext_type_len[id];
+	} else if (ext->offset[id]) {
+		newoff = ext->offset[id] & ~XDP_SKB_EXT_ENTRY_ACTIVE;
+		newlen = ext->chunks;
+	} else {
+		newoff = ext->chunks;
+		newlen = newoff + xdp_skb_ext_type_len[id];
+	}
+
+	start = xdp_data_meta_unsupported(xdp) ? xdp->data : xdp->data_meta;
+	if ((void *)ext + newlen * XDP_SKB_EXT_ALIGN > start)
+		return ERR_PTR(-ENOSPC);
+
+	if (!valid) {
+		memset(ext->offset, 0, sizeof(ext->offset));
+		xdp->flags |= XDP_FLAGS_HAS_SKB_EXT;
+	}
+	ext->offset[id] = newoff | XDP_SKB_EXT_ENTRY_ACTIVE;
+	ext->chunks = newlen;
+
+	return (void *)ext + newoff * XDP_SKB_EXT_ALIGN;
+}
+
+__bpf_kfunc_start_defs();
+
+/**
+ * bpf_xdp_skb_ext_store_rx_hash - Store the RX hash of this packet.
+ * @ctx: XDP context pointer.
+ * @hash: RX hash value.
+ * @rss_type: RSS hash type, see &enum xdp_rss_hash_type.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-ENOSPC`` : not enough headroom for the entry
+ * * ``-EOPNOTSUPP`` : AF_XDP zero-copy buffer
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_store_rx_hash(struct xdp_md *ctx, u32 hash,
+					      enum xdp_rss_hash_type rss_type)
 {
-	return register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &xdp_metadata_kfunc_set);
+	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
+	struct xdp_skb_ext_hash *entry;
+
+	entry = xdp_skb_ext_add(xdp, XDP_SKB_EXT_HASH);
+	if (IS_ERR(entry))
+		return PTR_ERR(entry);
+
+	entry->val = hash;
+	entry->type = rss_type;
+
+	return 0;
+}
+
+/**
+ * bpf_xdp_skb_ext_store_rx_vlan - Store the VLAN tag of this packet.
+ * @ctx: XDP context pointer.
+ * @vlan_proto: VLAN tag protocol identifier (TPID), in network byte order.
+ * @vlan_tci: VLAN TCI, in host byte order.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-EINVAL`` : @vlan_proto is not 802.1Q or 802.1AD
+ * * ``-ENOSPC`` : not enough headroom for the entry
+ * * ``-EOPNOTSUPP`` : AF_XDP zero-copy buffer
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_store_rx_vlan(struct xdp_md *ctx,
+					      __be16 vlan_proto, u16 vlan_tci)
+{
+	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
+	struct xdp_skb_ext_vlan *entry;
+
+	if (vlan_proto != htons(ETH_P_8021Q) &&
+	    vlan_proto != htons(ETH_P_8021AD))
+		return -EINVAL;
+
+	entry = xdp_skb_ext_add(xdp, XDP_SKB_EXT_VLAN);
+	if (IS_ERR(entry))
+		return PTR_ERR(entry);
+
+	entry->proto = vlan_proto;
+	entry->tci = vlan_tci;
+
+	return 0;
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(xdp_skb_ext_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_hash)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_vlan)
+BTF_KFUNCS_END(xdp_skb_ext_kfunc_ids)
+
+static const struct btf_kfunc_id_set xdp_skb_ext_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set   = &xdp_skb_ext_kfunc_ids,
+};
+
+static int __init xdp_kfunc_init(void)
+{
+	int ret;
+
+	xdp_skb_ext_struct_check();
+
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
+					&xdp_skb_ext_kfunc_set);
+	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
+						&xdp_metadata_kfunc_set);
 }
-late_initcall(xdp_metadata_init);
+late_initcall(xdp_kfunc_init);
 
 void xdp_set_features_flag_locked(struct net_device *dev, xdp_features_t val)
 {
-- 
2.56.0

[PATCH bpf-next 04/15] xdp: add a kfunc to delete skb core extension entries

[Tianyi Gao]

bpf_xdp_skb_ext_del() deletes the stored entry for an id. A program on
cpumap or veth can use it to drop RX metadata that an earlier program
stored.

Without the XDP_FLAGS_HAS_SKB_EXT flag, the kfunc does nothing, because
the header bytes may then belong to the packet.

The kfunc only clears XDP_SKB_EXT_ENTRY_ACTIVE, so a later store of the
same id reuses its offset. The flag and the size of the area stay the
same, also after the last delete. To give the bytes back, the kfunc
would have to move the entries after the deleted one. The cost is
headroom that bpf_xdp_adjust_head() and bpf_xdp_adjust_meta() cannot
use.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
__skb_ext_del() frees struct skb_ext after the last delete, because it
comes from a slab cache. The skb core extension area uses existing
headroom, so it is not freed.
---
 include/net/xdp.h |  8 ++++++++
 net/core/xdp.c    | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index c888353425d4..23c976e92898 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -486,6 +486,14 @@ static __always_inline struct xdp_skb_ext *xdp_skb_ext(struct xdp_frame *frame)
 	return (void *)(frame + 1);
 }
 
+static __always_inline struct xdp_skb_ext *
+xdp_buff_skb_ext(const struct xdp_buff *xdp)
+{
+	if (!xdp_buff_has_skb_ext(xdp))
+		return NULL;
+	return xdp_skb_ext(xdp->data_hard_start);
+}
+
 static __always_inline void *xdp_skb_ext_end(const struct xdp_buff *xdp)
 {
 	struct xdp_skb_ext *ext = xdp_skb_ext(xdp->data_hard_start);
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 97dc0e403a78..2c1d5ec3311b 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -991,6 +991,7 @@ bool bpf_dev_bound_kfunc_id(u32 btf_id)
 }
 
 #define XDP_SKB_EXT_CHUNKSIZEOF(x) DIV_ROUND_UP(sizeof(x), XDP_SKB_EXT_ALIGN)
+/* A deleted entry keeps its offset, without this bit */
 #define XDP_SKB_EXT_ENTRY_ACTIVE BIT(7)
 
 struct xdp_skb_ext_hash {
@@ -1119,11 +1120,36 @@ __bpf_kfunc int bpf_xdp_skb_ext_store_rx_vlan(struct xdp_md *ctx,
 	return 0;
 }
 
+/**
+ * bpf_xdp_skb_ext_del - Delete a stored entry of this packet.
+ * @ctx: XDP context pointer.
+ * @id: Entry id, see &enum xdp_skb_ext_id.
+ *
+ * Return:
+ * * Returns 0 on success, also when the entry is not stored.
+ * * ``-EINVAL`` : @id is not a valid entry id
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_del(struct xdp_md *ctx, enum xdp_skb_ext_id id)
+{
+	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
+	struct xdp_skb_ext *ext;
+
+	if ((u32)id >= XDP_SKB_EXT_NUM)
+		return -EINVAL;
+
+	ext = xdp_buff_skb_ext(xdp);
+	if (ext)
+		ext->offset[id] &= ~XDP_SKB_EXT_ENTRY_ACTIVE;
+
+	return 0;
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(xdp_skb_ext_kfunc_ids)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_hash)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_vlan)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_del)
 BTF_KFUNCS_END(xdp_skb_ext_kfunc_ids)
 
 static const struct btf_kfunc_id_set xdp_skb_ext_kfunc_set = {
-- 
2.56.0

[PATCH bpf-next 05/15] xdp: add a kfunc to store the RX checksum status

[Tianyi Gao]

Add bpf_xdp_skb_ext_store_rx_csum(). It stores the RX checksum status in
the skb core extension area. A later patch sets the stored status on the
skb built from the xdp_frame. The program can read the status from the
NIC with bpf_xdp_metadata_rx_checksum() from Lorenzo's RX checksum kfunc
series [1].

The kfunc accepts XDP_CHECKSUM_NONE with csum_level 0, and
XDP_CHECKSUM_UNNECESSARY with csum_level up to SKB_MAX_CSUM_LEVEL:

- Storing XDP_CHECKSUM_UNNECESSARY asserts that the checksums are
  correct, as in [2]. A program must store it only after it has verified
  the checksums.
- Storing XDP_CHECKSUM_NONE invalidates the checksum status, as
  requested in [3][4].
- After a decap, storing a lower csum_level updates the status, as
  requested in [5].
- CHECKSUM_COMPLETE is not passed to the stack for now, as suggested in
  [5].

Link: https://lore.kernel.org/all/20260715-bpf-xdp-meta-rxcksum-v5-1-623d5c0d0ab7@kernel.org/ [1]
Link: https://lore.kernel.org/all/20260630191510.81402-2-deliran@verdict.gg/ [2]
Link: https://lore.kernel.org/all/20260227153231.78d16b69@kernel.org/ [3]
Link: https://lore.kernel.org/all/amEojEGsIGkUjIWn@devvm7509.cco0.facebook.com/ [4]
Link: https://lore.kernel.org/all/20260721183256.6f49d6e1@kernel.org/ [5]
Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
enum xdp_checksum has the same names and values as in [1], so this
patch does not depend on that series. The series merged later drops its
copy.

Stanislav was not sure that any NIC reports a level of 2 or more [4]. A
separate maximum for XDP would give the same skb field two limits, so
the kfunc uses SKB_MAX_CSUM_LEVEL.
---
 include/net/xdp.h |  6 +++++
 net/core/xdp.c    | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 23c976e92898..d8b77d80bfbe 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -701,6 +701,12 @@ enum xdp_rss_hash_type {
 	XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP | XDP_RSS_L3_DYNHDR,
 };
 
+enum xdp_checksum {
+	XDP_CHECKSUM_NONE		= BIT(CHECKSUM_NONE),
+	XDP_CHECKSUM_UNNECESSARY	= BIT(CHECKSUM_UNNECESSARY),
+	XDP_CHECKSUM_COMPLETE		= BIT(CHECKSUM_COMPLETE),
+};
+
 struct xdp_metadata_ops {
 	int	(*xmo_rx_timestamp)(const struct xdp_md *ctx, u64 *timestamp);
 	int	(*xmo_rx_hash)(const struct xdp_md *ctx, u32 *hash,
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 2c1d5ec3311b..eea04e7b7204 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -1004,9 +1004,15 @@ struct xdp_skb_ext_vlan {
 	u16 tci;
 };
 
+struct xdp_skb_ext_csum {
+	u8 ip_summed; /* CHECKSUM_* */
+	u8 level;
+};
+
 static const u8 xdp_skb_ext_type_len[] = {
 	[XDP_SKB_EXT_HASH] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_hash),
 	[XDP_SKB_EXT_VLAN] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_vlan),
+	[XDP_SKB_EXT_CSUM] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_csum),
 };
 
 /* Limit the header and all entries to 64 bytes, so that a new entry
@@ -1120,6 +1126,56 @@ __bpf_kfunc int bpf_xdp_skb_ext_store_rx_vlan(struct xdp_md *ctx,
 	return 0;
 }
 
+/**
+ * bpf_xdp_skb_ext_store_rx_csum - Store the RX checksum status of this packet.
+ * @ctx: XDP context pointer.
+ * @ip_summed: ``XDP_CHECKSUM_NONE`` or ``XDP_CHECKSUM_UNNECESSARY``.
+ * @csum_level: Number of consecutive verified checksums minus one, up to
+ *              ``SKB_MAX_CSUM_LEVEL``. Must be 0 with ``XDP_CHECKSUM_NONE``.
+ *
+ * Store ``XDP_CHECKSUM_UNNECESSARY`` only after the program has verified
+ * the checksums.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-EINVAL`` : @ip_summed is not supported, or @csum_level is not valid
+ *   for it
+ * * ``-ENOSPC`` : not enough headroom for the entry
+ * * ``-EOPNOTSUPP`` : AF_XDP zero-copy buffer
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_store_rx_csum(struct xdp_md *ctx,
+					      enum xdp_checksum ip_summed,
+					      u8 csum_level)
+{
+	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
+	struct xdp_skb_ext_csum *entry;
+	u8 val;
+
+	switch (ip_summed) {
+	case XDP_CHECKSUM_NONE:
+		if (csum_level)
+			return -EINVAL;
+		val = CHECKSUM_NONE;
+		break;
+	case XDP_CHECKSUM_UNNECESSARY:
+		if (csum_level > SKB_MAX_CSUM_LEVEL)
+			return -EINVAL;
+		val = CHECKSUM_UNNECESSARY;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	entry = xdp_skb_ext_add(xdp, XDP_SKB_EXT_CSUM);
+	if (IS_ERR(entry))
+		return PTR_ERR(entry);
+
+	entry->ip_summed = val;
+	entry->level = csum_level;
+
+	return 0;
+}
+
 /**
  * bpf_xdp_skb_ext_del - Delete a stored entry of this packet.
  * @ctx: XDP context pointer.
@@ -1149,6 +1205,7 @@ __bpf_kfunc_end_defs();
 BTF_KFUNCS_START(xdp_skb_ext_kfunc_ids)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_hash)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_vlan)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_csum)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_del)
 BTF_KFUNCS_END(xdp_skb_ext_kfunc_ids)
 
-- 
2.56.0

[PATCH bpf-next 06/15] selftests/bpf: Test the skb core extension kfuncs

[Tianyi Gao]

Add tests for the kfuncs that store and delete skb core extension
entries.

An XDP program cannot read the area, because it is below data_meta. So
the tests check the return values of the kfuncs and the lower bound of
bpf_xdp_adjust_head(), bpf_xdp_adjust_meta() and bpf_xdp_pull_data().
A later patch tests the skb fields.

xdp_skb_ext uses BPF_PROG_RUN. It has test cases for the size of the
area, invalid arguments, -ENOSPC, delete, and a tc program that calls
the kfuncs. The live_frames subtest checks that the area of a recycled
page is not valid for the next packet. The subtest fails if no page is
recycled.

test_xsk checks that the store kfuncs return 0 in copy mode and
-EOPNOTSUPP in zero-copy mode. xdp_pull_data checks that a stored hash
entry reduces the maximum pull length by 12 bytes.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       |  25 ++
 .../selftests/bpf/prog_tests/test_xsk.h       |   2 +
 .../selftests/bpf/prog_tests/xdp_pull_data.c  |  12 +-
 .../selftests/bpf/prog_tests/xdp_skb_ext.c    |  41 +++
 .../selftests/bpf/progs/test_xdp_pull_data.c  |   7 +
 .../selftests/bpf/progs/test_xdp_skb_ext.c    |  41 +++
 .../selftests/bpf/progs/xdp_skb_ext_failure.c |  16 ++
 .../selftests/bpf/progs/xdp_skb_ext_success.c | 245 ++++++++++++++++++
 .../selftests/bpf/progs/xsk_xdp_progs.c       |  13 +
 tools/testing/selftests/bpf/xdp_skb_ext.h     |  15 ++
 10 files changed, 416 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_skb_ext_failure.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_skb_ext_success.c
 create mode 100644 tools/testing/selftests/bpf/xdp_skb_ext.h

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 4549358cc8c2..f0d462a537e3 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -2461,6 +2461,31 @@ int testapp_xdp_metadata_mb(struct test_spec *test)
 	return testapp_xdp_metadata_copy(test);
 }
 
+/* User space can write the headroom of AF_XDP zero-copy buffers */
+int testapp_xdp_skb_ext_store(struct test_spec *test)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+	int expected = test->mode == TEST_MODE_ZC ? -EOPNOTSUPP : 0;
+	int ret;
+
+	test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_skb_ext_store,
+			       skel_tx->progs.xsk_xdp_skb_ext_store,
+			       skel_rx->maps.xsk, skel_tx->maps.xsk);
+
+	ret = testapp_validate_traffic(test);
+	if (ret)
+		return ret;
+
+	if (skel_rx->bss->store_ret != expected) {
+		ksft_print_msg("[%s] expected store result [%d], got [%d]\n",
+			       __func__, expected, skel_rx->bss->store_ret);
+		return TEST_FAILURE;
+	}
+
+	return TEST_PASS;
+}
+
 int testapp_hw_sw_min_ring_size(struct test_spec *test)
 {
 	int ret;
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
index 03753ddc5dcd..6f13440438b8 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.h
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
@@ -268,6 +268,7 @@ int testapp_xdp_metadata(struct test_spec *test);
 int testapp_xdp_metadata_mb(struct test_spec *test);
 int testapp_xdp_prog_cleanup(struct test_spec *test);
 int testapp_xdp_shared_umem(struct test_spec *test);
+int testapp_xdp_skb_ext_store(struct test_spec *test);
 
 void *worker_testapp_validate_rx(void *arg);
 void *worker_testapp_validate_tx(void *arg);
@@ -293,6 +294,7 @@ static const struct test_spec tests[] = {
 	{.name = "XDP_SHARED_UMEM", .test_func = testapp_xdp_shared_umem},
 	{.name = "XDP_METADATA_COPY", .test_func = testapp_xdp_metadata},
 	{.name = "XDP_METADATA_COPY_MULTI_BUFF", .test_func = testapp_xdp_metadata_mb},
+	{.name = "XDP_SKB_EXT_STORE", .test_func = testapp_xdp_skb_ext_store},
 	{.name = "ALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_aligned_inv_desc_mb},
 	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
 	{.name = "XDP_ADJUST_TAIL_SHRINK", .test_func = testapp_adjust_tail_shrink},
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c b/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c
index 910dabe95afd..570fef579f8b 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c
@@ -81,7 +81,8 @@ static void run_test(struct test_xdp_pull_data *skel, int retval,
 
 	skel->bss->data_len = data_len;
 	if (pull_len & PULL_MAX) {
-		int headroom = XDP_PACKET_HEADROOM - meta_len - skel->bss->xdpf_sz;
+		int headroom = XDP_PACKET_HEADROOM - meta_len -
+			       skel->bss->xdpf_sz - skel->bss->ext_sz;
 		int tailroom = frame_sz - XDP_PACKET_HEADROOM -
 			       data_len - skel->bss->sinfo_sz;
 
@@ -170,6 +171,15 @@ static void test_xdp_pull_data_basic(void)
 	run_test(skel, XDP_DROP, pg_sz, buff_len, 0, max_data_len,
 		 PULL_MAX | PULL_PLUS_ONE);
 
+	skel->bss->ext_sz = skel->bss->hash_ext_sz;
+
+	/* multi-buf pkt, hash stored, no tailroom, pull requires memmove */
+	run_test(skel, XDP_PASS, pg_sz, buff_len, 0, max_data_len, PULL_MAX);
+
+	/* multi-buf pkt, hash stored, no tailroom */
+	run_test(skel, XDP_DROP, pg_sz, buff_len, 0, max_data_len,
+		 PULL_MAX | PULL_PLUS_ONE);
+
 out:
 	test_xdp_pull_data__destroy(skel);
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c b/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
new file mode 100644
index 000000000000..bd36e22e22a6
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include "test_xdp_skb_ext.skel.h"
+#include "xdp_skb_ext_success.skel.h"
+#include "xdp_skb_ext_failure.skel.h"
+
+/* The area of a reused page is not valid for the next packet */
+static void test_live_frames(void)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts,
+		    .data_in = &pkt_v4,
+		    .data_size_in = sizeof(pkt_v4),
+		    .flags = BPF_F_TEST_XDP_LIVE_FRAMES,
+		    .repeat = 256,
+	);
+	struct test_xdp_skb_ext *skel;
+	int prog_fd;
+
+	skel = test_xdp_skb_ext__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "test_xdp_skb_ext__open_and_load"))
+		return;
+
+	prog_fd = bpf_program__fd(skel->progs.live_frames);
+	if (!ASSERT_OK(bpf_prog_test_run_opts(prog_fd, &opts), "test_run"))
+		goto out;
+	ASSERT_GT(skel->bss->reused_ok, 0, "reused_ok");
+	ASSERT_EQ(skel->bss->reused_bad, 0, "reused_bad");
+	ASSERT_EQ(skel->bss->store_err, 0, "store_err");
+out:
+	test_xdp_skb_ext__destroy(skel);
+}
+
+void test_xdp_skb_ext(void)
+{
+	RUN_TESTS(xdp_skb_ext_success);
+	RUN_TESTS(xdp_skb_ext_failure);
+
+	if (test__start_subtest("live_frames"))
+		test_live_frames();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c b/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c
index c41a21413eaa..49c8f4bb91f7 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c
@@ -7,6 +7,8 @@ int xdpf_sz;
 int sinfo_sz;
 int data_len;
 int pull_len;
+int ext_sz;
+int hash_ext_sz;
 
 #define XDP_PACKET_HEADROOM 256
 
@@ -14,6 +16,8 @@ SEC("xdp.frags")
 int xdp_find_sizes(struct xdp_md *ctx)
 {
 	xdpf_sz = sizeof(struct xdp_frame);
+	hash_ext_sz = sizeof(struct xdp_skb_ext) +
+		      sizeof(struct xdp_skb_ext_hash);
 	sinfo_sz = __PAGE_SIZE - XDP_PACKET_HEADROOM -
 		   (ctx->data_end - ctx->data);
 
@@ -31,6 +35,9 @@ int xdp_pull_data_prog(struct xdp_md *ctx)
 	if (data_len != data_end - data)
 		return XDP_DROP;
 
+	if (ext_sz && bpf_xdp_skb_ext_store_rx_hash(ctx, 0, XDP_RSS_TYPE_NONE))
+		return XDP_DROP;
+
 	err = bpf_xdp_pull_data(ctx, pull_len);
 	if (err)
 		return XDP_DROP;
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c b/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
new file mode 100644
index 000000000000..e072e051af48
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include "xdp_skb_ext.h"
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+
+/* Written to the first byte of the packet. A reused page keeps it. */
+#define MARK_STORED 0xaa
+
+int reused_ok, reused_bad, store_err;
+
+SEC("xdp")
+int live_frames(struct xdp_md *ctx)
+{
+	__u8 *mark = (void *)(long)ctx->data;
+
+	if (mark + 1 > (__u8 *)(long)ctx->data_end)
+		return XDP_DROP;
+	/* A reused page: MAX_META_LEN fits only if its area was reset */
+	if (*mark == MARK_STORED) {
+		if (bpf_xdp_adjust_meta(ctx, -MAX_META_LEN)) {
+			reused_bad++;
+		} else {
+			reused_ok++;
+			bpf_xdp_adjust_meta(ctx, MAX_META_LEN);
+		}
+	}
+	if (bpf_xdp_skb_ext_store_rx_hash(ctx, 1, XDP_RSS_TYPE_L4_IPV4_TCP) ||
+	    bpf_xdp_skb_ext_store_rx_vlan(ctx, bpf_htons(ETH_P_8021Q), 1) ||
+	    bpf_xdp_skb_ext_store_rx_csum(ctx, XDP_CHECKSUM_UNNECESSARY,
+					  SKB_MAX_CSUM_LEVEL))
+		store_err++;
+
+	mark = (void *)(long)ctx->data;
+	if (mark + 1 > (__u8 *)(long)ctx->data_end)
+		return XDP_DROP;
+	*mark = MARK_STORED;
+	return XDP_DROP;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/xdp_skb_ext_failure.c b/tools/testing/selftests/bpf/progs/xdp_skb_ext_failure.c
new file mode 100644
index 000000000000..d6030a2da236
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xdp_skb_ext_failure.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+/* The kfuncs are only for XDP programs */
+SEC("tc")
+__failure
+__msg("calling kernel function bpf_xdp_skb_ext_store_rx_hash is not allowed")
+int store_from_tc(struct __sk_buff *skb)
+{
+	return bpf_xdp_skb_ext_store_rx_hash((struct xdp_md *)skb, 1,
+					     XDP_RSS_TYPE_L4_IPV4_TCP);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/xdp_skb_ext_success.c b/tools/testing/selftests/bpf/progs/xdp_skb_ext_success.c
new file mode 100644
index 000000000000..158882f91c54
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xdp_skb_ext_success.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include "xdp_skb_ext.h"
+#include <errno.h>
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+#define HDR_SZ 4
+#define HASH_SZ 8
+#define VLAN_SZ 4
+#define CSUM_SZ 4
+#define EXT_SZ (HDR_SZ + HASH_SZ + VLAN_SZ + CSUM_SZ)
+
+static int store_hash(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_hash(ctx, 1, XDP_RSS_TYPE_L4_IPV4_TCP);
+}
+
+static int store_vlan(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_vlan(ctx, bpf_htons(ETH_P_8021Q), 1);
+}
+
+static int store_csum(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_csum(ctx, XDP_CHECKSUM_UNNECESSARY,
+					     SKB_MAX_CSUM_LEVEL);
+}
+
+static int store_all(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = store_hash(ctx);
+	if (!ret)
+		ret = store_vlan(ctx);
+	if (!ret)
+		ret = store_csum(ctx);
+	return ret;
+}
+
+/* Leave @len bytes of headroom above data_meta */
+static int leave_headroom(struct xdp_md *ctx, int len)
+{
+	return bpf_xdp_adjust_meta(ctx, -(MAX_META_LEN - len));
+}
+
+/*
+ * The test run copies data_meta out, so give the room back. Not inlined:
+ * clang would read ctx through a modified ctx pointer, which the verifier
+ * rejects.
+ */
+static __noinline int restore_meta(struct xdp_md *ctx, int ret)
+{
+	bpf_xdp_adjust_meta(ctx, ctx->data - ctx->data_meta);
+	return ret;
+}
+
+/* A second store of the same id takes the same place */
+SEC("xdp")
+__success __retval(0)
+int store_same_id(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = store_all(ctx);
+	if (!ret)
+		ret = store_hash(ctx);
+	if (!ret)
+		ret = leave_headroom(ctx, EXT_SZ);
+	return restore_meta(ctx, ret);
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int adjust_meta_into_area(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = store_all(ctx);
+	if (!ret)
+		ret = leave_headroom(ctx, EXT_SZ - CSUM_SZ);
+	return restore_meta(ctx, ret);
+}
+
+SEC("xdp")
+__success __retval(0)
+int adjust_head_to_area(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = store_all(ctx);
+	if (!ret)
+		ret = bpf_xdp_adjust_head(ctx, -(MAX_META_LEN - EXT_SZ));
+	if (!ret)
+		bpf_xdp_adjust_head(ctx, MAX_META_LEN - EXT_SZ);
+	return ret;
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int adjust_head_into_area(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = store_all(ctx);
+	if (!ret)
+		ret = bpf_xdp_adjust_head(ctx, -(MAX_META_LEN - EXT_SZ + 1));
+	return ret;
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int csum_complete(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_csum(ctx, XDP_CHECKSUM_COMPLETE, 0);
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int csum_none_level1(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_csum(ctx, XDP_CHECKSUM_NONE, 1);
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int csum_level_too_big(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_csum(ctx, XDP_CHECKSUM_UNNECESSARY,
+					     SKB_MAX_CSUM_LEVEL + 1);
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int vlan_bad_proto(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_store_rx_vlan(ctx, bpf_htons(0x1234), 1);
+}
+
+SEC("xdp")
+__success __retval(-EINVAL)
+int del_bad_id(struct xdp_md *ctx)
+{
+	return bpf_xdp_skb_ext_del(ctx, XDP_SKB_EXT_NUM);
+}
+
+/* Failed calls do not create the area */
+SEC("xdp")
+__success __retval(0)
+int einval_no_area(struct xdp_md *ctx)
+{
+	bpf_xdp_skb_ext_store_rx_csum(ctx, XDP_CHECKSUM_COMPLETE, 0);
+	bpf_xdp_skb_ext_store_rx_vlan(ctx, bpf_htons(0x1234), 1);
+	bpf_xdp_skb_ext_del(ctx, XDP_SKB_EXT_NUM);
+	return restore_meta(ctx, leave_headroom(ctx, 0));
+}
+
+SEC("xdp")
+__success __retval(0)
+int enospc_smaller_fits(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = leave_headroom(ctx, HDR_SZ + VLAN_SZ);
+	if (!ret && store_hash(ctx) != -ENOSPC)
+		ret = -1;
+	if (!ret)
+		ret = store_vlan(ctx);
+	return restore_meta(ctx, ret);
+}
+
+SEC("xdp")
+__success __retval(-ENOSPC)
+int enospc_full(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = leave_headroom(ctx, HDR_SZ + VLAN_SZ);
+	if (!ret)
+		ret = store_vlan(ctx);
+	if (!ret)
+		ret = store_csum(ctx);
+	return restore_meta(ctx, ret);
+}
+
+/* Without the area, a delete must not write where the header would be */
+SEC("xdp")
+__success __retval(0)
+int del_no_area(struct xdp_md *ctx)
+{
+	__u32 *meta;
+	int ret;
+
+	if (leave_headroom(ctx, 0))
+		return -1;
+	meta = (void *)(long)ctx->data_meta;
+	if (meta + 1 > (__u32 *)(long)ctx->data)
+		return restore_meta(ctx, -1);
+	*meta = ~0U;
+	ret = bpf_xdp_skb_ext_del(ctx, XDP_SKB_EXT_HASH);
+	meta = (void *)(long)ctx->data_meta;
+	if (meta + 1 > (__u32 *)(long)ctx->data)
+		return restore_meta(ctx, -1);
+	if (!ret && *meta != ~0U)
+		ret = -1;
+	return restore_meta(ctx, ret);
+}
+
+/* A delete does not shrink the area */
+SEC("xdp")
+__success __retval(-ENOSPC)
+int del_keeps_area(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = leave_headroom(ctx, HDR_SZ + HASH_SZ);
+	if (!ret)
+		ret = store_hash(ctx);
+	if (!ret)
+		ret = bpf_xdp_skb_ext_del(ctx, XDP_SKB_EXT_HASH);
+	if (!ret)
+		ret = store_vlan(ctx);
+	return restore_meta(ctx, ret);
+}
+
+/* A deleted entry keeps its place */
+SEC("xdp")
+__success __retval(0)
+int del_reuse(struct xdp_md *ctx)
+{
+	int ret;
+
+	ret = leave_headroom(ctx, HDR_SZ + HASH_SZ);
+	if (!ret)
+		ret = store_hash(ctx);
+	if (!ret)
+		ret = bpf_xdp_skb_ext_del(ctx, XDP_SKB_EXT_HASH);
+	if (!ret)
+		ret = store_hash(ctx);
+	return restore_meta(ctx, ret);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
index 023d8befd4ca..103f1469b4d6 100644
--- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
@@ -3,6 +3,7 @@
 
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
 #include <linux/if_ether.h>
 #include <linux/ip.h>
 #include <linux/errno.h>
@@ -18,6 +19,10 @@ struct {
 static unsigned int idx;
 int adjust_value = 0;
 int count = 0;
+int store_ret;
+
+extern int bpf_xdp_skb_ext_store_rx_vlan(struct xdp_md *ctx, __be16 vlan_proto,
+					 __u16 vlan_tci) __ksym;
 
 SEC("xdp.frags") int xsk_def_prog(struct xdp_md *xdp)
 {
@@ -58,6 +63,14 @@ SEC("xdp.frags") int xsk_xdp_populate_metadata(struct xdp_md *xdp)
 	return bpf_redirect_map(&xsk, 0, XDP_DROP);
 }
 
+SEC("xdp.frags") int xsk_xdp_skb_ext_store(struct xdp_md *xdp)
+{
+	store_ret = bpf_xdp_skb_ext_store_rx_vlan(xdp,
+						  bpf_htons(ETH_P_8021Q), 0);
+
+	return bpf_redirect_map(&xsk, 0, XDP_DROP);
+}
+
 SEC("xdp") int xsk_xdp_shared_umem(struct xdp_md *xdp)
 {
 	void *data = (void *)(long)xdp->data;
diff --git a/tools/testing/selftests/bpf/xdp_skb_ext.h b/tools/testing/selftests/bpf/xdp_skb_ext.h
new file mode 100644
index 000000000000..540ba7c6aa1b
--- /dev/null
+++ b/tools/testing/selftests/bpf/xdp_skb_ext.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#pragma once
+
+#ifndef ETH_P_8021Q
+#define ETH_P_8021Q 0x8100
+#endif
+
+#ifndef XDP_PACKET_HEADROOM
+#define XDP_PACKET_HEADROOM 256
+#endif
+
+/* Metadata in BPF_PROG_RUN must be this size or smaller */
+#define MAX_META_LEN (XDP_PACKET_HEADROOM - (int)sizeof(struct xdp_frame))
+
+#define SKB_MAX_CSUM_LEVEL 3
-- 
2.56.0

[PATCH bpf-next 07/15] xdp: set skb RX metadata from the skb core extension area of xdp_frame

[Tianyi Gao]

cpumap, veth and BPF_PROG_RUN build the skb from an xdp_frame with
__xdp_build_skb_from_frame(). It now calls the new helper
xdp_skb_ext_to_skb() if the xdp_frame has the XDP_FLAGS_HAS_SKB_EXT
flag.

Three drivers (atlantic, mana and netvsc) call the wrapper
xdp_build_skb_from_frame() in their RX and TX routines, and then set the
skb fields from the RX descriptor or transmit the skb. The stored values
must be ignored there to keep their behaviour consistent. So the wrapper
clears the flag. A driver that supports them calls xdp_skb_ext_to_skb()
itself, as ice does in a later patch.

xdp_skb_ext_to_skb() is based on [1].

Link: https://lore.kernel.org/all/175146831960.1421237.9105904582430357235.stgit@firesoul/ [1]
Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 include/net/xdp.h |  7 ++++++
 net/core/xdp.c    | 54 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index d8b77d80bfbe..7c9a812b6e86 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -329,6 +329,11 @@ static __always_inline bool xdp_frame_has_frags(const struct xdp_frame *frame)
 	return !!(frame->flags & XDP_FLAGS_HAS_FRAGS);
 }
 
+static __always_inline bool xdp_frame_has_skb_ext(const struct xdp_frame *frame)
+{
+	return !!(frame->flags & XDP_FLAGS_HAS_SKB_EXT);
+}
+
 static __always_inline u32
 xdp_frame_get_skb_flags(const struct xdp_frame *frame)
 {
@@ -503,6 +508,8 @@ static __always_inline void *xdp_skb_ext_end(const struct xdp_buff *xdp)
 	return (void *)ext + ext->chunks * XDP_SKB_EXT_ALIGN;
 }
 
+void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext);
+
 void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
 		  bool napi_direct, struct xdp_buff *xdp);
 void xdp_return_frame(struct xdp_frame *xdpf);
diff --git a/net/core/xdp.c b/net/core/xdp.c
index eea04e7b7204..9f311b3c0d2b 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -829,10 +829,10 @@ struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
 
 	/* Essential SKB info: protocol and skb->dev */
 	skb->protocol = eth_type_trans(skb, dev);
+	if (xdp_frame_has_skb_ext(xdpf))
+		xdp_skb_ext_to_skb(skb, xdp_skb_ext(xdpf));
 
 	/* Optional SKB info, currently missing:
-	 * - HW checksum info		(skb->ip_summed)
-	 * - HW RX hash			(skb_set_hash)
 	 * - RX ring dev queue index	(skb_record_rx_queue)
 	 */
 
@@ -857,6 +857,9 @@ struct sk_buff *xdp_build_skb_from_frame(struct xdp_frame *xdpf,
 
 	memset(skb, 0, offsetof(struct sk_buff, tail));
 
+	/* The callers set the RX fields themselves or transmit the skb */
+	xdpf->flags &= ~XDP_FLAGS_HAS_SKB_EXT;
+
 	return __xdp_build_skb_from_frame(xdpf, skb, dev);
 }
 EXPORT_SYMBOL_GPL(xdp_build_skb_from_frame);
@@ -1065,6 +1068,53 @@ static void *xdp_skb_ext_add(struct xdp_buff *xdp, enum xdp_skb_ext_id id)
 	return (void *)ext + newoff * XDP_SKB_EXT_ALIGN;
 }
 
+static const void *xdp_skb_ext_get(const struct xdp_skb_ext *ext,
+				   enum xdp_skb_ext_id id)
+{
+	u8 off = ext->offset[id];
+
+	if (!(off & XDP_SKB_EXT_ENTRY_ACTIVE))
+		return NULL;
+	off &= ~XDP_SKB_EXT_ENTRY_ACTIVE;
+
+	return (const void *)ext + off * XDP_SKB_EXT_ALIGN;
+}
+
+/**
+ * xdp_skb_ext_to_skb - Set the stored RX metadata on an skb.
+ * @skb: The skb that is built from the packet.
+ * @ext: The skb core extension area of the packet.
+ *
+ * Call it only when the packet has the XDP_FLAGS_HAS_SKB_EXT flag.
+ */
+void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext)
+{
+	const struct xdp_skb_ext_hash *hash;
+	const struct xdp_skb_ext_vlan *vlan;
+	const struct xdp_skb_ext_csum *csum;
+
+	hash = xdp_skb_ext_get(ext, XDP_SKB_EXT_HASH);
+	if (hash) {
+		enum pkt_hash_types type = PKT_HASH_TYPE_NONE;
+
+		if (hash->type & XDP_RSS_L4)
+			type = PKT_HASH_TYPE_L4;
+		else if (hash->type & (XDP_RSS_L3_IPV4 | XDP_RSS_L3_IPV6))
+			type = PKT_HASH_TYPE_L3;
+		skb_set_hash(skb, hash->val, type);
+	}
+
+	vlan = xdp_skb_ext_get(ext, XDP_SKB_EXT_VLAN);
+	if (vlan)
+		__vlan_hwaccel_put_tag(skb, vlan->proto, vlan->tci);
+
+	csum = xdp_skb_ext_get(ext, XDP_SKB_EXT_CSUM);
+	if (csum) {
+		skb->ip_summed = csum->ip_summed;
+		skb->csum_level = csum->level;
+	}
+}
+
 __bpf_kfunc_start_defs();
 
 /**
-- 
2.56.0

[PATCH bpf-next 08/15] selftests/bpf: Test RX metadata on skbs built from xdp_frames

[Tianyi Gao]

Add the xdp_skb_ext_to_skb test. An XDP program stores entries and
passes the packet to the stack. A tc program on the receiving veth
device then reads the skb fields. Three paths pass the packet to the
stack: XDP_PASS in BPF_PROG_RUN with BPF_F_TEST_XDP_LIVE_FRAMES,
XDP_REDIRECT into a cpumap, and XDP_REDIRECT into a veth device. Each
path has ten test cases for the hash, VLAN and checksum entries.

The veth_skb subtest runs the same XDP program on a veth device that
receives an skb. The skb is not built from an xdp_frame, so its fields
do not change. The veth_skb_cpumap subtest redirects that packet into
the cpumap. veth turns it into an xdp_frame, so the cpumap sets the
stored values on the new skb.

GRO is off on both veth devices. Otherwise GRO on cpumap verifies the
UDP checksum of the packet and changes the checksum status.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 .../selftests/bpf/prog_tests/xdp_skb_ext.c    | 319 ++++++++++++++++++
 .../selftests/bpf/progs/test_xdp_skb_ext.c    |  84 +++++
 tools/testing/selftests/bpf/xdp_skb_ext.h     |  38 +++
 3 files changed, 441 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c b/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
index bd36e22e22a6..64c6f0268d60 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
@@ -1,9 +1,139 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 #include <network_helpers.h>
+#include <net/if.h>
+#include <linux/if_packet.h>
+#include "xdp_skb_ext.h"
 #include "test_xdp_skb_ext.skel.h"
 #include "xdp_skb_ext_success.skel.h"
 #include "xdp_skb_ext_failure.skel.h"
+#include "xdp_dummy.skel.h"
+
+#define XDP_RSS_TYPE_L3_IPV4 BIT(0)
+#define XDP_RSS_TYPE_L4 BIT(3)
+#define XDP_RSS_TYPE_L4_IPV4_TCP (BIT(0) | BIT(3) | BIT(4))
+
+#define XDP_CHECKSUM_NONE BIT(0)
+#define XDP_CHECKSUM_UNNECESSARY BIT(1)
+
+#define CHECKSUM_NONE 0
+#define CHECKSUM_UNNECESSARY 1
+
+#define RX_HASH 0x5a3c91e2
+#define RX_VLAN_TCI 1
+
+#define UDP_PAYLOAD_LEN 22
+
+static __u8 pkt_skb_ext[ETH_HLEN + sizeof(struct iphdr) +
+			sizeof(struct udphdr) + UDP_PAYLOAD_LEN];
+
+/* Valid checksums, as in real traffic */
+static void build_pkt(void)
+{
+	struct ethhdr *eth = (void *)pkt_skb_ext;
+	struct iphdr *iph = (void *)(eth + 1);
+	struct udphdr *udph = (void *)(iph + 1);
+
+	eth->h_proto = htons(ETH_P_IP);
+
+	iph->version = 4;
+	iph->ihl = 5;
+	iph->tot_len = htons(sizeof(pkt_skb_ext) - ETH_HLEN);
+	iph->ttl = 64;
+	iph->protocol = IPPROTO_UDP;
+	iph->saddr = htonl(0x0a000001);
+	iph->daddr = htonl(0x0a000002);
+	iph->check = build_ip_csum(iph);
+
+	udph->source = htons(4000);
+	udph->dest = htons(4001);
+	udph->len = htons(sizeof(*udph) + UDP_PAYLOAD_LEN);
+	udph->check = build_udp_v4_csum(iph, udph);
+}
+
+/* The case that test_veth_skb() uses */
+enum { CASE_ALL };
+
+static const struct skb_ext_case {
+	const char *name;
+	struct xdp_skb_ext_vals want;
+	bool del_hash;
+} skb_ext_cases[] = {
+	[CASE_ALL] = {
+		.name = "all",
+		.want = {
+			.rx_hash = RX_HASH,
+			.rx_hash_type = XDP_RSS_TYPE_L4_IPV4_TCP,
+			.rx_vlan_proto = __bpf_constant_htons(ETH_P_8021Q),
+			.rx_vlan_tci = RX_VLAN_TCI,
+			.ip_summed = XDP_CHECKSUM_UNNECESSARY,
+			.csum_level = 1,
+			.fields = SKB_EXT_FIELD_HASH | SKB_EXT_FIELD_VLAN |
+				  SKB_EXT_FIELD_CSUM,
+		},
+	},
+	{
+		.name = "none",
+	},
+	{
+		.name = "hash_l4",
+		.want = {
+			.rx_hash = RX_HASH,
+			.rx_hash_type = XDP_RSS_TYPE_L4_IPV4_TCP,
+			.fields = SKB_EXT_FIELD_HASH,
+		},
+	},
+	{
+		.name = "hash_l3",
+		.want = {
+			.rx_hash = RX_HASH,
+			.rx_hash_type = XDP_RSS_TYPE_L3_IPV4,
+			.fields = SKB_EXT_FIELD_HASH,
+		},
+	},
+	{
+		.name = "del_hash",
+		.del_hash = true,
+	},
+	{
+		.name = "vlan",
+		.want = {
+			.rx_vlan_proto = __bpf_constant_htons(ETH_P_8021Q),
+			.rx_vlan_tci = RX_VLAN_TCI,
+			.fields = SKB_EXT_FIELD_VLAN,
+		},
+	},
+	{
+		.name = "vlan_ad",
+		.want = {
+			.rx_vlan_proto = __bpf_constant_htons(ETH_P_8021AD),
+			.rx_vlan_tci = RX_VLAN_TCI,
+			.fields = SKB_EXT_FIELD_VLAN,
+		},
+	},
+	{
+		.name = "csum_none",
+		.want = {
+			.ip_summed = XDP_CHECKSUM_NONE,
+			.fields = SKB_EXT_FIELD_CSUM,
+		},
+	},
+	{
+		.name = "csum_level0",
+		.want = {
+			.ip_summed = XDP_CHECKSUM_UNNECESSARY,
+			.fields = SKB_EXT_FIELD_CSUM,
+		},
+	},
+	{
+		.name = "csum_level3",
+		.want = {
+			.ip_summed = XDP_CHECKSUM_UNNECESSARY,
+			.csum_level = 3,
+			.fields = SKB_EXT_FIELD_CSUM,
+		},
+	},
+};
 
 /* The area of a reused page is not valid for the next packet */
 static void test_live_frames(void)
@@ -31,6 +161,124 @@ static void test_live_frames(void)
 	test_xdp_skb_ext__destroy(skel);
 }
 
+static void set_case(struct test_xdp_skb_ext *skel,
+		     const struct skb_ext_case *c)
+{
+	skel->bss->want = c->want;
+	skel->bss->del_hash = c->del_hash;
+	memset(&skel->bss->have, 0, sizeof(skel->bss->have));
+	skel->bss->store_err = 0;
+	skel->bss->pkts_seen_tc = 0;
+}
+
+/* cpumap builds the skb in its kthread */
+static bool wait_pkt(struct test_xdp_skb_ext *skel)
+{
+	int i;
+
+	for (i = 0; i < 100; i++) {
+		if (__atomic_load_n(&skel->bss->pkts_seen_tc, __ATOMIC_ACQUIRE))
+			break;
+		usleep(10000);
+	}
+	return ASSERT_EQ(skel->bss->pkts_seen_tc, 1, "pkts_seen_tc");
+}
+
+static void check_have(struct test_xdp_skb_ext *skel)
+{
+	const struct xdp_skb_ext_vals *want = &skel->bss->want;
+	const struct xdp_skb_ext_have *have = &skel->bss->have;
+	bool unnecessary = (want->fields & SKB_EXT_FIELD_CSUM) &&
+			   want->ip_summed == XDP_CHECKSUM_UNNECESSARY;
+
+	if (!wait_pkt(skel))
+		return;
+	ASSERT_EQ(skel->bss->store_err, 0, "store_err");
+	ASSERT_EQ(have->hash, want->rx_hash, "hash");
+	ASSERT_EQ(have->l4_hash, !!(want->rx_hash_type & XDP_RSS_TYPE_L4),
+		  "l4_hash");
+	ASSERT_EQ(have->vlan_present, !!(want->fields & SKB_EXT_FIELD_VLAN),
+		  "vlan_present");
+	ASSERT_EQ(have->vlan_proto, want->rx_vlan_proto, "vlan_proto");
+	ASSERT_EQ(have->vlan_tci, want->rx_vlan_tci, "vlan_tci");
+	ASSERT_EQ(have->ip_summed,
+		  unnecessary ? CHECKSUM_UNNECESSARY : CHECKSUM_NONE,
+		  "ip_summed");
+	ASSERT_EQ(have->csum_level, unnecessary ? want->csum_level : 0,
+		  "csum_level");
+}
+
+static void test_frames(struct test_xdp_skb_ext *skel, const char *path,
+			enum xdp_skb_ext_deliver deliver, int ifindex)
+{
+	struct xdp_md ctx_in = { .data_end = sizeof(pkt_skb_ext),
+				 .ingress_ifindex = ifindex };
+	LIBBPF_OPTS(bpf_test_run_opts, opts,
+		    .data_in = pkt_skb_ext,
+		    .data_size_in = sizeof(pkt_skb_ext),
+		    .ctx_in = &ctx_in,
+		    .ctx_size_in = sizeof(ctx_in),
+		    .flags = BPF_F_TEST_XDP_LIVE_FRAMES,
+		    .repeat = 1,
+	);
+	int prog_fd = bpf_program__fd(skel->progs.xdp_store);
+	char name[32];
+	int i;
+
+	skel->bss->deliver = deliver;
+	for (i = 0; i < ARRAY_SIZE(skb_ext_cases); i++) {
+		snprintf(name, sizeof(name), "%s_%s", path,
+			 skb_ext_cases[i].name);
+		if (!test__start_subtest(name))
+			continue;
+		set_case(skel, &skb_ext_cases[i]);
+		if (ASSERT_OK(bpf_prog_test_run_opts(prog_fd, &opts),
+			      "test_run"))
+			check_have(skel);
+	}
+}
+
+/* The XDP program on the veth device gets an skb, not an xdp_frame */
+static void test_veth_skb(struct test_xdp_skb_ext *skel, struct bpf_link *link,
+			  int ifindex, enum xdp_skb_ext_deliver deliver)
+{
+	struct sockaddr_ll addr = {
+		.sll_family = PF_PACKET,
+		.sll_ifindex = ifindex,
+		.sll_halen = ETH_ALEN,
+	};
+	const struct xdp_skb_ext_have *have = &skel->bss->have;
+	int sock;
+
+	skel->bss->deliver = deliver;
+	if (!ASSERT_OK(bpf_link__update_program(link, skel->progs.xdp_store),
+		       "update xdp_store"))
+		return;
+
+	sock = socket(AF_PACKET, SOCK_RAW, IPPROTO_RAW);
+	if (!ASSERT_GE(sock, 0, "socket"))
+		return;
+	set_case(skel, &skb_ext_cases[CASE_ALL]);
+	if (!ASSERT_EQ(sendto(sock, pkt_skb_ext, sizeof(pkt_skb_ext), 0,
+			      (struct sockaddr *)&addr, sizeof(addr)),
+		       sizeof(pkt_skb_ext), "sendto") ||
+	    !wait_pkt(skel))
+		goto out;
+	if (deliver == DELIVER_CPUMAP) {
+		check_have(skel);
+		goto out;
+	}
+	ASSERT_EQ(skel->bss->store_err, 0, "store_err");
+	ASSERT_NEQ(have->hash, RX_HASH, "hash");
+	ASSERT_EQ(have->vlan_present, 0, "vlan_present");
+	ASSERT_EQ(have->vlan_proto, 0, "vlan_proto");
+	ASSERT_EQ(have->vlan_tci, 0, "vlan_tci");
+	ASSERT_EQ(have->ip_summed, CHECKSUM_NONE, "ip_summed");
+	ASSERT_EQ(have->csum_level, 0, "csum_level");
+out:
+	close(sock);
+}
+
 void test_xdp_skb_ext(void)
 {
 	RUN_TESTS(xdp_skb_ext_success);
@@ -39,3 +287,74 @@ void test_xdp_skb_ext(void)
 	if (test__start_subtest("live_frames"))
 		test_live_frames();
 }
+
+void test_xdp_skb_ext_to_skb(void)
+{
+	struct bpf_cpumap_val val = { .qsize = 192 };
+	int ifindex_src, ifindex_dst, tc_prog_fd;
+	struct xdp_dummy *skel_dummy = NULL;
+	struct test_xdp_skb_ext *skel = NULL;
+	struct bpf_link *link = NULL;
+	struct netns_obj *ns = NULL;
+	__u32 key = 0;
+
+	build_pkt();
+
+	ns = netns_new("xdp_skb_ext", true);
+	if (!ASSERT_OK_PTR(ns, "netns_new"))
+		return;
+
+	SYS(out, "ip link add veth_src type veth peer name veth_dst");
+	SYS(out, "ip link set dev veth_src up");
+	SYS(out, "ip link set dev veth_dst up");
+	/* GRO would verify the UDP checksum and change ip_summed */
+	SYS(out, "ethtool -K veth_src gro off");
+	SYS(out, "ethtool -K veth_dst gro off");
+
+	ifindex_src = if_nametoindex("veth_src");
+	ifindex_dst = if_nametoindex("veth_dst");
+	if (!ASSERT_NEQ(ifindex_src, 0, "ifindex_src") ||
+	    !ASSERT_NEQ(ifindex_dst, 0, "ifindex_dst"))
+		goto out;
+
+	skel = test_xdp_skb_ext__open();
+	if (!ASSERT_OK_PTR(skel, "test_xdp_skb_ext__open"))
+		goto out;
+	skel->rodata->ifindex_out = ifindex_src;
+	if (!ASSERT_OK(test_xdp_skb_ext__load(skel), "test_xdp_skb_ext__load"))
+		goto out;
+
+	/* pass and cpumap build the skb on veth_src, veth on veth_dst */
+	tc_prog_fd = bpf_program__fd(skel->progs.tc_read_skb);
+	if (tc_prog_attach("veth_src", tc_prog_fd, -1) ||
+	    tc_prog_attach("veth_dst", tc_prog_fd, -1))
+		goto out;
+
+	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.cpu_map),
+					   &key, &val, 0), "add cpumap entry"))
+		goto out;
+
+	skel_dummy = xdp_dummy__open_and_load();
+	if (!ASSERT_OK_PTR(skel_dummy, "xdp_dummy__open_and_load"))
+		goto out;
+
+	/* veth_dst takes xdp_frames only with an XDP program */
+	link = bpf_program__attach_xdp(skel_dummy->progs.xdp_dummy_prog,
+				       ifindex_dst);
+	if (!ASSERT_OK_PTR(link, "attach xdp_dummy"))
+		goto out;
+
+	test_frames(skel, "pass", DELIVER_PASS, ifindex_src);
+	test_frames(skel, "cpumap", DELIVER_CPUMAP, ifindex_src);
+	test_frames(skel, "veth", DELIVER_VETH, ifindex_src);
+	if (test__start_subtest("veth_skb"))
+		test_veth_skb(skel, link, ifindex_src, DELIVER_PASS);
+	if (test__start_subtest("veth_skb_cpumap"))
+		test_veth_skb(skel, link, ifindex_src, DELIVER_CPUMAP);
+
+out:
+	bpf_link__destroy(link);
+	xdp_dummy__destroy(skel_dummy);
+	test_xdp_skb_ext__destroy(skel);
+	netns_free(ns);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c b/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
index e072e051af48..a63206ff993d 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
@@ -3,6 +3,7 @@
 #include "xdp_skb_ext.h"
 #include <bpf/bpf_endian.h>
 #include <bpf/bpf_helpers.h>
+#include "bpf_tracing_net.h"
 
 /* Written to the first byte of the packet. A reused page keeps it. */
 #define MARK_STORED 0xaa
@@ -38,4 +39,87 @@ int live_frames(struct xdp_md *ctx)
 	return XDP_DROP;
 }
 
+struct {
+	__uint(type, BPF_MAP_TYPE_CPUMAP);
+	__uint(key_size, sizeof(__u32));
+	__uint(value_size, sizeof(struct bpf_cpumap_val));
+	__uint(max_entries, 1);
+} cpu_map SEC(".maps");
+
+const volatile int ifindex_out;
+enum xdp_skb_ext_deliver deliver;
+struct xdp_skb_ext_vals want;
+bool del_hash;
+struct xdp_skb_ext_have have;
+int pkts_seen_tc;
+
+SEC("xdp")
+int xdp_store(struct xdp_md *ctx)
+{
+	int err;
+
+	if (want.fields & SKB_EXT_FIELD_HASH) {
+		err = bpf_xdp_skb_ext_store_rx_hash(ctx, want.rx_hash,
+						    want.rx_hash_type);
+		if (err)
+			store_err++;
+	}
+	/* A deleted entry must not reach the skb */
+	if (del_hash) {
+		err = bpf_xdp_skb_ext_store_rx_hash(ctx, 1,
+						    XDP_RSS_TYPE_L4_IPV4_TCP);
+		if (!err)
+			err = bpf_xdp_skb_ext_del(ctx, XDP_SKB_EXT_HASH);
+		if (err)
+			store_err++;
+	}
+	if (want.fields & SKB_EXT_FIELD_VLAN) {
+		err = bpf_xdp_skb_ext_store_rx_vlan(ctx, want.rx_vlan_proto,
+						    want.rx_vlan_tci);
+		if (err)
+			store_err++;
+	}
+	/* NONE must replace an earlier status */
+	if ((want.fields & SKB_EXT_FIELD_CSUM) &&
+	    want.ip_summed == XDP_CHECKSUM_NONE) {
+		err = bpf_xdp_skb_ext_store_rx_csum(ctx,
+						    XDP_CHECKSUM_UNNECESSARY,
+						    SKB_MAX_CSUM_LEVEL);
+		if (err)
+			store_err++;
+	}
+	if (want.fields & SKB_EXT_FIELD_CSUM) {
+		err = bpf_xdp_skb_ext_store_rx_csum(ctx, want.ip_summed,
+						    want.csum_level);
+		if (err)
+			store_err++;
+	}
+
+	if (deliver == DELIVER_CPUMAP)
+		return bpf_redirect_map(&cpu_map, 0, 0);
+	if (deliver == DELIVER_VETH)
+		return bpf_redirect(ifindex_out, 0);
+	return XDP_PASS;
+}
+
+SEC("tc")
+int tc_read_skb(struct __sk_buff *skb)
+{
+	struct sk_buff *kskb;
+
+	if (skb->protocol != bpf_htons(ETH_P_IP))
+		return TC_ACT_OK;
+
+	kskb = bpf_cast_to_kern_ctx(skb);
+	have.hash = skb->hash;
+	have.l4_hash = kskb->l4_hash;
+	have.vlan_present = skb->vlan_present;
+	have.vlan_proto = skb->vlan_proto;
+	have.vlan_tci = skb->vlan_tci;
+	have.ip_summed = kskb->ip_summed;
+	have.csum_level = kskb->csum_level;
+	__sync_fetch_and_add(&pkts_seen_tc, 1);
+	return TC_ACT_SHOT;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/xdp_skb_ext.h b/tools/testing/selftests/bpf/xdp_skb_ext.h
index 540ba7c6aa1b..11feee2fa31c 100644
--- a/tools/testing/selftests/bpf/xdp_skb_ext.h
+++ b/tools/testing/selftests/bpf/xdp_skb_ext.h
@@ -13,3 +13,41 @@
 #define MAX_META_LEN (XDP_PACKET_HEADROOM - (int)sizeof(struct xdp_frame))
 
 #define SKB_MAX_CSUM_LEVEL 3
+
+#ifndef BIT
+#define BIT(nr)			(1 << (nr))
+#endif
+
+enum xdp_skb_ext_field {
+	SKB_EXT_FIELD_HASH = BIT(0),
+	SKB_EXT_FIELD_VLAN = BIT(1),
+	SKB_EXT_FIELD_CSUM = BIT(2),
+};
+
+enum xdp_skb_ext_deliver {
+	DELIVER_PASS,
+	DELIVER_CPUMAP,
+	DELIVER_VETH,
+};
+
+/* The entries that the XDP program stores */
+struct xdp_skb_ext_vals {
+	__u32 rx_hash;
+	__u32 rx_hash_type;
+	__be16 rx_vlan_proto;
+	__u16 rx_vlan_tci;
+	__u32 ip_summed;
+	__u8 csum_level;
+	enum xdp_skb_ext_field fields;
+};
+
+/* The skb fields that the tc program reads */
+struct xdp_skb_ext_have {
+	__u32 hash;
+	__u32 l4_hash;
+	__u32 vlan_present;
+	__u32 vlan_proto;
+	__u32 vlan_tci;
+	__u32 ip_summed;
+	__u32 csum_level;
+};
-- 
2.56.0

[PATCH bpf-next 09/15] xdp: add the skb-ext-pass XDP feature and warn without it

[Tianyi Gao]

On XDP_PASS, only drivers that call xdp_skb_ext_to_skb() set the skb
fields from the skb core extension area. Other drivers silently ignore
the stored values.

Add the XDP feature NETDEV_XDP_ACT_SKB_EXT_PASS ("skb-ext-pass") for
such drivers. veth sets it, and ice sets it in a later patch. User
space reads it with netdev netlink, and an XDP program reads
xdp->rxq->dev->xdp_features after bpf_cast_to_kern_ctx().

On a native XDP device without the feature, bpf_prog_run_xdp() warns
once on XDP_PASS with stored values, like bpf_warn_invalid_xdp_action().
For a packet without stored values, this adds one check of xdp->flags.
Programs on cpumap and devmap are skipped, because the skb is built
later from the xdp_frame.

BPF_PROG_RUN with BPF_F_TEST_XDP_LIVE_FRAMES builds the skb from the
xdp_frame and sets the stored values. It still warns when its
ingress_ifindex names a native XDP device without the feature, because the
check only looks at the device features.

Update the veth and bond feature checks in the xdp_do_redirect and
xdp_bonding selftests.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 Documentation/netlink/specs/netdev.yaml       |  5 ++++
 drivers/net/veth.c                            |  3 ++-
 include/net/xdp.h                             |  6 +++++
 include/uapi/linux/netdev.h                   |  6 ++++-
 net/core/xdp.c                                | 24 +++++++++++++++++++
 tools/include/uapi/linux/netdev.h             |  6 ++++-
 .../selftests/bpf/prog_tests/xdp_bonding.c    |  8 +++----
 .../bpf/prog_tests/xdp_do_redirect.c          |  8 +++----
 8 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 4303593e5003..7979b1ad38c7 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -50,6 +50,11 @@ definitions:
         doc: >-
           This feature informs if netdev implements non-linear XDP buffer
           support in ndo_xdp_xmit callback.
+      -
+        name: skb-ext-pass
+        doc: >-
+          This feature informs if netdev sets the skb fields from the skb
+          core extension area when it builds the skb after XDP_PASS.
   -
     type: flags
     name: xdp-rx-metadata
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 3eca92d3dc60..b402f8ed5096 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1304,7 +1304,8 @@ static void veth_set_xdp_features(struct net_device *dev)
 		struct veth_priv *priv_peer = netdev_priv(peer);
 		xdp_features_t val = NETDEV_XDP_ACT_BASIC |
 				     NETDEV_XDP_ACT_REDIRECT |
-				     NETDEV_XDP_ACT_RX_SG;
+				     NETDEV_XDP_ACT_RX_SG |
+				     NETDEV_XDP_ACT_SKB_EXT_PASS;
 
 		if (priv_peer->_xdp_prog || veth_gro_requested(peer))
 			val |= NETDEV_XDP_ACT_NDO_XMIT |
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 7c9a812b6e86..7148e8ef00e1 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -509,6 +509,8 @@ static __always_inline void *xdp_skb_ext_end(const struct xdp_buff *xdp)
 }
 
 void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext);
+void xdp_skb_ext_warn_pass(const struct bpf_prog *prog,
+			   const struct xdp_buff *xdp);
 
 void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
 		  bool napi_direct, struct xdp_buff *xdp);
@@ -771,6 +773,10 @@ static __always_inline u32 bpf_prog_run_xdp(const struct bpf_prog *prog,
 			act = xdp_master_redirect(xdp);
 	}
 
+	if (unlikely(xdp_buff_has_skb_ext(xdp)) && act == XDP_PASS &&
+	    !(xdp->rxq->dev->xdp_features & NETDEV_XDP_ACT_SKB_EXT_PASS))
+		xdp_skb_ext_warn_pass(prog, xdp);
+
 	return act;
 }
 #endif /* __LINUX_NET_XDP_H__ */
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 9026e0414fbe..a450e63cb941 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -25,6 +25,9 @@
  *   XDP buffer support in the driver napi callback.
  * @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev implements
  *   non-linear XDP buffer support in ndo_xdp_xmit callback.
+ * @NETDEV_XDP_ACT_SKB_EXT_PASS: This feature informs if netdev sets the skb
+ *   fields from the skb core extension area when it builds the skb after
+ *   XDP_PASS.
  */
 enum netdev_xdp_act {
 	NETDEV_XDP_ACT_BASIC = 1,
@@ -34,9 +37,10 @@ enum netdev_xdp_act {
 	NETDEV_XDP_ACT_HW_OFFLOAD = 16,
 	NETDEV_XDP_ACT_RX_SG = 32,
 	NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
+	NETDEV_XDP_ACT_SKB_EXT_PASS = 128,
 
 	/* private: */
-	NETDEV_XDP_ACT_MASK = 127,
+	NETDEV_XDP_ACT_MASK = 255,
 };
 
 /**
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 9f311b3c0d2b..36d6bb38b719 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -1115,6 +1115,30 @@ void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext)
 	}
 }
 
+/**
+ * xdp_skb_ext_warn_pass - Warn if XDP_PASS does not use the stored values.
+ * @prog: The XDP program that returned XDP_PASS.
+ * @xdp: The packet, with the XDP_FLAGS_HAS_SKB_EXT flag, on a device
+ *	 without NETDEV_XDP_ACT_SKB_EXT_PASS.
+ */
+void xdp_skb_ext_warn_pass(const struct bpf_prog *prog,
+			   const struct xdp_buff *xdp)
+{
+	const struct net_device *dev = xdp->rxq->dev;
+
+	/* The skb is built later from the xdp_frame */
+	if (prog->expected_attach_type == BPF_XDP_CPUMAP ||
+	    prog->expected_attach_type == BPF_XDP_DEVMAP)
+		return;
+	/* No native XDP, so only generic XDP or BPF_PROG_RUN runs on dev */
+	if (!(dev->xdp_features & NETDEV_XDP_ACT_BASIC))
+		return;
+
+	pr_warn_once("XDP_PASS on prog %s (id %u): dev %s has no skb-ext-pass XDP feature, the stored RX metadata is lost\n",
+		     prog->aux->name, prog->aux->id, dev->name);
+}
+EXPORT_SYMBOL_GPL(xdp_skb_ext_warn_pass);
+
 __bpf_kfunc_start_defs();
 
 /**
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 9026e0414fbe..a450e63cb941 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -25,6 +25,9 @@
  *   XDP buffer support in the driver napi callback.
  * @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev implements
  *   non-linear XDP buffer support in ndo_xdp_xmit callback.
+ * @NETDEV_XDP_ACT_SKB_EXT_PASS: This feature informs if netdev sets the skb
+ *   fields from the skb core extension area when it builds the skb after
+ *   XDP_PASS.
  */
 enum netdev_xdp_act {
 	NETDEV_XDP_ACT_BASIC = 1,
@@ -34,9 +37,10 @@ enum netdev_xdp_act {
 	NETDEV_XDP_ACT_HW_OFFLOAD = 16,
 	NETDEV_XDP_ACT_RX_SG = 32,
 	NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
+	NETDEV_XDP_ACT_SKB_EXT_PASS = 128,
 
 	/* private: */
-	NETDEV_XDP_ACT_MASK = 127,
+	NETDEV_XDP_ACT_MASK = 255,
 };
 
 /**
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
index c42488e445c2..c61b01c58342 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
@@ -625,7 +625,7 @@ static void test_xdp_bonding_features(struct skeletons *skeletons)
 
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-		       NETDEV_XDP_ACT_RX_SG,
+		       NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "bond query_opts.feature_flags"))
 		goto out;
 
@@ -646,7 +646,7 @@ static void test_xdp_bonding_features(struct skeletons *skeletons)
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 		       NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_NDO_XMIT |
-		       NETDEV_XDP_ACT_NDO_XMIT_SG,
+		       NETDEV_XDP_ACT_NDO_XMIT_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "bond query_opts.feature_flags"))
 		goto out;
 
@@ -663,7 +663,7 @@ static void test_xdp_bonding_features(struct skeletons *skeletons)
 	 */
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-		       NETDEV_XDP_ACT_RX_SG,
+		       NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "bond query_opts.feature_flags"))
 		goto out;
 
@@ -678,7 +678,7 @@ static void test_xdp_bonding_features(struct skeletons *skeletons)
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 		       NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_NDO_XMIT |
-		       NETDEV_XDP_ACT_NDO_XMIT_SG,
+		       NETDEV_XDP_ACT_NDO_XMIT_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "bond query_opts.feature_flags"))
 		goto out;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
index dd34b0cc4b4e..e1cd3b951dac 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
@@ -170,7 +170,7 @@ void test_xdp_do_redirect(void)
 
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-		       NETDEV_XDP_ACT_RX_SG,
+		       NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "veth_src query_opts.feature_flags"))
 		goto out;
 
@@ -180,7 +180,7 @@ void test_xdp_do_redirect(void)
 
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-		       NETDEV_XDP_ACT_RX_SG,
+		       NETDEV_XDP_ACT_RX_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "veth_dst query_opts.feature_flags"))
 		goto out;
 
@@ -195,7 +195,7 @@ void test_xdp_do_redirect(void)
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 		       NETDEV_XDP_ACT_NDO_XMIT | NETDEV_XDP_ACT_RX_SG |
-		       NETDEV_XDP_ACT_NDO_XMIT_SG,
+		       NETDEV_XDP_ACT_NDO_XMIT_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "veth_src query_opts.feature_flags gro on"))
 		goto out;
 
@@ -206,7 +206,7 @@ void test_xdp_do_redirect(void)
 	if (!ASSERT_EQ(query_opts.feature_flags,
 		       NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 		       NETDEV_XDP_ACT_NDO_XMIT | NETDEV_XDP_ACT_RX_SG |
-		       NETDEV_XDP_ACT_NDO_XMIT_SG,
+		       NETDEV_XDP_ACT_NDO_XMIT_SG | NETDEV_XDP_ACT_SKB_EXT_PASS,
 		       "veth_dst query_opts.feature_flags gro on"))
 		goto out;
 
-- 
2.56.0

[PATCH bpf-next 10/15] xdp: warn on XDP_PASS with stored values when XDP runs on an skb

[Tianyi Gao]

On XDP_PASS, when XDP runs on an skb, the stored values are lost, because
the skb already exists. This happens in bpf_prog_run_generic_xdp() for
generic XDP and for cpumap and devmap programs on an skb. It also
happens in veth_xdp_rcv_skb() when veth receives an skb from its peer.
veth has the skb-ext-pass feature, so bpf_prog_run_xdp() does not warn
for it.

Add xdp_skb_ext_warn_skb(). bpf_prog_run_generic_xdp() and
veth_xdp_rcv_skb() call it on XDP_PASS with stored values, whatever the
device features.

Generic XDP also runs bpf_prog_run_xdp(). On a native XDP device without
the feature, it would warn twice. So xdp_skb_ext_warn_pass() skips
generic XDP.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
Tested on virtio_net and veth: generic XDP prints only the new warning,
and BPF_PROG_RUN without live frames prints none.
---
 drivers/net/veth.c |  2 ++
 include/net/xdp.h  |  2 ++
 net/core/dev.c     |  2 ++
 net/core/xdp.c     | 21 ++++++++++++++++++++-
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index b402f8ed5096..f0d279e2da6a 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -826,6 +826,8 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 
 	switch (act) {
 	case XDP_PASS:
+		if (unlikely(xdp_buff_has_skb_ext(xdp)))
+			xdp_skb_ext_warn_skb(xdp_prog, xdp);
 		break;
 	case XDP_TX:
 		veth_xdp_get(xdp);
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 7148e8ef00e1..596261ed9cca 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -511,6 +511,8 @@ static __always_inline void *xdp_skb_ext_end(const struct xdp_buff *xdp)
 void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext);
 void xdp_skb_ext_warn_pass(const struct bpf_prog *prog,
 			   const struct xdp_buff *xdp);
+void xdp_skb_ext_warn_skb(const struct bpf_prog *prog,
+			  const struct xdp_buff *xdp);
 
 void __xdp_return(netmem_ref netmem, enum xdp_mem_type mem_type,
 		  bool napi_direct, struct xdp_buff *xdp);
diff --git a/net/core/dev.c b/net/core/dev.c
index 3bf00e7e1a85..e37c881d968b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5705,6 +5705,8 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
 	orig_eth_type = eth->h_proto;
 
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
+	if (unlikely(xdp_buff_has_skb_ext(xdp)) && act == XDP_PASS)
+		xdp_skb_ext_warn_skb(xdp_prog, xdp);
 
 	/* check if bpf_xdp_adjust_head was used */
 	off = xdp->data - orig_data;
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 36d6bb38b719..984299199906 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -18,6 +18,7 @@
 
 #include <net/hotdata.h>
 #include <net/netdev_lock.h>
+#include <net/netdev_rx_queue.h>
 #include <net/xdp.h>
 #include <net/xdp_priv.h> /* struct xdp_mem_allocator */
 #include <trace/events/xdp.h>
@@ -1124,12 +1125,17 @@ void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext)
 void xdp_skb_ext_warn_pass(const struct bpf_prog *prog,
 			   const struct xdp_buff *xdp)
 {
-	const struct net_device *dev = xdp->rxq->dev;
+	const struct xdp_rxq_info *rxq = xdp->rxq;
+	const struct net_device *dev = rxq->dev;
 
 	/* The skb is built later from the xdp_frame */
 	if (prog->expected_attach_type == BPF_XDP_CPUMAP ||
 	    prog->expected_attach_type == BPF_XDP_DEVMAP)
 		return;
+	/* Generic XDP and BPF_PROG_RUN use the netdev_rx_queue xdp_rxq */
+	if (rxq->queue_index < dev->num_rx_queues &&
+	    rxq == &dev->_rx[rxq->queue_index].xdp_rxq)
+		return;
 	/* No native XDP, so only generic XDP or BPF_PROG_RUN runs on dev */
 	if (!(dev->xdp_features & NETDEV_XDP_ACT_BASIC))
 		return;
@@ -1139,6 +1145,19 @@ void xdp_skb_ext_warn_pass(const struct bpf_prog *prog,
 }
 EXPORT_SYMBOL_GPL(xdp_skb_ext_warn_pass);
 
+/**
+ * xdp_skb_ext_warn_skb - Warn that XDP_PASS on an skb loses the stored values.
+ * @prog: The XDP program that returned XDP_PASS.
+ * @xdp: The packet, with the XDP_FLAGS_HAS_SKB_EXT flag, built on an skb.
+ */
+void xdp_skb_ext_warn_skb(const struct bpf_prog *prog,
+			  const struct xdp_buff *xdp)
+{
+	pr_warn_once("XDP_PASS on prog %s (id %u): dev %s runs XDP on an skb, the stored RX metadata is lost\n",
+		     prog->aux->name, prog->aux->id, xdp->rxq->dev->name);
+}
+EXPORT_SYMBOL_GPL(xdp_skb_ext_warn_skb);
+
 __bpf_kfunc_start_defs();
 
 /**
-- 
2.56.0

[PATCH bpf-next 11/15] ice: set skb RX metadata from the skb core extension area of xdp_buff

[Tianyi Gao]

On XDP_PASS, ice builds the skb with xdp_build_skb_from_buff(). Then
ice_process_skb_fields() sets the hash and ip_summed from the RX
descriptor, and ice_receive_skb() adds the VLAN tag. The values that the
XDP program stored in the skb core extension area are not used.

ice_clean_rx_irq() takes the skb core extension pointer from the
xdp_buff before it builds the skb, and passes it to ice_receive_skb().
ice_receive_skb() calls xdp_skb_ext_to_skb() right before GRO. The
stored values then replace the values from the descriptor before the skb
leaves the driver. ice now sets the skb-ext-pass XDP feature.

Zero-copy is not supported in this patch. ice_clean_rx_irq_zc() passes
NULL for the skb core extension pointer to ice_receive_skb(). Hence the
skb-ext-pass feature does not cover AF_XDP zero-copy queues.

Export xdp_skb_ext_to_skb(), because ice can be a module.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c     | 3 ++-
 drivers/net/ethernet/intel/ice/ice_sf_eth.c   | 3 ++-
 drivers/net/ethernet/intel/ice/ice_txrx.c     | 4 +++-
 drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 7 ++++++-
 drivers/net/ethernet/intel/ice/ice_txrx_lib.h | 3 ++-
 drivers/net/ethernet/intel/ice/ice_xsk.c      | 2 +-
 net/core/xdp.c                                | 5 ++++-
 7 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 454fba07d4ec..cb619689d11a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3461,7 +3461,8 @@ static void ice_set_ops(struct ice_vsi *vsi)
 
 	netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 			       NETDEV_XDP_ACT_XSK_ZEROCOPY |
-			       NETDEV_XDP_ACT_RX_SG;
+			       NETDEV_XDP_ACT_RX_SG |
+			       NETDEV_XDP_ACT_SKB_EXT_PASS;
 	netdev->xdp_zc_max_segs = ICE_MAX_BUF_TXD;
 }
 
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.c b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
index a730aa368c92..90f2ce5da022 100644
--- a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
@@ -52,7 +52,8 @@ static int ice_sf_cfg_netdev(struct ice_dynamic_port *dyn_port,
 
 	netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 			       NETDEV_XDP_ACT_XSK_ZEROCOPY |
-			       NETDEV_XDP_ACT_RX_SG;
+			       NETDEV_XDP_ACT_RX_SG |
+			       NETDEV_XDP_ACT_SKB_EXT_PASS;
 	netdev->xdp_zc_max_segs = ICE_MAX_BUF_TXD;
 
 	eth_hw_addr_set(netdev, dyn_port->hw_addr);
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index a2c7c4962882..06a1a5fdc43b 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -966,6 +966,7 @@ static int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
 	/* start the loop to process Rx packets bounded by 'budget' */
 	while (likely(total_rx_pkts < (unsigned int)budget)) {
 		union ice_32b_rx_flex_desc *rx_desc;
+		const struct xdp_skb_ext *ext;
 		struct libeth_fqe *rx_buf;
 		struct sk_buff *skb;
 		unsigned int size;
@@ -1041,6 +1042,7 @@ static int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
 		continue;
 
 construct_skb:
+		ext = xdp_buff_skb_ext(&xdp->base);
 		skb = xdp_build_skb_from_buff(&xdp->base);
 		xdp->data = NULL;
 
@@ -1061,7 +1063,7 @@ static int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
 
 		ice_trace(clean_rx_irq_indicate, rx_ring, rx_desc, skb);
 		/* send completed skb up the stack */
-		ice_receive_skb(rx_ring, skb, vlan_tci);
+		ice_receive_skb(rx_ring, skb, vlan_tci, ext);
 
 		/* update budget accounting */
 		total_rx_pkts++;
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
index e695a664e53d..85439853596f 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
@@ -247,17 +247,22 @@ ice_process_skb_fields(struct ice_rx_ring *rx_ring,
  * @rx_ring: Rx ring in play
  * @skb: packet to send up
  * @vlan_tci: VLAN TCI for packet
+ * @ext: skb core extension area of the packet, or NULL
  *
  * This function sends the completed packet (via. skb) up the stack using
  * gro receive functions (with/without VLAN tag)
  */
 void
-ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tci)
+ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tci,
+		const struct xdp_skb_ext *ext)
 {
 	if ((vlan_tci & VLAN_VID_MASK) && rx_ring->vlan_proto)
 		__vlan_hwaccel_put_tag(skb, rx_ring->vlan_proto,
 				       vlan_tci);
 
+	if (ext)
+		xdp_skb_ext_to_skb(skb, ext);
+
 	napi_gro_receive(&rx_ring->q_vector->napi, skb);
 }
 
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
index f17990b68b62..cfc77847696e 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
@@ -133,6 +133,7 @@ ice_process_skb_fields(struct ice_rx_ring *rx_ring,
 		       union ice_32b_rx_flex_desc *rx_desc,
 		       struct sk_buff *skb);
 void
-ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tci);
+ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tci,
+		const struct xdp_skb_ext *ext);
 
 #endif /* !_ICE_TXRX_LIB_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 0643017541c3..f64659dc8313 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -670,7 +670,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring,
 		vlan_tci = ice_get_vlan_tci(rx_desc);
 
 		ice_process_skb_fields(rx_ring, rx_desc, skb);
-		ice_receive_skb(rx_ring, skb, vlan_tci);
+		ice_receive_skb(rx_ring, skb, vlan_tci, NULL);
 	}
 
 	rx_ring->next_to_clean = ntc;
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 984299199906..fdda072a5858 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -1086,7 +1086,9 @@ static const void *xdp_skb_ext_get(const struct xdp_skb_ext *ext,
  * @skb: The skb that is built from the packet.
  * @ext: The skb core extension area of the packet.
  *
- * Call it only when the packet has the XDP_FLAGS_HAS_SKB_EXT flag.
+ * Call it only when the packet has the XDP_FLAGS_HAS_SKB_EXT flag. A driver
+ * calls it after it sets the skb fields from the RX descriptor, and sets
+ * NETDEV_XDP_ACT_SKB_EXT_PASS.
  */
 void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext)
 {
@@ -1115,6 +1117,7 @@ void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext)
 		skb->csum_level = csum->level;
 	}
 }
+EXPORT_SYMBOL_GPL(xdp_skb_ext_to_skb);
 
 /**
  * xdp_skb_ext_warn_pass - Warn if XDP_PASS does not use the stored values.
-- 
2.56.0

[PATCH bpf-next 12/15] selftests: drv-net: xdp: test the stored RX metadata on XDP_PASS

[Tianyi Gao]

Add a drv-net test for the skb core extension area. An XDP program
stores an RX hash, a VLAN tag and a checksum status. A tc program on
ingress reads the skb fields, and the test compares them with the
stored values.

The stored hash and VLAN tag are values that the NIC does not report:
hash 0x12345678 and an 802.1ad tag with VID 0xabc. The checksum status
is tested as CHECKSUM_NONE and as CHECKSUM_UNNECESSARY with csum_level
1. The NIC reports only one of them, so a driver that ignores the
stored values fails at least one case.

The test runs only on devices with the skb-ext-pass XDP feature. GRO is
off, because it can change ip_summed before the tc program runs. Like
test_xdp_rss_hash, the test sets its config map by name with
bpf_map_set().

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
veth has the skb-ext-pass feature, but in drv-net the test packet reaches
veth as an skb. The stored values are then lost, so the test fails on
veth. NIPA runs drv-net on netdevsim, where the test skips, and on real
NICs.
---
 .../drivers/net/hw/lib/py/__init__.py         |   3 +-
 .../selftests/drivers/net/hw/xdp_metadata.py  | 118 ++++++++++++-
 .../drivers/net/hw/xdp_skb_ext.bpf.c          | 166 ++++++++++++++++++
 3 files changed, 285 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/hw/xdp_skb_ext.bpf.c

diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
index 4077f5f1a1be..8a6702abcdbe 100644
--- a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
@@ -24,7 +24,7 @@ try:
     from net.lib.py import CmdExitFailure
     from net.lib.py import bkg, cmd, bpftool, bpftrace, defer, ethtool, \
         fd_read_timeout, ip, rand_port, rand_ports, wait_port_listen, \
-        wait_file, ctl_file_write, tool
+        wait_file, ctl_file_write, tool, tc
     from net.lib.py import bpf_map_set, bpf_map_dump, bpf_prog_map_ids
     from net.lib.py import KsftSkipEx, KsftFailEx, KsftXfailEx
     from net.lib.py import ksft_disruptive, ksft_exit, ksft_pr, ksft_run, \
@@ -42,6 +42,7 @@ try:
                "bkg", "cmd", "bpftool", "bpftrace", "defer", "ethtool",
                "fd_read_timeout", "ip", "rand_port", "rand_ports",
                "wait_port_listen", "wait_file", "ctl_file_write", "tool",
+               "tc",
                "bpf_map_set", "bpf_map_dump", "bpf_prog_map_ids",
                "KsftSkipEx", "KsftFailEx", "KsftXfailEx",
                "ksft_disruptive", "ksft_exit", "ksft_pr", "ksft_run",
diff --git a/tools/testing/selftests/drivers/net/hw/xdp_metadata.py b/tools/testing/selftests/drivers/net/hw/xdp_metadata.py
index 33a1985356d9..88b242964cff 100644
--- a/tools/testing/selftests/drivers/net/hw/xdp_metadata.py
+++ b/tools/testing/selftests/drivers/net/hw/xdp_metadata.py
@@ -7,13 +7,18 @@ Tests for XDP metadata kfuncs (e.g. bpf_xdp_metadata_rx_hash).
 These tests load device-bound XDP programs from xdp_metadata.bpf.o
 that call metadata kfuncs, send traffic, and verify the extracted
 metadata via BPF maps.
+
+The skb core extension test loads xdp_skb_ext.bpf.o: an XDP program
+stores RX metadata and a tc program checks it on the skb.
 """
+import socket
+
 from lib.py import ksft_run, ksft_eq, ksft_exit, ksft_ge, ksft_ne, ksft_pr
 from lib.py import KsftNamedVariant, ksft_variants
 from lib.py import CmdExitFailure, KsftSkipEx, NetDrvEpEnv
 from lib.py import NetdevFamily
 from lib.py import bkg, cmd, rand_port, wait_port_listen
-from lib.py import ip, bpftool, defer
+from lib.py import ip, bpftool, defer, ethtool, tc, set_ethtool_feat
 from lib.py import bpf_map_set, bpf_map_dump, bpf_prog_map_ids
 
 
@@ -130,6 +135,116 @@ def test_xdp_rss_hash(cfg, proto):
             f"RSS hash type should include L4 for {proto.upper()} traffic")
 
 
+# BPF map keys matching the enums in xdp_skb_ext.bpf.c
+_CFG_PORT = 0
+_CFG_HASH = 1
+_CFG_VLAN_PROTO = 2
+_CFG_VLAN_TCI = 3
+_CFG_IP_SUMMED = 4
+_CFG_CSUM_LEVEL = 5
+
+_RES_PKT_CNT = 0
+_RES_STORE_ERR = 1
+_RES_HASH = 2
+_RES_L4_HASH = 3
+_RES_VLAN_PROTO = 4
+_RES_VLAN_TCI = 5
+_RES_IP_SUMMED = 6
+_RES_CSUM_LEVEL = 7
+
+ETH_P_8021AD = 0x88a8
+VLAN_PRIO_SHIFT = 13
+
+# Magic numbers the NIC does not report
+TEST_RX_HASH = 0x12345678
+TEST_VLAN_PCP = 5
+TEST_VLAN_VID = 0xabc
+TEST_VLAN_TCI = TEST_VLAN_PCP << VLAN_PRIO_SHIFT | TEST_VLAN_VID
+
+XDP_CHECKSUM_NONE = 1 << 0
+XDP_CHECKSUM_UNNECESSARY = 1 << 1
+CHECKSUM_NONE = 0
+CHECKSUM_UNNECESSARY = 1
+
+
+def _load_skb_ext_progs(cfg):
+    """Attach the store program to XDP and the check program to tc ingress.
+
+    Returns:
+        A dict mapping map name to map ID.
+    """
+    abs_path = cfg.test_dir / "xdp_skb_ext.bpf.o"
+    pin_dir = "/sys/fs/bpf/xdp_skb_ext_test"
+
+    cmd(f"rm -rf {pin_dir}", shell=True, fail=False)
+    cmd(f"mkdir -p {pin_dir}", shell=True)
+    defer(cmd, f"rm -rf {pin_dir}", shell=True, fail=False)
+
+    bpftool(f"prog loadall {abs_path} {pin_dir}")
+    ip(f"link set dev {cfg.ifname} xdpdrv pinned {pin_dir}/xdp_store")
+    defer(ip, f"link set dev {cfg.ifname} xdpdrv off")
+
+    qdiscs = tc(f"qdisc show dev {cfg.ifname}", json=True)
+    if not any(q["kind"] == "clsact" for q in qdiscs):
+        tc(f"qdisc add dev {cfg.ifname} clsact")
+        defer(tc, f"qdisc del dev {cfg.ifname} clsact")
+
+    tc(f"filter add dev {cfg.ifname} ingress bpf "
+       f"object-pinned {pin_dir}/tc_check direct-action")
+    prog_id = bpftool(f"prog show pinned {pin_dir}/tc_check", json=True)["id"]
+    for f in tc(f"filter show dev {cfg.ifname} ingress", json=True):
+        if f.get("options", {}).get("prog", {}).get("id") == prog_id:
+            defer(tc, f"filter del dev {cfg.ifname} ingress pref {f['pref']}")
+
+    return bpf_prog_map_ids(prog_id)
+
+
+@ksft_variants([
+    KsftNamedVariant("csum_none", XDP_CHECKSUM_NONE, 0, CHECKSUM_NONE),
+    KsftNamedVariant("csum_unnecessary", XDP_CHECKSUM_UNNECESSARY, 1,
+                     CHECKSUM_UNNECESSARY),
+])
+def test_xdp_skb_ext(cfg, xdp_csum, csum_level, ip_summed):
+    """Test that the skb gets the RX metadata stored on XDP_PASS.
+
+    The XDP program stores a hash, a VLAN tag and a checksum status that
+    differ from what the NIC reports. The tc program reads them from the
+    skb.
+    """
+    dev_info = cfg.netnl.dev_get({"ifindex": cfg.ifindex})
+    if "skb-ext-pass" not in dev_info.get("xdp-features", []):
+        raise KsftSkipEx("device does not support skb-ext-pass")
+
+    maps = _load_skb_ext_progs(cfg)
+
+    # GRO can change ip_summed before tc ingress
+    feat = ethtool(f"-k {cfg.ifname}", json=True)[0]
+    set_ethtool_feat(cfg.ifname, feat, {"generic-receive-offload": False})
+
+    cfg_map = "map_store_cfg"
+    bpf_map_set(cfg_map, _CFG_HASH, TEST_RX_HASH)
+    bpf_map_set(cfg_map, _CFG_VLAN_PROTO, ETH_P_8021AD)
+    bpf_map_set(cfg_map, _CFG_VLAN_TCI, TEST_VLAN_TCI)
+    bpf_map_set(cfg_map, _CFG_IP_SUMMED, xdp_csum)
+    bpf_map_set(cfg_map, _CFG_CSUM_LEVEL, csum_level)
+    # The port goes last: it enables the store for the test packets
+    port = rand_port(socket.SOCK_DGRAM)
+    bpf_map_set(cfg_map, _CFG_PORT, port)
+
+    _send_probe(cfg, port, proto="udp")
+
+    res = bpf_map_dump(maps["map_store_res"])
+    ksft_ge(res.get(_RES_PKT_CNT, 0), 1, "tc saw the test packet")
+    ksft_eq(res.get(_RES_STORE_ERR, 0), 0, "store kfunc errors")
+
+    ksft_eq(res.get(_RES_HASH, 0), TEST_RX_HASH, "skb->hash")
+    ksft_eq(res.get(_RES_L4_HASH, 0), 1, "skb->l4_hash")
+    ksft_eq(res.get(_RES_VLAN_PROTO, 0), ETH_P_8021AD, "skb->vlan_proto")
+    ksft_eq(res.get(_RES_VLAN_TCI, 0), TEST_VLAN_TCI, "skb->vlan_tci")
+    ksft_eq(res.get(_RES_IP_SUMMED, 0), ip_summed, "skb->ip_summed")
+    ksft_eq(res.get(_RES_CSUM_LEVEL, 0), csum_level, "skb->csum_level")
+
+
 def main():
     """Run XDP metadata kfunc tests against a real device."""
     with NetDrvEpEnv(__file__) as cfg:
@@ -137,6 +252,7 @@ def main():
         ksft_run(
             [
                 test_xdp_rss_hash,
+                test_xdp_skb_ext,
             ],
             args=(cfg,))
     ksft_exit()
diff --git a/tools/testing/selftests/drivers/net/hw/xdp_skb_ext.bpf.c b/tools/testing/selftests/drivers/net/hw/xdp_skb_ext.bpf.c
new file mode 100644
index 000000000000..b186bc1f7947
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/xdp_skb_ext.bpf.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdbool.h>
+#include <linux/bpf.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/pkt_cls.h>
+#include <linux/udp.h>
+#include <bpf/bpf_core_read.h>
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+
+enum {
+	CFG_PORT,
+	CFG_HASH,
+	CFG_VLAN_PROTO,
+	CFG_VLAN_TCI,
+	CFG_IP_SUMMED,
+	CFG_CSUM_LEVEL,
+	CFG_MAX,
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, CFG_MAX);
+	__type(key, __u32);
+	__type(value, __u32);
+} map_store_cfg SEC(".maps");
+
+enum {
+	RES_PKT_CNT,
+	RES_STORE_ERR,
+	RES_HASH,
+	RES_L4_HASH,
+	RES_VLAN_PROTO,
+	RES_VLAN_TCI,
+	RES_IP_SUMMED,
+	RES_CSUM_LEVEL,
+	RES_MAX,
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, RES_MAX);
+	__type(key, __u32);
+	__type(value, __u32);
+} map_store_res SEC(".maps");
+
+/* Mirrors of include/net/xdp.h, not part of UAPI */
+enum xdp_rss_hash_type {
+	XDP_RSS_L4 = 1U << 3,
+};
+
+enum xdp_checksum {
+	XDP_CHECKSUM_NONE = 1U << 0,
+	XDP_CHECKSUM_UNNECESSARY = 1U << 1,
+};
+
+struct sk_buff {
+	__u8 ip_summed:2;
+	__u8 csum_level:2;
+	__u8 l4_hash:1;
+} __attribute__((preserve_access_index));
+
+extern int bpf_xdp_skb_ext_store_rx_hash(struct xdp_md *, __u32,
+					 enum xdp_rss_hash_type) __ksym;
+extern int bpf_xdp_skb_ext_store_rx_vlan(struct xdp_md *, __be16,
+					 __u16) __ksym;
+extern int bpf_xdp_skb_ext_store_rx_csum(struct xdp_md *, enum xdp_checksum,
+					 __u8) __ksym;
+extern void *bpf_cast_to_kern_ctx(void *obj) __ksym;
+
+static __u32 cfg_get(__u32 key)
+{
+	__u32 *val = bpf_map_lookup_elem(&map_store_cfg, &key);
+
+	return val ? *val : 0;
+}
+
+static void res_set(__u32 key, __u32 val)
+{
+	bpf_map_update_elem(&map_store_res, &key, &val, BPF_ANY);
+}
+
+static void res_inc(__u32 key)
+{
+	__u32 *cnt = bpf_map_lookup_elem(&map_store_res, &key);
+
+	if (cnt)
+		__sync_fetch_and_add(cnt, 1);
+}
+
+static bool is_test_pkt(void *data, void *data_end)
+{
+	struct ethhdr *eth = data;
+	struct udphdr *udp;
+
+	if ((void *)(eth + 1) > data_end)
+		return false;
+
+	if (eth->h_proto == bpf_htons(ETH_P_IP)) {
+		struct iphdr *iph = (void *)(eth + 1);
+
+		if ((void *)(iph + 1) > data_end ||
+		    iph->protocol != IPPROTO_UDP)
+			return false;
+		udp = (void *)(iph + 1);
+	} else if (eth->h_proto == bpf_htons(ETH_P_IPV6)) {
+		struct ipv6hdr *ip6h = (void *)(eth + 1);
+
+		if ((void *)(ip6h + 1) > data_end ||
+		    ip6h->nexthdr != IPPROTO_UDP)
+			return false;
+		udp = (void *)(ip6h + 1);
+	} else {
+		return false;
+	}
+
+	if ((void *)(udp + 1) > data_end)
+		return false;
+
+	return bpf_ntohs(udp->dest) == cfg_get(CFG_PORT);
+}
+
+SEC("xdp")
+int xdp_store(struct xdp_md *ctx)
+{
+	if (!is_test_pkt((void *)(long)ctx->data, (void *)(long)ctx->data_end))
+		return XDP_PASS;
+
+	if (bpf_xdp_skb_ext_store_rx_hash(ctx, cfg_get(CFG_HASH), XDP_RSS_L4) ||
+	    bpf_xdp_skb_ext_store_rx_vlan(ctx,
+					  bpf_htons(cfg_get(CFG_VLAN_PROTO)),
+					  cfg_get(CFG_VLAN_TCI)) ||
+	    bpf_xdp_skb_ext_store_rx_csum(ctx, cfg_get(CFG_IP_SUMMED),
+					  cfg_get(CFG_CSUM_LEVEL)))
+		res_inc(RES_STORE_ERR);
+
+	return XDP_PASS;
+}
+
+SEC("tc")
+int tc_check(struct __sk_buff *skb)
+{
+	struct sk_buff *kskb;
+
+	if (!is_test_pkt((void *)(long)skb->data, (void *)(long)skb->data_end))
+		return TC_ACT_UNSPEC;
+
+	kskb = bpf_cast_to_kern_ctx(skb);
+	res_set(RES_HASH, skb->hash);
+	res_set(RES_L4_HASH, BPF_CORE_READ_BITFIELD(kskb, l4_hash));
+	if (skb->vlan_present) {
+		res_set(RES_VLAN_PROTO, bpf_ntohs(skb->vlan_proto));
+		res_set(RES_VLAN_TCI, skb->vlan_tci);
+	}
+	res_set(RES_IP_SUMMED, BPF_CORE_READ_BITFIELD(kskb, ip_summed));
+	res_set(RES_CSUM_LEVEL, BPF_CORE_READ_BITFIELD(kskb, csum_level));
+	res_inc(RES_PKT_CNT);
+
+	return TC_ACT_UNSPEC;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.56.0

[PATCH bpf-next 13/15] xdp: add kfuncs to load the stored RX metadata

[Tianyi Gao]

A program on cpumap or devmap cannot call the XDP RX metadata kfuncs,
because they need a device-bound program. A program on veth can be
bound to veth, but the veth callbacks return -ENODATA for an
xdp_frame. So a program after the redirect cannot read the values
that the program on the NIC stored.

Add three kfuncs that read the skb core extension area:
bpf_xdp_skb_ext_load_rx_hash(), bpf_xdp_skb_ext_load_rx_vlan() and
bpf_xdp_skb_ext_load_rx_csum(). Each returns -ENODATA when its entry
is not stored. They are in the same kfunc set as the store kfuncs, so
they are not device-bound and need no driver callback. A program on
the NIC can also read back the values that it stored.

The RX metadata kfuncs could return the stored values instead. This
patch does not do that. Relaxing bpf_dev_bound_kfunc_check() for
cpumap is not trivial, and it changes these kfuncs for existing
programs, which read the values from the RX descriptor of the bound
device.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 net/core/xdp.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/net/core/xdp.c b/net/core/xdp.c
index fdda072a5858..4df3984479a7 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -1081,6 +1081,15 @@ static const void *xdp_skb_ext_get(const struct xdp_skb_ext *ext,
 	return (const void *)ext + off * XDP_SKB_EXT_ALIGN;
 }
 
+static const void *xdp_skb_ext_load(const struct xdp_md *ctx,
+				    enum xdp_skb_ext_id id)
+{
+	const struct xdp_buff *xdp = (const struct xdp_buff *)ctx;
+	const struct xdp_skb_ext *ext = xdp_buff_skb_ext(xdp);
+
+	return ext ? xdp_skb_ext_get(ext, id) : NULL;
+}
+
 /**
  * xdp_skb_ext_to_skb - Set the stored RX metadata on an skb.
  * @skb: The skb that is built from the packet.
@@ -1296,6 +1305,85 @@ __bpf_kfunc int bpf_xdp_skb_ext_del(struct xdp_md *ctx, enum xdp_skb_ext_id id)
 	return 0;
 }
 
+/**
+ * bpf_xdp_skb_ext_load_rx_hash - Load the stored RX hash of this packet.
+ * @ctx: XDP context pointer.
+ * @hash: Return value pointer.
+ * @rss_type: Return value pointer for RSS type, see &enum xdp_rss_hash_type.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-ENODATA`` : the RX hash is not stored
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_load_rx_hash(const struct xdp_md *ctx,
+					     u32 *hash,
+					     enum xdp_rss_hash_type *rss_type)
+{
+	const struct xdp_skb_ext_hash *entry;
+
+	entry = xdp_skb_ext_load(ctx, XDP_SKB_EXT_HASH);
+	if (!entry)
+		return -ENODATA;
+
+	*hash = entry->val;
+	*rss_type = entry->type;
+
+	return 0;
+}
+
+/**
+ * bpf_xdp_skb_ext_load_rx_vlan - Load the stored VLAN tag of this packet.
+ * @ctx: XDP context pointer.
+ * @vlan_proto: Return value pointer for the TPID, in network byte order.
+ * @vlan_tci: Return value pointer for the TCI, in host byte order.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-ENODATA`` : the VLAN tag is not stored
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_load_rx_vlan(const struct xdp_md *ctx,
+					     __be16 *vlan_proto, u16 *vlan_tci)
+{
+	const struct xdp_skb_ext_vlan *entry;
+
+	entry = xdp_skb_ext_load(ctx, XDP_SKB_EXT_VLAN);
+	if (!entry)
+		return -ENODATA;
+
+	*vlan_proto = entry->proto;
+	*vlan_tci = entry->tci;
+
+	return 0;
+}
+
+/**
+ * bpf_xdp_skb_ext_load_rx_csum - Load the stored RX checksum status of this
+ *                                packet.
+ * @ctx: XDP context pointer.
+ * @ip_summed: Return value pointer, ``XDP_CHECKSUM_NONE`` or
+ *             ``XDP_CHECKSUM_UNNECESSARY``.
+ * @csum_level: Return value pointer for the checksum level.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-ENODATA`` : the checksum status is not stored
+ */
+__bpf_kfunc int bpf_xdp_skb_ext_load_rx_csum(const struct xdp_md *ctx,
+					     enum xdp_checksum *ip_summed,
+					     u8 *csum_level)
+{
+	const struct xdp_skb_ext_csum *entry;
+
+	entry = xdp_skb_ext_load(ctx, XDP_SKB_EXT_CSUM);
+	if (!entry)
+		return -ENODATA;
+
+	*ip_summed = BIT(entry->ip_summed);
+	*csum_level = entry->level;
+
+	return 0;
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(xdp_skb_ext_kfunc_ids)
@@ -1303,6 +1391,9 @@ BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_hash)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_vlan)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_csum)
 BTF_ID_FLAGS(func, bpf_xdp_skb_ext_del)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_load_rx_hash)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_load_rx_vlan)
+BTF_ID_FLAGS(func, bpf_xdp_skb_ext_load_rx_csum)
 BTF_KFUNCS_END(xdp_skb_ext_kfunc_ids)
 
 static const struct btf_kfunc_id_set xdp_skb_ext_kfunc_set = {
-- 
2.56.0

[PATCH bpf-next 14/15] selftests/bpf: Test the load kfuncs on cpumap and veth

[Tianyi Gao]

Add a test for the load kfuncs on each path of the xdp_skb_ext_to_skb
test. On XDP_PASS, the program that stores the values reads them back.
On cpumap and veth, the program after the redirect reads them. Each
test case compares them with the stored values. A load that finds no
entry, also after bpf_xdp_skb_ext_del(), must return -ENODATA.

With BPF_F_TEST_XDP_LIVE_FRAMES, a recycled page still holds the entries
of the previous packet. The live_frames test checks that a load returns
-ENODATA there.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 .../selftests/bpf/prog_tests/xdp_skb_ext.c    | 47 +++++++++++------
 .../selftests/bpf/progs/test_xdp_skb_ext.c    | 50 ++++++++++++++++++-
 tools/testing/selftests/bpf/xdp_skb_ext.h     |  2 +-
 3 files changed, 81 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c b/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
index 64c6f0268d60..0811c09b7747 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c
@@ -7,7 +7,6 @@
 #include "test_xdp_skb_ext.skel.h"
 #include "xdp_skb_ext_success.skel.h"
 #include "xdp_skb_ext_failure.skel.h"
-#include "xdp_dummy.skel.h"
 
 #define XDP_RSS_TYPE_L3_IPV4 BIT(0)
 #define XDP_RSS_TYPE_L4 BIT(3)
@@ -167,7 +166,9 @@ static void set_case(struct test_xdp_skb_ext *skel,
 	skel->bss->want = c->want;
 	skel->bss->del_hash = c->del_hash;
 	memset(&skel->bss->have, 0, sizeof(skel->bss->have));
+	memset(&skel->bss->loaded, 0, sizeof(skel->bss->loaded));
 	skel->bss->store_err = 0;
+	skel->bss->load_err = 0;
 	skel->bss->pkts_seen_tc = 0;
 }
 
@@ -191,8 +192,6 @@ static void check_have(struct test_xdp_skb_ext *skel)
 	bool unnecessary = (want->fields & SKB_EXT_FIELD_CSUM) &&
 			   want->ip_summed == XDP_CHECKSUM_UNNECESSARY;
 
-	if (!wait_pkt(skel))
-		return;
 	ASSERT_EQ(skel->bss->store_err, 0, "store_err");
 	ASSERT_EQ(have->hash, want->rx_hash, "hash");
 	ASSERT_EQ(have->l4_hash, !!(want->rx_hash_type & XDP_RSS_TYPE_L4),
@@ -208,6 +207,24 @@ static void check_have(struct test_xdp_skb_ext *skel)
 		  "csum_level");
 }
 
+static void check_loaded(struct test_xdp_skb_ext *skel)
+{
+	const struct xdp_skb_ext_vals *want = &skel->bss->want;
+	const struct xdp_skb_ext_vals *loaded = &skel->bss->loaded;
+
+	ASSERT_EQ(skel->bss->load_err, 0, "load_err");
+	ASSERT_EQ(loaded->fields, want->fields, "loaded_fields");
+	ASSERT_EQ(loaded->rx_hash, want->rx_hash, "loaded_rx_hash");
+	ASSERT_EQ(loaded->rx_hash_type, want->rx_hash_type,
+		  "loaded_rx_hash_type");
+	ASSERT_EQ(loaded->rx_vlan_proto, want->rx_vlan_proto,
+		  "loaded_rx_vlan_proto");
+	ASSERT_EQ(loaded->rx_vlan_tci, want->rx_vlan_tci,
+		  "loaded_rx_vlan_tci");
+	ASSERT_EQ(loaded->ip_summed, want->ip_summed, "loaded_ip_summed");
+	ASSERT_EQ(loaded->csum_level, want->csum_level, "loaded_csum_level");
+}
+
 static void test_frames(struct test_xdp_skb_ext *skel, const char *path,
 			enum xdp_skb_ext_deliver deliver, int ifindex)
 {
@@ -232,9 +249,12 @@ static void test_frames(struct test_xdp_skb_ext *skel, const char *path,
 		if (!test__start_subtest(name))
 			continue;
 		set_case(skel, &skb_ext_cases[i]);
-		if (ASSERT_OK(bpf_prog_test_run_opts(prog_fd, &opts),
-			      "test_run"))
-			check_have(skel);
+		if (!ASSERT_OK(bpf_prog_test_run_opts(prog_fd, &opts),
+			       "test_run") ||
+		    !wait_pkt(skel))
+			continue;
+		check_have(skel);
+		check_loaded(skel);
 	}
 }
 
@@ -266,6 +286,7 @@ static void test_veth_skb(struct test_xdp_skb_ext *skel, struct bpf_link *link,
 		goto out;
 	if (deliver == DELIVER_CPUMAP) {
 		check_have(skel);
+		check_loaded(skel);
 		goto out;
 	}
 	ASSERT_EQ(skel->bss->store_err, 0, "store_err");
@@ -275,6 +296,7 @@ static void test_veth_skb(struct test_xdp_skb_ext *skel, struct bpf_link *link,
 	ASSERT_EQ(have->vlan_tci, 0, "vlan_tci");
 	ASSERT_EQ(have->ip_summed, CHECKSUM_NONE, "ip_summed");
 	ASSERT_EQ(have->csum_level, 0, "csum_level");
+	check_loaded(skel);
 out:
 	close(sock);
 }
@@ -292,7 +314,6 @@ void test_xdp_skb_ext_to_skb(void)
 {
 	struct bpf_cpumap_val val = { .qsize = 192 };
 	int ifindex_src, ifindex_dst, tc_prog_fd;
-	struct xdp_dummy *skel_dummy = NULL;
 	struct test_xdp_skb_ext *skel = NULL;
 	struct bpf_link *link = NULL;
 	struct netns_obj *ns = NULL;
@@ -330,18 +351,13 @@ void test_xdp_skb_ext_to_skb(void)
 	    tc_prog_attach("veth_dst", tc_prog_fd, -1))
 		goto out;
 
+	val.bpf_prog.fd = bpf_program__fd(skel->progs.cpumap_load);
 	if (!ASSERT_OK(bpf_map_update_elem(bpf_map__fd(skel->maps.cpu_map),
 					   &key, &val, 0), "add cpumap entry"))
 		goto out;
 
-	skel_dummy = xdp_dummy__open_and_load();
-	if (!ASSERT_OK_PTR(skel_dummy, "xdp_dummy__open_and_load"))
-		goto out;
-
-	/* veth_dst takes xdp_frames only with an XDP program */
-	link = bpf_program__attach_xdp(skel_dummy->progs.xdp_dummy_prog,
-				       ifindex_dst);
-	if (!ASSERT_OK_PTR(link, "attach xdp_dummy"))
+	link = bpf_program__attach_xdp(skel->progs.veth_load, ifindex_dst);
+	if (!ASSERT_OK_PTR(link, "attach veth_load"))
 		goto out;
 
 	test_frames(skel, "pass", DELIVER_PASS, ifindex_src);
@@ -354,7 +370,6 @@ void test_xdp_skb_ext_to_skb(void)
 
 out:
 	bpf_link__destroy(link);
-	xdp_dummy__destroy(skel_dummy);
 	test_xdp_skb_ext__destroy(skel);
 	netns_free(ns);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c b/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
index a63206ff993d..34db1a986f3b 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <vmlinux.h>
+#include <errno.h>
 #include "xdp_skb_ext.h"
 #include <bpf/bpf_endian.h>
 #include <bpf/bpf_helpers.h>
@@ -14,6 +15,8 @@ SEC("xdp")
 int live_frames(struct xdp_md *ctx)
 {
 	__u8 *mark = (void *)(long)ctx->data;
+	enum xdp_rss_hash_type rss_type;
+	__u32 hash;
 
 	if (mark + 1 > (__u8 *)(long)ctx->data_end)
 		return XDP_DROP;
@@ -25,6 +28,9 @@ int live_frames(struct xdp_md *ctx)
 			reused_ok++;
 			bpf_xdp_adjust_meta(ctx, MAX_META_LEN);
 		}
+		if (bpf_xdp_skb_ext_load_rx_hash(ctx, &hash, &rss_type) !=
+		    -ENODATA)
+			reused_bad++;
 	}
 	if (bpf_xdp_skb_ext_store_rx_hash(ctx, 1, XDP_RSS_TYPE_L4_IPV4_TCP) ||
 	    bpf_xdp_skb_ext_store_rx_vlan(ctx, bpf_htons(ETH_P_8021Q), 1) ||
@@ -51,7 +57,34 @@ enum xdp_skb_ext_deliver deliver;
 struct xdp_skb_ext_vals want;
 bool del_hash;
 struct xdp_skb_ext_have have;
-int pkts_seen_tc;
+struct xdp_skb_ext_vals loaded;
+int pkts_seen_tc, load_err;
+
+static void load_all(struct xdp_md *ctx)
+{
+	int err;
+
+	err = bpf_xdp_skb_ext_load_rx_hash(ctx, &loaded.rx_hash,
+					   &loaded.rx_hash_type);
+	if (!err)
+		loaded.fields |= SKB_EXT_FIELD_HASH;
+	else if (err != -ENODATA)
+		load_err++;
+
+	err = bpf_xdp_skb_ext_load_rx_vlan(ctx, &loaded.rx_vlan_proto,
+					   &loaded.rx_vlan_tci);
+	if (!err)
+		loaded.fields |= SKB_EXT_FIELD_VLAN;
+	else if (err != -ENODATA)
+		load_err++;
+
+	err = bpf_xdp_skb_ext_load_rx_csum(ctx, &loaded.ip_summed,
+					   &loaded.csum_level);
+	if (!err)
+		loaded.fields |= SKB_EXT_FIELD_CSUM;
+	else if (err != -ENODATA)
+		load_err++;
+}
 
 SEC("xdp")
 int xdp_store(struct xdp_md *ctx)
@@ -99,6 +132,21 @@ int xdp_store(struct xdp_md *ctx)
 		return bpf_redirect_map(&cpu_map, 0, 0);
 	if (deliver == DELIVER_VETH)
 		return bpf_redirect(ifindex_out, 0);
+	load_all(ctx);
+	return XDP_PASS;
+}
+
+SEC("xdp/cpumap")
+int cpumap_load(struct xdp_md *ctx)
+{
+	load_all(ctx);
+	return XDP_PASS;
+}
+
+SEC("xdp")
+int veth_load(struct xdp_md *ctx)
+{
+	load_all(ctx);
 	return XDP_PASS;
 }
 
diff --git a/tools/testing/selftests/bpf/xdp_skb_ext.h b/tools/testing/selftests/bpf/xdp_skb_ext.h
index 11feee2fa31c..64d4a0511d0a 100644
--- a/tools/testing/selftests/bpf/xdp_skb_ext.h
+++ b/tools/testing/selftests/bpf/xdp_skb_ext.h
@@ -30,7 +30,7 @@ enum xdp_skb_ext_deliver {
 	DELIVER_VETH,
 };
 
-/* The entries that the XDP program stores */
+/* The entries that the XDP programs store or load */
 struct xdp_skb_ext_vals {
 	__u32 rx_hash;
 	__u32 rx_hash_type;
-- 
2.56.0

[PATCH bpf-next 15/15] docs: net: document the skb core extension area

[Tianyi Gao]

Add a section on the skb core extension area to xdp-rx-metadata.rst.
It shows where the area is in the headroom and lists the store, delete
and load kfuncs. It also says when the kernel sets the stored values on
the skb. The Supported Devices section names the skb-ext-pass XDP
feature.

Update the XDP_PASS, bpf_redirect_map and bpf_tail_call sections,
which did not cover the area. Document xdp_skb_ext_to_skb(), the
skb-ext-pass XDP feature and xdp_build_skb_from_frame() in the driver
section, and point to the selftests in the example section.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 Documentation/networking/xdp-rx-metadata.rst | 127 +++++++++++++++++--
 1 file changed, 116 insertions(+), 11 deletions(-)

diff --git a/Documentation/networking/xdp-rx-metadata.rst b/Documentation/networking/xdp-rx-metadata.rst
index efdf5eeb49e7..61dcc24d7342 100644
--- a/Documentation/networking/xdp-rx-metadata.rst
+++ b/Documentation/networking/xdp-rx-metadata.rst
@@ -54,6 +54,83 @@ area in whichever format it chooses. Later consumers of the metadata
 will have to agree on the format by some out of band contract (like for
 the AF_XDP use case, see below).
 
+SKB Core Extension Area
+=======================
+
+An XDP program can also store RX metadata that the kernel sets on the
+``skb`` built from the packet. It stores and deletes the values with
+these kfuncs:
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_store_rx_hash
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_store_rx_vlan
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_store_rx_csum
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_del
+
+The values are kept in the skb core extension area. The area starts at
+``data_hard_start + sizeof(struct xdp_frame)``, right after the room for
+the ``xdp_frame``::
+
+  +-----------+-----------+----------------+---------------------+------+
+  | xdp_frame |  skb core |    headroom    |   custom metadata   | data |
+  |           | extension |                |                     |      |
+  +-----------+-----------+----------------+---------------------+------+
+  ^                                        ^                     ^
+  |                                        |                     |
+  xdp_buff->data_hard_start       xdp_buff->data_meta     xdp_buff->data
+
+The area is not an skb extension (``struct skb_ext``) and needs no
+extra allocation. It takes no space until the first store. It holds a
+4-byte header and the stored entries. The header and all three entries
+take 20 bytes. ``bpf_xdp_adjust_head``, ``bpf_xdp_adjust_meta`` and
+``bpf_xdp_pull_data`` do not move the packet or the metadata into the
+area.
+
+The kernel sets the stored values on the ``skb`` in these cases:
+
+- When cpumap or veth builds the ``skb`` from an ``xdp_frame`` after
+  ``XDP_REDIRECT``.
+- When ``BPF_PROG_RUN`` with ``BPF_F_TEST_XDP_LIVE_FRAMES`` builds the
+  ``skb`` after ``XDP_PASS``.
+- On ``XDP_PASS``, in a driver with the ``skb-ext-pass`` XDP feature,
+  see `Supported Devices`_.
+
+On ``XDP_PASS``, the stored values replace the values that the driver
+sets from the RX descriptor. On a native XDP driver without the feature,
+the kernel warns once when the program returns ``XDP_PASS`` with stored
+values. When XDP runs on an ``skb``, in generic XDP or when veth receives
+an ``skb`` from its peer, the kernel does not set the stored values on
+``XDP_PASS`` and also warns once.
+
+The program that stores the values can read them back. So can the
+programs that run on the ``xdp_frame`` after ``XDP_REDIRECT``, on cpumap,
+devmap and veth:
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_load_rx_hash
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_load_rx_vlan
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_skb_ext_load_rx_csum
+
+They return only the stored values. The RX metadata kfuncs above still
+read the values from the RX descriptor. Unlike those, the kfuncs in this
+section do not need a program that is bound to a device.
+
+On ``AF_XDP`` zero-copy, the store kfuncs return ``-EOPNOTSUPP``, because
+user space can write the headroom. The area would not reach the skb
+anyway. On ``XDP_REDIRECT``, ``xdp_convert_zc_to_xdp_frame`` copies only
+the custom metadata and the packet. On ``XDP_PASS``, drivers do not apply
+the area.
+
 AF_XDP
 ======
 
@@ -83,12 +160,13 @@ This is the path where the packets processed by the XDP program are passed
 into the kernel. The kernel creates the ``skb`` out of the ``xdp_buff``
 contents. Currently, every driver has custom kernel code to parse
 the descriptors and populate ``skb`` metadata when doing this ``xdp_buff->skb``
-conversion, and the XDP metadata is not used by the kernel when building
+conversion, and the custom metadata is not used by the kernel when building
 ``skbs``. However, TC-BPF programs can access the XDP metadata area using
 the ``data_meta`` pointer.
 
-In the future, we'd like to support a case where an XDP program
-can override some of the metadata used for building ``skbs``.
+On drivers with the ``skb-ext-pass`` XDP feature, an XDP program can use
+the `SKB Core Extension Area`_ to override the hash, VLAN tag and checksum
+status of the ``skb``.
 
 bpf_redirect_map
 ================
@@ -100,18 +178,19 @@ access to the original hardware descriptor and can't access any of
 the original metadata. The same applies to XDP programs installed
 into devmaps and cpumaps.
 
-This means that for redirected packets only custom metadata is
-currently supported, which has to be prepared by the initial XDP program
-before redirect. If the frame is eventually passed to the kernel, the
-``skb`` created from such a frame won't have any hardware metadata populated
-in its ``skb``. If such a packet is later redirected into an ``XSK``,
-that will also only have access to the custom metadata.
+This means that the initial XDP program has to prepare the metadata
+before the redirect, as custom metadata or in the `SKB Core Extension
+Area`_. If the frame is eventually passed to the kernel, the ``skb``
+created from such a frame gets its hash, VLAN tag and checksum status
+only from the area. If such a packet is later redirected into an
+``XSK``, that will only have access to the custom metadata.
 
 bpf_tail_call
 =============
 
 Adding programs that access metadata kfuncs to the ``BPF_MAP_TYPE_PROG_ARRAY``
-is currently not supported.
+is currently not supported. The kfuncs of the `SKB Core Extension Area`_
+do not have this limit.
 
 Supported Devices
 =================
@@ -120,6 +199,12 @@ It is possible to query which kfunc the particular netdev implements via
 netlink. See ``xdp-rx-metadata-features`` attribute set in
 ``Documentation/netlink/specs/netdev.yaml``.
 
+A netdev that sets the values from the `SKB Core Extension Area`_ on
+``XDP_PASS`` in native XDP has the ``skb-ext-pass`` flag in its
+``xdp-features`` attribute. The flag does not cover generic XDP or the
+veth ``skb`` path. An XDP program can also read ``xdp_features`` of the
+receiving netdev after ``bpf_cast_to_kern_ctx``.
+
 Driver Implementation
 =====================
 
@@ -153,9 +238,29 @@ xdp_frame. If these conditions are not met, it returns a negative error. In this
 case, the BPF program should not proceed to populate data into the ``data_meta``
 area.
 
+A driver that builds the ``skb`` on ``XDP_PASS`` applies the
+`SKB Core Extension Area`_ and sets ``NETDEV_XDP_ACT_SKB_EXT_PASS``. The
+driver sets the ``skb`` fields from the RX descriptor and then calls this
+function. The driver takes the area pointer with ``xdp_buff_skb_ext``
+before it reuses the ``xdp_buff``:
+
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: xdp_skb_ext_to_skb
+
+``xdp_build_skb_from_frame`` does not apply the area. Its callers set the
+RX fields themselves or transmit the ``skb``.
+
+A driver that moves ``data_hard_start`` after the XDP program ran calls
+``xdp_buff_clear_skb_ext_flag``, because the area is no longer at the
+expected place. The stored values are then lost.
+
 Example
 =======
 
 See ``tools/testing/selftests/bpf/progs/xdp_metadata.c`` and
 ``tools/testing/selftests/bpf/prog_tests/xdp_metadata.c`` for an example of
-BPF program that handles XDP metadata.
+BPF program that handles XDP metadata. See
+``tools/testing/selftests/bpf/progs/test_xdp_skb_ext.c`` and
+``tools/testing/selftests/bpf/prog_tests/xdp_skb_ext.c`` for the skb core
+extension area, and ``tools/testing/selftests/drivers/net/hw/xdp_metadata.py``
+for a driver on ``XDP_PASS``.
-- 
2.56.0

Re: [PATCH bpf-next 15/15] docs: net: document the skb core extension area

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3076 bytes --]

> docs: net: document the skb core extension area

TL;DR: The docs don't match the code: the "lost metadata" warning may fire
spuriously for BPF_PROG_RUN live frames on native drivers lacking
skb-ext-pass, and may hide a later real warning. Fix the code or the docs?

> diff --git a/Documentation/networking/xdp-rx-metadata.rst b/Documentation/networking/xdp-rx-metadata.rst
> --- a/Documentation/networking/xdp-rx-metadata.rst
> +++ b/Documentation/networking/xdp-rx-metadata.rst

[ ... ]

> +The kernel sets the stored values on the ``skb`` in these cases:
> +
> +- When cpumap or veth builds the ``skb`` from an ``xdp_frame`` after
> +  ``XDP_REDIRECT``.
> +- When ``BPF_PROG_RUN`` with ``BPF_F_TEST_XDP_LIVE_FRAMES`` builds the
> +  ``skb`` after ``XDP_PASS``.
> +- On ``XDP_PASS``, in a driver with the ``skb-ext-pass`` XDP feature,
> +  see `Supported Devices`_.
> +
> +On ``XDP_PASS``, the stored values replace the values that the driver
> +sets from the RX descriptor. On a native XDP driver without the feature,
> +the kernel warns once when the program returns ``XDP_PASS`` with stored
> +values.

This isn't a bug in the text itself, but it doesn't match the code.  Can the
warning fire for BPF_PROG_RUN with BPF_F_TEST_XDP_LIVE_FRAMES even though the
kernel does set the stored values there?

xdp_skb_ext_warn_pass() in net/core/xdp.c skips BPF_PROG_RUN only by testing
for the netdev_rx_queue xdp_rxq:

	/* Generic XDP and BPF_PROG_RUN use the netdev_rx_queue xdp_rxq */
	if (rxq->queue_index < dev->num_rx_queues &&
	    rxq == &dev->_rx[rxq->queue_index].xdp_rxq)
		return;
	/* No native XDP, so only generic XDP or BPF_PROG_RUN runs on dev */
	if (!(dev->xdp_features & NETDEV_XDP_ACT_BASIC))
		return;

In live frames mode the buffers use a private rxq instead.
xdp_test_run_setup() in net/bpf/test_run.c does:

	xdp_rxq_info_reg(&xdp->rxq, orig_ctx->rxq->dev, 0, 0);

and xdp_test_run_init_page() passes &xdp->rxq to xdp_init_buff().

So with ctx_in.ingress_ifindex set to a native XDP device that lacks
NETDEV_XDP_ACT_SKB_EXT_PASS (any native driver other than ice or veth), a
program that calls bpf_xdp_skb_ext_store_rx_hash() and returns XDP_PASS gets:

  bpf_prog_run_xdp()
    xdp_skb_ext_warn_pass()
      pr_warn_once("... the stored RX metadata is lost")

while xdp_test_run_batch()->xdp_recv_frames()->__xdp_build_skb_from_frame()
->xdp_skb_ext_to_skb() still applies the values to the skb.

Since this is pr_warn_once(), the false report also uses up the one warning,
so a later real loss on a native driver without the feature is not reported.

Should the warning skip the live frames rxq, or should the documentation say
that BPF_PROG_RUN live frames can warn?  The code comes from f5e7dade0555
("xdp: add the skb-ext-pass XDP feature and warn without it"), and no later
commit in the series changes it.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42755842687

Re: [PATCH bpf-next 10/15] xdp: warn on XDP_PASS with stored values when XDP runs on an skb

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

> xdp: warn on XDP_PASS with stored values when XDP runs on an skb

TL;DR: Comment nit in xdp_skb_ext_warn_pass(): the new
generic-XDP/BPF_PROG_RUN comment is inaccurate for BPF_F_TEST_XDP_LIVE_FRAMES,
which uses a separate rxq, and the next comment is now stale. Not a bug.

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c

[ ... ]

> @@ -1124,12 +1125,17 @@ void xdp_skb_ext_to_skb(struct sk_buff *skb, const struct xdp_skb_ext *ext)
>  void xdp_skb_ext_warn_pass(const struct bpf_prog *prog,
>  			   const struct xdp_buff *xdp)
>  {
> -	const struct net_device *dev = xdp->rxq->dev;
> +	const struct xdp_rxq_info *rxq = xdp->rxq;
> +	const struct net_device *dev = rxq->dev;
>  
>  	/* The skb is built later from the xdp_frame */
>  	if (prog->expected_attach_type == BPF_XDP_CPUMAP ||
>  	    prog->expected_attach_type == BPF_XDP_DEVMAP)
>  		return;
> +	/* Generic XDP and BPF_PROG_RUN use the netdev_rx_queue xdp_rxq */
> +	if (rxq->queue_index < dev->num_rx_queues &&
> +	    rxq == &dev->_rx[rxq->queue_index].xdp_rxq)
> +		return;
>  	/* No native XDP, so only generic XDP or BPF_PROG_RUN runs on dev */
>  	if (!(dev->xdp_features & NETDEV_XDP_ACT_BASIC))
>  		return;

This isn't a bug, but is the new comment accurate for BPF_PROG_RUN?

With BPF_F_TEST_XDP_LIVE_FRAMES, xdp_test_run_setup() registers a separate
rxq embedded in struct xdp_test_data:

    xdp_rxq_info_reg(&xdp->rxq, orig_ctx->rxq->dev, 0, 0)

and xdp_test_run_init_page() then does:

    xdp_init_buff(new_ctx, TEST_XDP_FRAME_SIZE, &xdp->rxq);

so in live-frames mode rxq != &dev->_rx[rxq->queue_index].xdp_rxq and the
early return is not taken.

That also makes the following comment stale: generic XDP and non-live
BPF_PROG_RUN now return at the new check, so only the live-frames
BPF_PROG_RUN can reach the NETDEV_XDP_ACT_BASIC test.

Would something like "Generic XDP and BPF_PROG_RUN without
BPF_F_TEST_XDP_LIVE_FRAMES use the netdev_rx_queue xdp_rxq" match the code
better?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42755842687

Re: [PATCH bpf-next 10/15] xdp: warn on XDP_PASS with stored values when XDP runs on an skb

[Tianyi Gao]

Hi,

On Tue, Nov 10, 2026 at 04:10:19AM +0000, sashiko-bot@kernel.org wrote:
> [Severity: Critical]
> This problem wasn't introduced by this patch, but can veth_disable_xdp()
> unregister rq->xdp_rxq [1] while veth_xdp_rcv_skb() or
> veth_xdp_rcv_one() is still using it?

[ ... ]

The list [2] and I [3] are aware of this problem. If it is still not
fixed in a week, I will send a fix to net.

[2] https://lore.kernel.org/all/20260917185814.0f944e78@kernel.org/
[3] https://lore.kernel.org/all/20261105143356.17062-1-tianyi@cloudflare.com/

Cheers,
Tianyi

Re: [PATCH bpf-next 00/15] xdp: add SKB core extensions to keep RX metadata until skb creation

[Jesper Dangaard Brouer]

On 11/10/26 04:56, Tianyi Gao wrote:
> An skb gets its RX hash, VLAN tag and checksum status from the RX
> descriptor, which describes the packet before the XDP program ran.
> 
> Without this series:
> 
> - On XDP_PASS, the values are stale. After the program strips a UDP
>    tunnel header, the stack applies the outer CHECKSUM_UNNECESSARY to the
>    inner packet [1]. If the outer UDP checksum is 0, a packet with a bad
>    inner checksum reaches the socket.

Our XDP Unimog load-balancer uses GUE like packet encapsulation, so it
hits above case all the time. If packet is end-host, it modify packet by
popping header and returns XDP_PASS for local netstack delivery.

> - On XDP_REDIRECT, the values are lost. The skb that cpumap or veth
>    builds from the xdp_frame has no RX hash and no VLAN tag, and its
>    checksum status is CHECKSUM_NONE.
> 
> In one use case, an XDP program redirects packets into a veth device.
> The checksum status is lost, so GRO verifies every checksum in software.
> With one TCP flow, this costs 34% of the receiving CPU.
> 
> Another use case [2] is load balancing IPsec tunnel traffic with cpumap.
> Without the RX hash, GRO on cpumap puts all flows into the same bucket.
> In a cpumap test with 16 TCP flows and no IPsec, the throughput is
> 17.6 Gbit/s instead of 23.4.
> 

Yes, GRO missing RX-hash for bucket split is real. Above IPsec XDP load-
balacing have been running in production for a year by now.
Our more general RX-pipeline CPUMAP redirect is currently being tested
in production.  This means all of our traffic will soon be hitting this
GRO limitation.

> This series introduces SKB core extensions for XDP. It reserves an skb
> core extension area in the headroom, right after the space for struct
> xdp_frame. An XDP program stores RX metadata in the area with kfuncs,
> and the kernel sets the skb fields from it. This design was initially
> proposed in [3].
> 

Yes, I agree. We don't want to add static members to xdp_frame, as it
will always reserve the space.  Instead this only takes up the space
when needed.  The most obvious case is the VLAN.


> The layout of the area follows skb extensions (struct skb_ext). A small
> header gives the offset of each entry. An entry takes headroom only when
> the program stores it, so the area does not grow with each new type of
> RX metadata, unlike [4]. Unlike skb_ext, the area needs no allocation.
> 

Hopefully other maintainers like this encoding format better than our
previous attempt.  As this is already used by existing skb extensions.
This is not expose as UAPI and BPF access this via helpers.


> This series supports three types of RX metadata. Each type has a store
> kfunc, and bpf_xdp_skb_ext_del() deletes an entry:
> 
> - The RX hash and its type: bpf_xdp_skb_ext_store_rx_hash().
> - The VLAN tag and its protocol: bpf_xdp_skb_ext_store_rx_vlan().
> - The checksum status: bpf_xdp_skb_ext_store_rx_csum(). Only
>    XDP_CHECKSUM_NONE, or XDP_CHECKSUM_UNNECESSARY with a csum_level up to
>    SKB_MAX_CSUM_LEVEL, are allowed [5][6].
> 
> The kernel sets the skb fields from the area:
> 
> - After XDP_REDIRECT, in __xdp_build_skb_from_frame().
> - On XDP_PASS, in the NIC driver. A driver that does this sets the new
>    XDP feature skb-ext-pass. On other native XDP drivers, the kernel
>    warns once when the program returns XDP_PASS with stored values, and
>    ignores the area. This series adds the support to ice.
> 
> The existing XDP RX metadata kfuncs are bpf_xdp_metadata_rx_*(). Only
> a device-bound program can call them. Programs on cpumap and devmap are
> not device-bound, so they cannot call them. Relaxing this check would
> change their semantics. So this series adds three load kfuncs,
> bpf_xdp_skb_ext_load_rx_*(). They read the stored values.
> 
> bpf_xdp_adjust_head(), bpf_xdp_adjust_meta() and bpf_xdp_pull_data() do
> not move the packet or the metadata into the area. In this series, the
> area takes at most 20 bytes of headroom. With the hardware RX timestamp,
> the RX queue index and CHECKSUM_COMPLETE, it would take up to 40 bytes.
> 

I like how the size usage is limited in this patchset. As I understood, 
this was previously a concern from Jakub/kuba in my previous patchset.
In my previous patchset we allowed more flexibility and others to use 
this area. This has been dropped. Notice how this patchset focus only on 
the fields needed for the SKB.  This is basically working towards that
xdp_frame becomes the "mini-SKB" concept.


> Lorenzo's RX checksum kfunc series v5 [7] also defines enum
> xdp_checksum. The series that is merged second drops its copy.
> 
> Performance:
> 
> We measured on ice (E810, 25 Gbit/s) with bpf-next 0fb18ce8a2e9, patches
> 1-3 of [7] and this series. In the RX metadata column, the XDP program
> reads a value with bpf_xdp_metadata_rx_*() and stores it. The veth test
> stores the checksum status, and the cpumap test stores the RX hash. The
> kernel then sets the value on the skb. Baseline does not store it. Each
> value is the median of 5 rounds:
> 
> test                  metric          baseline     RX metadata
> veth, 1 TCP flow      RX CPU busy     100%         66% (-34%)
> veth, 1 TCP flow      RX cycles/pkt   2027         1343 (-34%)
> cpumap, 16 TCP flows  throughput      17.6 Gbit/s  23.4 (+33%)
> cpumap, 16 TCP flows  pkts/GRO skb    3.4          7.5 (+122%)
> 
> Both veth columns reach line rate, so the gain is in CPU time. A store
> kfunc takes 2.9 to 4.9 ns. Details are in [8].
> 

Nice performance gains :-)

--Jesper

[... cut ...]
 > [1]https://lore.kernel.org/all/20260227153231.78d16b69@kernel.org/
 > 
[2]https://lore.kernel.org/all/ca38f2ed-999f-4ce1-8035-8ee9247f27f2@kernel.org/
 > 
[3]https://lore.kernel.org/all/1d5521eb-84fc-4417-a3f5-6366694d160a@kernel.org/
 > [4]https://lore.kernel.org/
 > all/175146824674.1421237.18351246421763677468.stgit@firesoul/
 > [5]https://lore.kernel.org/all/20260722113525.34092660@kernel.org/
 > [6]https://lore.kernel.org/all/20260721183256.6f49d6e1@kernel.org/
 > [7]https://lore.kernel.org/all/20260715-bpf-xdp-meta-rxcksum-
 > v5-0-623d5c0d0ab7@kernel.org/
 > [8]https://github.com/tgao-cf/skb-core-extensions-bench

Re: [PATCH bpf-next 01/15] xdp: reserve an skb core extension area after the xdp_frame

[Jesper Dangaard Brouer]

On 11/10/26 04:56, Tianyi Gao wrote:
> Reserve the skb core extension area in the headroom, right after the
> xdp_frame. An XDP program stores RX metadata there for the skb that is
> built later.
> 
> The area begins with a header, struct xdp_skb_ext. It records the offset
> of each entry and the end of the last entry, in 4-byte units.
> bpf_xdp_adjust_head(), bpf_xdp_adjust_meta() and bpf_xdp_pull_data() use
> xdp_skb_ext_end() as their lower bound, so packet data and metadata
> cannot overwrite the entries.
> 
> The XDP_FLAGS_HAS_SKB_EXT flag indicates that the header is valid. Most
> drivers reuse the xdp_buff between packets, so xdp_prepare_buff() clears
> it. With BPF_F_TEST_XDP_LIVE_FRAMES, reset_ctx() restores the flags
> before each run, and ctx_was_changed() compares them.
> 
> Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
> Link: https://lore.kernel.org/all/1d5521eb-84fc-4417-a3f5-6366694d160a@kernel.org/
> Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
> ---
>   include/net/xdp.h  | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   net/bpf/test_run.c |  4 +++-
>   net/core/filter.c  |  8 +++-----
>   3 files changed, 51 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 07231adfb5f8..c888353425d4 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -81,6 +81,9 @@ enum xdp_buff_flags {
>   	 * XDP program is not attached.
>   	 */
>   	XDP_FLAGS_FRAGS_UNREADABLE	= BIT(2),
> +	XDP_FLAGS_HAS_SKB_EXT		= BIT(3), /* skb core extension header
> +						   * is valid
> +						   */

This can be on 1-line (80 chars) of you change the comment to be:
  /* skb core ext hdr valid */

>   };
The rest looks good to me.

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

Re: [PATCH bpf-next 03/15] xdp: add kfuncs to store RX hash and VLAN tag

[Jesper Dangaard Brouer]

On 11/10/26 04:57, Tianyi Gao wrote:
> Add bpf_xdp_skb_ext_store_rx_hash() and bpf_xdp_skb_ext_store_rx_vlan().
> They store the RX hash and the VLAN tag in the skb core extension area.
> A later patch sets the stored values on the skb built from the
> xdp_frame.
> 
> Both kfuncs call xdp_skb_ext_add(), which reserves the entry for an id.
> On the first store for a packet, it clears the header and sets
> XDP_FLAGS_HAS_SKB_EXT, so stale headroom is never read as a header. A
> second store of the same id, also after a delete, reuses its offset.
> 
> xdp_skb_ext_add() returns -EOPNOTSUPP for AF_XDP zero-copy buffers. User
> space can write their headroom while the program runs, and could change
> the header between two stores.
> 
> The store kfuncs are in their own kfunc set. They do not need a
> device-bound program, so programs on cpumap and veth can call them.
> 
> bpf_xdp_skb_ext_store_rx_vlan() accepts only ETH_P_8021Q and
> ETH_P_8021AD, because vlan_proto_idx() warns on any other TPID.
> 
> A BUILD_BUG_ON() in xdp_skb_ext_struct_check() limits the area to 64
> bytes. As in skb_extensions_init(), the function is noinline
> __no_profile so that it builds with CONFIG_GCOV_PROFILE_ALL.
> 
> The two kfuncs are based on bpf_xdp_store_rx_hash() and
> bpf_xdp_store_rx_vlan() by Lorenzo Bianconi in Jesper's XDP_REDIRECT RX
> hints series V2 [1].
> 
> Link: https://lore.kernel.org/all/175146831297.1421237.17665319427079757435.stgit@firesoul/ [1]
> Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
> ---
>   net/core/xdp.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 151 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 1d679e8fd649..97dc0e403a78 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -990,11 +990,159 @@ bool bpf_dev_bound_kfunc_id(u32 btf_id)
>   	return btf_id_set8_contains(&xdp_metadata_kfunc_ids, btf_id);
>   }
>   
> -static int __init xdp_metadata_init(void)
> +#define XDP_SKB_EXT_CHUNKSIZEOF(x) DIV_ROUND_UP(sizeof(x), XDP_SKB_EXT_ALIGN)
> +#define XDP_SKB_EXT_ENTRY_ACTIVE BIT(7)
> +
> +struct xdp_skb_ext_hash {
> +	u32 val;
> +	u32 type; /* enum xdp_rss_hash_type */
> +};
> +
> +struct xdp_skb_ext_vlan {
> +	__be16 proto;
> +	u16 tci;
> +};
> +
> +static const u8 xdp_skb_ext_type_len[] = {
> +	[XDP_SKB_EXT_HASH] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_hash),
> +	[XDP_SKB_EXT_VLAN] = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext_vlan),
> +};
> +
> +/* Limit the header and all entries to 64 bytes, so that a new entry
> + * cannot grow the area without notice.
> + */
> +static noinline void __init __no_profile xdp_skb_ext_struct_check(void)
> +{
> +	unsigned int chunks = XDP_SKB_EXT_CHUNKSIZEOF(struct xdp_skb_ext);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(xdp_skb_ext_type_len); i++)
> +		chunks += xdp_skb_ext_type_len[i];
> +
> +	BUILD_BUG_ON(chunks * XDP_SKB_EXT_ALIGN > 64);
> +}
> +
> +static void *xdp_skb_ext_add(struct xdp_buff *xdp, enum xdp_skb_ext_id id)
> +{
> +	struct xdp_skb_ext *ext = xdp_skb_ext(xdp->data_hard_start);
> +	bool valid = xdp_buff_has_skb_ext(xdp);
> +	unsigned int newlen, newoff;
> +	void *start;
> +
> +	/* User space can write the headroom of AF_XDP zero-copy buffers */
> +	if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	if (!valid) {
> +		newoff = XDP_SKB_EXT_CHUNKSIZEOF(*ext);
> +		newlen = newoff + xdp_skb_ext_type_len[id];
> +	} else if (ext->offset[id]) {
> +		newoff = ext->offset[id] & ~XDP_SKB_EXT_ENTRY_ACTIVE;


Given the recent run of security bugs caused by manipulated offsets, I
would like to limit how far a bad offset can reach here. The offset is
read back from the packet headroom and is trusted only because
XDP_FLAGS_HAS_SKB_EXT is set; nothing checks it against ext->chunks or
the buffer. With the 7-bit mask it can point up to 508 bytes past the
ext header, beyond the default headroom and into packet data, and in
this branch the caller then writes the entry (up to 8 bytes) there. I
have not found a way to get a bad header with this series, but the
invariant depends on every driver leaving this area alone while the
flag is set.

The BUILD_BUG_ON() in xdp_skb_ext_struct_check() limits the header plus
all entries to 64 bytes, which is 16 chunks, so a 4-bit mask covers
every valid offset. A bad offset then starts at most 60 bytes after the
ext header. It is the same AND with another constant, so it costs
nothing in the fast path. The same mask should be used in
xdp_skb_ext_get() in patch 7.

--Jesper

> +		newlen = ext->chunks;
> +	} else {
> +		newoff = ext->chunks;
> +		newlen = newoff + xdp_skb_ext_type_len[id];
> +	}
> +
> +	start = xdp_data_meta_unsupported(xdp) ? xdp->data : xdp->data_meta;
> +	if ((void *)ext + newlen * XDP_SKB_EXT_ALIGN > start)
> +		return ERR_PTR(-ENOSPC);
> +
> +	if (!valid) {
> +		memset(ext->offset, 0, sizeof(ext->offset));
> +		xdp->flags |= XDP_FLAGS_HAS_SKB_EXT;
> +	}
> +	ext->offset[id] = newoff | XDP_SKB_EXT_ENTRY_ACTIVE;
> +	ext->chunks = newlen;
> +
> +	return (void *)ext + newoff * XDP_SKB_EXT_ALIGN;
> +}
> +
> +__bpf_kfunc_start_defs();
> +
> +/**
> + * bpf_xdp_skb_ext_store_rx_hash - Store the RX hash of this packet.
> + * @ctx: XDP context pointer.
> + * @hash: RX hash value.
> + * @rss_type: RSS hash type, see &enum xdp_rss_hash_type.
> + *
> + * Return:
> + * * Returns 0 on success or ``-errno`` on error.
> + * * ``-ENOSPC`` : not enough headroom for the entry
> + * * ``-EOPNOTSUPP`` : AF_XDP zero-copy buffer
> + */
> +__bpf_kfunc int bpf_xdp_skb_ext_store_rx_hash(struct xdp_md *ctx, u32 hash,
> +					      enum xdp_rss_hash_type rss_type)
>   {
> -	return register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &xdp_metadata_kfunc_set);
> +	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
> +	struct xdp_skb_ext_hash *entry;
> +
> +	entry = xdp_skb_ext_add(xdp, XDP_SKB_EXT_HASH);
> +	if (IS_ERR(entry))
> +		return PTR_ERR(entry);
> +
> +	entry->val = hash;
> +	entry->type = rss_type;
> +
> +	return 0;
> +}
> +
> +/**
> + * bpf_xdp_skb_ext_store_rx_vlan - Store the VLAN tag of this packet.
> + * @ctx: XDP context pointer.
> + * @vlan_proto: VLAN tag protocol identifier (TPID), in network byte order.
> + * @vlan_tci: VLAN TCI, in host byte order.
> + *
> + * Return:
> + * * Returns 0 on success or ``-errno`` on error.
> + * * ``-EINVAL`` : @vlan_proto is not 802.1Q or 802.1AD
> + * * ``-ENOSPC`` : not enough headroom for the entry
> + * * ``-EOPNOTSUPP`` : AF_XDP zero-copy buffer
> + */
> +__bpf_kfunc int bpf_xdp_skb_ext_store_rx_vlan(struct xdp_md *ctx,
> +					      __be16 vlan_proto, u16 vlan_tci)
> +{
> +	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
> +	struct xdp_skb_ext_vlan *entry;
> +
> +	if (vlan_proto != htons(ETH_P_8021Q) &&
> +	    vlan_proto != htons(ETH_P_8021AD))
> +		return -EINVAL;
> +
> +	entry = xdp_skb_ext_add(xdp, XDP_SKB_EXT_VLAN);
> +	if (IS_ERR(entry))
> +		return PTR_ERR(entry);
> +
> +	entry->proto = vlan_proto;
> +	entry->tci = vlan_tci;
> +
> +	return 0;
> +}
> +
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(xdp_skb_ext_kfunc_ids)
> +BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_hash)
> +BTF_ID_FLAGS(func, bpf_xdp_skb_ext_store_rx_vlan)
> +BTF_KFUNCS_END(xdp_skb_ext_kfunc_ids)
> +
> +static const struct btf_kfunc_id_set xdp_skb_ext_kfunc_set = {
> +	.owner = THIS_MODULE,
> +	.set   = &xdp_skb_ext_kfunc_ids,
> +};
> +
> +static int __init xdp_kfunc_init(void)
> +{
> +	int ret;
> +
> +	xdp_skb_ext_struct_check();
> +
> +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
> +					&xdp_skb_ext_kfunc_set);
> +	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
> +						&xdp_metadata_kfunc_set);
>   }
> -late_initcall(xdp_metadata_init);
> +late_initcall(xdp_kfunc_init);
>   
>   void xdp_set_features_flag_locked(struct net_device *dev, xdp_features_t val)
>   {

Re: [PATCH bpf-next 04/15] xdp: add a kfunc to delete skb core extension entries

[Jesper Dangaard Brouer]

On 11/10/26 04:57, Tianyi Gao wrote:
> bpf_xdp_skb_ext_del() deletes the stored entry for an id. A program on
> cpumap or veth can use it to drop RX metadata that an earlier program
> stored.
> 
> Without the XDP_FLAGS_HAS_SKB_EXT flag, the kfunc does nothing, because
> the header bytes may then belong to the packet.
> 
> The kfunc only clears XDP_SKB_EXT_ENTRY_ACTIVE, so a later store of the
> same id reuses its offset. The flag and the size of the area stay the
> same, also after the last delete. To give the bytes back, the kfunc
> would have to move the entries after the deleted one. The cost is
> headroom that bpf_xdp_adjust_head() and bpf_xdp_adjust_meta() cannot
> use.
> 
> Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
> ---
> __skb_ext_del() frees struct skb_ext after the last delete, because it
> comes from a slab cache. The skb core extension area uses existing
> headroom, so it is not freed.
> ---
>   include/net/xdp.h |  8 ++++++++
>   net/core/xdp.c    | 26 ++++++++++++++++++++++++++
>   2 files changed, 34 insertions(+)

I think this approach of simply disabling the entry is okay.
This is trading off performance vs small area memory usage.

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

--Jesper

Re: [PATCH bpf-next 00/15] xdp: add SKB core extensions to keep RX metadata until skb creation

[Stanislav Fomichev]

On 11/10, Tianyi Gao wrote:
> An skb gets its RX hash, VLAN tag and checksum status from the RX
> descriptor, which describes the packet before the XDP program ran.
> 
> Without this series:
> 
> - On XDP_PASS, the values are stale. After the program strips a UDP
>   tunnel header, the stack applies the outer CHECKSUM_UNNECESSARY to the
>   inner packet [1]. If the outer UDP checksum is 0, a packet with a bad
>   inner checksum reaches the socket.
> - On XDP_REDIRECT, the values are lost. The skb that cpumap or veth
>   builds from the xdp_frame has no RX hash and no VLAN tag, and its
>   checksum status is CHECKSUM_NONE.
> 
> In one use case, an XDP program redirects packets into a veth device.
> The checksum status is lost, so GRO verifies every checksum in software.
> With one TCP flow, this costs 34% of the receiving CPU.
> 
> Another use case [2] is load balancing IPsec tunnel traffic with cpumap.
> Without the RX hash, GRO on cpumap puts all flows into the same bucket.
> In a cpumap test with 16 TCP flows and no IPsec, the throughput is
> 17.6 Gbit/s instead of 23.4.
> 
> This series introduces SKB core extensions for XDP. It reserves an skb
> core extension area in the headroom, right after the space for struct
> xdp_frame. An XDP program stores RX metadata in the area with kfuncs,
> and the kernel sets the skb fields from it. This design was initially
> proposed in [3].

Awhile ago we discussed the alternative of adding setters for xdp metadata.
Then the veth (or whoever turns xdp frame into an skb) can apply whatever
metadata it needs (and there is some magic behind the scenes that takes
it into account when creating an skb). That keeps the layout and most
of the code outside of kernel. Have we considered that here?

Re: [PATCH bpf-next 00/15] xdp: add SKB core extensions to keep RX metadata until skb creation

[Tianyi Gao]

Hi Stanislav,

On 11/10, Stanislav Fomichev wrote:
> Awhile ago we discussed the alternative of adding setters for xdp
> metadata. Then the veth (or whoever turns xdp frame into an skb) can
> apply whatever metadata it needs (and there is some magic behind the
> scenes that takes it into account when creating an skb). That keeps
> the layout and most of the code outside of kernel. Have we considered
> that here?

Thanks for the reminder. Yes, we considered [1] and [2], and took
parts of them.

The alternative setter model in [1] requires a program on veth or
cpumap to call the setters for the RX metadata. We want to avoid that:

- On veth, an XDP program removes software GSO from the peer, limits
  its MTU, and can copy skbs from the peer.
- On cpumap, each entry would need a program only for this.

This series achieves the same with only the XDP program on the NIC,
which stores the values with kfuncs in the skb core extension area
after the xdp_frame. cpumap and veth take this area into account when
they create the skb, which is the "magic behind the scenes".

[1] https://lore.kernel.org/all/20250616145523.63bd2577@kernel.org/
[2] https://lore.kernel.org/all/aGvcb53APFXR8eJb@mini-arch/

Cheers,
Tianyi

Re: [PATCH bpf-next 00/15] xdp: add SKB core extensions to keep RX metadata until skb creation

[Stanislav Fomichev]

On 11/11, Tianyi Gao wrote:
> Hi Stanislav,
> 
> On 11/10, Stanislav Fomichev wrote:
> > Awhile ago we discussed the alternative of adding setters for xdp
> > metadata. Then the veth (or whoever turns xdp frame into an skb) can
> > apply whatever metadata it needs (and there is some magic behind the
> > scenes that takes it into account when creating an skb). That keeps
> > the layout and most of the code outside of kernel. Have we considered
> > that here?
> 
> Thanks for the reminder. Yes, we considered [1] and [2], and took
> parts of them.
> 
> The alternative setter model in [1] requires a program on veth or
> cpumap to call the setters for the RX metadata. We want to avoid that:
> 
> - On veth, an XDP program removes software GSO from the peer, limits
>   its MTU, and can copy skbs from the peer.
> - On cpumap, each entry would need a program only for this.
> 
> This series achieves the same with only the XDP program on the NIC,
> which stores the values with kfuncs in the skb core extension area
> after the xdp_frame. cpumap and veth take this area into account when
> they create the skb, which is the "magic behind the scenes".
> 
> [1] https://lore.kernel.org/all/20250616145523.63bd2577@kernel.org/
> [2] https://lore.kernel.org/all/aGvcb53APFXR8eJb@mini-arch/

My main concern is that we are adding a lot of code and one more special
way to plumb the metadata (and another metadata area), another set
of kfuncs and slowly creeping more features into xdp making it more
and more like skb.

For containers in general, and for fast af_/xdp, we seem to have been
moving into netkit with queue leasing. Maybe that's something that
can help with your veth performance?