On Wed, 6 May 2020 23:09:21 -0700 Martin KaFai Lau <kafai@xxxxxx> wrote: > On Wed, May 06, 2020 at 03:32:06PM -0700, Stanislav Fomichev wrote: > > Move the following routines that let us start a background listener > > thread and connect to a server by fd to the test_prog: > > * start_server_thread - start background INADDR_ANY thread > > * stop_server_thread - stop the thread > > * connect_to_fd - connect to the server identified by fd > > > > These will be used in the next commit. > > > > Also, extend these helpers to support AF_INET6 and accept the family > > as an argument. > > > > v3: > > * export extra helper to start server without a thread (Martin KaFai Lau) > > > > v2: > > * put helpers into network_helpers.c (Andrii Nakryiko) > > > > Cc: Andrey Ignatov <rdna@xxxxxx> > > Cc: Martin KaFai Lau <kafai@xxxxxx> > > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx> > > --- Thanks for extracting network_helpers. I've seen network syscall wrappers repeat between the tests I've touched. Will be super useful to have it in one place. [...] > I still don't see a thread is needed in this existing test_tcp_rtt also. > > I was hoping the start/stop_server_thread() helpers can be removed. > I am not positive the future tests will find start/stop_server_thread() > useful as is because it only does accept() and there is no easy way to > get the accept-ed() fd. > > If this test needs to keep the thread, may be only keep the > start/stop_server_thread() in this test for now until > there is another use case that can benefit from them. > > Keep the start_server() and connect_to_fd() in the new > network_helpers.c. I think at least sk_assign.c (and likely > a few others) should be able to reuse them later. They > are doing something very close to start_server() and > connect_to_fd(). > > Thoughts? IMHO starting a thread to accept() a connection and close it is an overkill. I wouldn't spawn a thread if I can get away with interleaving client and server operations on sockets on the main thread. Makes the test code easier to follow, and strace, for me.
