On Fri, 21 Aug 2020 at 05:24, Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> >
> > Since proto_ops are almost identical, and only one is used in a given kernel
> > build, why not just do:
> >
> > static struct proto_ops algif_rng_ops = {
> > ...
> > #ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
> > .sendmsg = rng_sendmsg,
> > #else
> > .sendmsg = sock_no_sendmsg,
> > #endif
> > ...
> > };
> >
> > Similarly for .recvmsg(), although I don't understand what's wrong with just
> > adding the lock_sock() instead... The RNG algorithms do locking anyway, so it's
> > not like that would regress the ability to recvmsg() in parallel. Also,
> > conditional locking depending on the kernel config makes it more difficult to
> > find kernel bugs like deadlocks.
>
> I want this to have minimal impact on anyone who's not using it.
> After all, this is something that only Google is asking for.
>
> Anyway, I wasn't looking for a compile-time ops switch, but a
> run-time one.
>
> I think what we can do is move the new newsock->ops assignment
> in af_alg_accept up above the type->accept call which would then
> allow it to be overridden by the accept call.
>
> After that you could just change newsock->ops depending on whether
> pctx->entropy is NULL or not in rng_accept_parent.
Ack, done in v6.
> As for the proto_ops duplication I don't think it's that big a
> deal, but if you're really bothered just create a macro for the
> identical bits in the struct.
I didn't create a macro to avoid complicating the code.
Thanks,
Elena
