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

[Vadim Fedorenko <vadim.fedorenko@xxxxxxxxx>]

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@xxxxxxxx>
> > ---
> >   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;