.../selftests/bpf/progs/netif_receive_skb.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)
The netif_receive_skb BPF program used in snprintf_btf test still uses
a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
available and provides the same functionality.
This commit refactors the test to use the bpf_strncmp helper, removing
the redundant custom implementation.
Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
---
.../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
index 9e067dcbf607..186b8c82b9e6 100644
--- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
+++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
@@ -31,19 +31,6 @@ struct {
__type(value, char[STRSIZE]);
} strdata SEC(".maps");
-static int __strncmp(const void *m1, const void *m2, size_t len)
-{
- const unsigned char *s1 = m1;
- const unsigned char *s2 = m2;
- int i, delta = 0;
-
- for (i = 0; i < len; i++) {
- delta = s1[i] - s2[i];
- if (delta || s1[i] == 0 || s2[i] == 0)
- break;
- }
- return delta;
-}
#if __has_builtin(__builtin_btf_type_id)
#define TEST_BTF(_str, _type, _flags, _expected, ...) \
@@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
&_ptr, sizeof(_ptr), _hflags); \
if (ret) \
break; \
- _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
+ _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
if (_cmp != 0) { \
bpf_printk("(%d) got %s", _cmp, _str); \
bpf_printk("(%d) expected %s", _cmp, \
--
2.51.1On Wed, Nov 5, 2025 at 12:14 PM Hoyeon Lee <hoyeon.lee@suse.com> wrote:
>
> The netif_receive_skb BPF program used in snprintf_btf test still uses
> a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
> available and provides the same functionality.
>
> This commit refactors the test to use the bpf_strncmp helper, removing
> the redundant custom implementation.
>
> Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
> ---
> .../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> index 9e067dcbf607..186b8c82b9e6 100644
> --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> @@ -31,19 +31,6 @@ struct {
> __type(value, char[STRSIZE]);
> } strdata SEC(".maps");
>
> -static int __strncmp(const void *m1, const void *m2, size_t len)
> -{
> - const unsigned char *s1 = m1;
> - const unsigned char *s2 = m2;
> - int i, delta = 0;
> -
> - for (i = 0; i < len; i++) {
> - delta = s1[i] - s2[i];
> - if (delta || s1[i] == 0 || s2[i] == 0)
> - break;
> - }
> - return delta;
> -}
>
> #if __has_builtin(__builtin_btf_type_id)
> #define TEST_BTF(_str, _type, _flags, _expected, ...) \
> @@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> &_ptr, sizeof(_ptr), _hflags); \
> if (ret) \
> break; \
> - _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> + _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
Though it's equivalent, the point of the test is to be heavy
for the verifier with open coded __strncmp().
pw-bot: cr
On Wed, 2025-11-05 at 14:45 -0800, Alexei Starovoitov wrote:
> On Wed, Nov 5, 2025 at 12:14 PM Hoyeon Lee <hoyeon.lee@suse.com> wrote:
> >
> > The netif_receive_skb BPF program used in snprintf_btf test still uses
> > a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
> > available and provides the same functionality.
> >
> > This commit refactors the test to use the bpf_strncmp helper, removing
> > the redundant custom implementation.
> >
> > Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
> > ---
> > .../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
> > 1 file changed, 1 insertion(+), 14 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > index 9e067dcbf607..186b8c82b9e6 100644
> > --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > @@ -31,19 +31,6 @@ struct {
> > __type(value, char[STRSIZE]);
> > } strdata SEC(".maps");
> >
> > -static int __strncmp(const void *m1, const void *m2, size_t len)
> > -{
> > - const unsigned char *s1 = m1;
> > - const unsigned char *s2 = m2;
> > - int i, delta = 0;
> > -
> > - for (i = 0; i < len; i++) {
> > - delta = s1[i] - s2[i];
> > - if (delta || s1[i] == 0 || s2[i] == 0)
> > - break;
> > - }
> > - return delta;
> > -}
> >
> > #if __has_builtin(__builtin_btf_type_id)
> > #define TEST_BTF(_str, _type, _flags, _expected, ...) \
> > @@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> > &_ptr, sizeof(_ptr), _hflags); \
> > if (ret) \
> > break; \
> > - _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> > + _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
>
> Though it's equivalent, the point of the test is to be heavy
> for the verifier with open coded __strncmp().
>
> pw-bot: cr
I double checked that before acking, the test was added as a part of [1].
So it seems to be focused on bpf_snprintf_btf(), not on scalability.
And it's not that heavy in terms of instructions budget:
File Program Verdict Insns States
----------------------- ----------------------- ------- ----- ------
netif_receive_skb.bpf.o trace_netif_receive_skb success 18152 629
[1] https://lore.kernel.org/bpf/1601292670-1616-5-git-send-email-alan.maguire@oracle.com/
On Wed, Nov 5, 2025 at 2:52 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2025-11-05 at 14:45 -0800, Alexei Starovoitov wrote:
> > On Wed, Nov 5, 2025 at 12:14 PM Hoyeon Lee <hoyeon.lee@suse.com> wrote:
> > >
> > > The netif_receive_skb BPF program used in snprintf_btf test still uses
> > > a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
> > > available and provides the same functionality.
> > >
> > > This commit refactors the test to use the bpf_strncmp helper, removing
> > > the redundant custom implementation.
> > >
> > > Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
> > > ---
> > > .../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
> > > 1 file changed, 1 insertion(+), 14 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > index 9e067dcbf607..186b8c82b9e6 100644
> > > --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > @@ -31,19 +31,6 @@ struct {
> > > __type(value, char[STRSIZE]);
> > > } strdata SEC(".maps");
> > >
> > > -static int __strncmp(const void *m1, const void *m2, size_t len)
> > > -{
> > > - const unsigned char *s1 = m1;
> > > - const unsigned char *s2 = m2;
> > > - int i, delta = 0;
> > > -
> > > - for (i = 0; i < len; i++) {
> > > - delta = s1[i] - s2[i];
> > > - if (delta || s1[i] == 0 || s2[i] == 0)
> > > - break;
> > > - }
> > > - return delta;
> > > -}
> > >
> > > #if __has_builtin(__builtin_btf_type_id)
> > > #define TEST_BTF(_str, _type, _flags, _expected, ...) \
> > > @@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> > > &_ptr, sizeof(_ptr), _hflags); \
> > > if (ret) \
> > > break; \
> > > - _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> > > + _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
> >
> > Though it's equivalent, the point of the test is to be heavy
> > for the verifier with open coded __strncmp().
> >
> > pw-bot: cr
>
> I double checked that before acking, the test was added as a part of [1].
> So it seems to be focused on bpf_snprintf_btf(), not on scalability.
> And it's not that heavy in terms of instructions budget:
>
> File Program Verdict Insns States
> ----------------------- ----------------------- ------- ----- ------
> netif_receive_skb.bpf.o trace_netif_receive_skb success 18152 629
Is this before or after?
What is the % decrease in insn_processed?
I'd like to better understand the impact of the change.
On Wed, 2025-11-05 at 15:33 -0800, Alexei Starovoitov wrote:
> On Wed, Nov 5, 2025 at 2:52 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Wed, 2025-11-05 at 14:45 -0800, Alexei Starovoitov wrote:
> > > On Wed, Nov 5, 2025 at 12:14 PM Hoyeon Lee <hoyeon.lee@suse.com> wrote:
> > > >
> > > > The netif_receive_skb BPF program used in snprintf_btf test still uses
> > > > a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
> > > > available and provides the same functionality.
> > > >
> > > > This commit refactors the test to use the bpf_strncmp helper, removing
> > > > the redundant custom implementation.
> > > >
> > > > Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
> > > > ---
> > > > .../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
> > > > 1 file changed, 1 insertion(+), 14 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > index 9e067dcbf607..186b8c82b9e6 100644
> > > > --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > @@ -31,19 +31,6 @@ struct {
> > > > __type(value, char[STRSIZE]);
> > > > } strdata SEC(".maps");
> > > >
> > > > -static int __strncmp(const void *m1, const void *m2, size_t len)
> > > > -{
> > > > - const unsigned char *s1 = m1;
> > > > - const unsigned char *s2 = m2;
> > > > - int i, delta = 0;
> > > > -
> > > > - for (i = 0; i < len; i++) {
> > > > - delta = s1[i] - s2[i];
> > > > - if (delta || s1[i] == 0 || s2[i] == 0)
> > > > - break;
> > > > - }
> > > > - return delta;
> > > > -}
> > > >
> > > > #if __has_builtin(__builtin_btf_type_id)
> > > > #define TEST_BTF(_str, _type, _flags, _expected, ...) \
> > > > @@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> > > > &_ptr, sizeof(_ptr), _hflags); \
> > > > if (ret) \
> > > > break; \
> > > > - _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> > > > + _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
> > >
> > > Though it's equivalent, the point of the test is to be heavy
> > > for the verifier with open coded __strncmp().
> > >
> > > pw-bot: cr
> >
> > I double checked that before acking, the test was added as a part of [1].
> > So it seems to be focused on bpf_snprintf_btf(), not on scalability.
> > And it's not that heavy in terms of instructions budget:
> >
> > File Program Verdict Insns States
> > ----------------------- ----------------------- ------- ----- ------
> > netif_receive_skb.bpf.o trace_netif_receive_skb success 18152 629
>
> Is this before or after?
> What is the % decrease in insn_processed?
> I'd like to better understand the impact of the change.
That's before, after the change it is as follows:
File Program Verdict Insns States
----------------------- ----------------------- ------- ----- ------
netif_receive_skb.bpf.o trace_netif_receive_skb success 4353 235
----------------------- ----------------------- ------- ----- ------
So, the overall impact is 18K -> 4K instructions processed.
On Wed, Nov 5, 2025 at 3:38 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2025-11-05 at 15:33 -0800, Alexei Starovoitov wrote:
> > On Wed, Nov 5, 2025 at 2:52 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Wed, 2025-11-05 at 14:45 -0800, Alexei Starovoitov wrote:
> > > > On Wed, Nov 5, 2025 at 12:14 PM Hoyeon Lee <hoyeon.lee@suse.com> wrote:
> > > > >
> > > > > The netif_receive_skb BPF program used in snprintf_btf test still uses
> > > > > a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
> > > > > available and provides the same functionality.
> > > > >
> > > > > This commit refactors the test to use the bpf_strncmp helper, removing
> > > > > the redundant custom implementation.
> > > > >
> > > > > Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
> > > > > ---
> > > > > .../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
> > > > > 1 file changed, 1 insertion(+), 14 deletions(-)
> > > > >
> > > > > diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > > index 9e067dcbf607..186b8c82b9e6 100644
> > > > > --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > > +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > > @@ -31,19 +31,6 @@ struct {
> > > > > __type(value, char[STRSIZE]);
> > > > > } strdata SEC(".maps");
> > > > >
> > > > > -static int __strncmp(const void *m1, const void *m2, size_t len)
> > > > > -{
> > > > > - const unsigned char *s1 = m1;
> > > > > - const unsigned char *s2 = m2;
> > > > > - int i, delta = 0;
> > > > > -
> > > > > - for (i = 0; i < len; i++) {
> > > > > - delta = s1[i] - s2[i];
> > > > > - if (delta || s1[i] == 0 || s2[i] == 0)
> > > > > - break;
> > > > > - }
> > > > > - return delta;
> > > > > -}
> > > > >
> > > > > #if __has_builtin(__builtin_btf_type_id)
> > > > > #define TEST_BTF(_str, _type, _flags, _expected, ...) \
> > > > > @@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> > > > > &_ptr, sizeof(_ptr), _hflags); \
> > > > > if (ret) \
> > > > > break; \
> > > > > - _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> > > > > + _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
> > > >
> > > > Though it's equivalent, the point of the test is to be heavy
> > > > for the verifier with open coded __strncmp().
> > > >
> > > > pw-bot: cr
> > >
> > > I double checked that before acking, the test was added as a part of [1].
> > > So it seems to be focused on bpf_snprintf_btf(), not on scalability.
> > > And it's not that heavy in terms of instructions budget:
> > >
> > > File Program Verdict Insns States
> > > ----------------------- ----------------------- ------- ----- ------
> > > netif_receive_skb.bpf.o trace_netif_receive_skb success 18152 629
> >
> > Is this before or after?
> > What is the % decrease in insn_processed?
> > I'd like to better understand the impact of the change.
>
> That's before, after the change it is as follows:
>
> File Program Verdict Insns States
> ----------------------- ----------------------- ------- ----- ------
> netif_receive_skb.bpf.o trace_netif_receive_skb success 4353 235
> ----------------------- ----------------------- ------- ----- ------
>
> So, the overall impact is 18K -> 4K instructions processed.
It's large enough impact for the verifier.
I agree that the test was mainly focusing on testing
bpf_snprintf_btf(), but it has a nice side effect by testing
bounded loops too.
I prefer to keep it as-is.
On Thu, Nov 6, 2025 at 8:43 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Nov 5, 2025 at 3:38 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Wed, 2025-11-05 at 15:33 -0800, Alexei Starovoitov wrote:
> > > On Wed, Nov 5, 2025 at 2:52 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > > >
> > > > On Wed, 2025-11-05 at 14:45 -0800, Alexei Starovoitov wrote:
> > > > > On Wed, Nov 5, 2025 at 12:14 PM Hoyeon Lee <hoyeon.lee@suse.com> wrote:
> > > > > >
> > > > > > The netif_receive_skb BPF program used in snprintf_btf test still uses
> > > > > > a custom __strncmp. This is unnecessary as the bpf_strncmp helper is
> > > > > > available and provides the same functionality.
> > > > > >
> > > > > > This commit refactors the test to use the bpf_strncmp helper, removing
> > > > > > the redundant custom implementation.
> > > > > >
> > > > > > Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com>
> > > > > > ---
> > > > > > .../selftests/bpf/progs/netif_receive_skb.c | 15 +--------------
> > > > > > 1 file changed, 1 insertion(+), 14 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > > > index 9e067dcbf607..186b8c82b9e6 100644
> > > > > > --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > > > +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> > > > > > @@ -31,19 +31,6 @@ struct {
> > > > > > __type(value, char[STRSIZE]);
> > > > > > } strdata SEC(".maps");
> > > > > >
> > > > > > -static int __strncmp(const void *m1, const void *m2, size_t len)
> > > > > > -{
> > > > > > - const unsigned char *s1 = m1;
> > > > > > - const unsigned char *s2 = m2;
> > > > > > - int i, delta = 0;
> > > > > > -
> > > > > > - for (i = 0; i < len; i++) {
> > > > > > - delta = s1[i] - s2[i];
> > > > > > - if (delta || s1[i] == 0 || s2[i] == 0)
> > > > > > - break;
> > > > > > - }
> > > > > > - return delta;
> > > > > > -}
> > > > > >
> > > > > > #if __has_builtin(__builtin_btf_type_id)
> > > > > > #define TEST_BTF(_str, _type, _flags, _expected, ...) \
> > > > > > @@ -69,7 +56,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> > > > > > &_ptr, sizeof(_ptr), _hflags); \
> > > > > > if (ret) \
> > > > > > break; \
> > > > > > - _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> > > > > > + _cmp = bpf_strncmp(_str, EXPECTED_STRSIZE, _expectedval); \
> > > > >
> > > > > Though it's equivalent, the point of the test is to be heavy
> > > > > for the verifier with open coded __strncmp().
> > > > >
> > > > > pw-bot: cr
> > > >
> > > > I double checked that before acking, the test was added as a part of [1].
> > > > So it seems to be focused on bpf_snprintf_btf(), not on scalability.
> > > > And it's not that heavy in terms of instructions budget:
> > > >
> > > > File Program Verdict Insns States
> > > > ----------------------- ----------------------- ------- ----- ------
> > > > netif_receive_skb.bpf.o trace_netif_receive_skb success 18152 629
> > >
> > > Is this before or after?
> > > What is the % decrease in insn_processed?
> > > I'd like to better understand the impact of the change.
> >
> > That's before, after the change it is as follows:
> >
> > File Program Verdict Insns States
> > ----------------------- ----------------------- ------- ----- ------
> > netif_receive_skb.bpf.o trace_netif_receive_skb success 4353 235
> > ----------------------- ----------------------- ------- ----- ------
> >
> > So, the overall impact is 18K -> 4K instructions processed.
>
> It's large enough impact for the verifier.
> I agree that the test was mainly focusing on testing
> bpf_snprintf_btf(), but it has a nice side effect by testing
> bounded loops too.
> I prefer to keep it as-is.
Thanks for the clarification.
Removing the open-coded __strncmp would drop the bounded-loop
coverage that this test currently provides (as a side effect),
and that stress on the verifier is still valuable.
I'll drop this patch.
Thank you all for the discussion and review.
On Thu, 2025-11-06 at 05:14 +0900, Hoyeon Lee wrote: > The netif_receive_skb BPF program used in snprintf_btf test still uses > a custom __strncmp. This is unnecessary as the bpf_strncmp helper is > available and provides the same functionality. > > This commit refactors the test to use the bpf_strncmp helper, removing > the redundant custom implementation. > > Signed-off-by: Hoyeon Lee <hoyeon.lee@suse.com> > --- Acked-by: Eduard Zingerman <eddyz87@gmail.com> [...]
© 2016 - 2025 Red Hat, Inc.