Re: [PATCH bpf-next v10 02/12] selftests/bpf: Use start_server_str in sockmap_ktls

[Geliang Tang <geliang@kernel.org>]

On Mon, 2024-07-08 at 10:29 +0800, Geliang Tang wrote:
> > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > 
> > > > Include network_helpers.h in prog_tests/sockmap_ktls.c, use
> > > > public
> > > > network
> > > > helper start_server_str() instead of local defined function
> > > > tcp_server().
> > > > This can avoid duplicate code.
> > > > 
> > > > Technically, this is not a one-for-one replacement, as
> > > > start_server_str()
> > > > also does bind(). But the difference does not seem to matter.

I'd like to add a "nobind" flag in network_helper_opts to handle this.

And another flag "nolisten" to allow to use start_server_str() in
test_sockmap_ktls_update_fails_when_sock_has_ulp() too.

These two flags are also useful for later commits such as "use
start_server_str() in test_insert_bound()" in
prog_tests/sockmap_listen.c.

I changed the states of patches 2-4 (for sockmap_ktls) as "Changes
Requested".

Other 9 patches for sk_lookup in this set are still valid, and no
conflict.

Thanks,
-Geliang

> > > > 
> > > > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > > > ---
> > > >  .../selftests/bpf/prog_tests/sockmap_ktls.c   | 23
> > > > +++++--------------
> > > >  1 file changed, 6 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git
> > > > a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> > > > b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> > > > index 2d0796314862..32be112967a5 100644
> > > > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> > > > @@ -6,25 +6,11 @@
> > > >  
> > > >  #include <netinet/tcp.h>
> > > >  #include "test_progs.h"
> > > > +#include "network_helpers.h"
> > > >  
> > > >  #define MAX_TEST_NAME 80
> > > >  #define TCP_ULP 31
> > > >  
> > > > -static int tcp_server(int family)
> > > > -{
> > > > - int err, s;
> > > > -
> > > > - s = socket(family, SOCK_STREAM, 0);
> > > > - if (!ASSERT_GE(s, 0, "socket"))
> > > > - return -1;
> > > > -
> > > > - err = listen(s, SOMAXCONN);
> > > > - if (!ASSERT_OK(err, "listen"))
> > > > - return -1;
> > > > -
> > > > - return s;
> > > > -}
> > > > -
> > > >  static int disconnect(int fd)
> > > >  {
> > > >   struct sockaddr unspec = { AF_UNSPEC };
> > > > @@ -35,12 +21,15 @@ static int disconnect(int fd)
> > > >  /* Disconnect (unhash) a kTLS socket after removing it from
> > > > sockmap.
> > > > */
> > > >  static void test_sockmap_ktls_disconnect_after_delete(int
> > > > family,
> > > > int map)
> > > >  {
> > > > + struct network_helper_opts opts = {
> > > > + .backlog = SOMAXCONN,
> > > > + };
> > > >   struct sockaddr_storage addr = {0};
> > > >   socklen_t len = sizeof(addr);
> > > >   int err, cli, srv, zero = 0;
> > > >  
> > > > - srv = tcp_server(family);
> > > > - if (srv == -1)
> > > > + srv = start_server_str(family, SOCK_STREAM, NULL, 0, &opts);
> > > > + if (!ASSERT_GE(srv, 0, "start_server_str"))
> > > >   return;
> > > >  
> > > >   err = getsockname(srv, (struct sockaddr *)&addr, &len);