Hi Akhil,
On 06/10/2022 19:49, Akhil Goyal wrote:
As some cryptode PMDs have multiprocess support, the secondary
process needs queue-pair to be configured by the primary process before
to use. This patch adds an IPC register function to help the primary
process to register IPC action that allow secondary process to configure
cryptodev queue-pair via IPC messages during the runtime.
Why are we forcing user another alternate API for secondary process to work?
Can we not register the IPC inside rte_cryptodev_queue_pair_setup() ?
As I understand till now,
You have introduced another API rte_cryptodev_mp_request_register(),
Which will be called by application if primary-secondary communication is
required.
And if it is registered, rte_cryptodev_ipc_request() will be called from
somewhere(not sure when this will be called).
And the call to rte_cryptodev_queue_pair_setup() from the secondary will do
nothing.
Is this a correct understanding? If it is correct, then it is an unnecessary
overhead for the application.
We should update the rte_cryptodev_queue_pair_setup instead to handle primary
and secondary configuration.
IMO, you do not need to change anything in the library.
Everything can be handled in the PMD. When the queue_pair_setup is called for
particular qp_id,
Store the getpid() of the calling process into the priv data of queue pair if
it is not already configured
And if configured return failure.
And in case of release you can also check the same.
The configuration of queues for multi process is specific to PMDs.
There may be PMDs which may support same queue pair to be used by different
processes.
Rx queue from the qp by one process and Tx queue from the qp by another process.
This will be needed if one process is doing only enqueue and the other only
dequeue on the same qp.
So in that case, your implementation will not work.
This is a question we didn't think as comprehensive as you did. With the
change Kai did at least all Intel PMDs will support that.
I assume we need some feature flag to state that?
After setup, a new "qp_in_used_pid" param stores the PID to provide
the ownership of the queue-pair so that only the PID matched queue-pair
free request is allowed in the future.
qp_in_used_pid looks very cryptic, I believe this should be part of queue pair
private data of PMD.
Adding this in cryptodev data is not justified. This property is per queue and
not per crypto device.
Hence adding in device data does not make sense to me.
Agreed. The PID storage is not mandatory for every PMD but only for some
(ipsec-mb for example) so we should store the PID info inside the PMD
queue pair data instead.
Regards,
Fan
