netdev - Re: [PATCH bpf-next v2 1/5] selftests/bpf: generalize helpers to control background listener

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 May 2020 10:48:44 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     <sdf@...gle.com>
CC:     <netdev@...r.kernel.org>, <bpf@...r.kernel.org>,
        <davem@...emloft.net>, <ast@...nel.org>, <daniel@...earbox.net>,
        Andrey Ignatov <rdna@...com>
Subject: Re: [PATCH bpf-next v2 1/5] selftests/bpf: generalize helpers to
 control background listener

On Wed, May 06, 2020 at 09:28:02AM -0700, sdf@...gle.com 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.

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.