* [PATCH bpf-next] selftests: xsk: read current MAX_SKB_FRAGS from sysctl knob
@ 2024-09-09 14:11 Maciej Fijalkowski
2024-09-10 11:48 ` Magnus Karlsson
0 siblings, 1 reply; 3+ messages in thread
From: Maciej Fijalkowski @ 2024-09-09 14:11 UTC (permalink / raw)
To: bpf, ast, daniel, andrii
Cc: netdev, magnus.karlsson, bjorn, Maciej Fijalkowski
Currently, xskxceiver assumes that MAX_SKB_FRAGS value is always 17
which is not true - since the introduction of BIG TCP this can now take
any value between 17 to 45 via CONFIG_MAX_SKB_FRAGS.
Adjust the TOO_MANY_FRAGS test case to read the currently configured
MAX_SKB_FRAGS value by reading it from /proc/sys/net/core/max_skb_frags.
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
tools/testing/selftests/bpf/xskxceiver.c | 41 +++++++++++++++++++++---
tools/testing/selftests/bpf/xskxceiver.h | 1 -
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 92af633faea8..595b6da26897 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -325,6 +325,25 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
return zc_avail;
}
+#define MAX_SKB_FRAGS_PATH "/proc/sys/net/core/max_skb_frags"
+static unsigned int get_max_skb_frags(void)
+{
+ unsigned int max_skb_frags = 0;
+ FILE *file;
+
+ file = fopen(MAX_SKB_FRAGS_PATH, "r");
+ if (!file) {
+ ksft_print_msg("Error opening %s\n", MAX_SKB_FRAGS_PATH);
+ return 0;
+ }
+
+ if (fscanf(file, "%u", &max_skb_frags) != 1)
+ ksft_print_msg("Error reading %s\n", MAX_SKB_FRAGS_PATH);
+
+ fclose(file);
+ return max_skb_frags;
+}
+
static struct option long_options[] = {
{"interface", required_argument, 0, 'i'},
{"busy-poll", no_argument, 0, 'b'},
@@ -2245,13 +2264,22 @@ static int testapp_poll_rxq_tmout(struct test_spec *test)
static int testapp_too_many_frags(struct test_spec *test)
{
- struct pkt pkts[2 * XSK_DESC__MAX_SKB_FRAGS + 2] = {};
+ struct pkt *pkts;
u32 max_frags, i;
+ int ret;
- if (test->mode == TEST_MODE_ZC)
+ if (test->mode == TEST_MODE_ZC) {
max_frags = test->ifobj_tx->xdp_zc_max_segs;
- else
- max_frags = XSK_DESC__MAX_SKB_FRAGS;
+ } else {
+ max_frags = get_max_skb_frags();
+ if (!max_frags)
+ return TEST_FAILURE;
+ max_frags += 1;
+ }
+
+ pkts = calloc(2 * max_frags + 2, sizeof(struct pkt));
+ if (!pkts)
+ return TEST_FAILURE;
test->mtu = MAX_ETH_JUMBO_SIZE;
@@ -2281,7 +2309,10 @@ static int testapp_too_many_frags(struct test_spec *test)
pkts[2 * max_frags + 1].valid = true;
pkt_stream_generate_custom(test, pkts, 2 * max_frags + 2);
- return testapp_validate_traffic(test);
+ ret = testapp_validate_traffic(test);
+
+ free(pkts);
+ return ret;
}
static int xsk_load_xdp_programs(struct ifobject *ifobj)
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 885c948c5d83..e46e823f6a1a 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -55,7 +55,6 @@
#define XSK_UMEM__LARGE_FRAME_SIZE (3 * 1024)
#define XSK_UMEM__MAX_FRAME_SIZE (4 * 1024)
#define XSK_DESC__INVALID_OPTION (0xffff)
-#define XSK_DESC__MAX_SKB_FRAGS 18
#define HUGEPAGE_SIZE (2 * 1024 * 1024)
#define PKT_DUMP_NB_TO_PRINT 16
#define RUN_ALL_TESTS UINT_MAX
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] selftests: xsk: read current MAX_SKB_FRAGS from sysctl knob
2024-09-09 14:11 [PATCH bpf-next] selftests: xsk: read current MAX_SKB_FRAGS from sysctl knob Maciej Fijalkowski
@ 2024-09-10 11:48 ` Magnus Karlsson
2024-09-10 12:00 ` Maciej Fijalkowski
0 siblings, 1 reply; 3+ messages in thread
From: Magnus Karlsson @ 2024-09-10 11:48 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: bpf, ast, daniel, andrii, netdev, magnus.karlsson, bjorn
On Mon, 9 Sept 2024 at 16:12, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> Currently, xskxceiver assumes that MAX_SKB_FRAGS value is always 17
> which is not true - since the introduction of BIG TCP this can now take
> any value between 17 to 45 via CONFIG_MAX_SKB_FRAGS.
>
> Adjust the TOO_MANY_FRAGS test case to read the currently configured
> MAX_SKB_FRAGS value by reading it from /proc/sys/net/core/max_skb_frags.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
> tools/testing/selftests/bpf/xskxceiver.c | 41 +++++++++++++++++++++---
> tools/testing/selftests/bpf/xskxceiver.h | 1 -
> 2 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 92af633faea8..595b6da26897 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -325,6 +325,25 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
> return zc_avail;
> }
>
> +#define MAX_SKB_FRAGS_PATH "/proc/sys/net/core/max_skb_frags"
> +static unsigned int get_max_skb_frags(void)
> +{
> + unsigned int max_skb_frags = 0;
> + FILE *file;
> +
> + file = fopen(MAX_SKB_FRAGS_PATH, "r");
> + if (!file) {
> + ksft_print_msg("Error opening %s\n", MAX_SKB_FRAGS_PATH);
> + return 0;
> + }
> +
> + if (fscanf(file, "%u", &max_skb_frags) != 1)
> + ksft_print_msg("Error reading %s\n", MAX_SKB_FRAGS_PATH);
> +
> + fclose(file);
> + return max_skb_frags;
> +}
> +
> static struct option long_options[] = {
> {"interface", required_argument, 0, 'i'},
> {"busy-poll", no_argument, 0, 'b'},
> @@ -2245,13 +2264,22 @@ static int testapp_poll_rxq_tmout(struct test_spec *test)
>
> static int testapp_too_many_frags(struct test_spec *test)
> {
> - struct pkt pkts[2 * XSK_DESC__MAX_SKB_FRAGS + 2] = {};
> + struct pkt *pkts;
> u32 max_frags, i;
> + int ret;
>
> - if (test->mode == TEST_MODE_ZC)
> + if (test->mode == TEST_MODE_ZC) {
> max_frags = test->ifobj_tx->xdp_zc_max_segs;
> - else
> - max_frags = XSK_DESC__MAX_SKB_FRAGS;
> + } else {
> + max_frags = get_max_skb_frags();
> + if (!max_frags)
> + return TEST_FAILURE;
Thanks for this fix Maciej. However, I think failing the test here is
a little bit too drastic. How about just returning TEST_SKIP and print
out that the max number of skbs is unknown as the reason for the skip?
Or even more optimistically, print out a warning that we could not
read the max number of skb but we are guessing 17 and then run the
test? If it passes, great we guessed correctly, but if it fails we are
not worse off than the current code. Do not know how often a file
system does not contain /proc/sys/net/core/max_skb_frags though.
> + max_frags += 1;
> + }
> +
> + pkts = calloc(2 * max_frags + 2, sizeof(struct pkt));
> + if (!pkts)
> + return TEST_FAILURE;
>
> test->mtu = MAX_ETH_JUMBO_SIZE;
>
> @@ -2281,7 +2309,10 @@ static int testapp_too_many_frags(struct test_spec *test)
> pkts[2 * max_frags + 1].valid = true;
>
> pkt_stream_generate_custom(test, pkts, 2 * max_frags + 2);
> - return testapp_validate_traffic(test);
> + ret = testapp_validate_traffic(test);
> +
> + free(pkts);
> + return ret;
> }
>
> static int xsk_load_xdp_programs(struct ifobject *ifobj)
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index 885c948c5d83..e46e823f6a1a 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -55,7 +55,6 @@
> #define XSK_UMEM__LARGE_FRAME_SIZE (3 * 1024)
> #define XSK_UMEM__MAX_FRAME_SIZE (4 * 1024)
> #define XSK_DESC__INVALID_OPTION (0xffff)
> -#define XSK_DESC__MAX_SKB_FRAGS 18
> #define HUGEPAGE_SIZE (2 * 1024 * 1024)
> #define PKT_DUMP_NB_TO_PRINT 16
> #define RUN_ALL_TESTS UINT_MAX
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] selftests: xsk: read current MAX_SKB_FRAGS from sysctl knob
2024-09-10 11:48 ` Magnus Karlsson
@ 2024-09-10 12:00 ` Maciej Fijalkowski
0 siblings, 0 replies; 3+ messages in thread
From: Maciej Fijalkowski @ 2024-09-10 12:00 UTC (permalink / raw)
To: Magnus Karlsson; +Cc: bpf, ast, daniel, andrii, netdev, magnus.karlsson, bjorn
On Tue, Sep 10, 2024 at 01:48:35PM +0200, Magnus Karlsson wrote:
> On Mon, 9 Sept 2024 at 16:12, Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > Currently, xskxceiver assumes that MAX_SKB_FRAGS value is always 17
> > which is not true - since the introduction of BIG TCP this can now take
> > any value between 17 to 45 via CONFIG_MAX_SKB_FRAGS.
> >
> > Adjust the TOO_MANY_FRAGS test case to read the currently configured
> > MAX_SKB_FRAGS value by reading it from /proc/sys/net/core/max_skb_frags.
> >
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> > tools/testing/selftests/bpf/xskxceiver.c | 41 +++++++++++++++++++++---
> > tools/testing/selftests/bpf/xskxceiver.h | 1 -
> > 2 files changed, 36 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 92af633faea8..595b6da26897 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -325,6 +325,25 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
> > return zc_avail;
> > }
> >
> > +#define MAX_SKB_FRAGS_PATH "/proc/sys/net/core/max_skb_frags"
> > +static unsigned int get_max_skb_frags(void)
> > +{
> > + unsigned int max_skb_frags = 0;
> > + FILE *file;
> > +
> > + file = fopen(MAX_SKB_FRAGS_PATH, "r");
> > + if (!file) {
> > + ksft_print_msg("Error opening %s\n", MAX_SKB_FRAGS_PATH);
> > + return 0;
> > + }
> > +
> > + if (fscanf(file, "%u", &max_skb_frags) != 1)
> > + ksft_print_msg("Error reading %s\n", MAX_SKB_FRAGS_PATH);
> > +
> > + fclose(file);
> > + return max_skb_frags;
> > +}
> > +
> > static struct option long_options[] = {
> > {"interface", required_argument, 0, 'i'},
> > {"busy-poll", no_argument, 0, 'b'},
> > @@ -2245,13 +2264,22 @@ static int testapp_poll_rxq_tmout(struct test_spec *test)
> >
> > static int testapp_too_many_frags(struct test_spec *test)
> > {
> > - struct pkt pkts[2 * XSK_DESC__MAX_SKB_FRAGS + 2] = {};
> > + struct pkt *pkts;
> > u32 max_frags, i;
> > + int ret;
> >
> > - if (test->mode == TEST_MODE_ZC)
> > + if (test->mode == TEST_MODE_ZC) {
> > max_frags = test->ifobj_tx->xdp_zc_max_segs;
> > - else
> > - max_frags = XSK_DESC__MAX_SKB_FRAGS;
> > + } else {
> > + max_frags = get_max_skb_frags();
> > + if (!max_frags)
> > + return TEST_FAILURE;
>
> Thanks for this fix Maciej. However, I think failing the test here is
> a little bit too drastic. How about just returning TEST_SKIP and print
> out that the max number of skbs is unknown as the reason for the skip?
> Or even more optimistically, print out a warning that we could not
> read the max number of skb but we are guessing 17 and then run the
> test? If it passes, great we guessed correctly, but if it fails we are
> not worse off than the current code.
makes sense to default to 17 if we couldn't read it from file. will fix in
v2.
> Do not know how often a file
> system does not contain /proc/sys/net/core/max_skb_frags though.
A mix of CONFIG_NET, CONFIG_PROC_FS and CONFIG_SYSCTL i think.
>
> > + max_frags += 1;
> > + }
> > +
> > + pkts = calloc(2 * max_frags + 2, sizeof(struct pkt));
> > + if (!pkts)
> > + return TEST_FAILURE;
> >
> > test->mtu = MAX_ETH_JUMBO_SIZE;
> >
> > @@ -2281,7 +2309,10 @@ static int testapp_too_many_frags(struct test_spec *test)
> > pkts[2 * max_frags + 1].valid = true;
> >
> > pkt_stream_generate_custom(test, pkts, 2 * max_frags + 2);
> > - return testapp_validate_traffic(test);
> > + ret = testapp_validate_traffic(test);
> > +
> > + free(pkts);
> > + return ret;
> > }
> >
> > static int xsk_load_xdp_programs(struct ifobject *ifobj)
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> > index 885c948c5d83..e46e823f6a1a 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -55,7 +55,6 @@
> > #define XSK_UMEM__LARGE_FRAME_SIZE (3 * 1024)
> > #define XSK_UMEM__MAX_FRAME_SIZE (4 * 1024)
> > #define XSK_DESC__INVALID_OPTION (0xffff)
> > -#define XSK_DESC__MAX_SKB_FRAGS 18
> > #define HUGEPAGE_SIZE (2 * 1024 * 1024)
> > #define PKT_DUMP_NB_TO_PRINT 16
> > #define RUN_ALL_TESTS UINT_MAX
> > --
> > 2.34.1
> >
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-10 12:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-09 14:11 [PATCH bpf-next] selftests: xsk: read current MAX_SKB_FRAGS from sysctl knob Maciej Fijalkowski
2024-09-10 11:48 ` Magnus Karlsson
2024-09-10 12:00 ` Maciej Fijalkowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).