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

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

> -----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
> 
> 
> 
> > -----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.
> 
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;
> }
> > 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