net/bpf/test_run.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
For platforms on powerpc architecture with a default page size greater
than 4096, there was an inconsistency in fragment size calculation.
This caused the BPF selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow
to fail on powerpc.
The issue occurred because the fragment buffer size in
bpf_prog_test_run_xdp() was set to 4096, while the actual data size in
the fragment within the shared skb was checked against PAGE_SIZE
(65536 on powerpc) in min_t, causing it to exceed 4096 and be set
accordingly. This discrepancy led to an overflow when
bpf_xdp_frags_increase_tail() checked for tailroom, as skb_frag_size(frag)
could be greater than rxq->frag_size (when PAGE_SIZE > 4096).
This commit updates the page size references to 4096 to ensure consistency
and prevent overflow issues in fragment size calculations.
Fixes: 1c1949982524 ("bpf: introduce frags support to bpf_prog_test_run_xdp()")
Signed-off-by: Saket Kumar Bhaskar <skb99@linux.ibm.com>
---
net/bpf/test_run.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 501ec4249..eb5476184 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -124,7 +124,7 @@ struct xdp_test_data {
* must be updated accordingly this gets changed, otherwise BPF selftests
* will fail.
*/
-#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head))
+#define TEST_XDP_FRAME_SIZE (4096 - sizeof(struct xdp_page_head))
#define TEST_XDP_MAX_BATCH 256
static void xdp_test_run_init_page(netmem_ref netmem, void *arg)
@@ -660,7 +660,7 @@ static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
void *data;
- if (size < ETH_HLEN || size > PAGE_SIZE - headroom - tailroom)
+ if (size < ETH_HLEN || size > 4096 - headroom - tailroom)
return ERR_PTR(-EINVAL);
if (user_size > size)
@@ -1297,7 +1297,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
frag = &sinfo->frags[sinfo->nr_frags++];
data_len = min_t(u32, kattr->test.data_size_in - size,
- PAGE_SIZE);
+ 4096);
skb_frag_fill_page_desc(frag, page, 0, data_len);
if (copy_from_user(page_address(page), data_in + size,
--
2.43.5On Wed, Jan 22, 2025 at 10:38 AM Saket Kumar Bhaskar <skb99@linux.ibm.com> wrote: > > For platforms on powerpc architecture with a default page size greater > than 4096, there was an inconsistency in fragment size calculation. > This caused the BPF selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow > to fail on powerpc. > > The issue occurred because the fragment buffer size in > bpf_prog_test_run_xdp() was set to 4096, while the actual data size in > the fragment within the shared skb was checked against PAGE_SIZE > (65536 on powerpc) in min_t, causing it to exceed 4096 and be set > accordingly. This discrepancy led to an overflow when > bpf_xdp_frags_increase_tail() checked for tailroom, as skb_frag_size(frag) > could be greater than rxq->frag_size (when PAGE_SIZE > 4096). > > This commit updates the page size references to 4096 to ensure consistency > and prevent overflow issues in fragment size calculations. This isn't right. Please fix the selftest instead. pw-bot: cr
From: Alexei Starovoitov <alexei.starovoitov@gmail.com> Date: Thu, 23 Jan 2025 21:14:04 -0800 > On Wed, Jan 22, 2025 at 10:38 AM Saket Kumar Bhaskar > <skb99@linux.ibm.com> wrote: >> >> For platforms on powerpc architecture with a default page size greater >> than 4096, there was an inconsistency in fragment size calculation. >> This caused the BPF selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow >> to fail on powerpc. >> >> The issue occurred because the fragment buffer size in >> bpf_prog_test_run_xdp() was set to 4096, while the actual data size in >> the fragment within the shared skb was checked against PAGE_SIZE >> (65536 on powerpc) in min_t, causing it to exceed 4096 and be set >> accordingly. This discrepancy led to an overflow when >> bpf_xdp_frags_increase_tail() checked for tailroom, as skb_frag_size(frag) >> could be greater than rxq->frag_size (when PAGE_SIZE > 4096). >> >> This commit updates the page size references to 4096 to ensure consistency >> and prevent overflow issues in fragment size calculations. > > This isn't right. Please fix the selftest instead. It's not _that_ easy, I had tried in the past. Anyway, this patch is *not* a good "solution". If you (Saket) really want to fix this, both test_run and the selftest must be in sync, so you need to (both are arch-dependent): 1) get the correct PAGE_SIZE; 2) calculate the correct tailroom in userspace (which depends on sizeof(shinfo) and SKB_DATA_ALIGN -> SMP_CACHE_BYTES). > > pw-bot: cr Thanks, Olek
On Tue, Jan 28, 2025 at 04:03:11PM +0100, Alexander Lobakin wrote:
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Date: Thu, 23 Jan 2025 21:14:04 -0800
>
> > On Wed, Jan 22, 2025 at 10:38 AM Saket Kumar Bhaskar
> > <skb99@linux.ibm.com> wrote:
> >>
> >> For platforms on powerpc architecture with a default page size greater
> >> than 4096, there was an inconsistency in fragment size calculation.
> >> This caused the BPF selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow
> >> to fail on powerpc.
> >>
> >> The issue occurred because the fragment buffer size in
> >> bpf_prog_test_run_xdp() was set to 4096, while the actual data size in
> >> the fragment within the shared skb was checked against PAGE_SIZE
> >> (65536 on powerpc) in min_t, causing it to exceed 4096 and be set
> >> accordingly. This discrepancy led to an overflow when
> >> bpf_xdp_frags_increase_tail() checked for tailroom, as skb_frag_size(frag)
> >> could be greater than rxq->frag_size (when PAGE_SIZE > 4096).
> >>
> >> This commit updates the page size references to 4096 to ensure consistency
> >> and prevent overflow issues in fragment size calculations.
> >
> > This isn't right. Please fix the selftest instead.
>
> It's not _that_ easy, I had tried in the past. Anyway, this patch is
> *not* a good "solution".
>
> If you (Saket) really want to fix this, both test_run and the selftest
> must be in sync, so you need to (both are arch-dependent): 1) get the
> correct PAGE_SIZE; 2) calculate the correct tailroom in userspace (which
> depends on sizeof(shinfo) and SKB_DATA_ALIGN -> SMP_CACHE_BYTES).
>
> >
> > pw-bot: cr
>
> Thanks,
> Olek
There is a mixup in kernel b/w 4096 and PAGE_SIZE and all selftest seem
to be based on 4096 as the size, so I changed the PAGE_SIZE to 4096,
but if we have to use PAGE_SIZE we need this change in kernel.
In place of PAGE_SIZE 4096 was used here:
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 501ec4249..6b7fddfbb 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1251,7 +1251,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
headroom -= ctx->data;
}
- max_data_sz = 4096 - headroom - tailroom;
+ max_data_sz = PAGE_SIZE - headroom - tailroom;
if (size > max_data_sz) {
/* disallow live data mode for jumbo frames */
if (do_live)
Assuming that change in kernel we should also update the selftest to
64K page size for ppc64:
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
index 53d6ad8c2..037142e21 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
@@ -226,7 +226,7 @@ static void test_xdp_adjust_frags_tail_grow(void)
prog_fd = bpf_program__fd(prog);
- buf = malloc(16384);
+ buf = malloc(262144);
if (!ASSERT_OK_PTR(buf, "alloc buf 16Kb"))
goto out;
@@ -254,12 +254,12 @@ static void test_xdp_adjust_frags_tail_grow(void)
ASSERT_EQ(buf[i], 1, "9Kb+10b-untouched");
/* Test a too large grow */
- memset(buf, 1, 16384);
- exp_size = 9001;
+ memset(buf, 1, 262144);
+ exp_size = 132001;
topts.data_in = topts.data_out = buf;
- topts.data_size_in = 9001;
- topts.data_size_out = 16384;
+ topts.data_size_in = 132001;
+ topts.data_size_out = 262144;
err = bpf_prog_test_run_opts(prog_fd, &topts);
ASSERT_OK(err, "9Kb+10b");
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c b/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c
index 81bb38d72..40a0c5469 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c
@@ -27,8 +27,8 @@ int _xdp_adjust_tail_grow(struct xdp_md *xdp)
offset = 4096 - 256 - tailroom - data_len;
} else if (data_len == 9000) {
offset = 10;
- } else if (data_len == 9001) {
- offset = 4096;
+ } else if (data_len == 132001) {
+ offset = 65536;
} else {
return XDP_ABORTED; /* No matching test */
}
The above change is intended for feedback. The date_len and other
values in the test cases can be adjusted to be based on the page
size, rather than being hard-coded, to ensure compatibility with
different page sizes.
Thanks,
Saket
From: Saket Kumar Bhaskar <skb99@linux.ibm.com>
Date: Tue, 4 Feb 2025 12:27:52 +0530
> On Tue, Jan 28, 2025 at 04:03:11PM +0100, Alexander Lobakin wrote:
>> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Date: Thu, 23 Jan 2025 21:14:04 -0800
>>
>>> On Wed, Jan 22, 2025 at 10:38 AM Saket Kumar Bhaskar
>>> <skb99@linux.ibm.com> wrote:
>>>>
>>>> For platforms on powerpc architecture with a default page size greater
>>>> than 4096, there was an inconsistency in fragment size calculation.
>>>> This caused the BPF selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow
>>>> to fail on powerpc.
>>>>
>>>> The issue occurred because the fragment buffer size in
>>>> bpf_prog_test_run_xdp() was set to 4096, while the actual data size in
>>>> the fragment within the shared skb was checked against PAGE_SIZE
>>>> (65536 on powerpc) in min_t, causing it to exceed 4096 and be set
>>>> accordingly. This discrepancy led to an overflow when
>>>> bpf_xdp_frags_increase_tail() checked for tailroom, as skb_frag_size(frag)
>>>> could be greater than rxq->frag_size (when PAGE_SIZE > 4096).
>>>>
>>>> This commit updates the page size references to 4096 to ensure consistency
>>>> and prevent overflow issues in fragment size calculations.
>>>
>>> This isn't right. Please fix the selftest instead.
>>
>> It's not _that_ easy, I had tried in the past. Anyway, this patch is
>> *not* a good "solution".
>>
>> If you (Saket) really want to fix this, both test_run and the selftest
>> must be in sync, so you need to (both are arch-dependent): 1) get the
>> correct PAGE_SIZE; 2) calculate the correct tailroom in userspace (which
>> depends on sizeof(shinfo) and SKB_DATA_ALIGN -> SMP_CACHE_BYTES).
>>
>>>
>>> pw-bot: cr
>>
>> Thanks,
>> Olek
> There is a mixup in kernel b/w 4096 and PAGE_SIZE and all selftest seem
> to be based on 4096 as the size, so I changed the PAGE_SIZE to 4096,
> but if we have to use PAGE_SIZE we need this change in kernel.
I know how it is done, I was working on adjacent code, that's why I
spoke up and told you what you need to account if you want to fix this
properly.
xdp->frame_sz is hard buffer len, usually in range
[PAGE_SIZE / 2, PAGE_SIZE], and it includes:
headroom (XDP_PACKET_HEADROOM + some drivers reserve NET_IP_ALIGN)
actual data buffer
tailroom (SKB_DATA_ALIGN(sizeof(skb_shared_info)))
So to determine the actual data buffer size, you need to:
* know PAGE_SIZE
* know headroom
* know tailroom
Hardcoding anything from the list will lead to selftest fails.
> In place of PAGE_SIZE 4096 was used here:
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 501ec4249..6b7fddfbb 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1251,7 +1251,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
> headroom -= ctx->data;
> }
>
> - max_data_sz = 4096 - headroom - tailroom;
> + max_data_sz = PAGE_SIZE - headroom - tailroom;
> if (size > max_data_sz) {
> /* disallow live data mode for jumbo frames */
> if (do_live)
>
> Assuming that change in kernel we should also update the selftest to
> 64K page size for ppc64:
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
> index 53d6ad8c2..037142e21 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
> @@ -226,7 +226,7 @@ static void test_xdp_adjust_frags_tail_grow(void)
>
> prog_fd = bpf_program__fd(prog);
>
> - buf = malloc(16384);
> + buf = malloc(262144);
> if (!ASSERT_OK_PTR(buf, "alloc buf 16Kb"))
> goto out;
>
> @@ -254,12 +254,12 @@ static void test_xdp_adjust_frags_tail_grow(void)
> ASSERT_EQ(buf[i], 1, "9Kb+10b-untouched");
>
> /* Test a too large grow */
> - memset(buf, 1, 16384);
> - exp_size = 9001;
> + memset(buf, 1, 262144);
> + exp_size = 132001;
>
> topts.data_in = topts.data_out = buf;
> - topts.data_size_in = 9001;
> - topts.data_size_out = 16384;
> + topts.data_size_in = 132001;
> + topts.data_size_out = 262144;
> err = bpf_prog_test_run_opts(prog_fd, &topts);
>
> ASSERT_OK(err, "9Kb+10b");
>
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c b/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c
> index 81bb38d72..40a0c5469 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_adjust_tail_grow.c
> @@ -27,8 +27,8 @@ int _xdp_adjust_tail_grow(struct xdp_md *xdp)
> offset = 4096 - 256 - tailroom - data_len;
> } else if (data_len == 9000) {
> offset = 10;
> - } else if (data_len == 9001) {
> - offset = 4096;
> + } else if (data_len == 132001) {
> + offset = 65536;
> } else {
> return XDP_ABORTED; /* No matching test */
> }
>
> The above change is intended for feedback. The date_len and other
> values in the test cases can be adjusted to be based on the page
> size, rather than being hard-coded, to ensure compatibility with
> different page sizes.
In the code above I only see one hardcode replaced with another one.
Note that PAGE_SIZE == 4096 was hardcoded to be able to run selftests
on x86_64 in the first place. If you want to enable them on
non-fixed-page-size arches, then I mentioned 2 times already what you
need to do.
>
> Thanks,
> Saket
Thanks,
Olek
© 2016 - 2026 Red Hat, Inc.