* [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
@ 2022-02-04 13:58 Lorenzo Bianconi
2022-02-04 17:37 ` Yonghong Song
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-02-04 13:58 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, brouer, toke, lorenzo.bianconi, andrii, netdev
Update test_xdp_update_frags adding a test for a buffer size
set to (MAX_SKB_FRAGS + 2) * PAGE_SIZE. The kernel is supposed
to return -ENOMEM.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
.../bpf/prog_tests/xdp_adjust_frags.c | 37 ++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
index 134d0ac32f59..61d5b585eb15 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
@@ -5,11 +5,12 @@
void test_xdp_update_frags(void)
{
const char *file = "./test_xdp_update_frags.o";
+ int err, prog_fd, max_skb_frags, buf_size, num;
struct bpf_program *prog;
struct bpf_object *obj;
- int err, prog_fd;
__u32 *offset;
__u8 *buf;
+ FILE *f;
LIBBPF_OPTS(bpf_test_run_opts, topts);
obj = bpf_object__open(file);
@@ -99,6 +100,40 @@ void test_xdp_update_frags(void)
ASSERT_EQ(buf[7621], 0xbb, "xdp_update_frag buf[7621]");
free(buf);
+
+ /* test_xdp_update_frags: unsupported buffer size */
+ f = fopen("/proc/sys/net/core/max_skb_frags", "r");
+ if (!ASSERT_OK_PTR(f, "max_skb_frag file pointer"))
+ goto out;
+
+ num = fscanf(f, "%d", &max_skb_frags);
+ fclose(f);
+
+ if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
+ goto out;
+
+ /* xdp_buff linear area size is always set to 4096 in the
+ * bpf_prog_test_run_xdp routine.
+ */
+ buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
+ buf = malloc(buf_size);
+ if (!ASSERT_OK_PTR(buf, "alloc buf"))
+ goto out;
+
+ memset(buf, 0, buf_size);
+ offset = (__u32 *)buf;
+ *offset = 16;
+ buf[*offset] = 0xaa;
+ buf[*offset + 15] = 0xaa;
+
+ topts.data_in = buf;
+ topts.data_out = buf;
+ topts.data_size_in = buf_size;
+ topts.data_size_out = buf_size;
+
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
+ free(buf);
out:
bpf_object__close(obj);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-04 13:58 [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags Lorenzo Bianconi
@ 2022-02-04 17:37 ` Yonghong Song
2022-02-04 17:52 ` Lorenzo Bianconi
0 siblings, 1 reply; 8+ messages in thread
From: Yonghong Song @ 2022-02-04 17:37 UTC (permalink / raw)
To: Lorenzo Bianconi, bpf
Cc: ast, daniel, brouer, toke, lorenzo.bianconi, andrii, netdev
On 2/4/22 5:58 AM, Lorenzo Bianconi wrote:
> Update test_xdp_update_frags adding a test for a buffer size
> set to (MAX_SKB_FRAGS + 2) * PAGE_SIZE. The kernel is supposed
> to return -ENOMEM.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../bpf/prog_tests/xdp_adjust_frags.c | 37 ++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> index 134d0ac32f59..61d5b585eb15 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> @@ -5,11 +5,12 @@
> void test_xdp_update_frags(void)
> {
> const char *file = "./test_xdp_update_frags.o";
> + int err, prog_fd, max_skb_frags, buf_size, num;
> struct bpf_program *prog;
> struct bpf_object *obj;
> - int err, prog_fd;
> __u32 *offset;
> __u8 *buf;
> + FILE *f;
> LIBBPF_OPTS(bpf_test_run_opts, topts);
>
> obj = bpf_object__open(file);
> @@ -99,6 +100,40 @@ void test_xdp_update_frags(void)
> ASSERT_EQ(buf[7621], 0xbb, "xdp_update_frag buf[7621]");
>
> free(buf);
> +
> + /* test_xdp_update_frags: unsupported buffer size */
> + f = fopen("/proc/sys/net/core/max_skb_frags", "r");
> + if (!ASSERT_OK_PTR(f, "max_skb_frag file pointer"))
> + goto out;
In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
but if /proc/sys/net/core/max_skb_flags is 2 or more less
than MAX_SKB_FRAGS, the test won't fail, right?
> +
> + num = fscanf(f, "%d", &max_skb_frags);
> + fclose(f);
> +
> + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
> + goto out;
> +
> + /* xdp_buff linear area size is always set to 4096 in the
> + * bpf_prog_test_run_xdp routine.
> + */
> + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
> + buf = malloc(buf_size);
> + if (!ASSERT_OK_PTR(buf, "alloc buf"))
> + goto out;
> +
> + memset(buf, 0, buf_size);
> + offset = (__u32 *)buf;
> + *offset = 16;
> + buf[*offset] = 0xaa;
> + buf[*offset + 15] = 0xaa;
> +
> + topts.data_in = buf;
> + topts.data_out = buf;
> + topts.data_size_in = buf_size;
> + topts.data_size_out = buf_size;
> +
> + err = bpf_prog_test_run_opts(prog_fd, &topts);
> + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
> + free(buf);
> out:
> bpf_object__close(obj);
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-04 17:37 ` Yonghong Song
@ 2022-02-04 17:52 ` Lorenzo Bianconi
2022-02-04 18:14 ` Yonghong Song
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-02-04 17:52 UTC (permalink / raw)
To: Yonghong Song
Cc: Lorenzo Bianconi, bpf, ast, daniel, brouer, toke, andrii, netdev
[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]
>
>
> On 2/4/22 5:58 AM, Lorenzo Bianconi wrote:
> > Update test_xdp_update_frags adding a test for a buffer size
> > set to (MAX_SKB_FRAGS + 2) * PAGE_SIZE. The kernel is supposed
> > to return -ENOMEM.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > .../bpf/prog_tests/xdp_adjust_frags.c | 37 ++++++++++++++++++-
> > 1 file changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> > index 134d0ac32f59..61d5b585eb15 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> > @@ -5,11 +5,12 @@
> > void test_xdp_update_frags(void)
> > {
> > const char *file = "./test_xdp_update_frags.o";
> > + int err, prog_fd, max_skb_frags, buf_size, num;
> > struct bpf_program *prog;
> > struct bpf_object *obj;
> > - int err, prog_fd;
> > __u32 *offset;
> > __u8 *buf;
> > + FILE *f;
> > LIBBPF_OPTS(bpf_test_run_opts, topts);
> > obj = bpf_object__open(file);
> > @@ -99,6 +100,40 @@ void test_xdp_update_frags(void)
> > ASSERT_EQ(buf[7621], 0xbb, "xdp_update_frag buf[7621]");
> > free(buf);
> > +
> > + /* test_xdp_update_frags: unsupported buffer size */
> > + f = fopen("/proc/sys/net/core/max_skb_frags", "r");
> > + if (!ASSERT_OK_PTR(f, "max_skb_frag file pointer"))
> > + goto out;
>
> In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
> but if /proc/sys/net/core/max_skb_flags is 2 or more less
> than MAX_SKB_FRAGS, the test won't fail, right?
yes, you are right. Should we use the same definition used in
include/linux/skbuff.h instead? Something like:
if (65536 / page_size + 1 < 16)
max_skb_flags = 16;
else
max_skb_flags = 65536/page_size + 1;
Regards,
Lorenzo
>
> > +
> > + num = fscanf(f, "%d", &max_skb_frags);
> > + fclose(f);
> > +
> > + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
> > + goto out;
> > +
> > + /* xdp_buff linear area size is always set to 4096 in the
> > + * bpf_prog_test_run_xdp routine.
> > + */
> > + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
> > + buf = malloc(buf_size);
> > + if (!ASSERT_OK_PTR(buf, "alloc buf"))
> > + goto out;
> > +
> > + memset(buf, 0, buf_size);
> > + offset = (__u32 *)buf;
> > + *offset = 16;
> > + buf[*offset] = 0xaa;
> > + buf[*offset + 15] = 0xaa;
> > +
> > + topts.data_in = buf;
> > + topts.data_out = buf;
> > + topts.data_size_in = buf_size;
> > + topts.data_size_out = buf_size;
> > +
> > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
> > + free(buf);
> > out:
> > bpf_object__close(obj);
> > }
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-04 17:52 ` Lorenzo Bianconi
@ 2022-02-04 18:14 ` Yonghong Song
2022-02-04 18:30 ` Lorenzo Bianconi
0 siblings, 1 reply; 8+ messages in thread
From: Yonghong Song @ 2022-02-04 18:14 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Lorenzo Bianconi, bpf, ast, daniel, brouer, toke, andrii, netdev
On 2/4/22 9:52 AM, Lorenzo Bianconi wrote:
>>
>>
>> On 2/4/22 5:58 AM, Lorenzo Bianconi wrote:
>>> Update test_xdp_update_frags adding a test for a buffer size
>>> set to (MAX_SKB_FRAGS + 2) * PAGE_SIZE. The kernel is supposed
>>> to return -ENOMEM.
>>>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>> ---
>>> .../bpf/prog_tests/xdp_adjust_frags.c | 37 ++++++++++++++++++-
>>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>>> index 134d0ac32f59..61d5b585eb15 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>>> @@ -5,11 +5,12 @@
>>> void test_xdp_update_frags(void)
>>> {
>>> const char *file = "./test_xdp_update_frags.o";
>>> + int err, prog_fd, max_skb_frags, buf_size, num;
>>> struct bpf_program *prog;
>>> struct bpf_object *obj;
>>> - int err, prog_fd;
>>> __u32 *offset;
>>> __u8 *buf;
>>> + FILE *f;
>>> LIBBPF_OPTS(bpf_test_run_opts, topts);
>>> obj = bpf_object__open(file);
>>> @@ -99,6 +100,40 @@ void test_xdp_update_frags(void)
>>> ASSERT_EQ(buf[7621], 0xbb, "xdp_update_frag buf[7621]");
>>> free(buf);
>>> +
>>> + /* test_xdp_update_frags: unsupported buffer size */
>>> + f = fopen("/proc/sys/net/core/max_skb_frags", "r");
>>> + if (!ASSERT_OK_PTR(f, "max_skb_frag file pointer"))
>>> + goto out;
>>
>> In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
>> but if /proc/sys/net/core/max_skb_flags is 2 or more less
>> than MAX_SKB_FRAGS, the test won't fail, right?
>
> yes, you are right. Should we use the same definition used in
> include/linux/skbuff.h instead? Something like:
>
> if (65536 / page_size + 1 < 16)
> max_skb_flags = 16;
> else
> max_skb_flags = 65536/page_size + 1;
The maximum packet size limit 64KB won't change anytime soon.
So the above should work. Some comments to explain why using
the above formula will be good.
>
> Regards,
> Lorenzo
>
>>
>>> +
>>> + num = fscanf(f, "%d", &max_skb_frags);
>>> + fclose(f);
>>> +
>>> + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
>>> + goto out;
>>> +
>>> + /* xdp_buff linear area size is always set to 4096 in the
>>> + * bpf_prog_test_run_xdp routine.
>>> + */
>>> + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
>>> + buf = malloc(buf_size);
>>> + if (!ASSERT_OK_PTR(buf, "alloc buf"))
>>> + goto out;
>>> +
>>> + memset(buf, 0, buf_size);
>>> + offset = (__u32 *)buf;
>>> + *offset = 16;
>>> + buf[*offset] = 0xaa;
>>> + buf[*offset + 15] = 0xaa;
>>> +
>>> + topts.data_in = buf;
>>> + topts.data_out = buf;
>>> + topts.data_size_in = buf_size;
>>> + topts.data_size_out = buf_size;
>>> +
>>> + err = bpf_prog_test_run_opts(prog_fd, &topts);
>>> + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
>>> + free(buf);
>>> out:
>>> bpf_object__close(obj);
>>> }
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-04 18:14 ` Yonghong Song
@ 2022-02-04 18:30 ` Lorenzo Bianconi
2022-02-04 19:08 ` Lorenzo Bianconi
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-02-04 18:30 UTC (permalink / raw)
To: Yonghong Song
Cc: Lorenzo Bianconi, bpf, ast, daniel, brouer, toke, andrii, netdev
[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]
>
[...]
> > >
> > > In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
> > > but if /proc/sys/net/core/max_skb_flags is 2 or more less
> > > than MAX_SKB_FRAGS, the test won't fail, right?
> >
> > yes, you are right. Should we use the same definition used in
> > include/linux/skbuff.h instead? Something like:
> >
> > if (65536 / page_size + 1 < 16)
> > max_skb_flags = 16;
> > else
> > max_skb_flags = 65536/page_size + 1;
>
> The maximum packet size limit 64KB won't change anytime soon.
> So the above should work. Some comments to explain why using
> the above formula will be good.
ack, I will do in v2.
Regards,
Lorenzo
>
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > > +
> > > > + num = fscanf(f, "%d", &max_skb_frags);
> > > > + fclose(f);
> > > > +
> > > > + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
> > > > + goto out;
> > > > +
> > > > + /* xdp_buff linear area size is always set to 4096 in the
> > > > + * bpf_prog_test_run_xdp routine.
> > > > + */
> > > > + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
> > > > + buf = malloc(buf_size);
> > > > + if (!ASSERT_OK_PTR(buf, "alloc buf"))
> > > > + goto out;
> > > > +
> > > > + memset(buf, 0, buf_size);
> > > > + offset = (__u32 *)buf;
> > > > + *offset = 16;
> > > > + buf[*offset] = 0xaa;
> > > > + buf[*offset + 15] = 0xaa;
> > > > +
> > > > + topts.data_in = buf;
> > > > + topts.data_out = buf;
> > > > + topts.data_size_in = buf_size;
> > > > + topts.data_size_out = buf_size;
> > > > +
> > > > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > > > + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
> > > > + free(buf);
> > > > out:
> > > > bpf_object__close(obj);
> > > > }
> > >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-04 18:30 ` Lorenzo Bianconi
@ 2022-02-04 19:08 ` Lorenzo Bianconi
2022-02-07 17:08 ` Yonghong Song
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-02-04 19:08 UTC (permalink / raw)
To: Yonghong Song
Cc: Lorenzo Bianconi, bpf, ast, daniel, brouer, toke, andrii, netdev
[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]
> >
>
> [...]
>
> > > >
> > > > In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
> > > > but if /proc/sys/net/core/max_skb_flags is 2 or more less
> > > > than MAX_SKB_FRAGS, the test won't fail, right?
> > >
> > > yes, you are right. Should we use the same definition used in
> > > include/linux/skbuff.h instead? Something like:
> > >
> > > if (65536 / page_size + 1 < 16)
> > > max_skb_flags = 16;
> > > else
> > > max_skb_flags = 65536/page_size + 1;
> >
> > The maximum packet size limit 64KB won't change anytime soon.
> > So the above should work. Some comments to explain why using
> > the above formula will be good.
>
> ack, I will do in v2.
I can see there is a on-going discussion here [0] about increasing
MAX_SKB_FRAGS. I guess we can put on-hold this patch and see how
MAX_SKB_FRAGS will be changed.
Regards,
Lorenzo
[0] https://lore.kernel.org/all/202202031315.B425Ipe8-lkp@intel.com/t/#ma1b2c7e71fe9bc69e24642a62dadf32fda7d5f03
>
> Regards,
> Lorenzo
>
> >
> > >
> > > Regards,
> > > Lorenzo
> > >
> > > >
> > > > > +
> > > > > + num = fscanf(f, "%d", &max_skb_frags);
> > > > > + fclose(f);
> > > > > +
> > > > > + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
> > > > > + goto out;
> > > > > +
> > > > > + /* xdp_buff linear area size is always set to 4096 in the
> > > > > + * bpf_prog_test_run_xdp routine.
> > > > > + */
> > > > > + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
> > > > > + buf = malloc(buf_size);
> > > > > + if (!ASSERT_OK_PTR(buf, "alloc buf"))
> > > > > + goto out;
> > > > > +
> > > > > + memset(buf, 0, buf_size);
> > > > > + offset = (__u32 *)buf;
> > > > > + *offset = 16;
> > > > > + buf[*offset] = 0xaa;
> > > > > + buf[*offset + 15] = 0xaa;
> > > > > +
> > > > > + topts.data_in = buf;
> > > > > + topts.data_out = buf;
> > > > > + topts.data_size_in = buf_size;
> > > > > + topts.data_size_out = buf_size;
> > > > > +
> > > > > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > > > > + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
> > > > > + free(buf);
> > > > > out:
> > > > > bpf_object__close(obj);
> > > > > }
> > > >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-04 19:08 ` Lorenzo Bianconi
@ 2022-02-07 17:08 ` Yonghong Song
2022-02-07 17:53 ` Lorenzo Bianconi
0 siblings, 1 reply; 8+ messages in thread
From: Yonghong Song @ 2022-02-07 17:08 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Lorenzo Bianconi, bpf, ast, daniel, brouer, toke, andrii, netdev
On 2/4/22 11:08 AM, Lorenzo Bianconi wrote:
>>>
>>
>> [...]
>>
>>>>>
>>>>> In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
>>>>> but if /proc/sys/net/core/max_skb_flags is 2 or more less
>>>>> than MAX_SKB_FRAGS, the test won't fail, right?
>>>>
>>>> yes, you are right. Should we use the same definition used in
>>>> include/linux/skbuff.h instead? Something like:
>>>>
>>>> if (65536 / page_size + 1 < 16)
>>>> max_skb_flags = 16;
>>>> else
>>>> max_skb_flags = 65536/page_size + 1;
>>>
>>> The maximum packet size limit 64KB won't change anytime soon.
>>> So the above should work. Some comments to explain why using
>>> the above formula will be good.
>>
>> ack, I will do in v2.
>
> I can see there is a on-going discussion here [0] about increasing
> MAX_SKB_FRAGS. I guess we can put on-hold this patch and see how
> MAX_SKB_FRAGS will be changed.
Thanks for the link. The new patch is going to increase
MAX_SKB_FRAGS and it is possible that will be changed again
(maybe under some config options).
The default value for
/proc/sys/net/core/max_skb_flags is MAX_SKB_FRAGS and I suspect
anybody is bothering to change it. So your patch is okay to me.
Maybe change a little bit -ENOMEM error message. current,
ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
to
ASSERT_EQ(err, -ENOMEM, "unsupported buffer size, possible
non-default /proc/sys/net/core/max_skb_flags?");
>
> Regards,
> Lorenzo
>
> [0] https://lore.kernel.org/all/202202031315.B425Ipe8-lkp@intel.com/t/#ma1b2c7e71fe9bc69e24642a62dadf32fda7d5f03
>
>>
>> Regards,
>> Lorenzo
>>
>>>
>>>>
>>>> Regards,
>>>> Lorenzo
>>>>
>>>>>
>>>>>> +
>>>>>> + num = fscanf(f, "%d", &max_skb_frags);
>>>>>> + fclose(f);
>>>>>> +
>>>>>> + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
>>>>>> + goto out;
>>>>>> +
>>>>>> + /* xdp_buff linear area size is always set to 4096 in the
>>>>>> + * bpf_prog_test_run_xdp routine.
>>>>>> + */
>>>>>> + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
>>>>>> + buf = malloc(buf_size);
>>>>>> + if (!ASSERT_OK_PTR(buf, "alloc buf"))
>>>>>> + goto out;
>>>>>> +
>>>>>> + memset(buf, 0, buf_size);
>>>>>> + offset = (__u32 *)buf;
>>>>>> + *offset = 16;
>>>>>> + buf[*offset] = 0xaa;
>>>>>> + buf[*offset + 15] = 0xaa;
>>>>>> +
>>>>>> + topts.data_in = buf;
>>>>>> + topts.data_out = buf;
>>>>>> + topts.data_size_in = buf_size;
>>>>>> + topts.data_size_out = buf_size;
>>>>>> +
>>>>>> + err = bpf_prog_test_run_opts(prog_fd, &topts);
>>>>>> + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
>>>>>> + free(buf);
>>>>>> out:
>>>>>> bpf_object__close(obj);
>>>>>> }
>>>>>
>>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
2022-02-07 17:08 ` Yonghong Song
@ 2022-02-07 17:53 ` Lorenzo Bianconi
0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Bianconi @ 2022-02-07 17:53 UTC (permalink / raw)
To: Yonghong Song
Cc: Lorenzo Bianconi, bpf, ast, daniel, brouer, toke, andrii, netdev
[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]
>
>
> On 2/4/22 11:08 AM, Lorenzo Bianconi wrote:
> > > >
> > >
> > > [...]
> > >
> > > > > >
> > > > > > In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
> > > > > > but if /proc/sys/net/core/max_skb_flags is 2 or more less
> > > > > > than MAX_SKB_FRAGS, the test won't fail, right?
> > > > >
> > > > > yes, you are right. Should we use the same definition used in
> > > > > include/linux/skbuff.h instead? Something like:
> > > > >
> > > > > if (65536 / page_size + 1 < 16)
> > > > > max_skb_flags = 16;
> > > > > else
> > > > > max_skb_flags = 65536/page_size + 1;
> > > >
> > > > The maximum packet size limit 64KB won't change anytime soon.
> > > > So the above should work. Some comments to explain why using
> > > > the above formula will be good.
> > >
> > > ack, I will do in v2.
> >
> > I can see there is a on-going discussion here [0] about increasing
> > MAX_SKB_FRAGS. I guess we can put on-hold this patch and see how
> > MAX_SKB_FRAGS will be changed.
>
> Thanks for the link. The new patch is going to increase
> MAX_SKB_FRAGS and it is possible that will be changed again
> (maybe under some config options).
>
> The default value for
> /proc/sys/net/core/max_skb_flags is MAX_SKB_FRAGS and I suspect
> anybody is bothering to change it. So your patch is okay to me.
> Maybe change a little bit -ENOMEM error message. current,
> ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
> to
> ASSERT_EQ(err, -ENOMEM, "unsupported buffer size, possible non-default
> /proc/sys/net/core/max_skb_flags?");
ack, I am fine with it.
@Alexei, Andrii: any hints about it?
Regards,
Lorenzo
>
> >
> > Regards,
> > Lorenzo
> >
> > [0] https://lore.kernel.org/all/202202031315.B425Ipe8-lkp@intel.com/t/#ma1b2c7e71fe9bc69e24642a62dadf32fda7d5f03
> >
> > >
> > > Regards,
> > > Lorenzo
> > >
> > > >
> > > > >
> > > > > Regards,
> > > > > Lorenzo
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > + num = fscanf(f, "%d", &max_skb_frags);
> > > > > > > + fclose(f);
> > > > > > > +
> > > > > > > + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
> > > > > > > + goto out;
> > > > > > > +
> > > > > > > + /* xdp_buff linear area size is always set to 4096 in the
> > > > > > > + * bpf_prog_test_run_xdp routine.
> > > > > > > + */
> > > > > > > + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
> > > > > > > + buf = malloc(buf_size);
> > > > > > > + if (!ASSERT_OK_PTR(buf, "alloc buf"))
> > > > > > > + goto out;
> > > > > > > +
> > > > > > > + memset(buf, 0, buf_size);
> > > > > > > + offset = (__u32 *)buf;
> > > > > > > + *offset = 16;
> > > > > > > + buf[*offset] = 0xaa;
> > > > > > > + buf[*offset + 15] = 0xaa;
> > > > > > > +
> > > > > > > + topts.data_in = buf;
> > > > > > > + topts.data_out = buf;
> > > > > > > + topts.data_size_in = buf_size;
> > > > > > > + topts.data_size_out = buf_size;
> > > > > > > +
> > > > > > > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > > > > > > + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
> > > > > > > + free(buf);
> > > > > > > out:
> > > > > > > bpf_object__close(obj);
> > > > > > > }
> > > > > >
> > > >
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-02-07 17:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-04 13:58 [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags Lorenzo Bianconi
2022-02-04 17:37 ` Yonghong Song
2022-02-04 17:52 ` Lorenzo Bianconi
2022-02-04 18:14 ` Yonghong Song
2022-02-04 18:30 ` Lorenzo Bianconi
2022-02-04 19:08 ` Lorenzo Bianconi
2022-02-07 17:08 ` Yonghong Song
2022-02-07 17:53 ` Lorenzo Bianconi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.