[RFC bpf-next v3 0/3] bpf: Add PPPoE encap/decap kfuncs

[RFC bpf-next v3 0/3] bpf: Add PPPoE encap/decap kfuncs

[ThisSeanZhang]

Hello,

The motivating use case is a TC BPF PPPoE forwarding path. When such a
path forwards a GSO packet larger than the negotiated PPPoE MTU,
inserting the PPPoE header manually leaves skb->protocol set to the
original IP protocol, so the stack cannot select the PPPoE GSO path
and the packet may be dropped.

The kernel already supports PPPoE GRO/GSO for ETH_P_PPP_SES
(introduced by commit 55a5d8fca836 ("net: pppoe: implement GRO/GSO
support")), but a TC BPF program has no direct way to update
skb->protocol and the associated skb metadata when adding or removing
a PPPoE header, and bpf_skb_adjust_room() cannot decapsulate PPPoE
packets either, as it only accepts ETH_P_IP and ETH_P_IPV6. The
current workaround uses BPF_F_ADJ_ROOM_ENCAP_L3_IPV4/IPV6, which
marks the skb as IP-in-IP even though the wire format is PPPoE. The
BPF program must also adjust gso_size manually [1].

This series adds bpf_skb_pppoe_encap() and bpf_skb_pppoe_decap(),
which insert or remove the PPPoE session header and keep the skb
metadata consistent with the kernel's PPPoE GRO/GSO handlers; the
encap kfunc writes the PPPoE session header itself, mirroring
pppoe_xmit(), and the BPF program sets the MAC header's EtherType, as
with the existing BPF_F_ADJ_ROOM_ENCAP_* flags:

    bpf_skb_pppoe_encap(skb, sid, flags);
    bpf_skb_pppoe_decap(skb, flags);

Based on the v1 discussion about whether new UAPI flags were
warranted, this version uses kfuncs instead, following the existing
bpf_skb_set_fou_encap() interface. No UAPI changes are needed.

Since the kfuncs may reallocate the skb head, patch 1 adds the
KF_CHANGES_PKT flag so the verifier invalidates packet pointers after
calls to packet-modifying kfuncs, analogous to its handling of
packet-modifying helpers. bpf_xdp_pull_data() and dynptr writes retain
their existing handling.

The selftests cover IPv4 and IPv6 round trips, the PPPoE header fields
written by the encap kfunc, rejection of non-IP encapsulation input,
non-PPPoE decapsulation input, unsupported PPP protocols and truncated
input, and verifier rejection of packet pointers held across a kfunc
call. All 9 subtests pass in QEMU/KVM.

[1] https://github.com/ThisSeanZhang/landscape/blob/80bc43eef7143ad029283fe2a8d510718c22af12/landscape-ebpf/src/bpf/tc_chain/tc_pppoe.bpf.c#L38-L47

ThisSeanZhang (3):
  bpf: Add KF_CHANGES_PKT kfunc flag
  bpf: Add PPPoE encap/decap kfuncs
  selftests/bpf: Add a test for the PPPoE encap/decap kfuncs

 include/linux/btf.h                           |   1 +
 kernel/bpf/verifier.c                         |   1 +
 net/core/filter.c                             | 192 +++++++++++++
 .../selftests/bpf/prog_tests/tc_pppoe.c       | 269 ++++++++++++++++++
 tools/testing/selftests/bpf/progs/tc_pppoe.c  |  77 +++++
 .../selftests/bpf/progs/tc_pppoe_fail.c       |  36 +++
 6 files changed, 576 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_pppoe.c
 create mode 100644 tools/testing/selftests/bpf/progs/tc_pppoe.c
 create mode 100644 tools/testing/selftests/bpf/progs/tc_pppoe_fail.c

---

Changes since v2:
- Replace the proposed BPF_F_ADJ_ROOM_ENCAP_PPPOE and
  BPF_F_ADJ_ROOM_DECAP_PPPOE UAPI flags with
  bpf_skb_pppoe_encap() and bpf_skb_pppoe_decap() kfuncs.
- Add KF_CHANGES_PKT so the verifier invalidates packet pointers
  across calls to packet-modifying kfuncs. bpf_xdp_pull_data() and
  dynptr writes retain their existing handling.
- Make the encap kfunc write the PPPoE session header itself,
  mirroring pppoe_xmit().
- Rebase onto the current bpf-next.

v2: https://lore.kernel.org/bpf/20261003193347.1137527-1-thisseanzhang@gmail.com/
v1: https://lore.kernel.org/bpf/20260926210757.2152159-1-thisseanzhang@gmail.com/

base-commit: 0fb18ce8a2e9de830433923cf65838933d615359
-- 
2.47.3

[RFC bpf-next v3 1/3] bpf: Add KF_CHANGES_PKT kfunc flag

[ThisSeanZhang]

The verifier invalidates packet pointers after a call to a helper that
may change packet data, see bpf_helper_changes_pkt_data(). Kfuncs have
no equivalent: bpf_is_kfunc_pkt_changing() special-cases
bpf_xdp_pull_data() by ID, and the kfuncs that write through a dynptr
are listed in dynptr_write_kfuncs, because the dynptr type is not known
when the arguments are checked.

Add KF_CHANGES_PKT so that a kfunc can declare this where it is
defined, instead of adding another entry to the kfunc lists in the
verifier. The verifier then invalidates packet pointers after a call to
such a kfunc, like it does for the equivalent helpers; a stale packet
pointer kept in a callee-saved register across the call is rejected at
verification time.

The same predicate is used by the CFG pass to propagate the
changes_pkt_data property to the callers of a subprogram, so a kfunc
called through a subprogram is covered as well.

This is needed by the PPPoE encap/decap kfuncs added later in this
series, which adjust the skb like bpf_skb_adjust_room() does.

Signed-off-by: ThisSeanZhang <thisseanzhang@gmail.com>
---
 include/linux/btf.h   | 1 +
 kernel/bpf/verifier.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 87fad4076..0ec8b939a 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -83,6 +83,7 @@
 #define KF_SPINLOCK_SAFE (1 << 17) /* kfunc is allowed inside bpf_spin_lock-ed region */
 #define KF_PERFMON      (1 << 18) /* kfunc requires CAP_PERFMON */
 #define KF_ITER_RET_INDEPENDENT (1 << 19) /* next result outlives the current element */
+#define KF_CHANGES_PKT  (1 << 20) /* kfunc may change packet data */
 
 /*
  * Tag marking a kernel function as a kfunc. This is meant to minimize the
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 20b1b9ff5..62536bb98 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14411,6 +14411,7 @@ static bool is_kfunc_dynptr_write(struct bpf_call_arg_meta *meta)
 bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta)
 {
 	return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]) ||
+	       (meta->kfunc_flags & KF_CHANGES_PKT) ||
 	       is_kfunc_dynptr_write(meta);
 }
 
-- 
2.47.3

[RFC bpf-next v3 2/3] bpf: Add PPPoE encap/decap kfuncs

[ThisSeanZhang]

Add bpf_skb_pppoe_encap() and bpf_skb_pppoe_decap() kfuncs so that a
TC BPF program can encapsulate an IPv4 or IPv6 packet into a PPPoE
session frame, and decapsulate one again:

	bpf_skb_pppoe_encap(skb, sid, flags);
	bpf_skb_pppoe_decap(skb, flags);

The kfuncs insert or remove the 8 byte PPPoE session header between
the MAC header and the network header, and update the skb metadata to
match: skb->protocol is set to ETH_P_PPP_SES on encapsulation, or
restored from the PPP protocol field on decapsulation, mac_len is
reset, and the GSO metadata is adjusted like bpf_skb_adjust_room()
does (gso_size, gso_segs, SKB_GSO_DODGY).

Both kfuncs accept BPF_F_ADJ_ROOM_FIXED_GSO to leave gso_size
unchanged and only recompute gso_segs, reusing the existing flag
instead of adding a new one; UDP GSO packets require it, as their
gso_size is the datagram size. Unknown flags are rejected.

Previously, a TC program could add or remove the header bytes
manually, but the skb would keep a stale skb->protocol: forwarded GSO
packets larger than the PPPoE MTU could not reach the PPPoE
segmentation path registered for ETH_P_PPP_SES and could be dropped.
bpf_skb_adjust_room() cannot be used for the decapsulation direction
either, because it only accepts ETH_P_IP or ETH_P_IPV6 in
skb->protocol.

The encap kfunc fills in the PPPoE header itself from the session id:
version 1, type 1, code 0, and the PPP protocol derived from
skb->protocol, mirroring what pppoe_xmit() writes. The ethertype of
the MAC header remains for the BPF program, like with the existing
BPF_F_ADJ_ROOM_ENCAP_* flags. skb->encapsulation is deliberately left
unset, like pppoe_xmit() does: PPPoE is not an IP tunnel, and setting
it would narrow the device features to hw_enc_features in
netif_skb_features(). The decap kfunc accepts only PPP_IP and PPP_IPV6
payloads and requires a full inner L3 header to remain.

The kfuncs are registered for SCHED_CLS and SCHED_ACT with the new
KF_CHANGES_PKT flag, so the verifier invalidates packet pointers after
a call, like it does for bpf_skb_adjust_room().

PPPoE GRO/GSO support must be built in or loaded before this path is
used; loading pppoe.ko is left to user space.

Signed-off-by: ThisSeanZhang <thisseanzhang@gmail.com>
---
 net/core/filter.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 192 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 9245009d9..3d9143552 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -47,6 +47,8 @@
 #include <linux/ratelimit.h>
 #include <linux/seccomp.h>
 #include <linux/if_vlan.h>
+#include <linux/if_pppox.h>
+#include <linux/ppp_defs.h>
 #include <linux/bpf.h>
 #include <linux/btf.h>
 #include <net/sch_generic.h>
@@ -12919,6 +12921,184 @@ __bpf_kfunc int bpf_icmp_send(struct __sk_buff *skb_ctx, int type, int code)
 	return 0;
 }
 
+/**
+ * bpf_skb_pppoe_encap() - Encapsulate an IPv4 or IPv6 packet in a
+ * PPPoE session header.
+ *
+ * Insert an 8 byte PPPoE session header, made of the 6 byte session
+ * header plus the 2 byte PPP protocol field, between the MAC header
+ * and the network header, and fill it in for the given session id.
+ * skb->protocol is set to ETH_P_PPP_SES so the skb reaches the PPPoE
+ * GRO/GSO support; the ethertype of the MAC header is left to the BPF
+ * program.
+ *
+ * A packet whose VLAN tag is still present in the packet data is
+ * rejected, as skb->protocol is then the VLAN ethertype.
+ *
+ * On GSO skbs, gso_size is reduced by the size of the inserted header
+ * and gso_segs is recomputed, like bpf_skb_adjust_room() does. With
+ * BPF_F_ADJ_ROOM_FIXED_GSO, gso_size is left unchanged and only
+ * gso_segs is recomputed; UDP GSO packets require this, as their
+ * gso_size is the datagram size.
+ *
+ * A call to this kfunc changes packet data. Therefore, at load time,
+ * all checks on pointers previously done by the verifier are
+ * invalidated and must be performed again, if the kfunc is used in
+ * combination with direct packet access.
+ *
+ * Parameters:
+ * @skb_ctx: Pointer to ctx (__sk_buff) in TC program. Cannot be NULL
+ * @sid: PPPoE session id, in network byte order
+ * @flags: BPF_F_ADJ_ROOM_FIXED_GSO to leave gso_size unchanged
+ *
+ * Return:
+ * * %0          - success
+ * * %-EINVAL    - unknown flags
+ * * %-ENOTSUPP  - skb->protocol is neither ETH_P_IP nor ETH_P_IPV6, the
+ *                 new length would exceed the maximum skb length, or the
+ *                 GSO metadata cannot be adjusted
+ * * %-ENOMEM    - the skb head could not be expanded
+ */
+__bpf_kfunc int bpf_skb_pppoe_encap(struct __sk_buff *skb_ctx, __be16 sid,
+				    u64 flags)
+{
+	struct sk_buff *skb = (struct sk_buff *)skb_ctx;
+	struct pppoe_hdr *ph;
+	u16 ppp_proto;
+	u32 off;
+	int ret;
+
+	if (flags & ~BPF_F_ADJ_ROOM_FIXED_GSO)
+		return -EINVAL;
+
+	if (skb->protocol != htons(ETH_P_IP) &&
+	    skb->protocol != htons(ETH_P_IPV6))
+		return -ENOTSUPP;
+
+	if (skb->len + PPPOE_SES_HLEN > BPF_SKB_MAX_LEN && !skb_is_gso(skb))
+		return -ENOTSUPP;
+
+	off = skb_mac_header_len(skb);
+
+	ret = bpf_skb_net_grow(skb, off, PPPOE_SES_HLEN, flags);
+	if (ret < 0)
+		return ret;
+
+	if (skb->protocol == htons(ETH_P_IP))
+		ppp_proto = PPP_IP;
+	else
+		ppp_proto = PPP_IPV6;
+
+	ph = (struct pppoe_hdr *)(skb->data + off);
+	ph->ver = 1;
+	ph->type = 1;
+	ph->code = 0;
+	ph->sid = sid;
+	ph->length = htons(skb->len - off - sizeof(*ph));
+	put_unaligned_be16(ppp_proto, (u8 *)ph + sizeof(*ph));
+
+	skb->protocol = htons(ETH_P_PPP_SES);
+	skb_reset_mac_len(skb);
+	if (skb_valid_dst(skb))
+		skb_dst_drop(skb);
+
+	__skb_reset_checksum_unnecessary(skb);
+	bpf_compute_data_pointers(skb);
+
+	return 0;
+}
+
+/**
+ * bpf_skb_pppoe_decap() - Remove the PPPoE session header from a
+ * packet.
+ *
+ * Remove the 8 byte PPPoE session header between the MAC header and
+ * the network header. skb->protocol is restored from the PPP protocol
+ * field; a payload with any other PPP protocol is rejected. The
+ * ethertype of the MAC header is left to the BPF program.
+ *
+ * A packet whose VLAN tag is still present in the packet data is
+ * rejected, as skb->protocol is then the VLAN ethertype.
+ *
+ * On GSO skbs, gso_size is increased by the size of the removed
+ * header and gso_segs is recomputed, like bpf_skb_adjust_room() does.
+ * With BPF_F_ADJ_ROOM_FIXED_GSO, gso_size is left unchanged and only
+ * gso_segs is recomputed.
+ *
+ * A call to this kfunc changes packet data. Therefore, at load time,
+ * all checks on pointers previously done by the verifier are
+ * invalidated and must be performed again, if the kfunc is used in
+ * combination with direct packet access.
+ *
+ * Parameters:
+ * @skb_ctx: Pointer to ctx (__sk_buff) in TC program. Cannot be NULL
+ * @flags: BPF_F_ADJ_ROOM_FIXED_GSO to leave gso_size unchanged
+ *
+ * Return:
+ * * %0          - success
+ * * %-ENOTSUPP  - skb->protocol is not ETH_P_PPP_SES, or the PPP
+ *                 protocol is neither PPP_IP nor PPP_IPV6
+ * * %-EINVAL    - unknown flags, the packet is shorter than the PPPoE
+ *                 session header, or no full L3 header would remain
+ *                 after decapsulation
+ * * %-ENOMEM    - the header is not linearly accessible
+ */
+__bpf_kfunc int bpf_skb_pppoe_decap(struct __sk_buff *skb_ctx, u64 flags)
+{
+	struct sk_buff *skb = (struct sk_buff *)skb_ctx;
+	__be16 inner_proto;
+	u32 inner_len, off;
+	u16 ppp_proto;
+	int ret;
+
+	if (flags & ~BPF_F_ADJ_ROOM_FIXED_GSO)
+		return -EINVAL;
+
+	if (skb->protocol != htons(ETH_P_PPP_SES))
+		return -ENOTSUPP;
+
+	off = skb_mac_header_len(skb);
+
+	if (skb->len < off + PPPOE_SES_HLEN)
+		return -EINVAL;
+
+	if (!pskb_may_pull(skb, off + PPPOE_SES_HLEN))
+		return -ENOMEM;
+
+	ppp_proto = get_unaligned_be16(skb->data + off +
+				       sizeof(struct pppoe_hdr));
+	switch (ppp_proto) {
+	case PPP_IP:
+		inner_proto = htons(ETH_P_IP);
+		inner_len = sizeof(struct iphdr);
+		break;
+	case PPP_IPV6:
+		inner_proto = htons(ETH_P_IPV6);
+		inner_len = sizeof(struct ipv6hdr);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	/* A full inner L3 header must remain after decapsulation. */
+	if (skb->len - off - PPPOE_SES_HLEN < inner_len)
+		return -EINVAL;
+
+	ret = bpf_skb_net_shrink(skb, off, PPPOE_SES_HLEN, flags);
+	if (ret < 0)
+		return ret;
+
+	skb->protocol = inner_proto;
+	skb_reset_mac_len(skb);
+	if (skb_valid_dst(skb))
+		skb_dst_drop(skb);
+
+	__skb_reset_checksum_unnecessary(skb);
+	bpf_compute_data_pointers(skb);
+
+	return 0;
+}
+
 __bpf_kfunc_end_defs();
 
 int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
@@ -12965,6 +13145,11 @@ BTF_KFUNCS_START(bpf_kfunc_check_set_icmp_send)
 BTF_ID_FLAGS(func, bpf_icmp_send)
 BTF_KFUNCS_END(bpf_kfunc_check_set_icmp_send)
 
+BTF_KFUNCS_START(bpf_kfunc_check_set_skb_pppoe)
+BTF_ID_FLAGS(func, bpf_skb_pppoe_encap, KF_CHANGES_PKT)
+BTF_ID_FLAGS(func, bpf_skb_pppoe_decap, KF_CHANGES_PKT)
+BTF_KFUNCS_END(bpf_kfunc_check_set_skb_pppoe)
+
 static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
 	.owner = THIS_MODULE,
 	.set = &bpf_kfunc_check_set_skb,
@@ -13000,6 +13185,11 @@ static const struct btf_kfunc_id_set bpf_kfunc_set_icmp_send = {
 	.set = &bpf_kfunc_check_set_icmp_send,
 };
 
+static const struct btf_kfunc_id_set bpf_kfunc_set_skb_pppoe = {
+	.owner = THIS_MODULE,
+	.set = &bpf_kfunc_check_set_skb_pppoe,
+};
+
 static int __init bpf_kfunc_init(void)
 {
 	int ret;
@@ -13023,6 +13213,8 @@ static int __init bpf_kfunc_init(void)
 					       &bpf_kfunc_set_sock_addr);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_tcp_reqsk);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SKB, &bpf_kfunc_set_icmp_send);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_skb_pppoe);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT, &bpf_kfunc_set_skb_pppoe);
 	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SOCK_OPS, &bpf_kfunc_set_sock_ops);
 }
 late_initcall(bpf_kfunc_init);
