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

[PATCH bpf-next v2 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.

---
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       |  48 +++++++
 tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
 tools/testing/selftests/bpf/xskxceiver.c      | 131 ++++++++++++++++--
 tools/testing/selftests/bpf/xskxceiver.h      |   2 +
 4 files changed, 174 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH bpf-next v2 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 v2 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       |  48 +++++++
 tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
 tools/testing/selftests/bpf/xskxceiver.c      | 118 +++++++++++++++++-
 tools/testing/selftests/bpf/xskxceiver.h      |   2 +
 4 files changed, 167 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..2e8e2faf17e0 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 {
@@ -70,4 +72,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, count);
+	if (ret < 0) {
+		/* Handle unsupported cases */
+		if (ret == -EOPNOTSUPP) {
+			/* Set count to -EOPNOTSUPP to indicate to userspace that this case is
+			 * unsupported
+			 */
+			count = -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 + count)
+		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 'count' 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 - count, &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..ee196b638662 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 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, &value);
+	if (ret) {
+		ksft_print_msg("Error: bpf_map_lookup_elem failed with error %d\n", ret);
+		return false;
+	}
+
+	/* Set the 'count' 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 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,84 @@ 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 count)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+	struct bpf_map *data_map;
+	int key = 0;
+
+	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);
+
+	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");
+		return TEST_FAILURE;
+	}
+
+	if (bpf_map_update_elem(bpf_map__fd(data_map), &key, &count, BPF_ANY)) {
+		ksft_print_msg("Error: could not update count element\n");
+		return TEST_FAILURE;
+	}
+
+	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 +2732,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 v2 2/2] selftests/xsk: Add tail adjustment tests and support check

[Maciej Fijalkowski]

On Thu, Feb 27, 2025 at 02:27:37PM +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       |  48 +++++++
>  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
>  tools/testing/selftests/bpf/xskxceiver.c      | 118 +++++++++++++++++-
>  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
>  4 files changed, 167 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..2e8e2faf17e0 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 {
> @@ -70,4 +72,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, count);
> +	if (ret < 0) {
> +		/* Handle unsupported cases */
> +		if (ret == -EOPNOTSUPP) {
> +			/* Set count to -EOPNOTSUPP to indicate to userspace that this case is
> +			 * unsupported
> +			 */
> +			count = -EOPNOTSUPP;
> +			return bpf_redirect_map(&xsk, 0, XDP_DROP);

is this whole eopnotsupp dance worth the hassle?

this basically breaks down to underlying driver not supporting xdp
multi-buffer. we already store this state in ifobj->multi_buff_supp.

could we just check for that and skip the test case instead of using the
count global variable to store the error code which is counter intuitive?

> +		}
> +
> +		return XDP_DROP;
> +	}
> +
> +	curr_buff_len = bpf_xdp_get_buff_len(xdp);
> +	if (curr_buff_len != buff_len + count)
> +		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 'count' 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 - count, &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..ee196b638662 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 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, &value);
> +	if (ret) {
> +		ksft_print_msg("Error: bpf_map_lookup_elem failed with error %d\n", ret);
> +		return false;
> +	}
> +
> +	/* Set the 'count' 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 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,84 @@ 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 count)
> +{
> +	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> +	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> +	struct bpf_map *data_map;
> +	int key = 0;
> +
> +	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);
> +
> +	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");
> +		return TEST_FAILURE;
> +	}
> +
> +	if (bpf_map_update_elem(bpf_map__fd(data_map), &key, &count, BPF_ANY)) {
> +		ksft_print_msg("Error: could not update count element\n");
> +		return TEST_FAILURE;
> +	}
> +
> +	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;

couldn't we base this on BPF_F_XDP_HAS_FRAGS in some way instead of
boolean var?

> +	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 +2732,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 v2 2/2] selftests/xsk: Add tail adjustment tests and support check

[Vyavahare, Tushar]

> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Sent: Thursday, February 27, 2025 11:52 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 v2 2/2] selftests/xsk: Add tail adjustment tests
> and support check
> 
> On Thu, Feb 27, 2025 at 02:27:37PM +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       |  48 +++++++
> >  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
> >  tools/testing/selftests/bpf/xskxceiver.c      | 118 +++++++++++++++++-
> >  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
> >  4 files changed, 167 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..2e8e2faf17e0 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 {
> > @@ -70,4 +72,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, count);
> > +	if (ret < 0) {
> > +		/* Handle unsupported cases */
> > +		if (ret == -EOPNOTSUPP) {
> > +			/* Set count to -EOPNOTSUPP to indicate to userspace
> that this case is
> > +			 * unsupported
> > +			 */
> > +			count = -EOPNOTSUPP;
> > +			return bpf_redirect_map(&xsk, 0, XDP_DROP);
> 
> is this whole eopnotsupp dance worth the hassle?
> 
> this basically breaks down to underlying driver not supporting xdp multi-
> buffer. we already store this state in ifobj->multi_buff_supp.
> 
> could we just check for that and skip the test case instead of using the count
> global variable to store the error code which is counter intuitive?
> 

Thanks, Multi-buff is supported it might be that growing is not supported
but shrinking is supported. We have difference in result for shrinking and
growing tests. We are handling these cases with the existing 'count'
variable instead of introducing another variable to indicate or access in
userspace.

Here's the result matrix:
Driver/Mode	XDP_ADJUST_TAIL_SHRINK	XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF	XDP_ADJUST_TAIL_GROW	XDP_ADJUST_TAIL_GROW_MULTI_BUFF
virt-eth DRV		PASS					PASS					FAIL(EINNVAL)			SKIP (EOPNOTSUPP)
virt-eth SKB		PASS					PASS					FAIL(EINNVAL)			SKIP (EOPNOTSUPP)
i40e SKB		PASS					PASS					FAIL(EINNVAL)			SKIP (EOPNOTSUPP)
i40e DRV		PASS					PASS					PASS				PASS
i40e ZC			PASS					PASS					PASS				PASS
i40e SKB BUSY-POLL	PASS					PASS					FAIL(EINNVAL)			SKIP (Not supported)
i40e DRV BUSY-POLL	PASS					PASS					PASS				PASS
i40e ZC BUSY-POLL	PASS					PASS					PASS				PASS
ice SKB			PASS					PASS					FAIL(EINNVAL)			SKIP (Not supported)
ice DRV			PASS					PASS					PASS				PASS
ice ZC			PASS					PASS					PASS				PASS
ice SKB BUSY-POLL	PASS					PASS					FAIL(EINNVAL)			SKIP (Not supported)
ice DRV BUSY-POLL	PASS					PASS					PASS				PASS
ice ZC BUSY-POLL	PASS					PASS					PASS				PASS

> > +		}
> > +
> > +		return XDP_DROP;
> > +	}
> > +
> > +	curr_buff_len = bpf_xdp_get_buff_len(xdp);
> > +	if (curr_buff_len != buff_len + count)
> > +		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 'count' 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 - count, &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..ee196b638662 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 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, &value);
> > +	if (ret) {
> > +		ksft_print_msg("Error: bpf_map_lookup_elem failed with
> error %d\n", ret);
> > +		return false;
> > +	}
> > +
> > +	/* Set the 'count' 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 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,84 @@ 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 count)
> > +{
> > +	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> > +	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> > +	struct bpf_map *data_map;
> > +	int key = 0;
> > +
> > +	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);
> > +
> > +	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");
> > +		return TEST_FAILURE;
> > +	}
> > +
> > +	if (bpf_map_update_elem(bpf_map__fd(data_map), &key, &count,
> BPF_ANY)) {
> > +		ksft_print_msg("Error: could not update count element\n");
> > +		return TEST_FAILURE;
> > +	}
> > +
> > +	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;
> 
> couldn't we base this on BPF_F_XDP_HAS_FRAGS in some way instead of
> boolean var?
> 

No, it is being set much later. Test framework needs mtu to be set to
enable multi-buffer.

> > +	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 +2732,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 v2 2/2] selftests/xsk: Add tail adjustment tests and support check

[Maciej Fijalkowski]

On Fri, Feb 28, 2025 at 10:56:19AM +0100, Vyavahare, Tushar wrote:
> 
> 
> > -----Original Message-----
> > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> > Sent: Thursday, February 27, 2025 11:52 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 v2 2/2] selftests/xsk: Add tail adjustment tests
> > and support check
> > 
> > On Thu, Feb 27, 2025 at 02:27:37PM +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       |  48 +++++++
> > >  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
> > >  tools/testing/selftests/bpf/xskxceiver.c      | 118 +++++++++++++++++-
> > >  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
> > >  4 files changed, 167 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..2e8e2faf17e0 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 {
> > > @@ -70,4 +72,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, count);
> > > +	if (ret < 0) {
> > > +		/* Handle unsupported cases */
> > > +		if (ret == -EOPNOTSUPP) {
> > > +			/* Set count to -EOPNOTSUPP to indicate to userspace
> > that this case is
> > > +			 * unsupported
> > > +			 */
> > > +			count = -EOPNOTSUPP;
> > > +			return bpf_redirect_map(&xsk, 0, XDP_DROP);
> > 
> > is this whole eopnotsupp dance worth the hassle?
> > 
> > this basically breaks down to underlying driver not supporting xdp multi-
> > buffer. we already store this state in ifobj->multi_buff_supp.
> > 
> > could we just check for that and skip the test case instead of using the count
> > global variable to store the error code which is counter intuitive?
> > 
> 
> Thanks, Multi-buff is supported it might be that growing is not supported
> but shrinking is supported. We have difference in result for shrinking and
> growing tests. We are handling these cases with the existing 'count'
> variable instead of introducing another variable to indicate or access in
> userspace.

These tests were supposed to exercise bugs against tail adjustment in
multi-buffer scenarios, hence my comment to base this on this setting.

I won't insist on simplifying it if you decide to keep this but please use
different variable for communication with user space. We're not short on
resources and count = -EOPNOTSUPP looks awkward.

> 
> Here's the result matrix:
> Driver/Mode	XDP_ADJUST_TAIL_SHRINK	XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF	XDP_ADJUST_TAIL_GROW	XDP_ADJUST_TAIL_GROW_MULTI_BUFF
> virt-eth DRV		PASS					PASS					FAIL(EINNVAL)			SKIP (EOPNOTSUPP)
> virt-eth SKB		PASS					PASS					FAIL(EINNVAL)			SKIP (EOPNOTSUPP)
> i40e SKB		PASS					PASS					FAIL(EINNVAL)			SKIP (EOPNOTSUPP)
> i40e DRV		PASS					PASS					PASS				PASS
> i40e ZC			PASS					PASS					PASS				PASS
> i40e SKB BUSY-POLL	PASS					PASS					FAIL(EINNVAL)			SKIP (Not supported)
> i40e DRV BUSY-POLL	PASS					PASS					PASS				PASS
> i40e ZC BUSY-POLL	PASS					PASS					PASS				PASS
> ice SKB			PASS					PASS					FAIL(EINNVAL)			SKIP (Not supported)
> ice DRV			PASS					PASS					PASS				PASS
> ice ZC			PASS					PASS					PASS				PASS
> ice SKB BUSY-POLL	PASS					PASS					FAIL(EINNVAL)			SKIP (Not supported)
> ice DRV BUSY-POLL	PASS					PASS					PASS				PASS
> ice ZC BUSY-POLL	PASS					PASS					PASS				PASS
> 
> > > +		}
> > > +
> > > +		return XDP_DROP;
> > > +	}
> > > +
> > > +	curr_buff_len = bpf_xdp_get_buff_len(xdp);
> > > +	if (curr_buff_len != buff_len + count)
> > > +		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 'count' 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 - count, &seq_num,
> > sizeof(seq_num));
> > > +	}
> > > +
> > > +	return bpf_redirect_map(&xsk, 0, XDP_DROP); }
> > > +
> > >  char _license[] SEC("license") = "GPL"; diff --git

