[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z963wAD8HhOKdcAG@boxer>
Date: Sat, 22 Mar 2025 14:14:40 +0100
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Tushar Vyavahare <tushar.vyavahare@...el.com>
CC: <bpf@...r.kernel.org>, <netdev@...r.kernel.org>, <bjorn@...nel.org>,
<magnus.karlsson@...el.com>, <jonathan.lemon@...il.com>,
<davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
<ast@...nel.org>, <daniel@...earbox.net>, <tirthendu.sarkar@...el.com>,
<bastien.curutchet@...tlin.com>
Subject: Re: [PATCH bpf-next v4 2/2] selftests/xsk: Add tail adjustment tests
and support check
On Fri, Mar 21, 2025 at 12:54:19AM +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@...el.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
We carry the description of executed tests at the beginning of
xskxceiver.c and you have not updated it with these adjust tail tests but
it's not a show-stopper to me. I'm okay with current state of this patch.
Bastien, you probably would want to take into consideration these changes
if they go in before your bigger work.
> ---
> .../selftests/bpf/progs/xsk_xdp_progs.c | 50 +++++++++
> tools/testing/selftests/bpf/xsk_xdp_common.h | 1 +
> tools/testing/selftests/bpf/xskxceiver.c | 105 +++++++++++++++++-
> tools/testing/selftests/bpf/xskxceiver.h | 2 +
> 4 files changed, 156 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..683306db8594 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,51 @@ 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 - sizeof(seq_num), &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..0ced4026ee44 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);
> + exit_with_error(errno);
> + }
> +
> + /* 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,71 @@ 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)
> +{
> + int ret;
> +
> + test->adjust_tail_support = true;
> + test->adjust_tail = true;
> + test->total_steps = 1;
> +
> + pkt_stream_replace_ifobject(test->ifobj_tx, DEFAULT_BATCH_SIZE, pkt_len);
> + pkt_stream_replace_ifobject(test->ifobj_rx, DEFAULT_BATCH_SIZE, 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_shrink(struct test_spec *test)
> +{
> + /* Shrink by 4 bytes for testing purpose */
> + return testapp_adjust_tail(test, -4, MIN_PKT_SIZE * 2);
> +}
> +
> +static int testapp_adjust_tail_shrink_mb(struct test_spec *test)
> +{
> + test->mtu = MAX_ETH_JUMBO_SIZE;
> + /* Shrink by the frag size */
> + return testapp_adjust_tail(test, -XSK_UMEM__MAX_FRAME_SIZE, XSK_UMEM__LARGE_FRAME_SIZE * 2);
> +}
> +
> +static int testapp_adjust_tail_grow(struct test_spec *test)
> +{
> + /* Grow by 4 bytes for testing purpose */
> + return testapp_adjust_tail(test, 4, MIN_PKT_SIZE * 2);
> +}
> +
> +static int testapp_adjust_tail_grow_mb(struct test_spec *test)
> +{
> + test->mtu = MAX_ETH_JUMBO_SIZE;
> + /* Grow by (frag_size - last_frag_Size) - 1 to stay inside the last fragment */
> + return testapp_adjust_tail(test, (XSK_UMEM__MAX_FRAME_SIZE / 2) - 1,
> + XSK_UMEM__LARGE_FRAME_SIZE * 2);
> +}
> +
> static void run_pkt_test(struct test_spec *test)
> {
> int ret;
> @@ -2622,6 +2719,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
>
Powered by blists - more mailing lists