lkml.org 
[lkml]   [2022]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 3/3] virtio-crypto: implement RSA algorithm
From
Date

On 2/18/22 11:12 AM, zhenwei pi wrote:
>>> +void virtio_crypto_akcipher_algs_unregister(struct virtio_crypto
>>> +*vcrypto) {
>>> +    int i = 0;
>>> +
>>> +    mutex_lock(&algs_lock);
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(virtio_crypto_akcipher_algs); i++) {
>>> +        uint32_t service = virtio_crypto_akcipher_algs[i].service;
>>> +        uint32_t algonum = virtio_crypto_akcipher_algs[i].algonum;
>>> +
>>> +        if (virtio_crypto_akcipher_algs[i].active_devs == 0 ||
>>> +            !virtcrypto_algo_is_supported(vcrypto, service, algonum))
>>> +            continue;
>>> +
>>> +        if (virtio_crypto_akcipher_algs[i].active_devs == 1)
>>> +
>>>     crypto_unregister_akcipher(&virtio_crypto_akcipher_algs[i].algo);
>>> +
>>> +        virtio_crypto_akcipher_algs[i].active_devs--;
>>> +    }
>>> +
>>> +    mutex_unlock(&algs_lock);
>>> +}
>>
>> Why don't you reuse the virtio_crypto_algs_register/unregister functions?
>> The current code is too repetitive. Maybe we don't need create the new
>> file virtio_crypto_akcipher_algo.c
>> because we had virtio_crypto_algs.c which includes all algorithms.
>>
>
> Yes, this looks similar to virtio_crypto_algs_register/unregister.
>
> Let's look at the difference:
> struct virtio_crypto_akcipher_algo {
>         uint32_t algonum;
>         uint32_t service;
>         unsigned int active_devs;
>         struct akcipher_alg algo;
> };
>
> struct virtio_crypto_algo {
>         uint32_t algonum;
>         uint32_t service;
>         unsigned int active_devs;
>         struct skcipher_alg algo; /* akcipher_alg VS skcipher_alg */
> };
>
> If reusing virtio_crypto_algs_register/unregister, we need to modify the
> data structure like this:
> struct virtio_crypto_akcipher_algo {
>         uint32_t algonum;
>         uint32_t service;    /* use service to distinguish
> akcipher/skcipher */
>         unsigned int active_devs;
>     union {
>         struct skcipher_alg skcipher;
>             struct akcipher_alg akcipher;
>     } alg;
> };
>
> int virtio_crypto_akcipher_algs_register(struct virtio_crypto *vcrypto)
> {
>     ...
>         for (i = 0; i < ARRAY_SIZE(virtio_crypto_akcipher_algs); i++) {
>                 uint32_t service = virtio_crypto_akcipher_algs[i].service;
>         ...
>         /* test service type then call
> crypto_register_akcipher/crypto_register_skcipher */
>                 if (service == VIRTIO_CRYPTO_SERVICE_AKCIPHER)
>
> crypto_register_akcipher(&virtio_crypto_akcipher_algs[i].algo.akcipher);
>         else
>
> crypto_register_skcipher(&virtio_crypto_skcipher_algs[i].algo.skcipher);
>         ...
>         }
>     ...
> }
>
> Also test service type and call
> crypto_unregister_skcipher/crypto_unregister_akcipher.
>
> This gets unclear from current v2 version.
>
> On the other hand, the kernel side prefers to separate skcipher and
> akcipher(separated header files and implementations).
>
Hi, Lei
I also take a look at other crypto drivers at qat/ccp/hisilicon, they
separate akcipher/skcipher algo. If you consider that reusing
virtio_crypto_algs_register/unregister seems better, I will try to merge
them into a single function.

--
zhenwei pi

\
 
 \ /
  Last update: 2022-02-23 11:20    [W:0.091 / U:0.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site