Re: [PATCH bpf-next] selftests/bpf: Handle EAGAIN in bpf_tcp_ca

[Geliang Tang <geliang@kernel.org>]

Hi Martin,

On Thu, 2024-03-28 at 09:55 -0700, Martin KaFai Lau wrote:
> On 3/28/24 3:23 AM, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > bpf_tcp_ca tests may emit EAGAIN sometimes. In that case, tests
> > fail with
> > "bytes != total_bytes" errors. Sending should continue, not break
> > when
> > errno is EAGAIN. This patch can make bpf_tcp_ca tests stable.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >   tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
> > b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
> > index 077b107130f6..fbc219c2d53b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
> > @@ -56,7 +56,7 @@ static void *server(void *arg)
> >   	while (bytes < total_bytes && !READ_ONCE(stop)) {
> >   		nr_sent = send(fd, &batch,
> >   			       MIN(total_bytes - bytes,
> > sizeof(batch)), 0);
> > -		if (nr_sent == -1 && errno == EINTR)
> > +		if (nr_sent == -1 && (errno == EINTR || errno ==
> > EAGAIN))
> 
> This is a non blocking socket. EAGAIN is hitting the timeout
> situation?
> 
> The default timeout is 3s and it has not been changed after the
> recent 
> connect_fd_to_fd and start_server simplifications. I don't find bpf
> CI failing 
> in this test in the last month also.
> 
> I would prefer to fail after timeout instead of keep retrying. Do you
> really hit 
> that in your environment for this specific bpf_tcp_ca test? There are
> many tests 
> using this timeout value also.

This is the 2nd patch of "refactor mptcp bpf tests" series:

https://patchwork.kernel.org/project/mptcp/cover/cover.1711688054.git.tanggeliang@kylinos.cn/

I didn't get the mentioned EAGAIN errors in bpf_tcp_ca tests, but got
them in MPTCP BPF sched tests (see patch 1). MPTCP BPF sched tests (not
upstream yet) use the same sending and receiving functions as
bpf_tcp_ca tests (patch 15). So it makes sense to add this fix for
bpf_tcp_ca tests too.

And here's another reason. I want to move these functions from
bpf_tcp_ca into network_helpers as public ones (patch 4), which can be
used by both bpf_tcp_ca and MPTCP BPF sched tests. So we must add this
fix to the public ones too.

Maybe the commit log of this patch needs to be updated. Or I should
send patches 2, 3 and 4 together to bpf-next?

I'd like to hear your opinion.

Thanks,
-Geliang