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 for Android: free password hash cracker in your pocket

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

Message-Id: <20200506180909.GI241848@google.com>
Date:   Wed, 6 May 2020 11:09:09 -0700
From:   sdf@...gle.com
To:     Martin KaFai Lau <kafai@...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 05/06, Martin KaFai Lau wrote:
> 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.
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.