Jakub Sitnicki wrote: > On Thu, Dec 21, 2023 at 03:23 PM -08, John Fastabend wrote: > > There was a memleak when streaming af_unix sockets were inserted into > > multiple sockmap slots and/or maps. This is because each insert would > > call a proto update operatino and these must be allowed to be called > > multiple times. The streaming af_unix implementation recently added > > a refcnt to handle a use after free issue, however it introduced a > > memleak when inserted into multiple maps. > > > > This series fixes the memleak, adds a note in the code so we remember > > that proto updates need to support this. And then we add three tests > > for each of the slightly different iterations of adding sockets into > > multiple maps. I kept them as 3 independent test cases here. I have > > some slight preference for this they could however be a single test, > > but then you don't get to run them independently which was sort of > > useful while debugging. > > > > John Fastabend (5): > > bpf: sockmap, fix proto update hook to avoid dup calls > > bpf: sockmap, added comments describing update proto rules > > bpf: sockmap, add tests for proto updates many to single map > > bpf: sockmap, add tests for proto updates single socket to many map > > bpf: sockmap, add tests for proto updates replace socket > > > > include/linux/skmsg.h | 5 + > > net/unix/unix_bpf.c | 21 +- > > .../selftests/bpf/prog_tests/sockmap_basic.c | 199 +++++++++++++++++- > > 3 files changed, 221 insertions(+), 4 deletions(-) > > Sorry for the delay. I was out. Thanks for the review. > > This LGTM with some room for improvement in tests. > You repeat the code to create different kind of sockets in each test. > That could be refactored to use some kind of a factory helper. Yeah, my first attempt was uglier than the repeated setup in my opinion. So figured I would get this out and think a bit more about it. Lets see if BPF maintainers want me to fix the typo on Reported-by or if it can be fixed on merged. > > Reviewed-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>