Hi Arek, > > > > Hi Arek, > > > > > Subject: [PATCH v3 5/5] doc: add documentation for multi process > > > crypto app > > > > Please do not make separate patches for documentation. > > Your first patch description says more info can be found in mp_crypto.rst. > > But it is not there in that patch. > > > > You should add the relevant documentation in the first patch so that people > > can review and understand the patchset before actually reviewing the patch. > > > > You should split the documentation patch and add in the patches where they > > are relevant. > [AK] - sure, I will. > > > > MAINTAINERS file update is also missing for this app. > > > > > > > > > > This commit adds documentation for multi process crypto test > > > application. > > > > > > Signed-off-by: Arek Kusztal <arkadiuszx.kusz...@intel.com>
We first need to decide where to put this application. Thomas, does not like the idea to have it in app/ Please analyze if it can be combined with l2fwd-crypto. Regards, Akhil > > > --- > > > doc/guides/tools/index.rst | 1 + > > > doc/guides/tools/mp_crypto.rst | 151 > > > +++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 152 insertions(+) > > > create mode 100644 doc/guides/tools/mp_crypto.rst > > > > > > diff --git a/doc/guides/tools/index.rst b/doc/guides/tools/index.rst > > > index 4840cf4..a360307 100644 > > > --- a/doc/guides/tools/index.rst > > > +++ b/doc/guides/tools/index.rst > > > @@ -17,3 +17,4 @@ DPDK Tools User Guides > > > cryptoperf > > > comp_perf > > > testeventdev > > > + mp-crypto > > > diff --git a/doc/guides/tools/mp_crypto.rst > > > b/doc/guides/tools/mp_crypto.rst new file mode 100644 index > > > 0000000..201834f > > > --- /dev/null > > > +++ b/doc/guides/tools/mp_crypto.rst > > > @@ -0,0 +1,151 @@ > > > +.. SPDX-License-Identifier: BSD-3-Clause > > > + Copyright(c) 2020 Intel Corporation. > > > + > > > +dpdk-test-mp-crypto Application > > > +=============================== > > > + > > > +The Multi-process Crypto application is a simple application that > > > +allows to run crypto related operations in a multiple process > > > +environment. It builds on the EAL primary/secondary process > > infrastructure. > > > + > > > +The application allows a user to configure devices, setup > > > +queue-pairs, create and init sessions and specify data-path flow > > > +(enqueue/dequeue) in different processes. The app can help to check > > > +if the PMD behaves correctly in scenarios like the following: > > > + > > > +* device is configured in primary process, queue-pairs are setup in > > > +secondary > > > process > > > + > > > +* queue pair is shared across processes, i.e. enqueue in one process > > > +and > > > dequeue in another > > > + > > > + > > > +Compiling the Application > > > +------------------------- > > > + > > > +To compile the sample application see :doc:`compiling`. > > > + > > > +The application is located in the ``app/test-mp_crypto`` directory. > > > + > > > +Running the Application > > > +----------------------- > > > + > > > +App binary: dpdk-test-mp-crypto (in build/app) > > > + > > > +For running PRIMARY or SECONDARY process standard EAL options apply: > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto --proc-type primary > > > + > > > + ./dpdk-test-mp-crypto --proc-type secondary > > > + > > > +.. Note:: > > > + > > > + The same set of BDFs must be passed to all processes. > > > > BDFs ?? Statement should be generic. Not all devices are PCI based. > [Arek] Sure, will change. > > > > > + > > > +.. Note:: > > > + The same crypto devices must be created in all processes, e.g. in qat > > > + case if asym and sym devices are enabled in the primary process, > > they > > > + must be enabled in all secondary processes. > > In all secondary processes as well. > > > > > + > > > +General help can by checked by running: > > > + > > > +. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -- -h > > > + > > > +The application has a number of command line options: > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -- --devtype [dev-name] > > > + > > > +This option specifies which driver to use by its name (for example > > "crypto_qat"). > > > +The same name must be passed to all processes. > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -- --config_dev [devA, devB,] > > > + > > > +This option specifies the list of devices that should be configured > > > +by this > > > process, > > > +this results in a call to the ``rte_cryptodev_configure`` API. devX > > > +is a positive integer (including zero), the value is according to > > > +probe order (from the > > > smallest > > > +BDF number), not necessarily the cmdline order. > > > > Isn't it better to have a cryptodev_mask instead of config_dev? Same as we > > do in Ipsec-secgw and l2fwd-crypto. > > Mask will specify the devices which will be configured in the current > > process. > > > [AK] - actually we initially have done that, in code actually list is > translated to > mask because of this. We though later list can be easier to use. > > > > > + > > > +Example command: > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -w 03:01.2 -w 03:01.1 -w 03:01.3 > > > + --config-dev 0,2 > > > + > > > ++will configure devices 03:01.1 and 03:01.3. > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -- --qp-config=[devA]:[qp_A, > > > + qp_B,];[devB]:[qp_A, > > > qp_C]; > > > + > > > +devX - positive integer (including zero), as in config_dev command > > > + > > > ++qp_X - positive integer (including zero), specifies which queue pair > > > ++should be > > > setup > > > + > > > +This command specifies which queue pairs should be setup, resulting > > > +in a call to ``rte_cryptodev_queue_pair_setup`` API. > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -w 03:01.2 -w 03:01.1 -w 03:01.3 --qp- > > > config="0:0,1;1:1;2:0,1;" > > > + > > > +This command will configure queue pairs 0 and 1 on device 0 > > > +(03:01.1), queue > > > pair 1 > > > +on device 1 (03:01.2), queue pairs 0 and 1 on device 2 (03:01.3). The > > > +device in > > > question > > > +should be configured before that, though not necessarily by the same > > process. > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -- --enq=[devX]:[qpX]:[ops]:[vector_id] > > > + ./dpdk-test-mp-crypto -- --deq=[devX]:[qpX]:[ops]:[vector_id] > > > + > > > +devX - positive integer (including zero), as in config_dev command > > > + > > > +qp_X - positive integer (including zero), as in qp-config command > > > > qpX > [Arek] Ok. > > > > > + > > > +ops - when positive integer - number of operations to > > > +enqueue/dequeue, when > > > 0 infinite loop > > > > This should be an optional parameter and should not be part of enq/deq > > configs And default value should be 0 > [Arek] So something like (if vector number removed): > --enq=0:0, -> dev 0, qp 0 , infinite loop > --enq=0:0:5000, -> dev 0, qp 0, 5000 packets > ? > > > > > + > > > +vector_id - positive integer (including zero), vector_id used by this > > > +process > > > > What is vector id? I do not see it changing in any of your examples. Do we > > really need it? > [Arek] - with current implementation it could be dropped, but initially it was > created for multiple vectors. Eventually may be bring back in future. > > > > It looks that the dev id, qp_id are redundant in all three. We can probably > > squeeze it in a single config > > --enq=(devA:qpX,qpY),(devB:qpX) > > --deq=(devC:qpX,qpY),(devB:qpY,qpZ) > [Arek] - comma separates processes here? So 3 processes for enqueue, 4 for > dequeue? > > > > And remove config-dev and qp-config > [Arek] - not sure about that, initially first use case we were targeting was > to do > all config in one process and enq/deq in other process which can be quite > popular use case. > > > > Cumulative sum of all the devices(dev - A,B,C)/queues(A->X,Y; B->X,Y,Z;C- > > >X,Y) in enq and deq can be configured in that process. > > This will be simple to configure from user perspective and will reduce the > > risk > > of setting mismatch configuration. > > > > And if the enq and deq are same, we should be able to skip either of the > > configs and assume that it is same as the other one. > > > > What say? > > > > > + > > > +This commands will enqueue/dequeue "ops" number of packets to qp_X > > on > > > devX. > > > +Example usage: > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto -- --enq=2:0:0:0, --deq=2:0:0:0, > > > + > > > +Note. ',' comma character is necessary at the end due to some parser > > > shortcomings. > > > + > > > +To close the application when running in an infinite loop a signal > > > +handler is registered to catch interrupt signals i.e. ``ctrl-c`` > > > +should be used. When used in primary process other processes will be > > > +notified about exiting intention and will close after collecting > > > remaining > > packets (if dequeuing). > > > + > > > +Example commands > > > +---------------- > > > + > > > +Use two different devices on 3 separate queues: > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto --proc-type primary -c 1 -w 03:01.1 -w > > > + 03:01.2 -- -- > > > devtype "crypto_qat" --config-dev 0,1 --qp-config="0:0,1;1:0,1;" > > > --session- > > > mask=0x3 --enq=0:0:0:0, --deq=0:0:0:0, --print-stats > > > > You did not explain session-mask above. > > > > > + ./dpdk-test-mp-crypto --proc-type secondary -c 2 -w 03:01.1 -w > > > + 03:01.2 -- -- > > > devtype "crypto_qat" --enq=0:1:0:0, --deq=0:1:0:0, --print-stats > > > + ./dpdk-test-mp-crypto --proc-type secondary -c 4 -w 03:01.1 -w > > > + 03:01.2 -- -- > > > devtype "crypto_qat" --enq=1:0:0:0, --deq=1:0:0:0, --print-stats > > > + > > > +Use different processes to enqueue and dequeue to one queue pair: > > > + > > > +.. code-block:: console > > > + > > > + ./dpdk-test-mp-crypto --proc-type primary -c 1 -w 03:01.1 -- > > > + --devtype > > > "crypto_qat" --config-dev 0 --session-mask=0x3 --qp-config="0:1;" -- > > > enq=0:1:0:0, --print-stats > > > + ./dpdk-test-mp-crypto --proc-type secondary -c 2 -w 03:01.1 -- > > > + --devtype > > > "crypto_qat" --deq=0:1:0:0, --print-stats > > > > We can probably add a sample print-stats output here > [Arek] - Sure. > > > > > + > > > +Limitations > > > +----------- > > > + > > > +Only one crypto vector and session type is possible to chose right > > > +now and it is > > > AES-GCM test case. > > > + > > > +Number of descriptors if set by default to 4096 > > > > Number of descriptors is set by default to 4096 > > > > > -- > > > 2.1.0
