[PATCH bpf-next v3 0/2] selftests/xsk: Add tests for XDP tail adjustment in AF_XDP

[PATCH bpf-next v3 0/2] selftests/xsk: Add tests for XDP tail adjustment in AF_XDP

[Tushar Vyavahare]

This patch series adds tests to validate the XDP tail adjustment
functionality, focusing on its use within the AF_XDP context. The tests
verify dynamic packet size manipulation using the bpf_xdp_adjust_tail()
helper function, covering both single and multi-buffer scenarios.

v1 -> v2:
1. Retain and extend stream replacement: Keep `pkt_stream_replace`
   unchanged. Add `pkt_stream_replace_ifobject` for targeted ifobject
   handling.

2. Consolidate patches: Merge patches 2 to 6 for tail adjustment tests and
   check.

v2 -> v3:
1. Introduce `adjust_value` to replace `count` for clearer communication
   with userspace.

---
Patch Summary:

1. Packet stream replacement: Add `pkt_stream_replace_ifobject` to manage
   packet streams efficiently.

2. Tail adjustment tests and support check: Implement dynamic packet
   resizing in xskxceiver by adding `xsk_xdp_adjust_tail` and extend this
   functionality to userspace with `testapp_xdp_adjust_tail` for
   validation. Ensure support by adding `is_adjust_tail_supported` to
   verify the availability of `bpf_xdp_adjust_tail()`. Introduce tests for
   shrinking and growing packets using `bpf_xdp_adjust_tail()`, covering
   both single and multi-buffer scenarios when used with AF_XDP.
---

Tushar Vyavahare (2):
  selftests/xsk: Add packet stream replacement function
  selftests/xsk: Add tail adjustment tests and support check

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>

 .../selftests/bpf/progs/xsk_xdp_progs.c       |  49 +++++++
 tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
 tools/testing/selftests/bpf/xskxceiver.c      | 120 ++++++++++++++++--
 tools/testing/selftests/bpf/xskxceiver.h      |   2 +
 4 files changed, 164 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH bpf-next v3 1/2] selftests/xsk: Add packet stream replacement function

[Tushar Vyavahare]

Add pkt_stream_replace_ifobject function to replace the packet stream for
a given ifobject.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 11f047b8af75..d60ee6a31c09 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -757,14 +757,15 @@ static struct pkt_stream *pkt_stream_clone(struct pkt_stream *pkt_stream)
 	return pkt_stream_generate(pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
 }
 
-static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
+static void pkt_stream_replace_ifobject(struct ifobject *ifobj, u32 nb_pkts, u32 pkt_len)
 {
-	struct pkt_stream *pkt_stream;
+	ifobj->xsk->pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
+}
 