-- 
2.47.3

[RFC bpf-next v3 3/3] selftests/bpf: Add a test for the PPPoE encap/decap kfuncs

[ThisSeanZhang]

Add a tc test that exercises the bpf_skb_pppoe_encap() and
bpf_skb_pppoe_decap() kfuncs.

The encap program is run over plain Ethernet/IPv4 and Ethernet/IPv6
packets. The test checks that the kernel writes a correct PPPoE
session header between the Ethernet and network headers (version,
type, code, session id, length and PPP protocol), that skb->protocol
becomes ETH_P_PPP_SES, and that the payload is preserved. It then
runs the decap program and checks that the original packet and
protocol metadata are restored.

The rejection paths are driven by the packet input: encapsulation of
a non-IP packet (ARP), decapsulation of a packet whose protocol is
not ETH_P_PPP_SES (plain IPv4, and an IPv4 packet whose bytes at the
PPP protocol offset decode as PPP_IP), decapsulation of a PPPoE
packet with an unsupported PPP protocol (IPCP), and decapsulation of
a packet too short to still hold an IP header.

A separate program that holds a packet pointer across a call to
bpf_skb_pppoe_encap() must fail verification, covering the
KF_CHANGES_PKT invalidation from the first patch.

Signed-off-by: ThisSeanZhang <thisseanzhang@gmail.com>
---
 .../selftests/bpf/prog_tests/tc_pppoe.c       | 269 ++++++++++++++++++
 tools/testing/selftests/bpf/progs/tc_pppoe.c  |  77 +++++
 .../selftests/bpf/progs/tc_pppoe_fail.c       |  36 +++
 3 files changed, 382 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_pppoe.c
 create mode 100644 tools/testing/selftests/bpf/progs/tc_pppoe.c
 create mode 100644 tools/testing/selftests/bpf/progs/tc_pppoe_fail.c

