Le mar. 27 oct. 2026 à 22:25, Jakub Kicinski <[email protected]> a écrit :
>
> On Tue, 27 Oct 2026 09:25:59 +0100 Eric Dumazet wrote:
> > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > NIC drivers always provide this, and they should keep using the fast
> > version, without an extra pskb_may_pull().
> >
> > Devices looping skbs from their transmit path back to their receive
> > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > the BUG() in __skb_pull().
> >
> > Instead of fixing every producer, or adding a check to all
> > eth_type_trans() users, fix the few consumers exposed to this:
>
> Bot says:
>
> BPF CI (bpf-ci) is reporting a consistent regression with this patch
> applied: every single job variant (x86-64/aarch64/s390x, gcc/llvm,
> no-alu32/cpuv4/ASAN) fails the exact same selftest subtest:
>
> #135 empty_skb
>
> Specifically, the "veth ETH_HLEN+1 packet ingress [redirect_ingress]"
> case in tools/testing/selftests/bpf/prog_tests/empty_skb.c now fails:
>
> test_empty_skb:FAIL:ret: veth ETH_HLEN+1 packet ingress [redirect_ingress]
> unexpected ret: actual 1 != expected 0
>
> That test redirects a minimal (ETH_HLEN+1-byte) skb to a veth's RX path
> via bpf_clone_redirect(..., BPF_F_INGRESS), which goes through
> dev_forward_skb() -> __dev_forward_skb2(). Before this change the skb
> was forwarded successfully (ret == 0); after switching to
> eth_type_trans_careful() it is now dropped (actual == 1, i.e.
> NET_RX_DROP).
>
> Could you take a look at whether the new drop-on-failure behavior in
> __dev_forward_skb2() is intentional for this previously-accepted
> ETH_HLEN+1 packet, and if so, update the empty_skb selftest's expected
> return value alongside this change? Right now the selftest and the new
> kernel behavior disagree, and it's failing 100% of the time in CI on
> every arch/toolchain/config combination we run.
This test needs an update.
The failing program is the lwt_xmit one (redirect_ingress), not the tc
one. For lwt, BPF_PROG_TEST_RUN strips the 14-byte Ethernet header, so
the program sees a 1-byte skb, with skb->data at the network header.
bpf_clone_redirect(veth, BPF_F_INGRESS) then calls
eth_type_trans() on a 1-byte skb.
Before this patch, eth_type_trans() did not pull anything because
skb->len < ETH_HLEN, read the Ethernet header 13 bytes past skb->tail,
and sent a 1-byte "frame" up the stack. Now __dev_forward_skb2() drops
it and returns NET_RX_DROP (1).
The same packet sent with redirect_egress is already expected to fail
with 1, because veth_xmit() does pskb_may_pull(skb, ETH_HLEN) and drops it.
Ingress and egress now behave the same way. The tc variants (skb->len=15)
and the ipip variant (no MAC header, __bpf_rx_skb_no_mac()) are unchanged.
In v2, patch 3 will also update the selftest, renaming lwt_egress_ret
to lwt_ret so that it applies to both lwt programs. empty_skb passes
with it.
Thanks.
# ./test_progs -t empty_skb
[ 50.888061] bpf_testmod: loading out-of-tree module taints kernel.
#115 empty_skb:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0/0 FAILED
diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
index
c9fcb70cbafbce571d7826e233920193625dead1..a3938c16557b140e3ee5c116139cf8b636f85e35
100644
--- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
@@ -24,7 +24,7 @@ void test_empty_skb(void)
int *ifindex;
int err;
int ret;
- int lwt_egress_ret; /* expected retval at lwt/egress */
+ int lwt_ret; /* expected retval at lwt */
__be16 h_proto;
bool success_on_tc;
bool adjust_room;
@@ -82,7 +82,7 @@ void test_empty_skb(void)
.data_size_in = sizeof(eth_hlen),
.ifindex = &veth_ifindex,
.ret = -ERANGE,
- .lwt_egress_ret = -ERANGE,
+ .lwt_ret = -ERANGE,
.success_on_tc = true,
},
{
@@ -96,17 +96,23 @@ void test_empty_skb(void)
.data_size_in = sizeof(eth_hlen),
.ifindex = &ipip_ifindex,
.ret = -ERANGE,
- .lwt_egress_ret = -ERANGE,
+ .lwt_ret = -ERANGE,
},
- /* ETH_HLEN+1-sized packet should be redirected. */
+ /* ETH_HLEN+1-sized packet should be redirected at TC.
+ * At LWT, the Ethernet header is not part of the skb, and
+ * a 1-byte packet is too short for an Ethernet device.
+ */
{
.msg = "veth ETH_HLEN+1 packet ingress",
.data_in = eth_hlen_pp,
.data_size_in = sizeof(eth_hlen_pp),
.ifindex = &veth_ifindex,
- .lwt_egress_ret = 1, /* veth_xmit NET_XMIT_DROP */
+ /* lwt: skb->len=1, dropped by veth_xmit() at egress
+ * and by __dev_forward_skb2() at ingress.
+ */
+ .lwt_ret = 1, /* NET_XMIT_DROP / NET_RX_DROP */
},
{
.msg = "ipip ETH_HLEN+1 packet ingress",
@@ -144,7 +150,6 @@ void test_empty_skb(void)
eth_hlen.h_proto = tests[i].h_proto;
bpf_object__for_each_program(prog, bpf_obj->obj) {
- bool at_egress =
strstr(bpf_program__name(prog), "egress") != NULL;
bool at_tc =
!strncmp(bpf_program__section_name(prog), "tc", 2);
bool is_adjust_room =
!strcmp(bpf_program__name(prog), "tc_adjust_room");
int expected_ret;
@@ -153,7 +158,7 @@ void test_empty_skb(void)
if (tests[i].adjust_room != is_adjust_room)
continue;
- expected_ret = at_egress && !at_tc ?
tests[i].lwt_egress_ret : tests[i].ret;
+ expected_ret = at_tc ? tests[i].ret : tests[i].lwt_ret;
tattr.data_in = tests[i].data_in;
tattr.data_size_in = tests[i].data_size_in;
Thanks.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