-	pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
-	test->ifobj_tx->xsk->pkt_stream = pkt_stream;
-	pkt_stream = pkt_stream_generate(nb_pkts, pkt_len);
-	test->ifobj_rx->xsk->pkt_stream = pkt_stream;
+static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
+{
+	pkt_stream_replace_ifobject(test->ifobj_tx, nb_pkts, pkt_len);
+	pkt_stream_replace_ifobject(test->ifobj_rx, nb_pkts, pkt_len);
 }
 
 static void __pkt_stream_replace_half(struct ifobject *ifobj, u32 pkt_len,
-- 
2.34.1

[PATCH bpf-next v3 2/2] selftests/xsk: Add tail adjustment tests and support check

[Tushar Vyavahare]

Introduce tail adjustment functionality in xskxceiver using
bpf_xdp_adjust_tail(). Add `xsk_xdp_adjust_tail` to modify packet sizes
and drop unmodified packets. Implement `is_adjust_tail_supported` to check
helper availability. Develop packet resizing tests, including shrinking
and growing scenarios, with functions for both single-buffer and
multi-buffer cases. Update the test framework to handle various scenarios
and adjust MTU settings. These changes enhance the testing of packet tail
adjustments, improving AF_XDP framework reliability.

Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 .../selftests/bpf/progs/xsk_xdp_progs.c       |  49 ++++++++
 tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
 tools/testing/selftests/bpf/xskxceiver.c      | 107 +++++++++++++++++-
 tools/testing/selftests/bpf/xskxceiver.h      |   2 +
 4 files changed, 157 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
index ccde6a4c6319..34c832a5a98c 100644
--- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
@@ -4,6 +4,8 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include <linux/if_ether.h>
+#include <linux/ip.h>
+#include <linux/errno.h>
 #include "xsk_xdp_common.h"
 
 struct {
@@ -14,6 +16,7 @@ struct {
 } xsk SEC(".maps");
 
 static unsigned int idx;
+int adjust_value = 0;
 int count = 0;
 
 SEC("xdp.frags") int xsk_def_prog(struct xdp_md *xdp)
@@ -70,4 +73,50 @@ SEC("xdp") int xsk_xdp_shared_umem(struct xdp_md *xdp)
 	return bpf_redirect_map(&xsk, idx, XDP_DROP);
 }
 
+SEC("xdp.frags") int xsk_xdp_adjust_tail(struct xdp_md *xdp)
+{
+	__u32 buff_len, curr_buff_len;
+	int ret;
+
+	buff_len = bpf_xdp_get_buff_len(xdp);
+	if (buff_len == 0)
+		return XDP_DROP;
+
+	ret = bpf_xdp_adjust_tail(xdp, adjust_value);
+	if (ret < 0) {
+		/* Handle unsupported cases */
+		if (ret == -EOPNOTSUPP) {
+			/* Set adjust_value to -EOPNOTSUPP to indicate to userspace that this case
+			 * is unsupported
+			 */
+			adjust_value = -EOPNOTSUPP;
+			return bpf_redirect_map(&xsk, 0, XDP_DROP);
+		}
+
+		return XDP_DROP;
+	}
+
+	curr_buff_len = bpf_xdp_get_buff_len(xdp);
+	if (curr_buff_len != buff_len + adjust_value)
+		return XDP_DROP;
+
+	if (curr_buff_len > buff_len) {
+		__u32 *pkt_data = (void *)(long)xdp->data;
+		__u32 len, words_to_end, seq_num;
+
+		len = curr_buff_len - PKT_HDR_ALIGN;
+		words_to_end = len / sizeof(*pkt_data) - 1;
+		seq_num = words_to_end;
+
+		/* Convert sequence number to network byte order. Store this in the last 4 bytes of
+		 * the packet. Use 'adjust_value' to determine the position at the end of the
+		 * packet for storing the sequence number.
+		 */
+		seq_num = __constant_htonl(words_to_end);
+		bpf_xdp_store_bytes(xdp, curr_buff_len - adjust_value, &seq_num, sizeof(seq_num));
+	}
+
+	return bpf_redirect_map(&xsk, 0, XDP_DROP);
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/xsk_xdp_common.h b/tools/testing/selftests/bpf/xsk_xdp_common.h
index 5a6f36f07383..45810ff552da 100644
--- a/tools/testing/selftests/bpf/xsk_xdp_common.h
+++ b/tools/testing/selftests/bpf/xsk_xdp_common.h
@@ -4,6 +4,7 @@
 #define XSK_XDP_COMMON_H_
 
 #define MAX_SOCKETS 2
+#define PKT_HDR_ALIGN (sizeof(struct ethhdr) + 2) /* Just to align the data in the packet */
 
 struct xdp_info {
 	__u64 count;
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index d60ee6a31c09..bcc265fc784c 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -524,6 +524,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 	test->nb_sockets = 1;
 	test->fail = false;
 	test->set_ring = false;
+	test->adjust_tail = false;
+	test->adjust_tail_support = false;
 	test->mtu = MAX_ETH_PKT_SIZE;
 	test->xdp_prog_rx = ifobj_rx->xdp_progs->progs.xsk_def_prog;
 	test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk;
@@ -992,6 +994,31 @@ static bool is_metadata_correct(struct pkt *pkt, void *buffer, u64 addr)
 	return true;
 }
 
+static bool is_adjust_tail_supported(struct xsk_xdp_progs *skel_rx)
+{
+	struct bpf_map *data_map;
+	int adjust_value = 0;
+	int key = 0;
+	int ret;
+
+	data_map = bpf_object__find_map_by_name(skel_rx->obj, "xsk_xdp_.bss");
+	if (!data_map || !bpf_map__is_internal(data_map)) {
+		ksft_print_msg("Error: could not find bss section of XDP program\n");
+		exit_with_error(errno);
+	}
+
+	ret = bpf_map_lookup_elem(bpf_map__fd(data_map), &key, &adjust_value);
+	if (ret) {
+		ksft_print_msg("Error: bpf_map_lookup_elem failed with error %d\n", ret);
+		return false;
+	}
+
+	/* Set the 'adjust_value' variable to -EOPNOTSUPP in the XDP program if the adjust_tail
+	 * helper is not supported. Skip the adjust_tail test case in this scenario.
+	 */
+	return adjust_value != -EOPNOTSUPP;
+}
+
 static bool is_frag_valid(struct xsk_umem_info *umem, u64 addr, u32 len, u32 expected_pkt_nb,
 			  u32 bytes_processed)
 {
@@ -1768,8 +1795,13 @@ static void *worker_testapp_validate_rx(void *arg)
 
 	if (!err && ifobject->validation_func)
 		err = ifobject->validation_func(ifobject);
-	if (err)
-		report_failure(test);
+
+	if (err) {
+		if (test->adjust_tail && !is_adjust_tail_supported(ifobject->xdp_progs))
+			test->adjust_tail_support = false;
+		else
+			report_failure(test);
+	}
 
 	pthread_exit(NULL);
 }
@@ -2516,6 +2548,73 @@ static int testapp_hw_sw_max_ring_size(struct test_spec *test)
 	return testapp_validate_traffic(test);
 }
 
+static int testapp_xdp_adjust_tail(struct test_spec *test, int adjust_value)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+
+	test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_adjust_tail,
+			       skel_tx->progs.xsk_xdp_adjust_tail,
+			       skel_rx->maps.xsk, skel_tx->maps.xsk);
+
+	skel_rx->bss->adjust_value = adjust_value;
+
+	return testapp_validate_traffic(test);
+}
+
+static int testapp_adjust_tail(struct test_spec *test, u32 value, u32 pkt_len)
+{
+	u32 pkt_cnt = DEFAULT_BATCH_SIZE;
+	int ret;
+
+	test->adjust_tail_support = true;
+	test->adjust_tail = true;
+	test->total_steps = 1;
+
+	pkt_stream_replace_ifobject(test->ifobj_tx, pkt_cnt, pkt_len);
+	pkt_stream_replace_ifobject(test->ifobj_rx, pkt_cnt, pkt_len + value);
+
+	ret = testapp_xdp_adjust_tail(test, value);
+	if (ret)
+		return ret;
+
+	if (!test->adjust_tail_support) {
+		ksft_test_result_skip("%s %sResize pkt with bpf_xdp_adjust_tail() not supported\n",
+				      mode_string(test), busy_poll_string(test));
+	return TEST_SKIP;
+	}
+
+	return 0;
+}
+
+static int testapp_adjust_tail_common(struct test_spec *test, int adjust_value, u32 len,
+				      bool set_mtu)
+{
+	if (set_mtu)
+		test->mtu = MAX_ETH_JUMBO_SIZE;
+	return testapp_adjust_tail(test, adjust_value, len);
+}
+
+static int testapp_adjust_tail_shrink(struct test_spec *test)
+{
+	return testapp_adjust_tail_common(test, -4, MIN_PKT_SIZE, false);
+}
+
+static int testapp_adjust_tail_shrink_mb(struct test_spec *test)
+{
+	return testapp_adjust_tail_common(test, -4, XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);
+}
+
+static int testapp_adjust_tail_grow(struct test_spec *test)
+{
+	return testapp_adjust_tail_common(test, 4, MIN_PKT_SIZE, false);
+}
+
+static int testapp_adjust_tail_grow_mb(struct test_spec *test)
+{
+	return testapp_adjust_tail_common(test, 4, XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);
+}
+
 static void run_pkt_test(struct test_spec *test)
 {
 	int ret;
@@ -2622,6 +2721,10 @@ static const struct test_spec tests[] = {
 	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
 	{.name = "HW_SW_MIN_RING_SIZE", .test_func = testapp_hw_sw_min_ring_size},
 	{.name = "HW_SW_MAX_RING_SIZE", .test_func = testapp_hw_sw_max_ring_size},
+	{.name = "XDP_ADJUST_TAIL_SHRINK", .test_func = testapp_adjust_tail_shrink},
+	{.name = "XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF", .test_func = testapp_adjust_tail_shrink_mb},
+	{.name = "XDP_ADJUST_TAIL_GROW", .test_func = testapp_adjust_tail_grow},
+	{.name = "XDP_ADJUST_TAIL_GROW_MULTI_BUFF", .test_func = testapp_adjust_tail_grow_mb},
 	};
 
 static void print_tests(void)
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index e46e823f6a1a..67fc44b2813b 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -173,6 +173,8 @@ struct test_spec {
 	u16 nb_sockets;
 	bool fail;
 	bool set_ring;
+	bool adjust_tail;
+	bool adjust_tail_support;
 	enum test_mode mode;
 	char name[MAX_TEST_NAME_SIZE];
 };
-- 
2.34.1

Re: [PATCH bpf-next v3 2/2] selftests/xsk: Add tail adjustment tests and support check

[Maciej Fijalkowski]

On Wed, Mar 05, 2025 at 02:18:13PM +0000, Tushar Vyavahare wrote:
> Introduce tail adjustment functionality in xskxceiver using
> bpf_xdp_adjust_tail(). Add `xsk_xdp_adjust_tail` to modify packet sizes
> and drop unmodified packets. Implement `is_adjust_tail_supported` to check
> helper availability. Develop packet resizing tests, including shrinking
> and growing scenarios, with functions for both single-buffer and
> multi-buffer cases. Update the test framework to handle various scenarios
> and adjust MTU settings. These changes enhance the testing of packet tail
> adjustments, improving AF_XDP framework reliability.
> 
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> ---
>  .../selftests/bpf/progs/xsk_xdp_progs.c       |  49 ++++++++
>  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
>  tools/testing/selftests/bpf/xskxceiver.c      | 107 +++++++++++++++++-
>  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
>  4 files changed, 157 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> index ccde6a4c6319..34c832a5a98c 100644
> --- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> +++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> @@ -4,6 +4,8 @@
>  #include <linux/bpf.h>
>  #include <bpf/bpf_helpers.h>
>  #include <linux/if_ether.h>
> +#include <linux/ip.h>
> +#include <linux/errno.h>
>  #include "xsk_xdp_common.h"
>  
>  struct {
> @@ -14,6 +16,7 @@ struct {
>  } xsk SEC(".maps");
>  
>  static unsigned int idx;
> +int adjust_value = 0;
>  int count = 0;
>  
>  SEC("xdp.frags") int xsk_def_prog(struct xdp_md *xdp)
> @@ -70,4 +73,50 @@ SEC("xdp") int xsk_xdp_shared_umem(struct xdp_md *xdp)
>  	return bpf_redirect_map(&xsk, idx, XDP_DROP);
>  }
>  
> +SEC("xdp.frags") int xsk_xdp_adjust_tail(struct xdp_md *xdp)
> +{
> +	__u32 buff_len, curr_buff_len;
> +	int ret;
> +
> +	buff_len = bpf_xdp_get_buff_len(xdp);
> +	if (buff_len == 0)
> +		return XDP_DROP;
> +
> +	ret = bpf_xdp_adjust_tail(xdp, adjust_value);
> +	if (ret < 0) {
> +		/* Handle unsupported cases */
> +		if (ret == -EOPNOTSUPP) {
> +			/* Set adjust_value to -EOPNOTSUPP to indicate to userspace that this case
> +			 * is unsupported
> +			 */
> +			adjust_value = -EOPNOTSUPP;
> +			return bpf_redirect_map(&xsk, 0, XDP_DROP);

so in this case you will proceed with running whole traffic? IMHO test
should be stopped for very first notsupp case, but not sure if it's worth
the hassle.

> +		}
> +
> +		return XDP_DROP;
> +	}
> +
> +	curr_buff_len = bpf_xdp_get_buff_len(xdp);
> +	if (curr_buff_len != buff_len + adjust_value)
> +		return XDP_DROP;
> +
> +	if (curr_buff_len > buff_len) {
> +		__u32 *pkt_data = (void *)(long)xdp->data;
> +		__u32 len, words_to_end, seq_num;
> +
> +		len = curr_buff_len - PKT_HDR_ALIGN;
> +		words_to_end = len / sizeof(*pkt_data) - 1;
> +		seq_num = words_to_end;
> +
> +		/* Convert sequence number to network byte order. Store this in the last 4 bytes of
> +		 * the packet. Use 'adjust_value' to determine the position at the end of the
> +		 * packet for storing the sequence number.
> +		 */
> +		seq_num = __constant_htonl(words_to_end);
> +		bpf_xdp_store_bytes(xdp, curr_buff_len - adjust_value, &seq_num, sizeof(seq_num));

what is the purpose of that? test suite expects seq_num to be at the end
of the packet so you have to double it here?

> +	}
> +
> +	return bpf_redirect_map(&xsk, 0, XDP_DROP);
> +}
> +
>  char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/xsk_xdp_common.h b/tools/testing/selftests/bpf/xsk_xdp_common.h
> index 5a6f36f07383..45810ff552da 100644
> --- a/tools/testing/selftests/bpf/xsk_xdp_common.h
> +++ b/tools/testing/selftests/bpf/xsk_xdp_common.h
> @@ -4,6 +4,7 @@
>  #define XSK_XDP_COMMON_H_
>  
>  #define MAX_SOCKETS 2
> +#define PKT_HDR_ALIGN (sizeof(struct ethhdr) + 2) /* Just to align the data in the packet */
>  
>  struct xdp_info {
>  	__u64 count;
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index d60ee6a31c09..bcc265fc784c 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -524,6 +524,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  	test->nb_sockets = 1;
>  	test->fail = false;
>  	test->set_ring = false;
> +	test->adjust_tail = false;
> +	test->adjust_tail_support = false;
>  	test->mtu = MAX_ETH_PKT_SIZE;
>  	test->xdp_prog_rx = ifobj_rx->xdp_progs->progs.xsk_def_prog;
>  	test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk;
> @@ -992,6 +994,31 @@ static bool is_metadata_correct(struct pkt *pkt, void *buffer, u64 addr)
>  	return true;
>  }
>  
> +static bool is_adjust_tail_supported(struct xsk_xdp_progs *skel_rx)
> +{
> +	struct bpf_map *data_map;
> +	int adjust_value = 0;
> +	int key = 0;
> +	int ret;
> +
> +	data_map = bpf_object__find_map_by_name(skel_rx->obj, "xsk_xdp_.bss");
> +	if (!data_map || !bpf_map__is_internal(data_map)) {
> +		ksft_print_msg("Error: could not find bss section of XDP program\n");
> +		exit_with_error(errno);
> +	}
> +
> +	ret = bpf_map_lookup_elem(bpf_map__fd(data_map), &key, &adjust_value);
> +	if (ret) {
> +		ksft_print_msg("Error: bpf_map_lookup_elem failed with error %d\n", ret);
> +		return false;

exit_with_error(errno) to be consistent?

> +	}
> +
> +	/* Set the 'adjust_value' variable to -EOPNOTSUPP in the XDP program if the adjust_tail
> +	 * helper is not supported. Skip the adjust_tail test case in this scenario.
> +	 */
> +	return adjust_value != -EOPNOTSUPP;
> +}
> +
>  static bool is_frag_valid(struct xsk_umem_info *umem, u64 addr, u32 len, u32 expected_pkt_nb,
>  			  u32 bytes_processed)
>  {
> @@ -1768,8 +1795,13 @@ static void *worker_testapp_validate_rx(void *arg)
>  
>  	if (!err && ifobject->validation_func)
>  		err = ifobject->validation_func(ifobject);
> -	if (err)
> -		report_failure(test);
> +
> +	if (err) {
> +		if (test->adjust_tail && !is_adjust_tail_supported(ifobject->xdp_progs))
> +			test->adjust_tail_support = false;

how about setting is_adjust_tail_supported() as validation_func? Would
that work? Special casing this check specially for tail manipulation tests
seems a bit odd.

> +		else
> +			report_failure(test);
> +	}
>  
>  	pthread_exit(NULL);
>  }
> @@ -2516,6 +2548,73 @@ static int testapp_hw_sw_max_ring_size(struct test_spec *test)
>  	return testapp_validate_traffic(test);
>  }
>  
> +static int testapp_xdp_adjust_tail(struct test_spec *test, int adjust_value)
> +{
> +	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> +	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> +
> +	test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_adjust_tail,
> +			       skel_tx->progs.xsk_xdp_adjust_tail,
> +			       skel_rx->maps.xsk, skel_tx->maps.xsk);
> +
> +	skel_rx->bss->adjust_value = adjust_value;
> +
> +	return testapp_validate_traffic(test);
> +}
> +
> +static int testapp_adjust_tail(struct test_spec *test, u32 value, u32 pkt_len)
> +{
> +	u32 pkt_cnt = DEFAULT_BATCH_SIZE;

you don't need this variable

> +	int ret;
> +
> +	test->adjust_tail_support = true;
> +	test->adjust_tail = true;
> +	test->total_steps = 1;
> +
> +	pkt_stream_replace_ifobject(test->ifobj_tx, pkt_cnt, pkt_len);
> +	pkt_stream_replace_ifobject(test->ifobj_rx, pkt_cnt, pkt_len + value);
> +
> +	ret = testapp_xdp_adjust_tail(test, value);
> +	if (ret)
> +		return ret;
> +
> +	if (!test->adjust_tail_support) {
> +		ksft_test_result_skip("%s %sResize pkt with bpf_xdp_adjust_tail() not supported\n",
> +				      mode_string(test), busy_poll_string(test));
> +	return TEST_SKIP;

missing indent

> +	}
> +
> +	return 0;
> +}
> +
> +static int testapp_adjust_tail_common(struct test_spec *test, int adjust_value, u32 len,
> +				      bool set_mtu)
> +{
> +	if (set_mtu)
> +		test->mtu = MAX_ETH_JUMBO_SIZE;

could you remove this and instead of bool_set_mtu just pass the value of
mtu?

static int testapp_adjust_tail(struct test_spec *test, u32 value, u32 pkt_len, u32 mtu)
{
	(...)
	if (test->mtu != mtu)
		test->mtu = mtu;
	(...)
}

static int testapp_adjust_tail_shrink(struct test_spec *test)
{
	return testapp_adjust_tail(test, -4, MIN_PKT_SIZE, MAX_ETH_PKT_SIZE);
}

static int testapp_adjust_tail_shrink_mb(struct test_spec *test)
{
	return testapp_adjust_tail(test, -4, XSK_RING_PROD__DEFAULT_NUM_DESCS * 3,
				   MAX_ETH_JUMBO_SIZE);
}

no need for _common func.

> +	return testapp_adjust_tail(test, adjust_value, len);
> +}
> +
> +static int testapp_adjust_tail_shrink(struct test_spec *test)
> +{
> +	return testapp_adjust_tail_common(test, -4, MIN_PKT_SIZE, false);
> +}
> +
> +static int testapp_adjust_tail_shrink_mb(struct test_spec *test)
> +{
> +	return testapp_adjust_tail_common(test, -4, XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);

Am I reading this right that you are modifying the size by just 4 bytes?
The bugs that drivers had were for cases when packets got modified by
value bigger than frag size which caused for example underlying page being
freed.

If that is the case tests do nothing valuable from my perspective.

> +}
> +
> +static int testapp_adjust_tail_grow(struct test_spec *test)
> +{
> +	return testapp_adjust_tail_common(test, 4, MIN_PKT_SIZE, false);
> +}
> +
> +static int testapp_adjust_tail_grow_mb(struct test_spec *test)
> +{
> +	return testapp_adjust_tail_common(test, 4, XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);
> +}
> +
>  static void run_pkt_test(struct test_spec *test)
>  {
>  	int ret;
> @@ -2622,6 +2721,10 @@ static const struct test_spec tests[] = {
>  	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
>  	{.name = "HW_SW_MIN_RING_SIZE", .test_func = testapp_hw_sw_min_ring_size},
>  	{.name = "HW_SW_MAX_RING_SIZE", .test_func = testapp_hw_sw_max_ring_size},
> +	{.name = "XDP_ADJUST_TAIL_SHRINK", .test_func = testapp_adjust_tail_shrink},
> +	{.name = "XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF", .test_func = testapp_adjust_tail_shrink_mb},
> +	{.name = "XDP_ADJUST_TAIL_GROW", .test_func = testapp_adjust_tail_grow},
> +	{.name = "XDP_ADJUST_TAIL_GROW_MULTI_BUFF", .test_func = testapp_adjust_tail_grow_mb},
>  	};
>  
>  static void print_tests(void)
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index e46e823f6a1a..67fc44b2813b 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -173,6 +173,8 @@ struct test_spec {
>  	u16 nb_sockets;
>  	bool fail;
>  	bool set_ring;
> +	bool adjust_tail;
> +	bool adjust_tail_support;
>  	enum test_mode mode;
>  	char name[MAX_TEST_NAME_SIZE];
>  };
> -- 
> 2.34.1
> 

RE: [PATCH bpf-next v3 2/2] selftests/xsk: Add tail adjustment tests and support check

[Vyavahare, Tushar]

> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Sent: Wednesday, March 12, 2025 3:41 AM
> 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 v3 2/2] selftests/xsk: Add tail adjustment tests
> and support check
> 
> On Wed, Mar 05, 2025 at 02:18:13PM +0000, Tushar Vyavahare wrote:
> > Introduce tail adjustment functionality in xskxceiver using
> > bpf_xdp_adjust_tail(). Add `xsk_xdp_adjust_tail` to modify packet
> > sizes and drop unmodified packets. Implement
> > `is_adjust_tail_supported` to check helper availability. Develop
> > packet resizing tests, including shrinking and growing scenarios, with
> > functions for both single-buffer and multi-buffer cases. Update the
> > test framework to handle various scenarios and adjust MTU settings.
> > These changes enhance the testing of packet tail adjustments, improving
> AF_XDP framework reliability.
> >
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> > ---
> >  .../selftests/bpf/progs/xsk_xdp_progs.c       |  49 ++++++++
> >  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
> >  tools/testing/selftests/bpf/xskxceiver.c      | 107 +++++++++++++++++-
> >  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
> >  4 files changed, 157 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > index ccde6a4c6319..34c832a5a98c 100644
> > --- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > +++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > @@ -4,6 +4,8 @@
> >  #include <linux/bpf.h>
> >  #include <bpf/bpf_helpers.h>
> >  #include <linux/if_ether.h>
> > +#include <linux/ip.h>
> > +#include <linux/errno.h>
> >  #include "xsk_xdp_common.h"
> >
> >  struct {
> > @@ -14,6 +16,7 @@ struct {
> >  } xsk SEC(".maps");
> >
> >  static unsigned int idx;
> > +int adjust_value = 0;
> >  int count = 0;
> >
> >  SEC("xdp.frags") int xsk_def_prog(struct xdp_md *xdp) @@ -70,4 +73,50
> > @@ SEC("xdp") int xsk_xdp_shared_umem(struct xdp_md *xdp)
> >  	return bpf_redirect_map(&xsk, idx, XDP_DROP);  }
> >
> > +SEC("xdp.frags") int xsk_xdp_adjust_tail(struct xdp_md *xdp) {
> > +	__u32 buff_len, curr_buff_len;
> > +	int ret;
> > +
> > +	buff_len = bpf_xdp_get_buff_len(xdp);
> > +	if (buff_len == 0)
> > +		return XDP_DROP;
> > +
> > +	ret = bpf_xdp_adjust_tail(xdp, adjust_value);
> > +	if (ret < 0) {
> > +		/* Handle unsupported cases */
> > +		if (ret == -EOPNOTSUPP) {
> > +			/* Set adjust_value to -EOPNOTSUPP to indicate to
> userspace that this case
> > +			 * is unsupported
> > +			 */
> > +			adjust_value = -EOPNOTSUPP;
> > +			return bpf_redirect_map(&xsk, 0, XDP_DROP);
> 
> so in this case you will proceed with running whole traffic? IMHO test should
> be stopped for very first notsupp case, but not sure if it's worth the hassle.
> 

When the expected packet length does not match, the test will fail on the
very first packet in receive_pkts(). After this initial failure, check if
the adjust_tail function is supported, but only for cases where the test
involves adjust_tail and fails.

> > +		}
> > +
> > +		return XDP_DROP;
> > +	}
> > +
> > +	curr_buff_len = bpf_xdp_get_buff_len(xdp);
> > +	if (curr_buff_len != buff_len + adjust_value)
> > +		return XDP_DROP;
> > +
> > +	if (curr_buff_len > buff_len) {
> > +		__u32 *pkt_data = (void *)(long)xdp->data;
> > +		__u32 len, words_to_end, seq_num;
> > +
> > +		len = curr_buff_len - PKT_HDR_ALIGN;
> > +		words_to_end = len / sizeof(*pkt_data) - 1;
> > +		seq_num = words_to_end;
> > +
> > +		/* Convert sequence number to network byte order. Store this
> in the last 4 bytes of
> > +		 * the packet. Use 'adjust_value' to determine the position at
> the end of the
> > +		 * packet for storing the sequence number.
> > +		 */
> > +		seq_num = __constant_htonl(words_to_end);
> > +		bpf_xdp_store_bytes(xdp, curr_buff_len - adjust_value,
> &seq_num,
> > +sizeof(seq_num));
> 
> what is the purpose of that? test suite expects seq_num to be at the end of
> the packet so you have to double it here?
> 

The test suite expects the seq_num to be at the end of the packet, so it
needs to be doubled to ensure it is placed correctly in the final packet
structure.

> > +	}
> > +
> > +	return bpf_redirect_map(&xsk, 0, XDP_DROP); }
> > +
> >  char _license[] SEC("license") = "GPL"; diff --git
> > a/tools/testing/selftests/bpf/xsk_xdp_common.h
> > b/tools/testing/selftests/bpf/xsk_xdp_common.h
> > index 5a6f36f07383..45810ff552da 100644
> > --- a/tools/testing/selftests/bpf/xsk_xdp_common.h
> > +++ b/tools/testing/selftests/bpf/xsk_xdp_common.h
> > @@ -4,6 +4,7 @@
> >  #define XSK_XDP_COMMON_H_
> >
> >  #define MAX_SOCKETS 2
> > +#define PKT_HDR_ALIGN (sizeof(struct ethhdr) + 2) /* Just to align
> > +the data in the packet */
> >
> >  struct xdp_info {
> >  	__u64 count;
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > b/tools/testing/selftests/bpf/xskxceiver.c
> > index d60ee6a31c09..bcc265fc784c 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -524,6 +524,8 @@ static void __test_spec_init(struct test_spec *test,
> struct ifobject *ifobj_tx,
> >  	test->nb_sockets = 1;
> >  	test->fail = false;
> >  	test->set_ring = false;
> > +	test->adjust_tail = false;
> > +	test->adjust_tail_support = false;
> >  	test->mtu = MAX_ETH_PKT_SIZE;
> >  	test->xdp_prog_rx = ifobj_rx->xdp_progs->progs.xsk_def_prog;
> >  	test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk; @@ -992,6 +994,31
> > @@ static bool is_metadata_correct(struct pkt *pkt, void *buffer, u64 addr)
> >  	return true;
> >  }
> >
> > +static bool is_adjust_tail_supported(struct xsk_xdp_progs *skel_rx) {
> > +	struct bpf_map *data_map;
> > +	int adjust_value = 0;
> > +	int key = 0;
> > +	int ret;
> > +
> > +	data_map = bpf_object__find_map_by_name(skel_rx->obj,
> "xsk_xdp_.bss");
> > +	if (!data_map || !bpf_map__is_internal(data_map)) {
> > +		ksft_print_msg("Error: could not find bss section of XDP
> program\n");
> > +		exit_with_error(errno);
> > +	}
> > +
> > +	ret = bpf_map_lookup_elem(bpf_map__fd(data_map), &key,
> &adjust_value);
> > +	if (ret) {
> > +		ksft_print_msg("Error: bpf_map_lookup_elem failed with error
> %d\n", ret);
> > +		return false;
> 
> exit_with_error(errno) to be consistent?
> 

Will do.

> > +	}
> > +
> > +	/* Set the 'adjust_value' variable to -EOPNOTSUPP in the XDP program
> if the adjust_tail
> > +	 * helper is not supported. Skip the adjust_tail test case in this
> scenario.
> > +	 */
> > +	return adjust_value != -EOPNOTSUPP;
> > +}
> > +
> >  static bool is_frag_valid(struct xsk_umem_info *umem, u64 addr, u32 len,
> u32 expected_pkt_nb,
> >  			  u32 bytes_processed)
> >  {
> > @@ -1768,8 +1795,13 @@ static void *worker_testapp_validate_rx(void
> > *arg)
> >
> >  	if (!err && ifobject->validation_func)
> >  		err = ifobject->validation_func(ifobject);
> > -	if (err)
> > -		report_failure(test);
> > +
> > +	if (err) {
> > +		if (test->adjust_tail && !is_adjust_tail_supported(ifobject-
> >xdp_progs))
> > +			test->adjust_tail_support = false;
> 
> how about setting is_adjust_tail_supported() as validation_func? Would that
> work? Special casing this check specially for tail manipulation tests seems a bit
> odd.
> 

Setting is_adjust_tail_supported() as the validation_func would not work
directly. This function is designed to check if the bpf_xdp_adjust_tail
helper is supported by the XDP program, rather than to validate test
results. It assesses the capability of the XDP program, not the outcome of
the tests. 

> > +		else
> > +			report_failure(test);
> > +	}
> >
> >  	pthread_exit(NULL);
> >  }
> > @@ -2516,6 +2548,73 @@ static int testapp_hw_sw_max_ring_size(struct
> test_spec *test)
> >  	return testapp_validate_traffic(test);  }
> >
> > +static int testapp_xdp_adjust_tail(struct test_spec *test, int
> > +adjust_value) {
> > +	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> > +	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> > +
> > +	test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_adjust_tail,
> > +			       skel_tx->progs.xsk_xdp_adjust_tail,
> > +			       skel_rx->maps.xsk, skel_tx->maps.xsk);
> > +
> > +	skel_rx->bss->adjust_value = adjust_value;
> > +
> > +	return testapp_validate_traffic(test); }
> > +
> > +static int testapp_adjust_tail(struct test_spec *test, u32 value, u32
> > +pkt_len) {
> > +	u32 pkt_cnt = DEFAULT_BATCH_SIZE;
> 
> you don't need this variable
> 

will do.

> > +	int ret;
> > +
> > +	test->adjust_tail_support = true;
> > +	test->adjust_tail = true;
> > +	test->total_steps = 1;
> > +
> > +	pkt_stream_replace_ifobject(test->ifobj_tx, pkt_cnt, pkt_len);
> > +	pkt_stream_replace_ifobject(test->ifobj_rx, pkt_cnt, pkt_len +
> > +value);
> > +
> > +	ret = testapp_xdp_adjust_tail(test, value);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (!test->adjust_tail_support) {
> > +		ksft_test_result_skip("%s %sResize pkt with
> bpf_xdp_adjust_tail() not supported\n",
> > +				      mode_string(test), busy_poll_string(test));
> > +	return TEST_SKIP;
> 
> missing indent
> 

will do.

> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int testapp_adjust_tail_common(struct test_spec *test, int
> adjust_value, u32 len,
> > +				      bool set_mtu)
> > +{
> > +	if (set_mtu)
> > +		test->mtu = MAX_ETH_JUMBO_SIZE;
> 
> could you remove this and instead of bool_set_mtu just pass the value of
> mtu?
> 

will do.

> static int testapp_adjust_tail(struct test_spec *test, u32 value, u32 pkt_len,
> u32 mtu) {
> 	(...)
> 	if (test->mtu != mtu)
> 		test->mtu = mtu;
> 	(...)
> }
> 
> static int testapp_adjust_tail_shrink(struct test_spec *test) {
> 	return testapp_adjust_tail(test, -4, MIN_PKT_SIZE,
> MAX_ETH_PKT_SIZE); }
> 
> static int testapp_adjust_tail_shrink_mb(struct test_spec *test) {
> 	return testapp_adjust_tail(test, -4,
> XSK_RING_PROD__DEFAULT_NUM_DESCS * 3,
> 				   MAX_ETH_JUMBO_SIZE);
> }
> 
> no need for _common func.
> 

will do.

> > +	return testapp_adjust_tail(test, adjust_value, len); }
> > +
> > +static int testapp_adjust_tail_shrink(struct test_spec *test) {
> > +	return testapp_adjust_tail_common(test, -4, MIN_PKT_SIZE, false); }
> > +
> > +static int testapp_adjust_tail_shrink_mb(struct test_spec *test) {
> > +	return testapp_adjust_tail_common(test, -4,
> > +XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);
> 
> Am I reading this right that you are modifying the size by just 4 bytes?
> The bugs that drivers had were for cases when packets got modified by value
> bigger than frag size which caused for example underlying page being freed.
> 
> If that is the case tests do nothing valuable from my perspective.
> 

In the v4 patchset, I have updated the code to modify the packet size by
1024 bytes instead of just 4 bytes.
I will send v4.

> > +}
> > +
> > +static int testapp_adjust_tail_grow(struct test_spec *test) {
> > +	return testapp_adjust_tail_common(test, 4, MIN_PKT_SIZE, false); }
> > +
> > +static int testapp_adjust_tail_grow_mb(struct test_spec *test) {
> > +	return testapp_adjust_tail_common(test, 4,
> > +XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true); }
> > +
> >  static void run_pkt_test(struct test_spec *test)  {
> >  	int ret;
> > @@ -2622,6 +2721,10 @@ static const struct test_spec tests[] = {
> >  	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
> >  	{.name = "HW_SW_MIN_RING_SIZE", .test_func =
> testapp_hw_sw_min_ring_size},
> >  	{.name = "HW_SW_MAX_RING_SIZE", .test_func =
> > testapp_hw_sw_max_ring_size},
> > +	{.name = "XDP_ADJUST_TAIL_SHRINK", .test_func =
> testapp_adjust_tail_shrink},
> > +	{.name = "XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF", .test_func =
> testapp_adjust_tail_shrink_mb},
> > +	{.name = "XDP_ADJUST_TAIL_GROW", .test_func =
> testapp_adjust_tail_grow},
> > +	{.name = "XDP_ADJUST_TAIL_GROW_MULTI_BUFF", .test_func =
> > +testapp_adjust_tail_grow_mb},
> >  	};
> >
> >  static void print_tests(void)
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h
> > b/tools/testing/selftests/bpf/xskxceiver.h
> > index e46e823f6a1a..67fc44b2813b 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -173,6 +173,8 @@ struct test_spec {
> >  	u16 nb_sockets;
> >  	bool fail;
> >  	bool set_ring;
> > +	bool adjust_tail;
> > +	bool adjust_tail_support;
> >  	enum test_mode mode;
> >  	char name[MAX_TEST_NAME_SIZE];
> >  };
> > --
> > 2.34.1
> >

Re: [PATCH bpf-next v3 2/2] selftests/xsk: Add tail adjustment tests and support check

[Maciej Fijalkowski]

On Tue, Mar 18, 2025 at 10:22:55AM +0100, Vyavahare, Tushar wrote:
> 
> 
> > -----Original Message-----
> > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> > Sent: Wednesday, March 12, 2025 3:41 AM
> > 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 v3 2/2] selftests/xsk: Add tail adjustment tests
> > and support check
> > 
> > On Wed, Mar 05, 2025 at 02:18:13PM +0000, Tushar Vyavahare wrote:
> > > Introduce tail adjustment functionality in xskxceiver using
> > > bpf_xdp_adjust_tail(). Add `xsk_xdp_adjust_tail` to modify packet
> > > sizes and drop unmodified packets. Implement
> > > `is_adjust_tail_supported` to check helper availability. Develop
> > > packet resizing tests, including shrinking and growing scenarios, with
> > > functions for both single-buffer and multi-buffer cases. Update the
> > > test framework to handle various scenarios and adjust MTU settings.
> > > These changes enhance the testing of packet tail adjustments, improving
> > AF_XDP framework reliability.
> > >
> > > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> > > ---
> > >  .../selftests/bpf/progs/xsk_xdp_progs.c       |  49 ++++++++
> > >  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
> > >  tools/testing/selftests/bpf/xskxceiver.c      | 107 +++++++++++++++++-
> > >  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
> > >  4 files changed, 157 insertions(+), 2 deletions(-)
> > >
> > > +	return testapp_adjust_tail(test, adjust_value, len); }
> > > +
> > > +static int testapp_adjust_tail_shrink(struct test_spec *test) {
> > > +	return testapp_adjust_tail_common(test, -4, MIN_PKT_SIZE, false); }
> > > +
> > > +static int testapp_adjust_tail_shrink_mb(struct test_spec *test) {
> > > +	return testapp_adjust_tail_common(test, -4,
> > > +XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);
> > 
> > Am I reading this right that you are modifying the size by just 4 bytes?
> > The bugs that drivers had were for cases when packets got modified by value
> > bigger than frag size which caused for example underlying page being freed.
> > 
> > If that is the case tests do nothing valuable from my perspective.
> > 
> 
> In the v4 patchset, I have updated the code to modify the packet size by
> 1024 bytes instead of just 4 bytes.

Why this value?

> I will send v4.

RE: [PATCH bpf-next v3 2/2] selftests/xsk: Add tail adjustment tests and support check

[Vyavahare, Tushar]

> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Sent: Tuesday, March 18, 2025 8:28 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 v3 2/2] selftests/xsk: Add tail adjustment tests
> and support check
> 
> On Tue, Mar 18, 2025 at 10:22:55AM +0100, Vyavahare, Tushar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> > > Sent: Wednesday, March 12, 2025 3:41 AM
> > > 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 v3 2/2] selftests/xsk: Add tail
> > > adjustment tests and support check
> > >
> > > On Wed, Mar 05, 2025 at 02:18:13PM +0000, Tushar Vyavahare wrote:
> > > > Introduce tail adjustment functionality in xskxceiver using
> > > > bpf_xdp_adjust_tail(). Add `xsk_xdp_adjust_tail` to modify packet
> > > > sizes and drop unmodified packets. Implement
> > > > `is_adjust_tail_supported` to check helper availability. Develop
> > > > packet resizing tests, including shrinking and growing scenarios,
> > > > with functions for both single-buffer and multi-buffer cases.
> > > > Update the test framework to handle various scenarios and adjust MTU
> settings.
> > > > These changes enhance the testing of packet tail adjustments,
> > > > improving
> > > AF_XDP framework reliability.
> > > >
> > > > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
> > > > ---
> > > >  .../selftests/bpf/progs/xsk_xdp_progs.c       |  49 ++++++++
> > > >  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
> > > >  tools/testing/selftests/bpf/xskxceiver.c      | 107 +++++++++++++++++-
> > > >  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
> > > >  4 files changed, 157 insertions(+), 2 deletions(-)
> > > >
> > > > +	return testapp_adjust_tail(test, adjust_value, len); }
> > > > +
> > > > +static int testapp_adjust_tail_shrink(struct test_spec *test) {
> > > > +	return testapp_adjust_tail_common(test, -4, MIN_PKT_SIZE,
> > > > +false); }
> > > > +
> > > > +static int testapp_adjust_tail_shrink_mb(struct test_spec *test) {
> > > > +	return testapp_adjust_tail_common(test, -4,
> > > > +XSK_RING_PROD__DEFAULT_NUM_DESCS * 3, true);
> > >
> > > Am I reading this right that you are modifying the size by just 4 bytes?
> > > The bugs that drivers had were for cases when packets got modified
> > > by value bigger than frag size which caused for example underlying page
> being freed.
> > >
> > > If that is the case tests do nothing valuable from my perspective.
> > >
> >
> > In the v4 patchset, I have updated the code to modify the packet size
> > by
> > 1024 bytes instead of just 4 bytes.
> 
> Why this value?
> 

Thanks for the clarification, Maciej. Based on our discussion, add
comments and modify the code for buffer resizing logic in the test
cases to shrink/grow by specific byte sizes for testing purposes.

> > I will send v4.