diff mbox series

[bpf-next,v2] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test

Message ID 20231214130007.33281-1-tushar.vyavahare@intel.com (mailing list archive)
State Accepted
Commit 2e1d6a04116c373fbd25beddba4267178535bc60
Delegated to: BPF
Headers show
Series [bpf-next,v2] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
netdev/tree_selection success Clearly marked for bpf-next
netdev/apply fail Patch does not apply to bpf-next

Commit Message

Tushar Vyavahare Dec. 14, 2023, 1 p.m. UTC 
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@intel.com>
Fixes: 6d198a89c004 ("selftests/xsk: Add a test for shared umem feature")
Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.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(-)

Comments

Fijalkowski, Maciej Dec. 14, 2023, 1:23 p.m. UTC | #1 
On Thu, Dec 14, 2023 at 01:00:07PM +0000, Tushar Vyavahare wrote:

I think target tree should be bpf, not bpf-next

> 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@intel.com>
> Fixes: 6d198a89c004 ("selftests/xsk: Add a test for shared umem feature")
> Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>

besides subject fix,

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.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
>
Alexei Starovoitov Dec. 14, 2023, 1:36 p.m. UTC | #2 
On Thu, Dec 14, 2023 at 5:23 AM Maciej Fijalkowski
<maciej.fijalkowski@intel.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@intel.com>
> > Fixes: 6d198a89c004 ("selftests/xsk: Add a test for shared umem feature")
> > Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
>
> besides subject fix,
>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.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
> >
>
patchwork-bot+netdevbpf@kernel.org Dec. 14, 2023, 3:20 p.m. UTC | #3 
Hello:

This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu, 14 Dec 2023 13:00:07 +0000 you 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. Ensure that the expected value for valid_entries for
> the SEND_RECEIVE_UNALIGNED test equals the total number of packets sent,
> which is 4096.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test
    https://git.kernel.org/bpf/bpf-next/c/2e1d6a04116c

You are awesome, thank you!
diff mbox series

Patch

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)