diff --git a/tools/testing/selftests/bpf/prog_tests/tc_pppoe.c b/tools/testing/selftests/bpf/prog_tests/tc_pppoe.c
new file mode 100644
index 000000000..5420d3bf3
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/tc_pppoe.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 ThisSeanZhang */
+
+#include <arpa/inet.h>
+#include <test_progs.h>
+#include "tc_pppoe.skel.h"
+#include "tc_pppoe_fail.skel.h"
+
+/* Ethernet + IPv4 + TCP, 54 bytes in total. */
+static const __u8 ip4_pkt[] = {
+	/* Ethernet header. */
+	0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+	0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+	0x08, 0x00,
+	/* IPv4 header. */
+	0x45, 0x00, 0x00, 0x28,
+	0x12, 0x34, 0x40, 0x00,
+	0x40, 0x06, 0x00, 0x00,
+	0xc0, 0xa8, 0x01, 0x01,
+	0xc0, 0xa8, 0x01, 0x02,
+	/* TCP header. */
+	0x00, 0x50, 0x1f, 0x90,
+	0x00, 0x00, 0x00, 0x01,
+	0x00, 0x00, 0x00, 0x00,
+	0x50, 0x02, 0x10, 0x00,
+	0x00, 0x00, 0x00, 0x00,
+};
+
+/* Ethernet + IPv6 + TCP, 74 bytes in total. */
+static const __u8 ip6_pkt[] = {
+	/* Ethernet header. */
+	0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+	0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+	0x86, 0xdd,
+	/* IPv6 header. */
+	0x60, 0x00, 0x00, 0x00,
+	0x00, 0x14, 0x06, 0x40,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+	/* TCP header. */
+	0x00, 0x50, 0x1f, 0x90,
+	0x00, 0x00, 0x00, 0x01,
+	0x00, 0x00, 0x00, 0x00,
+	0x50, 0x02, 0x10, 0x00,
+	0x00, 0x00, 0x00, 0x00,
+};
+
+/* Ethernet + ARP request, 42 bytes in total. */
+static const __u8 arp_pkt[] = {
+	/* Ethernet header. */
+	0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+	0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+	0x08, 0x06,
+	/* ARP payload. */
+	0x00, 0x01,
+	0x08, 0x00,
+	0x06, 0x04, 0x00, 0x01,
+	0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+	0xc0, 0xa8, 0x01, 0x01,
+	0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+	0xc0, 0xa8, 0x01, 0x02,
+};
+
+#define PPPOE_SES_HLEN	8
+#define TC_ACT_SHOT	2
+
+static int run_prog(int prog_fd, const void *data_in, __u32 size_in,
+		    void *data_out, __u32 size_out, __u32 *retval,
+		    __u32 *size_out_actual)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts,
+		    .data_in = (void *)data_in,
+		    .data_size_in = size_in,
+		    .data_out = data_out,
+		    .data_size_out = size_out,
+	);
+	int ret;
+
+	ret = bpf_prog_test_run_opts(prog_fd, &opts);
+	if (!ret && retval)
+		*retval = opts.retval;
+	if (!ret && size_out_actual)
+		*size_out_actual = opts.data_size_out;
+
+	return ret;
+}
+
+static void test_encap_decap(struct tc_pppoe *skel, const char *subtest,
+			     const __u8 *pkt, __u32 pkt_len, int ethertype,
+			     __u8 ppp_proto)
+{
+	__u8 encap_pkt[128];
+	__u8 decap_pkt[128];
+	__u32 retval, out_len;
+	int ret;
+
+	if (!test__start_subtest(subtest))
+		return;
+
+	skel->bss->encap_proto = 0;
+	ret = run_prog(bpf_program__fd(skel->progs.tc_pppoe_encap),
+		       pkt, pkt_len, encap_pkt, sizeof(encap_pkt),
+		       &retval, &out_len);
+	ASSERT_OK(ret, "encap test_run");
+	ASSERT_OK(retval, "encap retval");
+	ASSERT_EQ(out_len, pkt_len + PPPOE_SES_HLEN, "encap pkt len");
+	ASSERT_EQ(skel->bss->encap_proto, htons(0x8864),
+		  "encap skb protocol");
+
+	/* Ethernet header, with the ethertype changed to PPPoE session. */
+	ASSERT_EQ(encap_pkt[12], 0x88, "encap eth h_proto");
+	ASSERT_EQ(encap_pkt[13], 0x64, "encap eth h_proto");
+	/* PPPoE session header written by the kernel. */
+	ASSERT_EQ(encap_pkt[14], 0x11, "encap pppoe ver/type");
+	ASSERT_EQ(encap_pkt[15], 0x00, "encap pppoe code");
+	ASSERT_EQ(encap_pkt[16], 0xde, "encap pppoe sid");
+	ASSERT_EQ(encap_pkt[17], 0xad, "encap pppoe sid");
+	ASSERT_EQ(encap_pkt[18], 0x00, "encap pppoe length");
+	ASSERT_EQ(encap_pkt[19], pkt_len - 14 + 2, "encap pppoe length");
+	ASSERT_EQ(encap_pkt[20], 0x00, "encap ppp proto");
+	ASSERT_EQ(encap_pkt[21], ppp_proto, "encap ppp proto");
+	/* The original packet moves behind the new header unchanged. */
+	ASSERT_MEMEQ(encap_pkt + 14 + PPPOE_SES_HLEN, pkt + 14,
+		     pkt_len - 14, "encap payload");
+
+	skel->bss->decap_proto = 0;
+	ret = run_prog(bpf_program__fd(skel->progs.tc_pppoe_decap),
+		       encap_pkt, pkt_len + PPPOE_SES_HLEN, decap_pkt,
+		       sizeof(decap_pkt), &retval, &out_len);
+	ASSERT_OK(ret, "decap test_run");
+	ASSERT_OK(retval, "decap retval");
+	ASSERT_EQ(out_len, pkt_len, "decap pkt len");
+	ASSERT_EQ(skel->bss->decap_proto, ethertype, "decap skb protocol");
+	ASSERT_MEMEQ(decap_pkt, pkt, pkt_len, "decap packet");
+}
+
+static void test_reject_input(struct tc_pppoe *skel, const char *prog_name,
+			      const __u8 *pkt, __u32 pkt_len,
+			      const char *subtest)
+{
+	struct bpf_program *prog;
+	__u8 out[128];
+	__u32 retval = 0;
+	int ret;
+
+	if (!test__start_subtest(subtest))
+		return;
+
+	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+	if (!ASSERT_OK_PTR(prog, "find program"))
+		return;
+
+	skel->bss->encap_proto = 0;
+	skel->bss->decap_proto = 0;
+	ret = run_prog(bpf_program__fd(prog), pkt, pkt_len, out,
+		       sizeof(out), &retval, NULL);
+	ASSERT_OK(ret, "reject test_run");
+	ASSERT_EQ(retval, TC_ACT_SHOT, "kfunc rejected the call");
+	ASSERT_EQ(skel->bss->encap_proto, 0, "skb protocol unchanged");
+	ASSERT_EQ(skel->bss->decap_proto, 0, "skb protocol unchanged");
+}
+
+static void test_kfunc_flags(struct tc_pppoe *skel, const char *prog_name,
+			     int expected, const char *subtest)
+{
+	struct bpf_program *prog;
+	__u8 out[128];
+	__u32 retval = 0;
+	int ret;
+
+	if (!test__start_subtest(subtest))
+		return;
+
+	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+	if (!ASSERT_OK_PTR(prog, "find program"))
+		return;
+
+	ret = run_prog(bpf_program__fd(prog), ip4_pkt, sizeof(ip4_pkt), out,
+		       sizeof(out), &retval, NULL);
+	ASSERT_OK(ret, "flags test_run");
+	ASSERT_EQ((int)retval, expected, "kfunc retval");
+}
+
+void test_tc_pppoe(void)
+{
+	/* PPPoE packet with PPP protocol 0x8021 (IPCP), neither IPv4 nor IPv6. */
+	static const __u8 bad_ppp_pkt[] = {
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x88, 0x64,
+		0x11, 0x00, 0x00, 0x00, 0x00, 0x28, 0x80, 0x21,
+		0x45, 0x00, 0x00, 0x28,
+		0x12, 0x34, 0x40, 0x00,
+		0x40, 0x06, 0x00, 0x00,
+		0xc0, 0xa8, 0x01, 0x01,
+		0xc0, 0xa8, 0x01, 0x02,
+		0x00, 0x50, 0x1f, 0x90,
+		0x00, 0x00, 0x00, 0x01,
+		0x00, 0x00, 0x00, 0x00,
+		0x50, 0x02, 0x10, 0x00,
+		0x00, 0x00, 0x00, 0x00,
+	};
+	/* PPPoE packet too short for a full inner IP header. */
+	static const __u8 truncated_pkt[] = {
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x88, 0x64,
+		0x11, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x21,
+		0x45, 0x00,
+	};
+	/* Non-PPPoE IPv4 packet whose bytes 20/21 decode as PPP_IP. */
+	static const __u8 fake_ppp_pkt[] = {
+		0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff,
+		0x08, 0x00,
+		0x45, 0x00, 0x00, 0x28,
+		0x12, 0x34, 0x00, 0x21,
+		0x40, 0x06, 0x00, 0x00,
+		0xc0, 0xa8, 0x01, 0x01,
+		0xc0, 0xa8, 0x01, 0x02,
+		0x00, 0x50, 0x1f, 0x90,
+		0x00, 0x00, 0x00, 0x01,
+		0x00, 0x00, 0x00, 0x00,
+		0x50, 0x02, 0x10, 0x00,
+		0x00, 0x00, 0x00, 0x00,
+	};
+	struct tc_pppoe_fail *fail_skel;
+	struct tc_pppoe *skel;
+
+	skel = tc_pppoe__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel open_and_load"))
+		return;
+
+	test_encap_decap(skel, "encap-decap-v4", ip4_pkt, sizeof(ip4_pkt),
+			 htons(0x0800), 0x21);
+	test_encap_decap(skel, "encap-decap-v6", ip6_pkt, sizeof(ip6_pkt),
+			 htons(0x86dd), 0x57);
+
+	/* BPF_F_ADJ_ROOM_FIXED_GSO is accepted, unknown flags are not. */
+	test_kfunc_flags(skel, "tc_pppoe_encap_fixed_gso", 0,
+			 "encap-fixed-gso");
+	test_kfunc_flags(skel, "tc_pppoe_encap_bad_flags", -EINVAL,
+			 "encap-bad-flags");
+
+	/* Encapsulation of a non-IP packet must be rejected. */
+	test_reject_input(skel, "tc_pppoe_encap", arp_pkt, sizeof(arp_pkt),
+			  "encap-arp-reject");
+	/* Decapsulation of a packet whose protocol is not PPPoE. */
+	test_reject_input(skel, "tc_pppoe_decap", ip4_pkt, sizeof(ip4_pkt),
+			  "decap-non-pppoe");
+	test_reject_input(skel, "tc_pppoe_decap", bad_ppp_pkt,
+			  sizeof(bad_ppp_pkt), "decap-bad-ppp-proto");
+	test_reject_input(skel, "tc_pppoe_decap", truncated_pkt,
+			  sizeof(truncated_pkt), "decap-truncated");
+	test_reject_input(skel, "tc_pppoe_decap", fake_ppp_pkt,
+			  sizeof(fake_ppp_pkt), "decap-non-pppoe-fake-proto");
+
+	tc_pppoe__destroy(skel);
+
+	/*
+	 * KF_CHANGES_PKT must make the verifier reject a program that
+	 * keeps a packet pointer across the call.
+	 */
+	fail_skel = tc_pppoe_fail__open_and_load();
+	if (!ASSERT_ERR_PTR(fail_skel, "stale packet pointer rejected"))
+		tc_pppoe_fail__destroy(fail_skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/tc_pppoe.c b/tools/testing/selftests/bpf/progs/tc_pppoe.c
new file mode 100644
index 000000000..880abfbd5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tc_pppoe.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 ThisSeanZhang */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+#define ETH_P_PPP_SES_TEST	0x8864
+
+#define TC_ACT_OK_TEST		0
+#define TC_ACT_SHOT_TEST	2
+
+extern int bpf_skb_pppoe_encap(struct __sk_buff *skb,
+			       __be16 sid, u64 flags) __ksym;
+extern int bpf_skb_pppoe_decap(struct __sk_buff *skb, u64 flags) __ksym;
+
+/* skb->protocol as observed after the kfunc call. */
+int encap_proto;
+int decap_proto;
+
+SEC("tc")
+int tc_pppoe_encap(struct __sk_buff *skb)
+{
+	struct ethhdr eth;
+
+	if (bpf_skb_pppoe_encap(skb, bpf_htons(0xdead), 0))
+		return TC_ACT_SHOT_TEST;
+
+	encap_proto = skb->protocol;
+
+	if (bpf_skb_load_bytes(skb, 0, &eth, sizeof(eth)))
+		return TC_ACT_SHOT_TEST;
+	eth.h_proto = bpf_htons(ETH_P_PPP_SES_TEST);
+	if (bpf_skb_store_bytes(skb, 0, &eth, sizeof(eth), 0))
+		return TC_ACT_SHOT_TEST;
+
+	return TC_ACT_OK_TEST;
+}
+
+SEC("tc")
+int tc_pppoe_decap(struct __sk_buff *skb)
+{
+	struct ethhdr eth;
+
+	if (bpf_skb_pppoe_decap(skb, 0))
+		return TC_ACT_SHOT_TEST;
+
+	decap_proto = skb->protocol;
+
+	/*
+	 * Restore the ethertype to the protocol of the decapsulated
+	 * payload, as picked by the kernel from the PPP protocol field.
+	 */
+	if (bpf_skb_load_bytes(skb, 0, &eth, sizeof(eth)))
+		return TC_ACT_SHOT_TEST;
+	eth.h_proto = (__be16)skb->protocol;
+	if (bpf_skb_store_bytes(skb, 0, &eth, sizeof(eth), 0))
+		return TC_ACT_SHOT_TEST;
+
+	return TC_ACT_OK_TEST;
+}
+
+/* BPF_F_ADJ_ROOM_FIXED_GSO is accepted; any other flag is rejected. */
+SEC("tc")
+int tc_pppoe_encap_fixed_gso(struct __sk_buff *skb)
+{
+	return bpf_skb_pppoe_encap(skb, bpf_htons(0xdead),
+				   BPF_F_ADJ_ROOM_FIXED_GSO);
+}
+
+SEC("tc")
+int tc_pppoe_encap_bad_flags(struct __sk_buff *skb)
+{
+	return bpf_skb_pppoe_encap(skb, bpf_htons(0xdead), 1ULL << 32);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/tc_pppoe_fail.c b/tools/testing/selftests/bpf/progs/tc_pppoe_fail.c
new file mode 100644
index 000000000..f816c7169
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tc_pppoe_fail.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 ThisSeanZhang */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+#define TC_ACT_SHOT_TEST	2
+
+extern int bpf_skb_pppoe_encap(struct __sk_buff *skb,
+			       __be16 sid, u64 flags) __ksym;
+
+/*
+ * bpf_skb_pppoe_encap() may reallocate the skb head, so the packet
+ * pointer checked before the call is stale afterwards: this program
+ * has to fail verification.
+ */
+SEC("tc")
+int tc_pppoe_stale_ptr(struct __sk_buff *skb)
+{
+	char *data = (char *)(long)skb->data;
+	char *data_end = (char *)(long)skb->data_end;
+	__u8 b = 0;
+
+	if (data + 1 > data_end)
+		return TC_ACT_SHOT_TEST;
+
+	if (bpf_skb_pppoe_encap(skb, bpf_htons(0xdead), 0))
+		return TC_ACT_SHOT_TEST;
+
+	b = *data;
+
+	return b;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.47.3

Re: [RFC bpf-next v3 1/3] bpf: Add KF_CHANGES_PKT kfunc flag

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] bpf: Signed-off-by trailer uses handle instead of full name
--

commit 59e7e83571f6e7abee77827b13a92abbf8a20431
Author: ThisSeanZhang <thisseanzhang@gmail.com>

> bpf: Add KF_CHANGES_PKT kfunc flag
>
> The verifier invalidates packet pointers after a call to a helper that
> may change packet data, see bpf_helper_changes_pkt_data(). Kfuncs have
> no equivalent: bpf_is_kfunc_pkt_changing() special-cases
> bpf_xdp_pull_data() by ID, and the kfuncs that write through a dynptr
> are listed in dynptr_write_kfuncs, because the dynptr type is not known
> when the arguments are checked.
>
> Add KF_CHANGES_PKT so that a kfunc can declare this where it is
> defined, instead of adding another entry to the kfunc lists in the
> verifier. The verifier then invalidates packet pointers after a call to
> such a kfunc, like it does for the equivalent helpers; a stale packet
> pointer kept in a callee-saved register across the call is rejected at
> verification time.
>
> The same predicate is used by the CFG pass to propagate the
> changes_pkt_data property to the callers of a subprogram, so a kfunc
> called through a subprogram is covered as well.
>
> This is needed by the PPPoE encap/decap kfuncs added later in this
> series, which adjust the skb like bpf_skb_adjust_room() does.
>
> Signed-off-by: ThisSeanZhang <thisseanzhang@gmail.com>

[Severity: High]
The Author header and Signed-off-by trailer use a single-word handle
(ThisSeanZhang) rather than a real full name (such as Sean Zhang), as
required by the Developer's Certificate of Origin in
Documentation/process/submitting-patches.rst.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111032141.525444-1-thisseanzhang@gmail.com?part=1

Re: [RFC bpf-next v3 2/3] bpf: Add PPPoE encap/decap kfuncs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] bpf: stale skb->cb data pointers on bpf_skb_pppoe_encap() and
  decap() error
- [High] bpf: Signed-off-by trailer uses handle instead of real full
  name
- [Medium] bpf: missing skb_postpush_rcsum() in bpf_skb_pppoe_encap()
  corrupts csum
--

commit 41cc0bc50e47c52d87a2f024362b196725052585
Author: ThisSeanZhang <thisseanzhang@gmail.com>

> bpf: Add PPPoE encap/decap kfuncs
>
> Add bpf_skb_pppoe_encap() and bpf_skb_pppoe_decap() kfuncs so that a
> TC BPF program can encapsulate an IPv4 or IPv6 packet into a PPPoE
> session frame, and decapsulate one again:
>
>         bpf_skb_pppoe_encap(skb, sid, flags);
>         bpf_skb_pppoe_decap(skb, flags);
>
> The kfuncs insert or remove the 8 byte PPPoE session header between
> the MAC header and the network header, and update the skb metadata to
> match: skb->protocol is set to ETH_P_PPP_SES on encapsulation, or
> restored from the PPP protocol field on decapsulation, mac_len is
> reset, and the GSO metadata is adjusted like bpf_skb_adjust_room()
> does (gso_size, gso_segs, SKB_GSO_DODGY).
>
> Both kfuncs accept BPF_F_ADJ_ROOM_FIXED_GSO to leave gso_size
> unchanged and only recompute gso_segs, reusing the existing flag
> instead of adding a new one; UDP GSO packets require it, as their
> gso_size is the datagram size. Unknown flags are rejected.
>
> Previously, a TC program could add or remove the header bytes
> manually, but the skb would keep a stale skb->protocol: forwarded GSO
> packets larger than the PPPoE MTU could not reach the PPPoE
> segmentation path registered for ETH_P_PPP_SES and could be dropped.
> bpf_skb_adjust_room() cannot be used for the decapsulation direction
> either, because it only accepts ETH_P_IP or ETH_P_IPV6 in
> skb->protocol.
>
> The encap kfunc fills in the PPPoE header itself from the session id:
> version 1, type 1, code 0, and the PPP protocol derived from
> skb->protocol, mirroring what pppoe_xmit() writes. The ethertype of
> the MAC header remains for the BPF program, like with the existing
> BPF_F_ADJ_ROOM_ENCAP_* flags. skb->encapsulation is deliberately left
> unset, like pppoe_xmit() does: PPPoE is not an IP tunnel, and setting
> it would narrow the device features to hw_enc_features in
> netif_skb_features(). The decap kfunc accepts only PPP_IP and PPP_IPV6
> payloads and requires a full inner L3 header to remain.
>
> The kfuncs are registered for SCHED_CLS and SCHED_ACT with the new
> KF_CHANGES_PKT flag, so the verifier invalidates packet pointers after
> a call, like it does for bpf_skb_adjust_room().
>
> PPPoE GRO/GSO support must be built in or loaded before this path is
> used; loading pppoe.ko is left to user space.
>
> Signed-off-by: ThisSeanZhang <thisseanzhang@gmail.com>

[Severity: High]
The Signed-off-by trailer and Author header use a single-word handle
("ThisSeanZhang") rather than a full real name, which does not meet the
Developer's Certificate of Origin requirements in
Documentation/process/submitting-patches.rst.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9245009d9f1eb..3d9143552f0d1 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> +__bpf_kfunc int bpf_skb_pppoe_encap(struct __sk_buff *skb_ctx, __be16 sid,
> +				    u64 flags)
> +{
> +	struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> +	struct pppoe_hdr *ph;
> +	u16 ppp_proto;
> +	u32 off;
> +	int ret;
> +
> +	if (flags & ~BPF_F_ADJ_ROOM_FIXED_GSO)
> +		return -EINVAL;
> +
> +	if (skb->protocol != htons(ETH_P_IP) &&
> +	    skb->protocol != htons(ETH_P_IPV6))
> +		return -ENOTSUPP;
> +
> +	if (skb->len + PPPOE_SES_HLEN > BPF_SKB_MAX_LEN && !skb_is_gso(skb))
> +		return -ENOTSUPP;
> +
> +	off = skb_mac_header_len(skb);
> +
> +	ret = bpf_skb_net_grow(skb, off, PPPOE_SES_HLEN, flags);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (skb->protocol == htons(ETH_P_IP))
> +		ppp_proto = PPP_IP;
> +	else
> +		ppp_proto = PPP_IPV6;
> +
> +	ph = (struct pppoe_hdr *)(skb->data + off);
> +	ph->ver = 1;
> +	ph->type = 1;
> +	ph->code = 0;
> +	ph->sid = sid;
> +	ph->length = htons(skb->len - off - sizeof(*ph));
> +	put_unaligned_be16(ppp_proto, (u8 *)ph + sizeof(*ph));
> +
> +	skb->protocol = htons(ETH_P_PPP_SES);
> +	skb_reset_mac_len(skb);
> +	if (skb_valid_dst(skb))
> +		skb_dst_drop(skb);
> +
> +	__skb_reset_checksum_unnecessary(skb);
> +	bpf_compute_data_pointers(skb);
> +
> +	return 0;
> +}

[Severity: Medium]
Does writing the PPPoE header without calling
skb_postpush_rcsum(skb, ph, PPPOE_SES_HLEN) leave skb->csum stale on
CHECKSUM_COMPLETE packets?

bpf_skb_net_grow() -> bpf_skb_net_hdr_push() -> bpf_skb_generic_push()
zeroes the pushed 8 bytes and skips skb_postpush_rcsum() because zeroed
bytes do not change skb->csum, and __skb_reset_checksum_unnecessary() only
clears CHECKSUM_UNNECESSARY.  Writing non-zero PPPoE header fields at
skb_network_header(skb) without updating skb->csum corrupts the checksum on
ingress packets with hardware CHECKSUM_COMPLETE (and fails
BPF_PROG_TEST_RUN with BPF_F_TEST_SKB_CHECKSUM_COMPLETE), whereas
bpf_skb_pppoe_decap() and pppoe_rcv() subtract the 8-byte header from
skb->csum via skb_postpull_rcsum().

[ ... ]

> +__bpf_kfunc int bpf_skb_pppoe_decap(struct __sk_buff *skb_ctx, u64 flags)
> +{
> +	struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> +	__be16 inner_proto;
> +	u32 inner_len, off;
> +	u16 ppp_proto;
> +	int ret;
> +
> +	if (flags & ~BPF_F_ADJ_ROOM_FIXED_GSO)
> +		return -EINVAL;
> +
> +	if (skb->protocol != htons(ETH_P_PPP_SES))
> +		return -ENOTSUPP;
> +
> +	off = skb_mac_header_len(skb);
> +
> +	if (skb->len < off + PPPOE_SES_HLEN)
> +		return -EINVAL;
> +
> +	if (!pskb_may_pull(skb, off + PPPOE_SES_HLEN))
> +		return -ENOMEM;
> +
> +	ppp_proto = get_unaligned_be16(skb->data + off +
> +				       sizeof(struct pppoe_hdr));
> +	switch (ppp_proto) {
> +	case PPP_IP:
> +		inner_proto = htons(ETH_P_IP);
> +		inner_len = sizeof(struct iphdr);
> +		break;
> +	case PPP_IPV6:
> +		inner_proto = htons(ETH_P_IPV6);
> +		inner_len = sizeof(struct ipv6hdr);
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
> +	/* A full inner L3 header must remain after decapsulation. */
> +	if (skb->len - off - PPPOE_SES_HLEN < inner_len)
> +		return -EINVAL;
> +
> +	ret = bpf_skb_net_shrink(skb, off, PPPOE_SES_HLEN, flags);
> +	if (ret < 0)
> +		return ret;

[Severity: High]
Can error returns in bpf_skb_pppoe_decap() and bpf_skb_pppoe_encap() leave
stale data_end and data_meta pointers in skb->cb after skb->head is
reallocated?

In bpf_skb_pppoe_decap(), pskb_may_pull() can reallocate skb->head via
pskb_expand_head() and free the old buffer when pulling bytes from
nonlinear fragments or expanding a cloned skb.  If ppp_proto is then
rejected as unsupported (for example, a PPP LCP/IPCP control frame),
inner_len is not met, or bpf_skb_net_shrink() fails, the function returns
without calling bpf_compute_data_pointers(skb).  Similarly, in
bpf_skb_pppoe_encap(), bpf_skb_net_grow() can reallocate skb->head in
skb_cow_head() and shift skb->data in bpf_skb_net_hdr_push() before failing
in pskb_may_pull() or skb_linearize().

Because TC programs read ctx->data from skb->data (now pointing to the new
buffer) while reading ctx->data_end and ctx->data_meta from skb->cb (still
pointing into the old freed buffer), a program that continues inspecting
the packet after an error return can pass verifier bounds checks and
trigger a kernel heap use-after-free or out-of-bounds access.  Calling
bpf_compute_data_pointers(skb) on the error paths after any potential
reallocation, as bpf_skb_adjust_room() does unconditionally, avoids leaving
dangling pointers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111032141.525444-1-thisseanzhang@gmail.com?part=2

Re: [RFC bpf-next v3 3/3] selftests/bpf: Add a test for the PPPoE encap/decap kfuncs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] selftests/bpf: single-word handle used in Signed-off-by trailer
- [Low] selftests/bpf: missing subtest isolation for tc_pppoe_fail in
  test_tc_pppoe()
- [Low] selftests/bpf: uninitialized reads on run_prog() error in
  test_encap_decap()
--

commit 0a18ebb6cc8079a048ec95f72882a33d91363974
Author: ThisSeanZhang <thisseanzhang@gmail.com>

> selftests/bpf: Add a test for the PPPoE encap/decap kfuncs
>
> Add a tc test that exercises the bpf_skb_pppoe_encap() and
> bpf_skb_pppoe_decap() kfuncs.
>
> The encap program is run over plain Ethernet/IPv4 and Ethernet/IPv6
> packets. The test checks that the kernel writes a correct PPPoE
> session header between the Ethernet and network headers (version,
> type, code, session id, length and PPP protocol), that skb->protocol
> becomes ETH_P_PPP_SES, and that the payload is preserved. It then
> runs the decap program and checks that the original packet and
> protocol metadata are restored.
>
> The rejection paths are driven by the packet input: encapsulation of
> a non-IP packet (ARP), decapsulation of a packet whose protocol is
> not ETH_P_PPP_SES (plain IPv4, and an IPv4 packet whose bytes at the
> PPP protocol offset decode as PPP_IP), decapsulation of a PPPoE
> packet with an unsupported PPP protocol (IPCP), and decapsulation of
> a packet too short to still hold an IP header.
>
> A separate program that holds a packet pointer across a call to
> bpf_skb_pppoe_encap() must fail verification, covering the
> KF_CHANGES_PKT invalidation from the first patch.
>
> Signed-off-by: ThisSeanZhang <thisseanzhang@gmail.com>

[Severity: High]
The Signed-off-by trailer, commit author, and file copyright headers use
the single-word handle "ThisSeanZhang".  Per
Documentation/process/submitting-patches.rst, the Developer's Certificate
of Origin requires a space-separated real full name.

> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_pppoe.c b/tools/testing/selftests/bpf/prog_tests/tc_pppoe.c
> new file mode 100644
> index 0000000000000..5420d3bf321ac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/tc_pppoe.c
[ ... ]
> +static int run_prog(int prog_fd, const void *data_in, __u32 size_in,
> +		    void *data_out, __u32 size_out, __u32 *retval,
> +		    __u32 *size_out_actual)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts,
> +		    .data_in = (void *)data_in,
> +		    .data_size_in = size_in,
> +		    .data_out = data_out,
> +		    .data_size_out = size_out,
> +	);
> +	int ret;
> +
> +	ret = bpf_prog_test_run_opts(prog_fd, &opts);
> +	if (!ret && retval)
> +		*retval = opts.retval;
> +	if (!ret && size_out_actual)
> +		*size_out_actual = opts.data_size_out;
> +
> +	return ret;
> +}
> +
> +static void test_encap_decap(struct tc_pppoe *skel, const char *subtest,
> +			     const __u8 *pkt, __u32 pkt_len, int ethertype,
> +			     __u8 ppp_proto)
> +{
> +	__u8 encap_pkt[128];
> +	__u8 decap_pkt[128];
> +	__u32 retval, out_len;
> +	int ret;
> +
> +	if (!test__start_subtest(subtest))
> +		return;
> +
> +	skel->bss->encap_proto = 0;
> +	ret = run_prog(bpf_program__fd(skel->progs.tc_pppoe_encap),
> +		       pkt, pkt_len, encap_pkt, sizeof(encap_pkt),
> +		       &retval, &out_len);
> +	ASSERT_OK(ret, "encap test_run");
> +	ASSERT_OK(retval, "encap retval");
> +	ASSERT_EQ(out_len, pkt_len + PPPOE_SES_HLEN, "encap pkt len");

[Severity: Low]
Can test_encap_decap() read uninitialized stack memory if run_prog()
fails?

run_prog() only populates *retval and *size_out_actual when
bpf_prog_test_run_opts() returns 0, and ASSERT_OK(ret, ...) does not
return on failure.  If either run_prog() call fails, execution continues
to evaluate assertions on uninitialized retval, out_len, encap_pkt, and
decap_pkt (which can also trigger -Werror=maybe-uninitialized when
run_prog() is inlined).  Returning early when !ASSERT_OK(ret, ...) avoids
this.

[ ... ]

> +	/*
> +	 * KF_CHANGES_PKT must make the verifier reject a program that
> +	 * keeps a packet pointer across the call.
> +	 */
> +	fail_skel = tc_pppoe_fail__open_and_load();
> +	if (!ASSERT_ERR_PTR(fail_skel, "stale packet pointer rejected"))
> +		tc_pppoe_fail__destroy(fail_skel);
> +}

[Severity: Low]
Does calling tc_pppoe_fail__open_and_load() without starting a subtest
attach its result to the preceding subtest?

In test_progs, the "decap-non-pppoe-fake-proto" subtest stays active until
the next test__start_subtest() call or until test_tc_pppoe() returns, and
filtered runs (such as test_progs -t tc_pppoe/encap-decap-v4) will still
execute tc_pppoe_fail__open_and_load() outside any subtest.  Using
RUN_TESTS(tc_pppoe_fail) with __failure and __msg() annotations from
bpf_misc.h on tc_pppoe_stale_ptr(), or guarding this check with
test__start_subtest(), would isolate it properly and verify the expected
verifier error message.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111032141.525444-1-thisseanzhang@gmail.com?part=3