On Wed, 29 Mar 2023 at 20:11, Kal Conley <kal.conley@xxxxxxxxxxx> wrote:
>
> Fix flaky STATS_RX_DROPPED test. The receiver calls getsockopt after
> receiving the last (valid) packet which is not the final packet sent in
> the test (valid and invalid packets are sent in alternating fashion with
> the final packet being invalid). Since the last packet may or may not
> have been dropped already, both outcomes must be allowed.
>
> This issue could also be fixed by making sure the last packet sent is
> valid. This alternative is left as an exercise to the reader (or the
> benevolent maintainers of this file).
>
> This problem was quite visible on certain setups. On one machine this
> failure was observed 50% of the time.
>
> Also, remove a redundant assignment of pkt_stream->nb_pkts. This field
> is already initialized by __pkt_stream_alloc.
This has been bugging me for a while so thanks for fixing this. Please
break this commit out of this patch set and send it as a separate bug
fix.
Acked-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
> Fixes: 27e934bec35b ("selftests: xsk: make stat tests not spin on getsockopt")
> Signed-off-by: Kal Conley <kal.conley@xxxxxxxxxxx>
> ---
> tools/testing/selftests/bpf/xskxceiver.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 34a1f32fe752..1a4bdd5aa78c 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -633,7 +633,6 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
> if (!pkt_stream)
> exit_with_error(ENOMEM);
>
> - pkt_stream->nb_pkts = nb_pkts;
> for (i = 0; i < nb_pkts; i++) {
> pkt_set(umem, &pkt_stream->pkts[i], (i % umem->num_frames) * umem->frame_size,
> pkt_len);
> @@ -1141,7 +1140,14 @@ static int validate_rx_dropped(struct ifobject *ifobject)
> if (err)
> return TEST_FAILURE;
>
> - if (stats.rx_dropped == ifobject->pkt_stream->nb_pkts / 2)
> + /* The receiver calls getsockopt after receiving the last (valid)
> + * packet which is not the final packet sent in this test (valid and
> + * invalid packets are sent in alternating fashion with the final
> + * packet being invalid). Since the last packet may or may not have
> + * been dropped already, both outcomes must be allowed.
> + */
> + if (stats.rx_dropped == ifobject->pkt_stream->nb_pkts / 2 ||
> + stats.rx_dropped == ifobject->pkt_stream->nb_pkts / 2 - 1)
> return TEST_PASS;
>
> return TEST_FAILURE;
> --
> 2.39.2
>
