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

[Martin KaFai Lau <martin.lau@linux.dev>]

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?

Why it only helps decrypt?

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;