On Fri, Apr 19, 2019 at 04:29:44PM -0700, Stanislav Fomichev wrote: > On 04/18, Alexei Starovoitov wrote: > > On Thu, Apr 18, 2019 at 05:43:50PM -0700, Stanislav Fomichev wrote: > > > On 04/18, Alexei Starovoitov wrote: > > > > On Mon, Apr 15, 2019 at 10:38:00AM -0700, Stanislav Fomichev wrote: > > > > > Update all users eth_get_headlen to pass network namespace > > > > > and pass it down to the flow dissector. This commit is a noop > > > > > until administrator inserts BPF flow dissector program. > > > > > > > > > > Cc: Maxim Krasnyansky <maxk@xxxxxxxxxxxxxxxx> > > > > > Cc: Saeed Mahameed <saeedm@xxxxxxxxxxxx> > > > > > Cc: Jeff Kirsher <jeffrey.t.kirsher@xxxxxxxxx> > > > > > Cc: intel-wired-lan@xxxxxxxxxxxxxxxx > > > > > Cc: Yisen Zhuang <yisen.zhuang@xxxxxxxxxx> > > > > > Cc: Salil Mehta <salil.mehta@xxxxxxxxxx> > > > > > Cc: Michael Chan <michael.chan@xxxxxxxxxxxx> > > > > > Cc: Igor Russkikh <igor.russkikh@xxxxxxxxxxxx> > > > > > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx> > > ... > > > > Also please add C based test for skb-less flow_dissector. > > > > Current test_flow_dissector.sh doesn't seem to cover it. > > > It doesn't look like we can exercise skb-less flow dissector from > > > test_flow_dissector.sh; we need to trigger some driver code, which is > > > hard when we send the packets on the localhost in > > > test_flow_dissector.sh. > > > > > > To test skb-less dissector I convert BPF_PROG_TEST_RUN to always use skb-less > > > mode. test_flow_dissector.sh tests skb-mode, prog_tests/flow_dissector.c > > > tests skb-less mode. > > > > I saw that but I'm afraid it's not enough. > > tun_get_user() is calling it, so it should be possible to test > > skb-less mode via tun. > Spent some time today looking into how to exercise this path in the tun > driver: doing writev() with IFF_NAPI_FRAGS IFF_TAP device would trigger > eth_get_headlen, but it looks like there is no way to do a test with > pass/no-pass result around that. > > The problem is - we don't actually do anything with the result of > eth_get_headlen, there is only a sanity check for "headlen > > skb_headlen(skb)" which can't trigger for BPF flow dissector; we > carefully clamp thoff/nhoff and should not return offset outside the > input buffer. > > By reading git history it looks like this call to eth_get_headlen was > added there to only make it possible for tools like syzbot to fuzz flow > dissector. That's why we don't care about the result, we just do that > simple sanity check. The main goal is to trigger some problem > (loop/warning) in the flow dissector code. > > tl;dr - no mater which bpf flow dissector is attached to the namespace, > it would not change behavior of the tun device; even empty 'return > false' program would not alter it. sure, but the program will run and the test can validate that the program saw valid packet, parsed it correctly and returned correct dissection. The results can be stored in a map and validated by the test. iirc you were saying that you'll have one program doing dissection for with-skb and skb-less cases. I think it's important to have such program in selftests and being run continuously for both cases.
