Re: [PATCH bpf-next 4/5] selftests/bpf: add benchmark for bpf_strncmp() helper

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Hou Tao <houtao1@huawei.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Martin KaFai Lau <kafai@fb.com>, Yonghong Song <yhs@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 4/5] selftests/bpf: add benchmark for bpf_strncmp() helper
Date: Wed, 8 Dec 2021 12:08:04 -0800	[thread overview]
Message-ID: <CAEf4BzYtKNs8ORZxSgFtWZA49-Rjp5UVww_s9MWKq2DNUMts_w@mail.gmail.com> (raw)
In-Reply-To: <97a9166f-7eec-49e2-1f94-ae2fc21dcf02@huawei.com>

On Wed, Dec 8, 2021 at 5:47 AM Hou Tao <houtao1@huawei.com> wrote:
>
> Hi,
>
> On 12/7/2021 11:01 AM, Andrii Nakryiko wrote:
> > On Tue, Nov 30, 2021 at 6:07 AM Hou Tao <houtao1@huawei.com> wrote:
> >> Add benchmark to compare the performance between home-made strncmp()
> >> in bpf program and bpf_strncmp() helper. In summary, the performance
> >> win of bpf_strncmp() under x86-64 is greater than 18% when the compared
> >> string length is greater than 64, and is 179% when the length is 4095.
> >> Under arm64 the performance win is even bigger: 33% when the length
> >> is greater than 64 and 600% when the length is 4095.
> snip
> >> +
> >> +long hits = 0;
> >> +char str[STRNCMP_STR_SZ];
> >> +
> >> +char _license[] SEC("license") = "GPL";
> >> +
> >> +static __always_inline int local_strncmp(const char *s1, unsigned int sz,
> >> +                                        const char *s2)
> >> +{
> >> +       int ret = 0;
> >> +       unsigned int i;
> >> +
> >> +       for (i = 0; i < sz; i++) {
> >> +               /* E.g. 0xff > 0x31 */
> >> +               ret = (unsigned char)s1[i] - (unsigned char)s2[i];
> > I'm actually not sure if it will perform subtraction in unsigned form
> > (and thus you'll never have a negative result) and then cast to int,
> > or not. Why not cast to int instead of unsigned char to be sure?
> It is used to handle the character which is greater than or equal with 0x80.
> When casting these character into int, the result will be a negative value,
> the compare result will always be negative and it is wrong because
> 0xff should be greater than 0x31.

I see about (unsigned char) cast, but I was worried that subtraction
result won't be negative I've tested with

$ cat test.c
#include <stdio.h>

int main() {
        int x = (unsigned char)190 - (unsigned char)255;
        printf("%d\n", x);
}

Seems like it behaves sanely (at least on this particular compiler),
so I'm fine with it.