netdev - Re: [PATCH bpf-next] 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] [day] [month] [year] [list]

Date: Tue, 14 Nov 2023 18:03:23 +0100
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: "Vyavahare, Tushar" <tushar.vyavahare@...el.com>
CC: "bpf@...r.kernel.org" <bpf@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "bjorn@...nel.org" <bjorn@...nel.org>, "Karlsson,
 Magnus" <magnus.karlsson@...el.com>, "jonathan.lemon@...il.com"
	<jonathan.lemon@...il.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"ast@...nel.org" <ast@...nel.org>, "daniel@...earbox.net"
	<daniel@...earbox.net>, "Sarkar, Tirthendu" <tirthendu.sarkar@...el.com>
Subject: Re: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED
 test.

On Mon, Nov 13, 2023 at 12:20:46PM +0100, Maciej Fijalkowski wrote:
> On Mon, Nov 13, 2023 at 07:42:09AM +0100, Vyavahare, Tushar wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Fijalkowski, Maciej <maciej.fijalkowski@...el.com>
> > > Sent: Wednesday, November 8, 2023 8:01 PM
> > > To: Vyavahare, Tushar <tushar.vyavahare@...el.com>
> > > Cc: bpf@...r.kernel.org; netdev@...r.kernel.org; bjorn@...nel.org; Karlsson,
> > > Magnus <magnus.karlsson@...el.com>; jonathan.lemon@...il.com;
> > > davem@...emloft.net; kuba@...nel.org; pabeni@...hat.com;
> > > ast@...nel.org; daniel@...earbox.net; Sarkar, Tirthendu
> > > <tirthendu.sarkar@...el.com>
> > > Subject: Re: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED
> > > test.
> > > 
> > > On Fri, Nov 03, 2023 at 02:29:36PM +0000, Tushar Vyavahare wrote:
> > > > 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.
> > > 
> > > Please be more specific - so what is the expected value for nb_valid_entries for
> > > unaligned mode test then, if not the half?
> > > 
> > 
> > The expected value for nb_valid_entries for the SEND_RECEIVE_UNALIGNED
> > test would be equal to the total number of packets sent.
> > 
> > > >
> > > > Create a new function called pkt_modify() 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.
> > > >
> > > > 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>
> > > > ---
> > > >  tools/testing/selftests/bpf/xskxceiver.c | 71
> > > > ++++++++++++++++--------
> > > >  1 file changed, 47 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > > > b/tools/testing/selftests/bpf/xskxceiver.c
> > > > index 591ca9637b23..f7d3a4a9013f 100644
> > > > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > > > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > > > @@ -634,16 +634,35 @@ static u32 pkt_nb_frags(u32 frame_size, struct
> > > pkt_stream *pkt_stream, struct pk
> > > >  	return nb_frags;
> > > >  }
> > > >
> > > > -static void pkt_set(struct pkt_stream *pkt_stream, struct pkt *pkt,
> > > > int offset, u32 len)
> > > > +static bool pkt_valid(bool unaligned_mode, int offset, u32 len)
> > > 
> > > kinda confusing to have is_pkt_valid() and pkt_valid() functions...
> > > maybe name this as set_pkt_valid() ? doesn't help much but anyways.
> > > 
> > 
> > will do it.
> > 
> > > > +{
> > > > +	if (len > MAX_ETH_JUMBO_SIZE || (!unaligned_mode && offset < 0))
> > > > +		return false;
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > > +static void pkt_set(struct pkt_stream *pkt_stream, struct xsk_umem_info
> > > *umem, struct pkt *pkt,
> > > > +		    int offset, u32 len)
> > > 
> > > How about adding a bool unaligned to pkt_stream instead of passing whole
> > > xsk_umem_info to pkt_set - wouldn't this make the diff smaller?
> > > 
> > 
> > We can also do it this way, but in this case, the difference will be
> > larger. Wherever we are using "struct pkt_stream *pkt_stream," we must set
> > this bool flag again. For example, in places like 
> > __pkt_stream_replace_half(), __pkt_stream_generate_custom() , and a few
> > more. I believe we should stick with the current approach.
> 
> We have a default pkt streams that are restored in run_pkt_test(), so I
> believe that setting this unaligned flag could be scoped to each test_func
> that is related to unaligned mode tests?

Ok now I see that we are sort of losing context when generating pkt
streams, that's a bit unfortunate in this case. Maybe we can think of some
refactor later on.