RE: [PATHC bpf v5 3/3] selftests, bpf: Add one test for sockmap with strparser

-----Original Message-----
From: liujian (CE)
Sent: Monday, October 25, 2021 6:17 PM
To: 'John Fastabend' <john.fastabend@gmail.com>; daniel@iogearbox.net;
jakub@cloudflare.com; lmb@cloudflare.com; davem@davemloft.net;
kuba@kernel.org; ast@kernel.org; andrii@kernel.org; kafai@fb.com;
songliubraving@fb.com; yhs@fb.com; kpsingh@kernel.org;
netdev@vger.kernel.org; bpf@vger.kernel.org; xiyou.wangcong@gmail.com
Subject: RE: [PATHC bpf v5 3/3] selftests, bpf: Add one test for sockmap with
strparser

quoted

-----Original Message-----
From: John Fastabend [mailto:john.fastabend@gmail.com]
Sent: Friday, October 22, 2021 11:31 PM
To: liujian (CE) <redacted>; john.fastabend@gmail.com;
daniel@iogearbox.net; jakub@cloudflare.com; lmb@cloudflare.com;
davem@davemloft.net; kuba@kernel.org; ast@kernel.org;
andrii@kernel.org; kafai@fb.com; songliubraving@fb.com; yhs@fb.com;
kpsingh@kernel.org; netdev@vger.kernel.org; bpf@vger.kernel.org;
xiyou.wangcong@gmail.com
Cc: liujian (CE) <redacted>
Subject: RE: [PATHC bpf v5 3/3] selftests, bpf: Add one test for
sockmap with strparser

Liu Jian wrote:

quoted

Add the test to check sockmap with strparser is working well.

Signed-off-by: Liu Jian <redacted>
---
 tools/testing/selftests/bpf/test_sockmap.c | 33
++++++++++++++++++++--
 1 file changed, 30 insertions(+), 3 deletions(-)

Hi Liu,

This is a good test, but we should also add one with a parser
returning a value that is not skb->len. This doesn't cover the case fixed in

patch 1/3 correct?

quoted

For that we would need to modify the BPF prog itself as well
sockmap_parse_prog.c.

Hi John,
This test patch use tools/testing/selftests/bpf/progs/test_sockmap_kern.c
not sockmap_parse_prog.c.

If we set skb_use_parser to nonzero, the bpf parser program will return
skb_use_parser not skb->len.
In this test case, I set skb_use_parser to 10, skb->len to 20 (opt->iov_length).
This can test 1/3 patch, it will check the recved data len is 10 not 20.

Sorry, it will check the recved data length is 20 not 40.

The parser prog in tools/testing/selftests/bpf/progs/test_sockmap_kern.h
SEC("sk_skb1")
int bpf_prog1(struct __sk_buff *skb)
{
        int *f, two = 2;

        f = bpf_map_lookup_elem(&sock_skb_opts, &two);
        if (f && *f) {
                return *f;
        }
        return skb->len;
}

quoted

For this patch though,

Acked-by: John Fastabend <john.fastabend@gmail.com>

Then one more patch is all we need something to break up the skb from
the parser. We really need the test because its not something we can
easily test otherwise and I don't have any use cases that do this so wouldn't

catch it.

quoted

Thanks!
John