* [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp @ 2025-11-05 20:14 Hoyeon Lee 2025-11-05 22:22 ` Eduard Zingerman 2025-11-05 22:45 ` Alexei Starovoitov 0 siblings, 2 replies; 8+ messages in thread From: Hoyeon Lee @ 2025-11-05 20:14 UTC (permalink / raw) To: ast, daniel, bpf Cc: Hoyeon Lee, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, linux-kselftest, linux-kernel 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.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 20:14 [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp Hoyeon Lee @ 2025-11-05 22:22 ` Eduard Zingerman 2025-11-05 22:45 ` Alexei Starovoitov 1 sibling, 0 replies; 8+ messages in thread From: Eduard Zingerman @ 2025-11-05 22:22 UTC (permalink / raw) To: Hoyeon Lee, ast, daniel, bpf Cc: andrii, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, linux-kselftest, linux-kernel 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> [...] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 20:14 [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp Hoyeon Lee 2025-11-05 22:22 ` Eduard Zingerman @ 2025-11-05 22:45 ` Alexei Starovoitov 2025-11-05 22:52 ` Eduard Zingerman 1 sibling, 1 reply; 8+ messages in thread From: Alexei Starovoitov @ 2025-11-05 22:45 UTC (permalink / raw) To: Hoyeon Lee Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Andrii Nakryiko, Martin KaFai Lau, Eduard, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, LKML 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 22:45 ` Alexei Starovoitov @ 2025-11-05 22:52 ` Eduard Zingerman 2025-11-05 23:33 ` Alexei Starovoitov 0 siblings, 1 reply; 8+ messages in thread From: Eduard Zingerman @ 2025-11-05 22:52 UTC (permalink / raw) To: Alexei Starovoitov, Hoyeon Lee Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, LKML 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/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 22:52 ` Eduard Zingerman @ 2025-11-05 23:33 ` Alexei Starovoitov 2025-11-05 23:37 ` Eduard Zingerman 0 siblings, 1 reply; 8+ messages in thread From: Alexei Starovoitov @ 2025-11-05 23:33 UTC (permalink / raw) To: Eduard Zingerman Cc: Hoyeon Lee, Alexei Starovoitov, Daniel Borkmann, bpf, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, LKML 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 23:33 ` Alexei Starovoitov @ 2025-11-05 23:37 ` Eduard Zingerman 2025-11-05 23:43 ` Alexei Starovoitov 0 siblings, 1 reply; 8+ messages in thread From: Eduard Zingerman @ 2025-11-05 23:37 UTC (permalink / raw) To: Alexei Starovoitov Cc: Hoyeon Lee, Alexei Starovoitov, Daniel Borkmann, bpf, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, LKML 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 23:37 ` Eduard Zingerman @ 2025-11-05 23:43 ` Alexei Starovoitov 2025-11-06 3:26 ` Hoyeon Lee 0 siblings, 1 reply; 8+ messages in thread From: Alexei Starovoitov @ 2025-11-05 23:43 UTC (permalink / raw) To: Eduard Zingerman Cc: Hoyeon Lee, Alexei Starovoitov, Daniel Borkmann, bpf, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, LKML 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp 2025-11-05 23:43 ` Alexei Starovoitov @ 2025-11-06 3:26 ` Hoyeon Lee 0 siblings, 0 replies; 8+ messages in thread From: Hoyeon Lee @ 2025-11-06 3:26 UTC (permalink / raw) To: Alexei Starovoitov Cc: Eduard Zingerman, Alexei Starovoitov, Daniel Borkmann, bpf, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, LKML 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-11-06 3:26 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-05 20:14 [bpf-next] selftests/bpf: refactor snprintf_btf test to use bpf_strncmp Hoyeon Lee 2025-11-05 22:22 ` Eduard Zingerman 2025-11-05 22:45 ` Alexei Starovoitov 2025-11-05 22:52 ` Eduard Zingerman 2025-11-05 23:33 ` Alexei Starovoitov 2025-11-05 23:37 ` Eduard Zingerman 2025-11-05 23:43 ` Alexei Starovoitov 2025-11-06 3:26 ` Hoyeon Lee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox