> -----Original Message----- > From: John Fastabend [mailto:john.fastabend@xxxxxxxxx] > Sent: Friday, October 22, 2021 11:31 PM > To: liujian (CE) <liujian56@xxxxxxxxxx>; john.fastabend@xxxxxxxxx; > daniel@xxxxxxxxxxxxx; jakub@xxxxxxxxxxxxxx; lmb@xxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; ast@xxxxxxxxxx; > andrii@xxxxxxxxxx; kafai@xxxxxx; songliubraving@xxxxxx; yhs@xxxxxx; > kpsingh@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx; > xiyou.wangcong@xxxxxxxxx > Cc: liujian (CE) <liujian56@xxxxxxxxxx> > 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@xxxxxxxxxx> > > --- > > 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@xxxxxxxxx> > > 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
