netdev - Re: [PATCH v3 bpf-next 10/11] selftests: xsk: remove struct xsk_socket_info::outstanding

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] [day] [month] [year] [list]

Message-ID: <CAJ8uoz1_ygfd9rU20MpxFLG=0sPnEutr8WtowaySL7z1ERPw_A@mail.gmail.com>
Date:   Thu, 16 Jun 2022 14:28:07 +0200
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn@...nel.org>
Subject: Re: [PATCH v3 bpf-next 10/11] selftests: xsk: remove struct xsk_socket_info::outstanding_tx

On Wed, Jun 15, 2022 at 6:18 PM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> Previous change makes xsk->outstanding_tx a dead code, so let's remove
> it.
>
> Acked-by: Magnus Karlsson <magnus.karlsson@...el.com>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 20 +++-----------------
>  tools/testing/selftests/bpf/xdpxceiver.h |  1 -
>  2 files changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index 13a3b2ac2399..ade9d87e7a7c 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -815,7 +815,7 @@ static void kick_rx(struct xsk_socket_info *xsk)
>                 exit_with_error(errno);
>  }
>
> -static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
> +static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
>  {
>         unsigned int rcvd;
>         u32 idx;
> @@ -824,20 +824,8 @@ static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
>                 kick_tx(xsk);
>
>         rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
> -       if (rcvd) {
> -               if (rcvd > xsk->outstanding_tx) {
> -                       u64 addr = *xsk_ring_cons__comp_addr(&xsk->umem->cq, idx + rcvd - 1);
> -
> -                       ksft_print_msg("[%s] Too many packets completed\n", __func__);
> -                       ksft_print_msg("Last completion address: %llx\n", addr);
> -                       return TEST_FAILURE;
> -               }

Actually Maciej, I would like to keep this test. The more sanity
checks we can do, the better. The bug that was just fixed in commit
a6e944f25cdb ("xsk: Fix generic transmit when completion queue
reservation fails") can be caught by this check as the bug introduced
more completions than packets received.

So please drop this patch for the v4.

Thanks: Magnus

> -
> +       if (rcvd)
>                 xsk_ring_cons__release(&xsk->umem->cq, rcvd);
> -               xsk->outstanding_tx -= rcvd;
> -       }
> -
> -       return TEST_PASS;
>  }
>
>  static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
> @@ -955,9 +943,7 @@ static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb)
>         pthread_mutex_unlock(&pacing_mutex);
>
>         xsk_ring_prod__submit(&xsk->tx, i);
> -       xsk->outstanding_tx += valid_pkts;
> -       if (complete_pkts(xsk, i))
> -               return TEST_FAILURE;
> +       complete_pkts(xsk, i);
>
>         usleep(10);
>         return TEST_PASS;
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index f364a92675f8..12b792004163 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -104,7 +104,6 @@ struct xsk_socket_info {
>         struct xsk_ring_prod tx;
>         struct xsk_umem_info *umem;
>         struct xsk_socket *xsk;
> -       u32 outstanding_tx;
>         u32 rxqsize;
>  };
>
> --
> 2.27.0
>