Re: [dpdk-dev] [PATCH v2 5/5] cryptodev: move device specific structures

["Zhang, Roy Fan" <roy.fan.zhang@intel.com>]

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Monday, October 11, 2021 1:43 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; david.marchand@redhat.com;
> hemant.agrawal@nxp.com; anoobj@marvell.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>;
> Doherty, Declan <declan.doherty@intel.com>; matan@nvidia.com;
> g.singh@nxp.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> jianjay.zhou@huawei.com; asomalap@amd.com; ruifeng.wang@arm.com;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>; ajit.khaparde@broadcom.com;
> rnagadheeraj@marvell.com; adwivedi@marvell.com; Power, Ciara
> <ciara.power@intel.com>; Akhil Goyal <gakhil@marvell.com>
> Subject: [PATCH v2 5/5] cryptodev: move device specific structures
> 
> The device specific structures - rte_cryptodev
> and rte_cryptodev_data are moved to cryptodev_pmd.h
> to hide it from the applications.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> ---
...
> diff --git a/lib/cryptodev/cryptodev_pmd.h
> b/lib/cryptodev/cryptodev_pmd.h
> index a71edbb991..0d1bef0e67 100644
> --- a/lib/cryptodev/cryptodev_pmd.h
> +++ b/lib/cryptodev/cryptodev_pmd.h
> @@ -52,6 +52,71 @@ struct rte_cryptodev_pmd_init_params {
>  	unsigned int max_nb_queue_pairs;
>  };
> 
> +/**
> + * @internal
> + * The data part, with no function pointers, associated with each device.
> + *
> + * This structure is safe to place in shared memory to be common among
> + * different processes in a multi-process configuration.
> + */
> +struct rte_cryptodev_data {
> +	/** Device ID for this instance */
> +	uint8_t dev_id;
> +	/** Socket ID where memory is allocated */
> +	uint8_t socket_id;
> +	/** Unique identifier name */
> +	char name[RTE_CRYPTODEV_NAME_MAX_LEN];
> +
> +	__extension__
> +	/** Device state: STARTED(1)/STOPPED(0) */
> +	uint8_t dev_started : 1;
> +
> +	/** Session memory pool */
> +	struct rte_mempool *session_pool;

Looks like we never used this pool ever - shall we take this chance to remove it?

> +	/** Array of pointers to queue pairs. */
> +	void **queue_pairs;
> +	/** Number of device queue pairs. */
> +	uint16_t nb_queue_pairs;

queue_pairs are likely the only item we use rte_cryptodev_data in the fast-path.
Also take this chance to move it to the beginning of the structure?

> +
> +	/** PMD-specific private data */
> +	void *dev_private;
> +} __rte_cache_aligned;
> +

Not really comments but some suggestions - could be done in the future as the
structure is moved to internal in this patchset.
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>