* [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test.
@ 2023-11-03 14:29 Tushar Vyavahare
2023-11-08 9:35 ` Magnus Karlsson
2023-11-08 14:30 ` Maciej Fijalkowski
0 siblings, 2 replies; 6+ messages in thread
From: Tushar Vyavahare @ 2023-11-03 14:29 UTC (permalink / raw)
To: bpf
Cc: netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
jonathan.lemon, davem, kuba, pabeni, ast, daniel,
tirthendu.sarkar, tushar.vyavahare
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.
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@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.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)
+{
+ 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)
{
pkt->offset = offset;
pkt->len = len;
- if (len > MAX_ETH_JUMBO_SIZE) {
- pkt->valid = false;
- } else {
- pkt->valid = true;
+
+ pkt->valid = pkt_valid(umem->unaligned_mode, offset, len);
+ if (pkt->valid)
pkt_stream->nb_valid_entries++;
- }
+}
+
+static void pkt_modify(struct pkt_stream *pkt_stream, struct xsk_umem_info *umem, struct pkt *pkt,
+ int offset, u32 len)
+{
+ bool mod_valid;
+
+ pkt->offset = offset;
+ pkt->len = len;
+ mod_valid = pkt_valid(umem->unaligned_mode, offset, len);
+ pkt_stream->nb_valid_entries += mod_valid - pkt->valid;
+ pkt->valid = mod_valid;
}
static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
@@ -651,7 +670,8 @@ static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
return ceil_u32(len, umem->frame_size) * umem->frame_size;
}
-static struct pkt_stream *__pkt_stream_generate(u32 nb_pkts, u32 pkt_len, u32 nb_start, u32 nb_off)
+static struct pkt_stream *__pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts,
+ u32 pkt_len, u32 nb_start, u32 nb_off)
{
struct pkt_stream *pkt_stream;
u32 i;
@@ -665,30 +685,31 @@ 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_set(pkt_stream, umem, pkt, 0, pkt_len);
pkt->pkt_nb = nb_start + i * nb_off;
}
return pkt_stream;
}
-static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
+static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
{
- return __pkt_stream_generate(nb_pkts, pkt_len, 0, 1);
+ return __pkt_stream_generate(umem, nb_pkts, pkt_len, 0, 1);
}
-static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream)
+static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream,
+ struct xsk_umem_info *umem)
{
- return pkt_stream_generate(pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
+ return pkt_stream_generate(umem, pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
}
static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
{
struct pkt_stream *pkt_stream;
- pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
+ pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, nb_pkts, pkt_len);
test->ifobj_tx->xsk->pkt_stream = pkt_stream;
- pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
+ pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
test->ifobj_rx->xsk->pkt_stream = pkt_stream;
}
@@ -698,12 +719,11 @@ static void __pkt_stream_replace_half(struct ifobject *ifobj, u32 pkt_len,
struct pkt_stream *pkt_stream;
u32 i;
- pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream);
+ pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream, ifobj->umem);
for (i = 1; i < ifobj->xsk->pkt_stream->nb_pkts; i += 2)
- pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
+ pkt_modify(pkt_stream, ifobj->umem, &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)
@@ -715,9 +735,10 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int off
static void pkt_stream_receive_half(struct test_spec *test)
{
struct pkt_stream *pkt_stream = test->ifobj_tx->xsk->pkt_stream;
+ struct xsk_umem_info *umem = test->ifobj_rx->umem;
u32 i;
- test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(pkt_stream->nb_pkts,
+ test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(umem, pkt_stream->nb_pkts,
pkt_stream->pkts[0].len);
pkt_stream = test->ifobj_rx->xsk->pkt_stream;
for (i = 1; i < pkt_stream->nb_pkts; i += 2)
@@ -733,12 +754,12 @@ static void pkt_stream_even_odd_sequence(struct test_spec *test)
for (i = 0; i < test->nb_sockets; i++) {
pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
- pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
+ pkt_stream = __pkt_stream_generate(test->ifobj_tx->umem, pkt_stream->nb_pkts / 2,
pkt_stream->pkts[0].len, i, 2);
test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
- pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
+ pkt_stream = __pkt_stream_generate(test->ifobj_rx->umem, pkt_stream->nb_pkts / 2,
pkt_stream->pkts[0].len, i, 2);
test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
}
@@ -1961,7 +1982,8 @@ static int testapp_stats_tx_invalid_descs(struct test_spec *test)
static int testapp_stats_rx_full(struct test_spec *test)
{
pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
- test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
+ test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
+ DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
test->ifobj_rx->release_rx = false;
@@ -1972,7 +1994,8 @@ static int testapp_stats_rx_full(struct test_spec *test)
static int testapp_stats_fill_empty(struct test_spec *test)
{
pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
- test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
+ test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
+ DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
test->ifobj_rx->use_fill_ring = false;
test->ifobj_rx->validation_func = validate_fill_empty;
@@ -2526,8 +2549,8 @@ int main(int argc, char **argv)
init_iface(ifobj_tx, worker_testapp_validate_tx);
test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
- tx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
- rx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
+ tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
+ rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
if (!tx_pkt_stream_default || !rx_pkt_stream_default)
exit_with_error(ENOMEM);
test.tx_pkt_stream_default = tx_pkt_stream_default;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test.
2023-11-03 14:29 [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test Tushar Vyavahare
@ 2023-11-08 9:35 ` Magnus Karlsson
2023-11-08 14:30 ` Maciej Fijalkowski
1 sibling, 0 replies; 6+ messages in thread
From: Magnus Karlsson @ 2023-11-08 9:35 UTC (permalink / raw)
To: Tushar Vyavahare
Cc: bpf, netdev, bjorn, magnus.karlsson, maciej.fijalkowski,
jonathan.lemon, davem, kuba, pabeni, ast, daniel,
tirthendu.sarkar
On Fri, 3 Nov 2023 at 15:41, Tushar Vyavahare
<tushar.vyavahare@intel.com> 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.
>
> 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.
Thanks for the fix Tushar. While long, this gives the packet stream
modification functionality a better structure.
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>
> ---
> 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)
> +{
> + 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)
> {
> pkt->offset = offset;
> pkt->len = len;
> - if (len > MAX_ETH_JUMBO_SIZE) {
> - pkt->valid = false;
> - } else {
> - pkt->valid = true;
> +
> + pkt->valid = pkt_valid(umem->unaligned_mode, offset, len);
> + if (pkt->valid)
> pkt_stream->nb_valid_entries++;
> - }
> +}
> +
> +static void pkt_modify(struct pkt_stream *pkt_stream, struct xsk_umem_info *umem, struct pkt *pkt,
> + int offset, u32 len)
> +{
> + bool mod_valid;
> +
> + pkt->offset = offset;
> + pkt->len = len;
> + mod_valid = pkt_valid(umem->unaligned_mode, offset, len);
> + pkt_stream->nb_valid_entries += mod_valid - pkt->valid;
> + pkt->valid = mod_valid;
> }
>
> static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
> @@ -651,7 +670,8 @@ static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
> return ceil_u32(len, umem->frame_size) * umem->frame_size;
> }
>
> -static struct pkt_stream *__pkt_stream_generate(u32 nb_pkts, u32 pkt_len, u32 nb_start, u32 nb_off)
> +static struct pkt_stream *__pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts,
> + u32 pkt_len, u32 nb_start, u32 nb_off)
> {
> struct pkt_stream *pkt_stream;
> u32 i;
> @@ -665,30 +685,31 @@ 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_set(pkt_stream, umem, pkt, 0, pkt_len);
> pkt->pkt_nb = nb_start + i * nb_off;
> }
>
> return pkt_stream;
> }
>
> -static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
> +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> {
> - return __pkt_stream_generate(nb_pkts, pkt_len, 0, 1);
> + return __pkt_stream_generate(umem, nb_pkts, pkt_len, 0, 1);
> }
>
> -static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream)
> +static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream,
> + struct xsk_umem_info *umem)
> {
> - return pkt_stream_generate(pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
> + return pkt_stream_generate(umem, pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
> }
>
> static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> {
> struct pkt_stream *pkt_stream;
>
> - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> + pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, nb_pkts, pkt_len);
> test->ifobj_tx->xsk->pkt_stream = pkt_stream;
> - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> + pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> test->ifobj_rx->xsk->pkt_stream = pkt_stream;
> }
>
> @@ -698,12 +719,11 @@ static void __pkt_stream_replace_half(struct ifobject *ifobj, u32 pkt_len,
> struct pkt_stream *pkt_stream;
> u32 i;
>
> - pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream);
> + pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream, ifobj->umem);
> for (i = 1; i < ifobj->xsk->pkt_stream->nb_pkts; i += 2)
> - pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
> + pkt_modify(pkt_stream, ifobj->umem, &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)
> @@ -715,9 +735,10 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int off
> static void pkt_stream_receive_half(struct test_spec *test)
> {
> struct pkt_stream *pkt_stream = test->ifobj_tx->xsk->pkt_stream;
> + struct xsk_umem_info *umem = test->ifobj_rx->umem;
> u32 i;
>
> - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(pkt_stream->nb_pkts,
> + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(umem, pkt_stream->nb_pkts,
> pkt_stream->pkts[0].len);
> pkt_stream = test->ifobj_rx->xsk->pkt_stream;
> for (i = 1; i < pkt_stream->nb_pkts; i += 2)
> @@ -733,12 +754,12 @@ static void pkt_stream_even_odd_sequence(struct test_spec *test)
>
> for (i = 0; i < test->nb_sockets; i++) {
> pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
> - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> + pkt_stream = __pkt_stream_generate(test->ifobj_tx->umem, pkt_stream->nb_pkts / 2,
> pkt_stream->pkts[0].len, i, 2);
> test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
>
> pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
> - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> + pkt_stream = __pkt_stream_generate(test->ifobj_rx->umem, pkt_stream->nb_pkts / 2,
> pkt_stream->pkts[0].len, i, 2);
> test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
> }
> @@ -1961,7 +1982,8 @@ static int testapp_stats_tx_invalid_descs(struct test_spec *test)
> static int testapp_stats_rx_full(struct test_spec *test)
> {
> pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
> + DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
>
> test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
> test->ifobj_rx->release_rx = false;
> @@ -1972,7 +1994,8 @@ static int testapp_stats_rx_full(struct test_spec *test)
> static int testapp_stats_fill_empty(struct test_spec *test)
> {
> pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
> + DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
>
> test->ifobj_rx->use_fill_ring = false;
> test->ifobj_rx->validation_func = validate_fill_empty;
> @@ -2526,8 +2549,8 @@ int main(int argc, char **argv)
> init_iface(ifobj_tx, worker_testapp_validate_tx);
>
> test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
> - tx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> - rx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> + tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> + rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> if (!tx_pkt_stream_default || !rx_pkt_stream_default)
> exit_with_error(ENOMEM);
> test.tx_pkt_stream_default = tx_pkt_stream_default;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test.
2023-11-03 14:29 [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test Tushar Vyavahare
2023-11-08 9:35 ` Magnus Karlsson
@ 2023-11-08 14:30 ` Maciej Fijalkowski
2023-11-13 6:42 ` Vyavahare, Tushar
1 sibling, 1 reply; 6+ messages in thread
From: Maciej Fijalkowski @ 2023-11-08 14:30 UTC (permalink / raw)
To: Tushar Vyavahare
Cc: bpf, netdev, bjorn, magnus.karlsson, jonathan.lemon, davem, kuba,
pabeni, ast, daniel, tirthendu.sarkar
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?
>
> 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@intel.com>
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.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.
> +{
> + 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?
> {
> pkt->offset = offset;
> pkt->len = len;
> - if (len > MAX_ETH_JUMBO_SIZE) {
> - pkt->valid = false;
> - } else {
> - pkt->valid = true;
> +
> + pkt->valid = pkt_valid(umem->unaligned_mode, offset, len);
> + if (pkt->valid)
> pkt_stream->nb_valid_entries++;
> - }
> +}
> +
> +static void pkt_modify(struct pkt_stream *pkt_stream, struct xsk_umem_info *umem, struct pkt *pkt,
> + int offset, u32 len)
> +{
> + bool mod_valid;
> +
> + pkt->offset = offset;
> + pkt->len = len;
> + mod_valid = pkt_valid(umem->unaligned_mode, offset, len);
double space
> + pkt_stream->nb_valid_entries += mod_valid - pkt->valid;
> + pkt->valid = mod_valid;
> }
>
> static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
> @@ -651,7 +670,8 @@ static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len)
> return ceil_u32(len, umem->frame_size) * umem->frame_size;
> }
>
> -static struct pkt_stream *__pkt_stream_generate(u32 nb_pkts, u32 pkt_len, u32 nb_start, u32 nb_off)
> +static struct pkt_stream *__pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts,
> + u32 pkt_len, u32 nb_start, u32 nb_off)
> {
> struct pkt_stream *pkt_stream;
> u32 i;
> @@ -665,30 +685,31 @@ 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_set(pkt_stream, umem, pkt, 0, pkt_len);
> pkt->pkt_nb = nb_start + i * nb_off;
> }
>
> return pkt_stream;
> }
>
> -static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
> +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> {
> - return __pkt_stream_generate(nb_pkts, pkt_len, 0, 1);
> + return __pkt_stream_generate(umem, nb_pkts, pkt_len, 0, 1);
> }
>
> -static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream)
> +static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream,
> + struct xsk_umem_info *umem)
> {
> - return pkt_stream_generate(pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
> + return pkt_stream_generate(umem, pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
> }
>
> static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> {
> struct pkt_stream *pkt_stream;
>
> - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> + pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, nb_pkts, pkt_len);
> test->ifobj_tx->xsk->pkt_stream = pkt_stream;
> - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> + pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> test->ifobj_rx->xsk->pkt_stream = pkt_stream;
> }
>
> @@ -698,12 +719,11 @@ static void __pkt_stream_replace_half(struct ifobject *ifobj, u32 pkt_len,
> struct pkt_stream *pkt_stream;
> u32 i;
>
> - pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream);
> + pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream, ifobj->umem);
> for (i = 1; i < ifobj->xsk->pkt_stream->nb_pkts; i += 2)
> - pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
> + pkt_modify(pkt_stream, ifobj->umem, &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)
> @@ -715,9 +735,10 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int off
> static void pkt_stream_receive_half(struct test_spec *test)
> {
> struct pkt_stream *pkt_stream = test->ifobj_tx->xsk->pkt_stream;
> + struct xsk_umem_info *umem = test->ifobj_rx->umem;
> u32 i;
>
> - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(pkt_stream->nb_pkts,
> + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(umem, pkt_stream->nb_pkts,
> pkt_stream->pkts[0].len);
> pkt_stream = test->ifobj_rx->xsk->pkt_stream;
> for (i = 1; i < pkt_stream->nb_pkts; i += 2)
> @@ -733,12 +754,12 @@ static void pkt_stream_even_odd_sequence(struct test_spec *test)
>
> for (i = 0; i < test->nb_sockets; i++) {
> pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
> - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> + pkt_stream = __pkt_stream_generate(test->ifobj_tx->umem, pkt_stream->nb_pkts / 2,
> pkt_stream->pkts[0].len, i, 2);
> test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
>
> pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
> - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> + pkt_stream = __pkt_stream_generate(test->ifobj_rx->umem, pkt_stream->nb_pkts / 2,
> pkt_stream->pkts[0].len, i, 2);
> test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
> }
> @@ -1961,7 +1982,8 @@ static int testapp_stats_tx_invalid_descs(struct test_spec *test)
> static int testapp_stats_rx_full(struct test_spec *test)
> {
> pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
> + DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
>
> test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
> test->ifobj_rx->release_rx = false;
> @@ -1972,7 +1994,8 @@ static int testapp_stats_rx_full(struct test_spec *test)
> static int testapp_stats_fill_empty(struct test_spec *test)
> {
> pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
> + DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
>
> test->ifobj_rx->use_fill_ring = false;
> test->ifobj_rx->validation_func = validate_fill_empty;
> @@ -2526,8 +2549,8 @@ int main(int argc, char **argv)
> init_iface(ifobj_tx, worker_testapp_validate_tx);
>
> test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
> - tx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> - rx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> + tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> + rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> if (!tx_pkt_stream_default || !rx_pkt_stream_default)
> exit_with_error(ENOMEM);
> test.tx_pkt_stream_default = tx_pkt_stream_default;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test.
2023-11-08 14:30 ` Maciej Fijalkowski
@ 2023-11-13 6:42 ` Vyavahare, Tushar
2023-11-13 11:20 ` Maciej Fijalkowski
0 siblings, 1 reply; 6+ messages in thread
From: Vyavahare, Tushar @ 2023-11-13 6:42 UTC (permalink / raw)
To: Fijalkowski, Maciej
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, bjorn@kernel.org,
Karlsson, Magnus, jonathan.lemon@gmail.com, davem@davemloft.net,
kuba@kernel.org, pabeni@redhat.com, ast@kernel.org,
daniel@iogearbox.net, Sarkar, Tirthendu
> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Sent: Wednesday, November 8, 2023 8:01 PM
> To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> Cc: bpf@vger.kernel.org; netdev@vger.kernel.org; bjorn@kernel.org; Karlsson,
> Magnus <magnus.karlsson@intel.com>; jonathan.lemon@gmail.com;
> davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> ast@kernel.org; daniel@iogearbox.net; Sarkar, Tirthendu
> <tirthendu.sarkar@intel.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@intel.com>
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.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.
> > {
> > pkt->offset = offset;
> > pkt->len = len;
> > - if (len > MAX_ETH_JUMBO_SIZE) {
> > - pkt->valid = false;
> > - } else {
> > - pkt->valid = true;
> > +
> > + pkt->valid = pkt_valid(umem->unaligned_mode, offset, len);
> > + if (pkt->valid)
> > pkt_stream->nb_valid_entries++;
> > - }
> > +}
> > +
> > +static void pkt_modify(struct pkt_stream *pkt_stream, struct
> xsk_umem_info *umem, struct pkt *pkt,
> > + int offset, u32 len)
> > +{
> > + bool mod_valid;
> > +
> > + pkt->offset = offset;
> > + pkt->len = len;
> > + mod_valid = pkt_valid(umem->unaligned_mode, offset, len);
>
> double space
>
will do it.
> > + pkt_stream->nb_valid_entries += mod_valid - pkt->valid;
> > + pkt->valid = mod_valid;
> > }
> >
> > static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len) @@
> > -651,7 +670,8 @@ static u32 pkt_get_buffer_len(struct xsk_umem_info
> *umem, u32 len)
> > return ceil_u32(len, umem->frame_size) * umem->frame_size; }
> >
> > -static struct pkt_stream *__pkt_stream_generate(u32 nb_pkts, u32
> > pkt_len, u32 nb_start, u32 nb_off)
> > +static struct pkt_stream *__pkt_stream_generate(struct xsk_umem_info
> *umem, u32 nb_pkts,
> > + u32 pkt_len, u32 nb_start,
> u32 nb_off)
> > {
> > struct pkt_stream *pkt_stream;
> > u32 i;
> > @@ -665,30 +685,31 @@ 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_set(pkt_stream, umem, pkt, 0, pkt_len);
> > pkt->pkt_nb = nb_start + i * nb_off;
> > }
> >
> > return pkt_stream;
> > }
> >
> > -static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32
> > pkt_len)
> > +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info
> > +*umem, u32 nb_pkts, u32 pkt_len)
> > {
> > - return __pkt_stream_generate(nb_pkts, pkt_len, 0, 1);
> > + return __pkt_stream_generate(umem, nb_pkts, pkt_len, 0, 1);
> > }
> >
> > -static struct pkt_stream *pkt_stream_clone(struct pkt_stream
> > *pkt_stream)
> > +static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream,
> > + struct xsk_umem_info *umem)
> > {
> > - return pkt_stream_generate(pkt_stream->nb_pkts, pkt_stream-
> >pkts[0].len);
> > + return pkt_stream_generate(umem, pkt_stream->nb_pkts,
> > +pkt_stream->pkts[0].len);
> > }
> >
> > static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts,
> > u32 pkt_len) {
> > struct pkt_stream *pkt_stream;
> >
> > - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> > + pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, nb_pkts,
> > +pkt_len);
> > test->ifobj_tx->xsk->pkt_stream = pkt_stream;
> > - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> > + pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts,
> > +pkt_len);
> > test->ifobj_rx->xsk->pkt_stream = pkt_stream; }
> >
> > @@ -698,12 +719,11 @@ static void __pkt_stream_replace_half(struct
> ifobject *ifobj, u32 pkt_len,
> > struct pkt_stream *pkt_stream;
> > u32 i;
> >
> > - pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream);
> > + pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream, ifobj-
> >umem);
> > for (i = 1; i < ifobj->xsk->pkt_stream->nb_pkts; i += 2)
> > - pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
> > + pkt_modify(pkt_stream, ifobj->umem, &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) @@ -715,9 +735,10 @@ static void
> > pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int off
> > static void pkt_stream_receive_half(struct test_spec *test) {
> > struct pkt_stream *pkt_stream = test->ifobj_tx->xsk->pkt_stream;
> > + struct xsk_umem_info *umem = test->ifobj_rx->umem;
> > u32 i;
> >
> > - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(pkt_stream-
> >nb_pkts,
> > + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(umem,
> > +pkt_stream->nb_pkts,
> > pkt_stream-
> >pkts[0].len);
> > pkt_stream = test->ifobj_rx->xsk->pkt_stream;
> > for (i = 1; i < pkt_stream->nb_pkts; i += 2) @@ -733,12 +754,12 @@
> > static void pkt_stream_even_odd_sequence(struct test_spec *test)
> >
> > for (i = 0; i < test->nb_sockets; i++) {
> > pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
> > - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts /
> 2,
> > + pkt_stream = __pkt_stream_generate(test->ifobj_tx->umem,
> > +pkt_stream->nb_pkts / 2,
> > pkt_stream->pkts[0].len, i,
> 2);
> > test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
> >
> > pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
> > - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts /
> 2,
> > + pkt_stream = __pkt_stream_generate(test->ifobj_rx->umem,
> > +pkt_stream->nb_pkts / 2,
> > pkt_stream->pkts[0].len, i,
> 2);
> > test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
> > }
> > @@ -1961,7 +1982,8 @@ static int testapp_stats_tx_invalid_descs(struct
> > test_spec *test) static int testapp_stats_rx_full(struct test_spec
> > *test) {
> > pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS +
> DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> > - test->ifobj_rx->xsk->pkt_stream =
> pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test-
> >ifobj_rx->umem,
> > +
> DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> >
> > test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
> > test->ifobj_rx->release_rx = false;
> > @@ -1972,7 +1994,8 @@ static int testapp_stats_rx_full(struct
> > test_spec *test) static int testapp_stats_fill_empty(struct test_spec
> > *test) {
> > pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS +
> DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> > - test->ifobj_rx->xsk->pkt_stream =
> pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test-
> >ifobj_rx->umem,
> > +
> DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> >
> > test->ifobj_rx->use_fill_ring = false;
> > test->ifobj_rx->validation_func = validate_fill_empty; @@ -2526,8
> > +2549,8 @@ int main(int argc, char **argv)
> > init_iface(ifobj_tx, worker_testapp_validate_tx);
> >
> > test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
> > - tx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT,
> MIN_PKT_SIZE);
> > - rx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT,
> MIN_PKT_SIZE);
> > + tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem,
> DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> > + rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem,
> > +DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> > if (!tx_pkt_stream_default || !rx_pkt_stream_default)
> > exit_with_error(ENOMEM);
> > test.tx_pkt_stream_default = tx_pkt_stream_default;
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test.
2023-11-13 6:42 ` Vyavahare, Tushar
@ 2023-11-13 11:20 ` Maciej Fijalkowski
2023-11-14 17:03 ` Maciej Fijalkowski
0 siblings, 1 reply; 6+ messages in thread
From: Maciej Fijalkowski @ 2023-11-13 11:20 UTC (permalink / raw)
To: Vyavahare, Tushar
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, bjorn@kernel.org,
Karlsson, Magnus, jonathan.lemon@gmail.com, davem@davemloft.net,
kuba@kernel.org, pabeni@redhat.com, ast@kernel.org,
daniel@iogearbox.net, Sarkar, Tirthendu
On Mon, Nov 13, 2023 at 07:42:09AM +0100, Vyavahare, Tushar wrote:
>
>
> > -----Original Message-----
> > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> > Sent: Wednesday, November 8, 2023 8:01 PM
> > To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> > Cc: bpf@vger.kernel.org; netdev@vger.kernel.org; bjorn@kernel.org; Karlsson,
> > Magnus <magnus.karlsson@intel.com>; jonathan.lemon@gmail.com;
> > davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> > ast@kernel.org; daniel@iogearbox.net; Sarkar, Tirthendu
> > <tirthendu.sarkar@intel.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@intel.com>
> > > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.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?
>
> > > {
> > > pkt->offset = offset;
> > > pkt->len = len;
> > > - if (len > MAX_ETH_JUMBO_SIZE) {
> > > - pkt->valid = false;
> > > - } else {
> > > - pkt->valid = true;
> > > +
> > > + pkt->valid = pkt_valid(umem->unaligned_mode, offset, len);
> > > + if (pkt->valid)
> > > pkt_stream->nb_valid_entries++;
> > > - }
> > > +}
> > > +
> > > +static void pkt_modify(struct pkt_stream *pkt_stream, struct
> > xsk_umem_info *umem, struct pkt *pkt,
> > > + int offset, u32 len)
> > > +{
> > > + bool mod_valid;
> > > +
> > > + pkt->offset = offset;
> > > + pkt->len = len;
> > > + mod_valid = pkt_valid(umem->unaligned_mode, offset, len);
> >
> > double space
> >
>
> will do it.
>
> > > + pkt_stream->nb_valid_entries += mod_valid - pkt->valid;
> > > + pkt->valid = mod_valid;
> > > }
> > >
> > > static u32 pkt_get_buffer_len(struct xsk_umem_info *umem, u32 len) @@
> > > -651,7 +670,8 @@ static u32 pkt_get_buffer_len(struct xsk_umem_info
> > *umem, u32 len)
> > > return ceil_u32(len, umem->frame_size) * umem->frame_size; }
> > >
> > > -static struct pkt_stream *__pkt_stream_generate(u32 nb_pkts, u32
> > > pkt_len, u32 nb_start, u32 nb_off)
> > > +static struct pkt_stream *__pkt_stream_generate(struct xsk_umem_info
> > *umem, u32 nb_pkts,
> > > + u32 pkt_len, u32 nb_start,
> > u32 nb_off)
> > > {
> > > struct pkt_stream *pkt_stream;
> > > u32 i;
> > > @@ -665,30 +685,31 @@ 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_set(pkt_stream, umem, pkt, 0, pkt_len);
> > > pkt->pkt_nb = nb_start + i * nb_off;
> > > }
> > >
> > > return pkt_stream;
> > > }
> > >
> > > -static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32
> > > pkt_len)
> > > +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info
> > > +*umem, u32 nb_pkts, u32 pkt_len)
> > > {
> > > - return __pkt_stream_generate(nb_pkts, pkt_len, 0, 1);
> > > + return __pkt_stream_generate(umem, nb_pkts, pkt_len, 0, 1);
> > > }
> > >
> > > -static struct pkt_stream *pkt_stream_clone(struct pkt_stream
> > > *pkt_stream)
> > > +static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream,
> > > + struct xsk_umem_info *umem)
> > > {
> > > - return pkt_stream_generate(pkt_stream->nb_pkts, pkt_stream-
> > >pkts[0].len);
> > > + return pkt_stream_generate(umem, pkt_stream->nb_pkts,
> > > +pkt_stream->pkts[0].len);
> > > }
> > >
> > > static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts,
> > > u32 pkt_len) {
> > > struct pkt_stream *pkt_stream;
> > >
> > > - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> > > + pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, nb_pkts,
> > > +pkt_len);
> > > test->ifobj_tx->xsk->pkt_stream = pkt_stream;
> > > - pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
> > > + pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts,
> > > +pkt_len);
> > > test->ifobj_rx->xsk->pkt_stream = pkt_stream; }
> > >
> > > @@ -698,12 +719,11 @@ static void __pkt_stream_replace_half(struct
> > ifobject *ifobj, u32 pkt_len,
> > > struct pkt_stream *pkt_stream;
> > > u32 i;
> > >
> > > - pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream);
> > > + pkt_stream = pkt_stream_clone(ifobj->xsk->pkt_stream, ifobj-
> > >umem);
> > > for (i = 1; i < ifobj->xsk->pkt_stream->nb_pkts; i += 2)
> > > - pkt_set(pkt_stream, &pkt_stream->pkts[i], offset, pkt_len);
> > > + pkt_modify(pkt_stream, ifobj->umem, &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) @@ -715,9 +735,10 @@ static void
> > > pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int off
> > > static void pkt_stream_receive_half(struct test_spec *test) {
> > > struct pkt_stream *pkt_stream = test->ifobj_tx->xsk->pkt_stream;
> > > + struct xsk_umem_info *umem = test->ifobj_rx->umem;
> > > u32 i;
> > >
> > > - test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(pkt_stream-
> > >nb_pkts,
> > > + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(umem,
> > > +pkt_stream->nb_pkts,
> > > pkt_stream-
> > >pkts[0].len);
> > > pkt_stream = test->ifobj_rx->xsk->pkt_stream;
> > > for (i = 1; i < pkt_stream->nb_pkts; i += 2) @@ -733,12 +754,12 @@
> > > static void pkt_stream_even_odd_sequence(struct test_spec *test)
> > >
> > > for (i = 0; i < test->nb_sockets; i++) {
> > > pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
> > > - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts /
> > 2,
> > > + pkt_stream = __pkt_stream_generate(test->ifobj_tx->umem,
> > > +pkt_stream->nb_pkts / 2,
> > > pkt_stream->pkts[0].len, i,
> > 2);
> > > test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
> > >
> > > pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
> > > - pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts /
> > 2,
> > > + pkt_stream = __pkt_stream_generate(test->ifobj_rx->umem,
> > > +pkt_stream->nb_pkts / 2,
> > > pkt_stream->pkts[0].len, i,
> > 2);
> > > test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
> > > }
> > > @@ -1961,7 +1982,8 @@ static int testapp_stats_tx_invalid_descs(struct
> > > test_spec *test) static int testapp_stats_rx_full(struct test_spec
> > > *test) {
> > > pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS +
> > DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> > > - test->ifobj_rx->xsk->pkt_stream =
> > pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > > + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test-
> > >ifobj_rx->umem,
> > > +
> > DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > >
> > > test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
> > > test->ifobj_rx->release_rx = false;
> > > @@ -1972,7 +1994,8 @@ static int testapp_stats_rx_full(struct
> > > test_spec *test) static int testapp_stats_fill_empty(struct test_spec
> > > *test) {
> > > pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS +
> > DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
> > > - test->ifobj_rx->xsk->pkt_stream =
> > pkt_stream_generate(DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > > + test->ifobj_rx->xsk->pkt_stream = pkt_stream_generate(test-
> > >ifobj_rx->umem,
> > > +
> > DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> > >
> > > test->ifobj_rx->use_fill_ring = false;
> > > test->ifobj_rx->validation_func = validate_fill_empty; @@ -2526,8
> > > +2549,8 @@ int main(int argc, char **argv)
> > > init_iface(ifobj_tx, worker_testapp_validate_tx);
> > >
> > > test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
> > > - tx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT,
> > MIN_PKT_SIZE);
> > > - rx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT,
> > MIN_PKT_SIZE);
> > > + tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem,
> > DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> > > + rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem,
> > > +DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> > > if (!tx_pkt_stream_default || !rx_pkt_stream_default)
> > > exit_with_error(ENOMEM);
> > > test.tx_pkt_stream_default = tx_pkt_stream_default;
> > > --
> > > 2.34.1
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test.
2023-11-13 11:20 ` Maciej Fijalkowski
@ 2023-11-14 17:03 ` Maciej Fijalkowski
0 siblings, 0 replies; 6+ messages in thread
From: Maciej Fijalkowski @ 2023-11-14 17:03 UTC (permalink / raw)
To: Vyavahare, Tushar
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, bjorn@kernel.org,
Karlsson, Magnus, jonathan.lemon@gmail.com, davem@davemloft.net,
kuba@kernel.org, pabeni@redhat.com, ast@kernel.org,
daniel@iogearbox.net, Sarkar, Tirthendu
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@intel.com>
> > > Sent: Wednesday, November 8, 2023 8:01 PM
> > > To: Vyavahare, Tushar <tushar.vyavahare@intel.com>
> > > Cc: bpf@vger.kernel.org; netdev@vger.kernel.org; bjorn@kernel.org; Karlsson,
> > > Magnus <magnus.karlsson@intel.com>; jonathan.lemon@gmail.com;
> > > davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com;
> > > ast@kernel.org; daniel@iogearbox.net; Sarkar, Tirthendu
> > > <tirthendu.sarkar@intel.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@intel.com>
> > > > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-14 17:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-03 14:29 [PATCH bpf-next] selftests/xsk: fix for SEND_RECEIVE_UNALIGNED test Tushar Vyavahare
2023-11-08 9:35 ` Magnus Karlsson
2023-11-08 14:30 ` Maciej Fijalkowski
2023-11-13 6:42 ` Vyavahare, Tushar
2023-11-13 11:20 ` Maciej Fijalkowski
2023-11-14 17:03 ` Maciej Fijalkowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).