Re: [PATCH bpf-next v2 4/4] selftests: bpf: crypto: adjust bench to use nullable IV

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 22/05/2024 19:01, Martin KaFai Lau wrote:
> On 5/10/24 5:28 AM, Vadim Fedorenko wrote:
>> The bench shows some improvements, around 4% faster on decrypt.
> 
> The original intention is to make the crypto kfunc more ergonomic to use 
> such that the bpf prog does not have to initialize a zero length dynptr 
> for the optional dynptr argument.
> 
> This performance boost is a decent surprise considering the crypto 
> operation should be pretty heavy. (thanks for having the crypto 
> benchmark handy).
> 
> Do you have a chance to get a perf record to confirm where the cycles is 
> saved?

Not really, but it's just initialization part changed, I was using the
same base commit to do the comparison.

> 
> Why it only helps decrypt?

It helps both, I just didn't show encrypt output, but it's the same 4%

> 
> Inlining it would be nice (as Eduard mentioned in another thread). I 
> also wonder if Eduard's work on the no caller saved registers could help 
> the dynptr kfunc? I think the dynptr kfunc optimization could be a 
> followup.
> 
>>
>> Before:
>>
>> Benchmark 'crypto-decrypt' started.
>> Iter   0 (325.719us): hits    5.105M/s (  5.105M/prod), drops 
>> 0.000M/s, total operations    5.105M/s
>> Iter   1 (-17.295us): hits    5.224M/s (  5.224M/prod), drops 
>> 0.000M/s, total operations    5.224M/s
>> Iter   2 (  5.504us): hits    4.630M/s (  4.630M/prod), drops 
>> 0.000M/s, total operations    4.630M/s
>> Iter   3 (  9.239us): hits    5.148M/s (  5.148M/prod), drops 
>> 0.000M/s, total operations    5.148M/s
>> Iter   4 ( 37.885us): hits    5.198M/s (  5.198M/prod), drops 
>> 0.000M/s, total operations    5.198M/s
>> Iter   5 (-53.282us): hits    5.167M/s (  5.167M/prod), drops 
>> 0.000M/s, total operations    5.167M/s
>> Iter   6 (-17.809us): hits    5.186M/s (  5.186M/prod), drops 
>> 0.000M/s, total operations    5.186M/s
>> Summary: hits    5.092 ± 0.228M/s (  5.092M/prod), drops    0.000 
>> ±0.000M/s, total operations    5.092 ± 0.228M/s
>>
>> After:
>>
>> Benchmark 'crypto-decrypt' started.
>> Iter   0 (268.912us): hits    5.312M/s (  5.312M/prod), drops 
>> 0.000M/s, total operations    5.312M/s
>> Iter   1 (124.869us): hits    5.354M/s (  5.354M/prod), drops 
>> 0.000M/s, total operations    5.354M/s
>> Iter   2 (-36.801us): hits    5.334M/s (  5.334M/prod), drops 
>> 0.000M/s, total operations    5.334M/s
>> Iter   3 (254.628us): hits    5.334M/s (  5.334M/prod), drops 
>> 0.000M/s, total operations    5.334M/s
>> Iter   4 (-77.691us): hits    5.275M/s (  5.275M/prod), drops 
>> 0.000M/s, total operations    5.275M/s
>> Iter   5 (-164.510us): hits    5.313M/s (  5.313M/prod), drops 
>> 0.000M/s, total operations    5.313M/s
>> Iter   6 (-81.376us): hits    5.346M/s (  5.346M/prod), drops 
>> 0.000M/s, total operations    5.346M/s
>> Summary: hits    5.326 ± 0.029M/s (  5.326M/prod), drops    0.000 
>> ±0.000M/s, total operations    5.326 ± 0.029M/s
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
>> ---
>>   tools/testing/selftests/bpf/progs/crypto_bench.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/crypto_bench.c 
>> b/tools/testing/selftests/bpf/progs/crypto_bench.c
>> index e61fe0882293..4ac956b26240 100644
>> --- a/tools/testing/selftests/bpf/progs/crypto_bench.c
>> +++ b/tools/testing/selftests/bpf/progs/crypto_bench.c
>> @@ -57,7 +57,7 @@ int crypto_encrypt(struct __sk_buff *skb)
>>   {
>>       struct __crypto_ctx_value *v;
>>       struct bpf_crypto_ctx *ctx;
>> -    struct bpf_dynptr psrc, pdst, iv;
>> +    struct bpf_dynptr psrc, pdst;
>>       v = crypto_ctx_value_lookup();
>>       if (!v) {
>> @@ -73,9 +73,8 @@ int crypto_encrypt(struct __sk_buff *skb)
>>       bpf_dynptr_from_skb(skb, 0, &psrc);
>>       bpf_dynptr_from_mem(dst, len, 0, &pdst);
>> -    bpf_dynptr_from_mem(dst, 0, 0, &iv);
>> -    status = bpf_crypto_encrypt(ctx, &psrc, &pdst, &iv);
>> +    status = bpf_crypto_encrypt(ctx, &psrc, &pdst, NULL);
>>       __sync_add_and_fetch(&hits, 1);
>>       return 0;
>> @@ -84,7 +83,7 @@ int crypto_encrypt(struct __sk_buff *skb)
>>   SEC("tc")
>>   int crypto_decrypt(struct __sk_buff *skb)
>>   {
>> -    struct bpf_dynptr psrc, pdst, iv;
>> +    struct bpf_dynptr psrc, pdst;
>>       struct __crypto_ctx_value *v;
>>       struct bpf_crypto_ctx *ctx;
>> @@ -98,9 +97,8 @@ int crypto_decrypt(struct __sk_buff *skb)
>>       bpf_dynptr_from_skb(skb, 0, &psrc);
>>       bpf_dynptr_from_mem(dst, len, 0, &pdst);
>> -    bpf_dynptr_from_mem(dst, 0, 0, &iv);
>> -    status = bpf_crypto_decrypt(ctx, &psrc, &pdst, &iv);
>> +    status = bpf_crypto_decrypt(ctx, &psrc, &pdst, NULL);
>>       __sync_add_and_fetch(&hits, 1);
>>       return 0;
>