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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx/
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
