Hi Kai,
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Kai Ji
> Sent: Tuesday, October 26, 2021 6:25 PM
> To: dev@dpdk.org
> Cc: Ji, Kai <kai...@intel.com>
> Subject: [dpdk-dev] [dpdk-dev v1 1/7] crypro/qat: qat driver refactor
> skeleton
>
> Add-in enqueue/dequeue op burst and build-request skeleton functions
> for qat crypto driver refactor.
>
> Signed-off-by: Kai Ji <kai...@intel.com>
> ---
...
> +/**
> + * @brief Function prototype to build a QAT request.
> + * @param opaque: an opaque data may be used to store context may be
> useful
> + * between 2 enqueue operations.
> + **/
The comments for the function prototype can be polished to make it clearer.
Also the formatting shall be inline with the rest of dpdk code. Such as
/**
* <brief introduction to the function prototype>
*
* @param x
* Description to the param x.
* @return
* - 0 if the crypto request is built successfully.
* - error code otherwise.
*/
> +typedef int (*qat_op_build_request_t)(void *in_op, uint8_t *out_msg,
> + void *op_cookie, uint64_t *opaque, enum qat_device_gen
> dev_gen);
> +
> +typedef int (*qat_op_dequeue_t)(void **op, uint8_t *resp, void
> *op_cookie,
> + uint64_t *dequeue_err_count __rte_unused);
The above function prototype worth some description too?
> +
> +#define QAT_BUILD_REQUEST_MAX_OPAQUE_SIZE 2
> +
> struct qat_qp {
> void *mmap_bar_addr;
> struct qat_queue tx_q;
> @@ -44,6 +57,7 @@ struct qat_qp {
> struct rte_mempool *op_cookie_pool;
> void **op_cookies;
> uint32_t nb_descriptors;
> + uint64_t opaque[QAT_BUILD_REQUEST_MAX_OPAQUE_SIZE];
> enum qat_device_gen qat_dev_gen;
> enum qat_service_type service_type;
> struct qat_pci_device *qat_dev;
> @@ -77,6 +91,14 @@ struct qat_qp_config {
> const char *service_str;
> };
>
> +uint16_t
> +refactor_qat_enqueue_op_burst(void *qp, qat_op_build_request_t
> op_build_request,
> + void **ops, uint16_t nb_ops);
> +
> +uint16_t
> +refactor_qat_dequeue_op_burst(void *qp, void **ops,
> + qat_op_dequeue_t qat_dequeue_process_response,
> uint16_t nb_ops);
> +
> uint16_t
> qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops);
>
> diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
> index 85973812a8..d5b4c66d68 100644
> --- a/drivers/crypto/qat/qat_asym.c
> +++ b/drivers/crypto/qat/qat_asym.c
> @@ -456,6 +456,41 @@ qat_asym_fill_arrays(struct rte_crypto_asym_op
> *asym_op,
> return 0;
> }
>
> +static __rte_always_inline int
> +refactor_qat_asym_build_request(__rte_unused void *in_op,
> + __rte_unused uint8_t *out_msg, __rte_unused void
> *op_cookie,
> + __rte_unused uint64_t *opaque,
> + __rte_unused enum qat_device_gen dev_gen)
> +{
> + return 0;
> +}
> +
> +int
> +refactor_qat_asym_process_response(__rte_unused void **op,
> + __rte_unused uint8_t *resp,
> + __rte_unused void *op_cookie,
> + __rte_unused uint64_t *dequeue_err_count)
> +{
> + return 0;
> +}
I assume the functions refactor_xxx() is removed in future patches right?
Maybe adding some comments to help reviewing.
> +
> +uint16_t
> +qat_asym_crypto_enqueue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> + uint16_t nb_ops)
> +{
> + return refactor_qat_enqueue_op_burst(qp,
> + refactor_qat_asym_build_request,
> + (void **)ops, nb_ops);
> +}
> +
> +uint16_t
> +qat_asym_crypto_dequeue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> + uint16_t nb_ops)
> +{
> + return refactor_qat_dequeue_op_burst(qp, (void **)ops,
> + refactor_qat_asym_process_response,
> nb_ops);
> +}
> +
> int
> qat_asym_build_request(void *in_op,
> uint8_t *out_msg,
> diff --git a/drivers/crypto/qat/qat_asym.h b/drivers/crypto/qat/qat_asym.h
> index 308b6b2e0b..50c2641eba 100644
> --- a/drivers/crypto/qat/qat_asym.h
> +++ b/drivers/crypto/qat/qat_asym.h
> @@ -92,4 +92,19 @@ void
> qat_asym_process_response(void __rte_unused **op, uint8_t *resp,
> void *op_cookie);
>
> +int
> +refactor_qat_asym_process_response(__rte_unused void **op,
> + __rte_unused uint8_t *resp,
> + __rte_unused void *op_cookie,
> + __rte_unused uint64_t *dequeue_err_count);
> +
> +uint16_t
> +qat_asym_crypto_enqueue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> + uint16_t nb_ops);
> +
> +
> +uint16_t
> +qat_asym_crypto_dequeue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> + uint16_t nb_ops);
> +
> #endif /* _QAT_ASYM_H_ */
> diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
> index 93b257522b..a92874cd27 100644
> --- a/drivers/crypto/qat/qat_sym.c
> +++ b/drivers/crypto/qat/qat_sym.c
> @@ -210,6 +210,31 @@ handle_spc_gmac(struct qat_sym_session *ctx,
> struct rte_crypto_op *op,
> ICP_QAT_FW_LA_NO_PROTO);
> }
>
> +static __rte_always_inline int
> +refactor_qat_sym_build_request(__rte_unused void *in_op,
> + __rte_unused uint8_t *out_msg, __rte_unused void
> *op_cookie,
> + __rte_unused uint64_t *opaque,
> + __rte_unused enum qat_device_gen dev_gen)
> +{
> + return 0;
> +}
> +
> +uint16_t
> +refactor_qat_sym_enqueue_burst(void *qp, struct rte_crypto_op **ops,
> + uint16_t nb_ops)
> +{
> + return refactor_qat_enqueue_op_burst(qp,
> refactor_qat_sym_build_request,
> + (void **)ops, nb_ops);
> +}
> +
Same as above, some comments would be helpful and shall remove altogether in
later patches.
> +uint16_t
> +refactor_qat_sym_dequeue_burst(void *qp, struct rte_crypto_op **ops,
> + uint16_t nb_ops)
> +{
> + return refactor_qat_dequeue_op_burst(qp, (void **)ops,
> + refactor_qat_sym_process_response,
> nb_ops);
> +}
> +
> int
> qat_sym_build_request(void *in_op, uint8_t *out_msg,
> void *op_cookie, enum qat_device_gen qat_dev_gen)
> diff --git a/drivers/crypto/qat/qat_sym.h b/drivers/crypto/qat/qat_sym.h
> index e3ec7f0de4..17b2c871bd 100644
> --- a/drivers/crypto/qat/qat_sym.h
> +++ b/drivers/crypto/qat/qat_sym.h
> @@ -54,6 +54,22 @@ struct qat_sym_op_cookie {
> } opt;
> };
>
> +static __rte_always_inline int
> +refactor_qat_sym_process_response(__rte_unused void **op,
> + __rte_unused uint8_t *resp, __rte_unused void *op_cookie,
> + __rte_unused uint64_t *dequeue_err_count)
> +{
> + return 0;
> +}
> +
> +uint16_t
> +refactor_qat_sym_enqueue_burst(void *qp, struct rte_crypto_op **ops,
> + uint16_t nb_ops);
> +
> +uint16_t
> +refactor_qat_sym_dequeue_burst(void *qp, struct rte_crypto_op **ops,
> + uint16_t nb_ops);
> +
> int
> qat_sym_build_request(void *in_op, uint8_t *out_msg,
> void *op_cookie, enum qat_device_gen qat_dev_gen);
> diff --git a/drivers/crypto/qat/qat_sym_session.h
> b/drivers/crypto/qat/qat_sym_session.h
> index 6ebc176729..73493ec864 100644
> --- a/drivers/crypto/qat/qat_sym_session.h
> +++ b/drivers/crypto/qat/qat_sym_session.h
> @@ -55,6 +55,11 @@
> #define QAT_SESSION_IS_SLICE_SET(flags, flag) \
> (!!((flags) & (flag)))
>
> +struct qat_sym_session;
> +
> +typedef int (*qat_sym_build_request_t)(void *in_op, struct
> qat_sym_session *ctx,
> + uint8_t *out_msg, void *op_cookie);
> +
> enum qat_sym_proto_flag {
> QAT_CRYPTO_PROTO_FLAG_NONE = 0,
> QAT_CRYPTO_PROTO_FLAG_CCM = 1,
> @@ -107,6 +112,7 @@ struct qat_sym_session {
> /* Some generations need different setup of counter */
> uint32_t slice_types;
> enum qat_sym_proto_flag qat_proto_flag;
> + qat_sym_build_request_t build_request[2];
> };
>
> int
> --
> 2.17.1
All comments are towards the comments - as of the rest
Acked-by: Fan Zhang <roy.fan.zh...@intel.com>