(...)

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

[Vyavahare, Tushar]

> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Sent: Monday, March 3, 2025 9:45 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 v2 2/2] selftests/xsk: Add tail adjustment tests
> and support check
> 
> On Fri, Feb 28, 2025 at 10:56:19AM +0100, Vyavahare, Tushar wrote:
> >
> >
> > > -----Original Message-----
> > > From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> > > Sent: Thursday, February 27, 2025 11:52 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 v2 2/2] selftests/xsk: Add tail
> > > adjustment tests and support check
> > >
> > > On Thu, Feb 27, 2025 at 02:27:37PM +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       |  48 +++++++
> > > >  tools/testing/selftests/bpf/xsk_xdp_common.h  |   1 +
> > > >  tools/testing/selftests/bpf/xskxceiver.c      | 118 +++++++++++++++++-
> > > >  tools/testing/selftests/bpf/xskxceiver.h      |   2 +
> > > >  4 files changed, 167 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..2e8e2faf17e0 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 {
> > > > @@ -70,4 +72,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, count);
> > > > +	if (ret < 0) {
> > > > +		/* Handle unsupported cases */
> > > > +		if (ret == -EOPNOTSUPP) {
> > > > +			/* Set count to -EOPNOTSUPP to indicate to userspace
> > > that this case is
> > > > +			 * unsupported
> > > > +			 */
> > > > +			count = -EOPNOTSUPP;
> > > > +			return bpf_redirect_map(&xsk, 0, XDP_DROP);
> > >
> > > is this whole eopnotsupp dance worth the hassle?
> > >
> > > this basically breaks down to underlying driver not supporting xdp
> > > multi- buffer. we already store this state in ifobj->multi_buff_supp.
> > >
> > > could we just check for that and skip the test case instead of using
> > > the count global variable to store the error code which is counter intuitive?
> > >
> >
> > Thanks, Multi-buff is supported it might be that growing is not
> > supported but shrinking is supported. We have difference in result for
> > shrinking and growing tests. We are handling these cases with the existing
> 'count'
> > variable instead of introducing another variable to indicate or access
> > in userspace.
> 
> These tests were supposed to exercise bugs against tail adjustment in multi-
> buffer scenarios, hence my comment to base this on this setting.
> 
> I won't insist on simplifying it if you decide to keep this but please use
> different variable for communication with user space. We're not short on
> resources and count = -EOPNOTSUPP looks awkward.
> 

I'll address the variable naming and include the changes in the v3 patchset.

> >
> > Here's the result matrix:
> > Driver/Mode	XDP_ADJUST_TAIL_SHRINK
> 	XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF	XDP_ADJUST_TAIL_GROW
> 	XDP_ADJUST_TAIL_GROW_MULTI_BUFF
> > virt-eth DRV		PASS					PASS
> 				FAIL(EINNVAL)			SKIP
> (EOPNOTSUPP)
> > virt-eth SKB		PASS					PASS
> 				FAIL(EINNVAL)			SKIP
> (EOPNOTSUPP)
> > i40e SKB		PASS					PASS
> 				FAIL(EINNVAL)			SKIP
> (EOPNOTSUPP)
> > i40e DRV		PASS					PASS
> 				PASS				PASS
> > i40e ZC			PASS					PASS
> 					PASS				PASS
> > i40e SKB BUSY-POLL	PASS					PASS
> 				FAIL(EINNVAL)			SKIP (Not
> supported)
> > i40e DRV BUSY-POLL	PASS					PASS
> 				PASS				PASS
> > i40e ZC BUSY-POLL	PASS					PASS
> 				PASS				PASS
> > ice SKB			PASS					PASS
> 					FAIL(EINNVAL)			SKIP
> (Not supported)
> > ice DRV			PASS					PASS
> 					PASS				PASS
> > ice ZC			PASS					PASS
> 				PASS				PASS
> > ice SKB BUSY-POLL	PASS					PASS
> 				FAIL(EINNVAL)			SKIP (Not
> supported)
> > ice DRV BUSY-POLL	PASS					PASS
> 				PASS				PASS
> > ice ZC BUSY-POLL	PASS					PASS
> 				PASS				PASS
> >
> > > > +		}
> > > > +
> > > > +		return XDP_DROP;
> > > > +	}
> > > > +
> > > > +	curr_buff_len = bpf_xdp_get_buff_len(xdp);
> > > > +	if (curr_buff_len != buff_len + count)
> > > > +		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 'count' 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 - count, &seq_num,
> > > sizeof(seq_num));
> > > > +	}
> > > > +
> > > > +	return bpf_redirect_map(&xsk, 0, XDP_DROP); }
> > > > +
> > > >  char _license[] SEC("license") = "GPL"; diff --git
> 
> (...)