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

["liujian (CE)" <liujian56@huawei.com>]

> -----Original Message-----
> From: John Fastabend [mailto:john.fastabend@gmail.com]
> Sent: Friday, October 22, 2021 11:31 PM
> To: liujian (CE) <liujian56@huawei.com>; 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) <liujian56@huawei.com>
> Subject: RE: [PATHC bpf v5 3/3] selftests, bpf: Add one test for sockmap with
> strparser
> 
> Liu Jian wrote:
> > Add the test to check sockmap with strparser is working well.
> >
> > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > ---
> >  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?
> 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.

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;
}
> 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.
> 
> Thanks!
> John