On Thu, May 28, 2020 at 08:08 AM CEST, Andrii Nakryiko wrote: > On Wed, May 27, 2020 at 12:16 PM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> wrote: >> >> Extend the existing test case for flow dissector attaching to cover: >> >> - link creation, >> - link updates, >> - link info querying, >> - mixing links with direct prog attachment. >> >> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> >> --- > > You are not using bpf_program__attach_netns() at all. Would be nice to > actually use higher-level API here... That's true. I didn't exercise the high-level API. I can cover that. > > Also... what's up with people using CHECK_FAIL + perror instead of > CHECK? Is CHECK being avoided for some reason or people are just not > aware of it (which is strange, because CHECK was there before > CHECK_FAIL)? I can only speak for myself. Funnily enough I think I've switched from CHECK to CHECK_FAIL when I touched on BPF flow dissector last time [0]. CHECK needs and "external" duration variable to be in scope, and so it was suggested to me that if I'm not measuring run-time with bpf_prog_test_run, CHECK_FAIL might be a better choice. CHECK is also perhaps too verbose because it emits a log message on success (to report duration, I assume). You have a better overview of all the tests than me, but if I had the cycles I'd see if renaming CHECK to something more specific, for those test that actually track prog run time, can work. -jkbs [0] https://lore.kernel.org/bpf/87imov1y5m.fsf@xxxxxxxxxxxxxx/ > >> .../bpf/prog_tests/flow_dissector_reattach.c | 500 +++++++++++++++++- >> 1 file changed, 471 insertions(+), 29 deletions(-) >> > > [...]
