Re: [PATCH v2 3/3] virtio-crypto: implement RSA algorithm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux