netdev - Re: [PATCH bpf-next v2] selftests/xsk: fix for SEND_RECEIVE

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

Message-ID: <CAADnVQLEXLO-5CHcY3mVPjtLq7TCgkyYXpqm73aakBCdr3Kg4w@mail.gmail.com>
Date: Thu, 14 Dec 2023 05:36:42 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: Tushar Vyavahare <tushar.vyavahare@...el.com>, bpf <bpf@...r.kernel.org>, 
	Network Development <netdev@...r.kernel.org>, Björn Töpel <bjorn@...nel.org>, 
	"Karlsson, Magnus" <magnus.karlsson@...el.com>, Jonathan Lemon <jonathan.lemon@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	"Sarkar, Tirthendu" <tirthendu.sarkar@...el.com>
Subject: Re: [PATCH bpf-next v2] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test

On Thu, Dec 14, 2023 at 5:23 AM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Thu, Dec 14, 2023 at 01:00:07PM +0000, Tushar Vyavahare wrote:
>
> I think target tree should be bpf, not bpf-next

no. selftests/bpf are fine with bpf-next 99% of the time.

> > Fix test broken by shared umem test and framework enhancement commit.
> >
> > Correct the current implementation of pkt_stream_replace_half() by
> > ensuring that nb_valid_entries are not set to half, as this is not true
> > for all the tests. Ensure that the expected value for valid_entries for
> > the SEND_RECEIVE_UNALIGNED test equals the total number of packets sent,
> > which is 4096.
> >
> > Create a new function called pkt_stream_pkt_set() that allows for packet
> > modification to meet specific requirements while ensuring the accurate
> > maintenance of the valid packet count to prevent inconsistencies in packet
> > tracking.
> >
> > Acked-by: Magnus Karlsson <magnus.karlsson@...el.com>
> > Fixes: 6d198a89c004 ("selftests/xsk: Add a test for shared umem feature")
> > Reported-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@...el.com>
>
> besides subject fix,
>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
>
> >
> > ---
> > v1->v2
> > - Updated git commit message for better clarity as suggested in the
> >   review. [Maciej]
> > - Renamed pkt_valid() to set_pkt_valid() for better clarity. [Maciej]
> > - Fixed double space issue. [Maciej]
> > - Included Magnus's acknowledgement.
> > - Remove the redundant part from the set_pkt_valid() if condition.
> >   [Maciej]
> > - remove pkt_modify().
> > - added pkt_stream_pkt_set(). [Magnus]
> > - renamed mod_valid to prev_pkt_valid. [Tirtha]
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 25 +++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index b604c570309a..b1102ee13faa 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -634,16 +634,24 @@ static u32 pkt_nb_frags(u32 frame_size, struct pkt_stream *pkt_stream, struct pk
> >       return nb_frags;
> >  }
> >
> > +static bool set_pkt_valid(int offset, u32 len)
> > +{
> > +     return len <= MAX_ETH_JUMBO_SIZE;
> > +}
> > +
> >  static void pkt_set(struct pkt_stream *pkt_stream, struct pkt *pkt, int offset, u32 len)
> >  {
> >       pkt->offset = offset;
> >       pkt->len = len;
> > -     if (len > MAX_ETH_JUMBO_SIZE) {
> > -             pkt->valid = false;
> > -     } else {
> > -             pkt->valid = true;
> > -             pkt_stream->nb_valid_entries++;
> > -     }
> > +     pkt->valid = set_pkt_valid(offset, len);
> > +}
> > +
> > +static void pkt_stream_pkt_set(struct pkt_stream *pkt_stream, struct pkt *pkt, int offset, u32 len)
> > +{
> > +     bool prev_pkt_valid = pkt->valid;
> > +
> > +     pkt_set(pkt_stream, pkt, offset, len);
> > +     pkt_stream->nb_valid_entries += pkt->valid - prev_pkt_valid;
> >  }
> >
> >  static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
> > @@ -665,7 +673,7 @@ static struct pkt_stream *__pkt_stream_generate(u32 nb_pkts, u32 pkt_len, u32 nb
> >       for (i = 0; i < nb_pkts; i++) {
> >               struct pkt *pkt = &pkt_stream->pkts[i];
> >
> > -             pkt_set(pkt_stream, pkt, 0, pkt_len);
> > +             pkt_stream_pkt_set(pkt_stream, pkt, 0, pkt_len);
> >               pkt->pkt_nb = nb_start + i * nb_off;
> >       }
> >
> > @@ -700,10 +708,9 @@ static void __pkt_stream_replace_half(struct ifobject *ifobj, u32 pkt_len,
> >
> >       pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream);
> >       for (i = 1; i < ifobj->xsk->pkt_stream->nb_pkts; i += 2)
> > -             pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
> > +             pkt_stream_pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
> >
> >       ifobj->xsk->pkt_stream = pkt_stream;
> > -     pkt_stream->nb_valid_entries /= 2;
> >  }
> >
> >  static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int offset)
> > --
> > 2.34.1
> >
>