Re: [PATCH v4 2/2] selftests/bpf: cover stale CB after LWT IP encapsulation

[bot+bpf-ci@kernel.org]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_ip_encap.c b/tools/testing/selftests/bpf/prog_tests/lwt_ip_encap.c
> index 5c5560d45c5b4..ef90dd8d4b87c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/lwt_ip_encap.c
> +++ b/tools/testing/selftests/bpf/prog_tests/lwt_ip_encap.c

[ ... ]

> @@ -739,3 +739,48 @@ void test_lwt_ip_encap_vxlan_ipv6(void)
>  {
>  	lwt_ip_encap_vxlan(IPV6_ENCAP);
>  }
> +
> +void test_lwt_ip_encap_stale_cb(void)
> +{

[ ... ]

> +	SYS_NOFAIL("ip netns exec %s ping -q -R -c 1 -W 1 -I veth1 %s >/dev/null 2>&1",
> +		   ns1, IP4_ADDR_DST);
> +	ASSERT_TRUE(skel->bss->stale_cb_seen, "stale_cb_seen");
> +	ASSERT_TRUE(skel->bss->stale_cb_cleared, "stale_cb_cleared");

Could the test confirm that the probe packet actually carries an IP option?

Looking at ip_rcv_core() in net/ipv4/ip_input.c, IPCB(skb) is zeroed
unconditionally on receive:

  memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));

The opt.optlen and opt.rr fields are only written by ip_rcv_options(),
which ip_rcv_finish_core() calls only when the IP header has options:

  if (iph->ihl > 5) {
      drop_reason = ip_rcv_options(skb, dev);

If ping -R does not place a Record Route option on the wire (a ping
implementation that accepts but ignores -R, an option stripped en route,
or a non-iputils ping), the inner header has ihl == 5, IPCB(skb)->opt
stays all-zero through the LWT run, and the fentry hook records
stale_cb_cleared = true whether or not bpf_lwt_reset_cb() exists.

The stale_cb_seen flag still becomes true, because the ICMP_TIME_EXCEEDED
is produced by the TTL-one outer header the BPF program prepends and does
not depend on options at all, so neither assertion catches the missing
precondition and the test silently stops guarding the fix.

Could a pre-encapsulation check record whether opt.optlen was non-zero,
so the test fails loudly when the probe packet is not what it expects?
For example, an fentry or fexit on bpf_lwt_push_ip_encap that reads
IPCB(skb)->opt.optlen before the reset, then asserts that value was
non-zero.

> +
> +out:
> +	test_lwt_ip_encap__destroy(skel);
> +	SYS_NOFAIL("ip netns del %s", ns1);
> +	SYS_NOFAIL("ip netns del %s", ns2);
> +	SYS_NOFAIL("ip netns del %s", ns3);
> +}

[ ... ]


---
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/35634260486