Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Subject: Re: RE: [PATCH v2 3/3] virtio-crypto: implement RSA algorithm
To:     "Gonglei (Arei)" <arei.gonglei@huawei.com>
Cc:     "jasowang@redhat.com" <jasowang@redhat.com>,
        "mst@redhat.com" <mst@redhat.com>,
        "virtualization@lists.linux-foundation.org" 
        <virtualization@lists.linux-foundation.org>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "helei.sig11@bytedance.com" <helei.sig11@bytedance.com>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        kernel test robot <lkp@intel.com>
References: <20220211084108.1254218-1-pizhenwei@bytedance.com>
 <20220211084108.1254218-4-pizhenwei@bytedance.com>
 <c9144b0d82e34566a960f210ddc32696@huawei.com>
From:   zhenwei pi <pizhenwei@bytedance.com>
Message-ID: <8ef2f660-bd84-de70-1539-402c73795dfe@bytedance.com>
Date:   Fri, 18 Feb 2022 11:12:51 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <c9144b0d82e34566a960f210ddc32696@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

>> +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).

-- 
zhenwei pi