Hi Kai, Some small comments below.
> -----Original Message----- > From: Kai Ji <kai...@intel.com> > Sent: Sunday 2 October 2022 23:45 > To: dev@dpdk.org > Cc: gak...@marvell.com; Ji, Kai <kai...@intel.com> > Subject: [dpdk-dev v3 1/1] lib/cryptodev: multi-process IPC request handler > > This patch add a function to support queue-pair configuration request to allow > the primary or secondary process to setup/free the queue-pair via IPC handler. > Add in queue pair in-used by process id array in rte_cryptodev_data for pid > tracking. > > Signed-off-by: Kai Ji <kai...@intel.com> <snip> > +static int > +rte_cryptodev_ipc_request(const struct rte_mp_msg *mp_msg, const void > +*peer) { > + struct rte_mp_msg mp_res; > + struct rte_cryptodev_mp_param *res = > + (struct rte_cryptodev_mp_param *)mp_res.param; > + const struct rte_cryptodev_mp_param *param = > + (const struct rte_cryptodev_mp_param *)mp_msg->param; [CP] The "param" name could be improved I think - maybe follow the "res" naming and call this one "msg" Both are "mp_***->param" so I think it is confusing to have one named "param" > + > + int ret; > + struct rte_cryptodev *dev; > + uint16_t *qps_in_used_by_pid; [CP] Possible typo - change to "qp_in_use_by_pid" to match the naming of "dev->data->qp_in_use_by_pid" <snip> > +int rte_cryptodev_mp_request_register(void) > +{ > + RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY); > + return rte_mp_action_register(CRYPTODEV_MP_REQ, > + rte_cryptodev_ipc_request); > +} > + > +void rte_cryptodev_mp_request_unregister(void) > +{ > + RTE_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY); > + rte_mp_action_unregister(CRYPTODEV_MP_REQ); > +} > + [CP] The above two functions should have the function return type on its own line. <snip> > +/** > + * Register multi process request IPC handler > + * > + * @return > + * - 0: Success registered > + * - 1: Failed registration failed > + * - -EINVAL: device was not configured [CP] Indent is off for success result <snip> Thanks, Ciara