Jakub Sitnicki wrote:
> On Sun, Feb 09, 2020 at 03:41 AM CET, Alexei Starovoitov wrote:
> > On Thu, Feb 6, 2020 at 3:28 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> wrote:
> >>
> >> Commit 7e81a3530206 ("bpf: Sockmap, ensure sock lock held during tear
> >> down") introduced sleeping issues inside RCU critical sections and while
> >> holding a spinlock on sockmap/sockhash tear-down. There has to be at least
> >> one socket in the map for the problem to surface.
> >>
> >> This adds a test that triggers the warnings for broken locking rules. Not a
> >> fix per se, but rather tooling to verify the accompanying fixes. Run on a
> >> VM with 1 vCPU to reproduce the warnings.
> >>
> >> Fixes: 7e81a3530206 ("bpf: Sockmap, ensure sock lock held during tear down")
> >> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> >
> > selftests/bpf no longer builds for me.
> > make
> > BINARY test_maps
> > TEST-OBJ [test_progs] sockmap_basic.test.o
> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c:
> > In function ‘connected_socket_v4’:
> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c:20:11:
> > error: ‘TCP_REPAIR_ON’ undeclared (first use in this function); did
> > you mean ‘TCP_REPAIR’?
> > 20 | repair = TCP_REPAIR_ON;
> > | ^~~~~~~~~~~~~
> > | TCP_REPAIR
> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c:20:11:
> > note: each undeclared identifier is reported only once for each
> > function it appears in
> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c:29:11:
> > error: ‘TCP_REPAIR_OFF_NO_WP’ undeclared (first use in this function);
> > did you mean ‘TCP_REPAIR_OPTIONS’?
> > 29 | repair = TCP_REPAIR_OFF_NO_WP;
> > | ^~~~~~~~~~~~~~~~~~~~
> > | TCP_REPAIR_OPTIONS
> >
> > Clearly /usr/include/linux/tcp.h is too old.
> > Suggestions?
>
> Sorry for the inconvenience. I see that tcp.h header is missing under
> linux/tools/include/uapi/.
How about we just add the couple defines needed to sockmap_basic.c I don't
see a need to pull in all of tcp.h just for a couple defines that wont
change anyways.
>
> I have been building against my distro kernel headers, completely
> unaware of this. This is an oversight on my side.
>
> Can I ask for a revert? I'm traveling today with limited ability to
> post patches.
I don't think we need a full revert.
>
> I can resubmit the test with the missing header for bpf-next once it
> reopens.
If you are traveling I'll post a patch with the defines.
