On 05/06, Martin KaFai Lau wrote:
On Wed, May 06, 2020 at 09:28:02AM -0700, sdf@xxxxxxxxxx wrote: > On 05/06, Martin KaFai Lau wrote: > > On Tue, May 05, 2020 at 01:27:26PM -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. > > The refactoring itself looks fine. > > > If I read it correctly, it is a simple connect() test. > > I am not sure a thread is even needed. accept() is also unnecessary. > > Can all be done in one thread? > I'm looking at the socket address after connection is established (to If I read it correctly, it is checking the local address (getsockname()) of the client's connect-ed() fd instead of the server's accept-ed() fd.
> verify that the port is the one we were supposed to be using), so > I fail to understand how accept() is unnecessary. Care to clarify? > > I thought about doing a "listen() > non-blocking connect() > accept()" It should not need non-blocking connect(). The client connect() (3WHS) can still finish before the server side accept() is called. If the test does not need the accept-ed() fd, then calling it or not is optional.
Ah, I see what you're saying, in this case I can expose extra helper from network_helpers to do only socket+bind part for the server. Thanks for the explanation!
Just took a quick look, sk_assign.c and test_sock_addr.c could be good examples. They use SO_RCVTIMEO/SO_SNDTIMEO for timeout also.
> in a single thread instead of background thread, but then decided that > it's better to reuse existing helpers and do proper connection instead > of writing all this new code.