Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test to cover ktls with bpf_msg_pop_data

[John Fastabend]

On 2025-06-09 10:08:53, Jiayuan Chen wrote:
> The selftest can reproduce an issue where using bpf_msg_pop_data() in
> ktls causes errors on the receiving end.
> 
> Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> ---

Reviewed-by: John Fastabend <john.fastabend@xxxxxxxxx>

>  .../selftests/bpf/prog_tests/sockmap_ktls.c   | 91 +++++++++++++++++++
>  .../selftests/bpf/progs/test_sockmap_ktls.c   |  4 +
>  2 files changed, 95 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> index b6c471da5c28..b87e7f39e15a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> @@ -314,6 +314,95 @@ static void test_sockmap_ktls_tx_no_buf(int family, int sotype, bool push)
>  	test_sockmap_ktls__destroy(skel);
>  }
>  
> +static void test_sockmap_ktls_tx_pop(int family, int sotype)
> +{
> +	char msg[37] = "0123456789abcdefghijklmnopqrstuvwxyz\0";
> +	int c = 0, p = 0, one = 1, sent, recvd;
> +	struct test_sockmap_ktls *skel;
> +	int prog_fd, map_fd;
> +	char rcv[50] = {0};
> +	int err;
> +	int i, m, r;
> +
> +	skel = test_sockmap_ktls__open_and_load();
> +	if (!ASSERT_TRUE(skel, "open ktls skel"))
> +		return;
> +
> +	err = create_pair(family, sotype, &c, &p);
> +	if (!ASSERT_OK(err, "create_pair()"))
> +		goto out;
> +
> +	prog_fd = bpf_program__fd(skel->progs.prog_sk_policy);
> +	map_fd = bpf_map__fd(skel->maps.sock_map);
> +
> +	err = bpf_prog_attach(prog_fd, map_fd, BPF_SK_MSG_VERDICT, 0);
> +	if (!ASSERT_OK(err, "bpf_prog_attach sk msg"))
> +		goto out;
> +
> +	err = bpf_map_update_elem(map_fd, &one, &c, BPF_NOEXIST);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem(c)"))
> +		goto out;
> +
> +	err = init_ktls_pairs(c, p);
> +	if (!ASSERT_OK(err, "init_ktls_pairs(c, p)"))
> +		goto out;
> +
> +	struct {
> +		int	pop_start;
> +		int	pop_len;
> +	} pop_policy[] = {
> +		/* trim the start */
> +		{0, 2},
> +		{0, 10},
> +		{1, 2},
> +		{1, 10},
> +		/* trim the end */
> +		{35, 2},
> +		/* New entries should be added before this line */
> +		{-1, -1},
> +	};
> +
> +	i = 0;
> +	while (pop_policy[i].pop_start >= 0) {
> +		skel->bss->pop_start = pop_policy[i].pop_start;
> +		skel->bss->pop_end =  pop_policy[i].pop_len;
> +
> +		sent = send(c, msg, sizeof(msg), 0);
> +		if (!ASSERT_EQ(sent, sizeof(msg), "send(msg)"))
> +			goto out;

Its possible this could actually not send 38B (sent < 38), but then again
it is only 38B so I guess it should never fail? Anyways we have this
case in a few places already I think and its not tripping CI so lets go
for it.

Thanks,
John

> +
> +		recvd = recv_timeout(p, rcv, sizeof(rcv), MSG_DONTWAIT, 1);
> +		if (!ASSERT_EQ(recvd, sizeof(msg) - pop_policy[i].pop_len, "pop len mismatch"))
> +			goto out;