[v2,0/4] introduce multi-function processing support

  PLEASE NOTE: This patchset supercedes the following v1 patches which were
mistakenly added as stand-alone patches (apologies for any confusion this
may cause)

https://patchwork.dpdk.org/patch/66733/
https://patchwork.dpdk.org/patch/66735/
https://patchwork.dpdk.org/patch/66736/

PLEASE NOTE ALSO: Support for QAT, which the following patch addressed,
has been dropped from this patchset and is now targetted at the next
release (v20.08)

https://patchwork.dpdk.org/patch/66819/


Introduction
============

This patchset adds a new multi-function interface and a aesni_mb raw device
PMD which uses this interface.

This patchset has already been discussed as part of the following RFC:

http://mails.dpdk.org/archives/dev/2020-February/157045.html
http://mails.dpdk.org/archives/dev/2020-March/159189.html

The main aim of this interface and raw PMDs is to provide a flexible and
extensible way of combining one or more packet-processing functions into a
single operation, thereby allowing these to be performed in parallel in
optimized software libraries or in a hardware accelerator. These functions
can include cryptography, compression and CRC/checksum calculation, while
others can potentially be added in the future. Performing these functions
in parallel as a single operation can enable a significant performance
improvement.


Background
==========

There are a number of byte-wise operations which are present and common
across many access network data-plane pipelines, such as Cipher,
Authentication, CRC, Bit-Interleaved-Parity (BIP), other checksums etc.
Some prototyping has been done at Intel in relation to the 01.org
access-network-dataplanes project to prove that a significant performance
improvement is possible when such byte-wise operations are combined into a
single pass of packet data processing. This performance boost has been
prototyped for both XGS-PON MAC data-plane and DOCSIS MAC data-plane
pipelines.

The prototypes used some protocol-specific modifications to the DPDK
cryptodev library. In order to make this performance improvement consumable
by network access equipment vendors, a more extensible and correct solution
was required.

Hence, the introduction of a multi-function interface, initially for use by
raw devices. In this patchset, a new aesni_mb raw device has been created
which uses this interface.

NOTE: In a future DPDK release (currently targetting DPDK v20.08), a qat
raw device will also be added. As multiple raw devices will share the same
interface, the approach taken was to create a common interface
(i.e. multi-function) which can be used by these devices. This both cuts
down on code duplication across the devices and allows an application
access multiple devices using the same interface.


Use Cases
=========

The primary use cases for the multi-function interface and raw PMDs have
already been mentioned. These are:

- DOCSIS MAC: Crypto-CRC
	- Order:
		- Downstream: CRC, Encrypt
		- Upstream: Decrypt, CRC
	- Specifications:
		- Crypto: 128-bit AES-CFB encryption variant for DOCSIS as
		  described in section 11.1 of DOCSIS 3.1 Security
		  Specification
		  (https://apps.cablelabs.com/specification/CM-SP-SECv3.1)
		- CRC: Ethernet 32-bit CRC as defined in
		  Ethernet/[ISO/IEC 8802-3]

- XGS-PON MAC: Crypto-CRC-BIP
	- Order:
		- Downstream: CRC, Encrypt, BIP
		- Upstream: BIP, Decrypt, CRC
	- Specifications:
		- Crypto: AES-128 [NIST FIPS-197] cipher, used in counter
		  mode (AES-CTR), as described in [NIST SP800-38A].
		- CRC: Ethernet 32-bit CRC as defined in
		  Ethernet/[ISO/IEC 8802-3]
		- BIP: 4-byte bit-interleaved even parity (BIP) field
		  computed over the entire FS frame, refer to
		  ITU-T G.989.3, sections 8.1.1.5 and 8.1.2.3
		  (https://www.itu.int/rec/dologin_pub.asp?lang=e&id=
		   T-REC-G.989.3-201510-I!!PDF-E)

Note that support for both these chained operations is already available in
the Intel IPSec Multi-Buffer library.

However, it is not limited to these. The following are some of the other
possible use-cases, which multi-function will allow for:

- Storage:
	- Compression followed by Encryption
- IPSec over UDP:
	- UDP Checksum calculation followed by Encryption

While DPDK's rte_cryptodev and rte_compressdev allow many cryptographic and
compression algorithms to be chained together in one operation, there is no
way to chain these with any error detection or checksum algorithms. And
there is no way to chain crypto and compression algorithms together. The
multi-function  interface will allow these chains to be created, and also
allow any future type of operation to be easily added.


Architecture
============

The following diagram shows where the multi-function interface and raw
devices fit in an overall application architecture.

  +------------------------------------------------+
  |                                                |
  |                  Application                   |
  |   (e.g. vCMTS (DOCSIS), vOLT (XGS-PON), etc.)  |
  |                                                |
  +------------------------------------------------+
                          |
  +-----------------------|------------------------+
  |                       |                  DPDK  |
  |                       |                        |
  |             +---------------------+            |
  |             |                     |            |
  |             |     rte_rawdev      |            |
  |             |                     |            |            NOTE:
  |             +---------------------+        ____|______ 'MULTI-FUNCTION
  |                    /      \              /     |          INTERFACE'
  |                   /        \            /      |         is opaque to
  |                  /          \          /       |          rte_rawdev
  |       +--------------------------------+       |
  |       |    MULTI-FUNCTION INTERFACE    |       |
  |       +--------------------------------+       |
  |       +------------+      +------------+       |
  |       |   RAWDEV   |      |   RAWDEV   |       |
  |       |  AESNI-MB  |      |    QAT     |       |
  |       |    PMD     |      |    PMD     |       |
  |       +------------+      +------------+       |              NOTE:
  |              |                  |     \________|_____  'RAWDEV QAT PMD'
  +--------------|------------------|--------------+       will be added in
                 |                  |                         next release
          +------------+      +------------+
          |  AESNI-MB  |      |   QAT HW   |
          |   SW LIB   |      |            |
          +------------+      +------------+

David Coyle (4):
  raw/common: add multi-function interface
  raw/aesni_mb: add aesni_mb raw device
  test/rawdev: add aesni_mb raw device tests
  app/crypto-perf: add support for multi-function processing

 app/test-crypto-perf/Makefile                 |    5 +
 app/test-crypto-perf/cperf_ops.c              |  265 +++
 app/test-crypto-perf/cperf_options.h          |   37 +-
 app/test-crypto-perf/cperf_options_parsing.c  |  396 ++++-
 app/test-crypto-perf/cperf_test_common.c      |   88 +-
 app/test-crypto-perf/cperf_test_latency.c     |  176 +-
 .../cperf_test_pmd_cyclecount.c               |   96 +-
 app/test-crypto-perf/cperf_test_throughput.c  |  164 +-
 .../cperf_test_vector_parsing.c               |   35 +-
 app/test-crypto-perf/cperf_test_vectors.c     |   53 +
 app/test-crypto-perf/cperf_test_vectors.h     |    9 +
 app/test-crypto-perf/cperf_test_verify.c      |  205 ++-
 app/test-crypto-perf/main.c                   |  255 ++-
 app/test-crypto-perf/meson.build              |    6 +
 app/test/test_rawdev.c                        |   16 +
 config/common_base                            |   11 +
 drivers/meson.build                           |    5 +
 drivers/raw/Makefile                          |    3 +
 drivers/raw/aesni_mb/Makefile                 |   47 +
 drivers/raw/aesni_mb/aesni_mb_rawdev.c        | 1536 +++++++++++++++++
 drivers/raw/aesni_mb/aesni_mb_rawdev.h        |  112 ++
 drivers/raw/aesni_mb/aesni_mb_rawdev_test.c   | 1102 ++++++++++++
 .../aesni_mb/aesni_mb_rawdev_test_vectors.h   | 1183 +++++++++++++
 drivers/raw/aesni_mb/meson.build              |   26 +
 .../aesni_mb/rte_rawdev_aesni_mb_version.map  |    3 +
 drivers/raw/common/Makefile                   |    8 +
 drivers/raw/common/meson.build                |    7 +
 drivers/raw/common/multi_fn/Makefile          |   27 +
 drivers/raw/common/multi_fn/meson.build       |    9 +
 .../multi_fn/rte_common_multi_fn_version.map  |   11 +
 drivers/raw/common/multi_fn/rte_multi_fn.c    |  166 ++
 drivers/raw/common/multi_fn/rte_multi_fn.h    |  350 ++++
 .../raw/common/multi_fn/rte_multi_fn_driver.h |   55 +
 drivers/raw/meson.build                       |    3 +-
 meson.build                                   |    4 +
 mk/rte.app.mk                                 |    3 +
 36 files changed, 6270 insertions(+), 207 deletions(-)
 create mode 100644 drivers/raw/aesni_mb/Makefile
 create mode 100644 drivers/raw/aesni_mb/aesni_mb_rawdev.c
 create mode 100644 drivers/raw/aesni_mb/aesni_mb_rawdev.h
 create mode 100644 drivers/raw/aesni_mb/aesni_mb_rawdev_test.c
 create mode 100644 drivers/raw/aesni_mb/aesni_mb_rawdev_test_vectors.h
 create mode 100644 drivers/raw/aesni_mb/meson.build
 create mode 100644 drivers/raw/aesni_mb/rte_rawdev_aesni_mb_version.map
 create mode 100644 drivers/raw/common/Makefile
 create mode 100644 drivers/raw/common/meson.build
 create mode 100644 drivers/raw/common/multi_fn/Makefile
 create mode 100644 drivers/raw/common/multi_fn/meson.build
 create mode 100644 drivers/raw/common/multi_fn/rte_common_multi_fn_version.map
 create mode 100644 drivers/raw/common/multi_fn/rte_multi_fn.c
 create mode 100644 drivers/raw/common/multi_fn/rte_multi_fn.h
 create mode 100644 drivers/raw/common/multi_fn/rte_multi_fn_driver.h
  

On 4/3/2020 5:36 PM, David Coyle wrote:
> PLEASE NOTE: This patchset supercedes the following v1 patches which were
> mistakenly added as stand-alone patches (apologies for any confusion this
> may cause)
> 
> https://patchwork.dpdk.org/patch/66733/
> https://patchwork.dpdk.org/patch/66735/
> https://patchwork.dpdk.org/patch/66736/
> 
> PLEASE NOTE ALSO: Support for QAT, which the following patch addressed,
> has been dropped from this patchset and is now targetted at the next
> release (v20.08)
> 
> https://patchwork.dpdk.org/patch/66819/
> 
> 
> Introduction
> ============
> 
> This patchset adds a new multi-function interface and a aesni_mb raw device
> PMD which uses this interface.
> 
> This patchset has already been discussed as part of the following RFC:
> 
> http://mails.dpdk.org/archives/dev/2020-February/157045.html
> http://mails.dpdk.org/archives/dev/2020-March/159189.html
> 
> The main aim of this interface and raw PMDs is to provide a flexible and
> extensible way of combining one or more packet-processing functions into a
> single operation, thereby allowing these to be performed in parallel in
> optimized software libraries or in a hardware accelerator. These functions
> can include cryptography, compression and CRC/checksum calculation, while
> others can potentially be added in the future. Performing these functions
> in parallel as a single operation can enable a significant performance
> improvement.
> 
> 
> Background
> ==========
> 
> There are a number of byte-wise operations which are present and common
> across many access network data-plane pipelines, such as Cipher,
> Authentication, CRC, Bit-Interleaved-Parity (BIP), other checksums etc.
> Some prototyping has been done at Intel in relation to the 01.org
> access-network-dataplanes project to prove that a significant performance
> improvement is possible when such byte-wise operations are combined into a
> single pass of packet data processing. This performance boost has been
> prototyped for both XGS-PON MAC data-plane and DOCSIS MAC data-plane
> pipelines.
> 
> The prototypes used some protocol-specific modifications to the DPDK
> cryptodev library. In order to make this performance improvement consumable
> by network access equipment vendors, a more extensible and correct solution
> was required.
> 
> Hence, the introduction of a multi-function interface, initially for use by
> raw devices. In this patchset, a new aesni_mb raw device has been created
> which uses this interface.
> 
> NOTE: In a future DPDK release (currently targetting DPDK v20.08), a qat
> raw device will also be added. As multiple raw devices will share the same
> interface, the approach taken was to create a common interface
> (i.e. multi-function) which can be used by these devices. This both cuts
> down on code duplication across the devices and allows an application
> access multiple devices using the same interface.
> 
> 
> Use Cases
> =========
> 
> The primary use cases for the multi-function interface and raw PMDs have
> already been mentioned. These are:
> 
> - DOCSIS MAC: Crypto-CRC
> 	- Order:
> 		- Downstream: CRC, Encrypt
> 		- Upstream: Decrypt, CRC
> 	- Specifications:
> 		- Crypto: 128-bit AES-CFB encryption variant for DOCSIS as
> 		  described in section 11.1 of DOCSIS 3.1 Security
> 		  Specification
> 		  (https://apps.cablelabs.com/specification/CM-SP-SECv3.1)
> 		- CRC: Ethernet 32-bit CRC as defined in
> 		  Ethernet/[ISO/IEC 8802-3]
> 
> - XGS-PON MAC: Crypto-CRC-BIP
> 	- Order:
> 		- Downstream: CRC, Encrypt, BIP
> 		- Upstream: BIP, Decrypt, CRC
> 	- Specifications:
> 		- Crypto: AES-128 [NIST FIPS-197] cipher, used in counter
> 		  mode (AES-CTR), as described in [NIST SP800-38A].
> 		- CRC: Ethernet 32-bit CRC as defined in
> 		  Ethernet/[ISO/IEC 8802-3]
> 		- BIP: 4-byte bit-interleaved even parity (BIP) field
> 		  computed over the entire FS frame, refer to
> 		  ITU-T G.989.3, sections 8.1.1.5 and 8.1.2.3
> 		  (https://www.itu.int/rec/dologin_pub.asp?lang=e&id=
> 		   T-REC-G.989.3-201510-I!!PDF-E)
> 
> Note that support for both these chained operations is already available in
> the Intel IPSec Multi-Buffer library.
> 
> However, it is not limited to these. The following are some of the other
> possible use-cases, which multi-function will allow for:
> 
> - Storage:
> 	- Compression followed by Encryption
> - IPSec over UDP:
> 	- UDP Checksum calculation followed by Encryption
> 
> While DPDK's rte_cryptodev and rte_compressdev allow many cryptographic and
> compression algorithms to be chained together in one operation, there is no
> way to chain these with any error detection or checksum algorithms. And
> there is no way to chain crypto and compression algorithms together. The
> multi-function  interface will allow these chains to be created, and also
> allow any future type of operation to be easily added.

I was thinking if the cryptodev can be used instead but this paragraph already
seems explained it. But again can you please elaborate why rawdev is used?

> 
> 
> Architecture
> ============
> 
> The following diagram shows where the multi-function interface and raw
> devices fit in an overall application architecture.
> 
>   +------------------------------------------------+
>   |                                                |
>   |                  Application                   |
>   |   (e.g. vCMTS (DOCSIS), vOLT (XGS-PON), etc.)  |
>   |                                                |
>   +------------------------------------------------+
>                           |
>   +-----------------------|------------------------+
>   |                       |                  DPDK  |
>   |                       |                        |
>   |             +---------------------+            |
>   |             |                     |            |
>   |             |     rte_rawdev      |            |
>   |             |                     |            |            NOTE:
>   |             +---------------------+        ____|______ 'MULTI-FUNCTION
>   |                    /      \              /     |          INTERFACE'
>   |                   /        \            /      |         is opaque to
>   |                  /          \          /       |          rte_rawdev
>   |       +--------------------------------+       |
>   |       |    MULTI-FUNCTION INTERFACE    |       |
>   |       +--------------------------------+       |
>   |       +------------+      +------------+       |
>   |       |   RAWDEV   |      |   RAWDEV   |       |
>   |       |  AESNI-MB  |      |    QAT     |       |
>   |       |    PMD     |      |    PMD     |       |
>   |       +------------+      +------------+       |              NOTE:
>   |              |                  |     \________|_____  'RAWDEV QAT PMD'
>   +--------------|------------------|--------------+       will be added in
>                  |                  |                         next release
>           +------------+      +------------+
>           |  AESNI-MB  |      |   QAT HW   |
>           |   SW LIB   |      |            |
>           +------------+      +------------+
> 
> David Coyle (4):
>   raw/common: add multi-function interface
>   raw/aesni_mb: add aesni_mb raw device
>   test/rawdev: add aesni_mb raw device tests
>   app/crypto-perf: add support for multi-function processing
> 

<...>
  

Hi Ferruh, see below

> >
> > While DPDK's rte_cryptodev and rte_compressdev allow many
> > cryptographic and compression algorithms to be chained together in one
> > operation, there is no way to chain these with any error detection or
> > checksum algorithms. And there is no way to chain crypto and
> > compression algorithms together. The multi-function  interface will
> > allow these chains to be created, and also allow any future type of
> operation to be easily added.
> 
> I was thinking if the cryptodev can be used instead but this paragraph already
> seems explained it. But again can you please elaborate why rawdev is used?

[DC] There are a number of reasons the rawdev approach was ultimately chosen:

1) As the paragraph above explains, our primary use-case was to chain a crypto operation with error detection algorithms such as CRC or BIP as this could leverage optimized multi-function implementations such as in the IPSec Multi-Buffer library and have a significant impact on performance of network access dataplane processing such as for vCMTS (DOCSIS MAC).
However such error detection algorithms are not Crypto functions so some early advice we took was that it would not be suitable to add these to cryptodev.
Also, with a view to the future, the multi-function rawdev approach allows crypto operations to be chained with compression operations.
Again, neither cryptodev or compressdev allows this type chaining.

2) An earlier version of multi-function suggested adding a new library called rte_accelerator, as described here http://mails.dpdk.org/archives/dev/2020-February/157045.html
We received some comments on the dev mailing list that we should not add yet another acceleration library to DPDK.
And we also subsequently felt that the rawdev approach is better - that rationale is described below.

rte_accelerator was also built on top of crypto and compress devices which already existed e.g. drivers/crypto/aesni_mb, drivers/crypto/qat and drivers/compress/qat .
We subsequently realized that this was somewhat confusing when performing multi-function type operations. For example, for combined Crypto-Compression operations in the future, it would use either an existing crypto or compress device, but neither really made sense when the operations are combined.
What was needed was a raw device which allowed an application to configure any type of device and it's queue pairs and send any type of operation to that device.

For both of these reasons, we decided to go down the rawdev route, with a multi-function interface which can be used by several raw device drivers.

3) rawdev is the ideal place to try out a new approach like this to accessing devices.
Adding it here allows potential consumers of this such as VNF solution providers to study and try out this approach, and take advantage of the multi-function operations already supported in the IPSec Multi-Buffer library such as Crypto-CRC and Crypto-CRC-BIP, all without DPDK committing to a new library upfront.
We would hope that the multi-function rawdev approach will mature over time (through feedback from customers, new use-cases arising etc.), at which point it could be potentially be moved into the main DPDK library set.

> 
> >
> >
> >
> 
> <...>
  

Hi David, Ferruh,

> -----Original Message-----
> From: Coyle, David <david.coyle@intel.com>
> Sent: Tuesday, April 7, 2020 12:28 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Ryan, Brendan <brendan.ryan@intel.com>;
> shreyansh.jain@nxp.com; hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/4] introduce multi-function processing support
> 
> Hi Ferruh, see below
> 
> > >
> > > While DPDK's rte_cryptodev and rte_compressdev allow many
> > > cryptographic and compression algorithms to be chained together in one
> > > operation, there is no way to chain these with any error detection or
> > > checksum algorithms. And there is no way to chain crypto and
> > > compression algorithms together. The multi-function  interface will
> > > allow these chains to be created, and also allow any future type of
> > operation to be easily added.
> >
> > I was thinking if the cryptodev can be used instead but this paragraph already
> > seems explained it. But again can you please elaborate why rawdev is used?
> 
> [DC] There are a number of reasons the rawdev approach was ultimately chosen:
> 
> 1) As the paragraph above explains, our primary use-case was to chain a crypto operation with error
> detection algorithms such as CRC or BIP as this could leverage optimized multi-function
> implementations such as in the IPSec Multi-Buffer library and have a significant impact on
> performance of network access dataplane processing such as for vCMTS (DOCSIS MAC).
> However such error detection algorithms are not Crypto functions so some early advice we took was
> that it would not be suitable to add these to cryptodev.
> Also, with a view to the future, the multi-function rawdev approach allows crypto operations to be
> chained with compression operations.
> Again, neither cryptodev or compressdev allows this type chaining.
> 
> 2) An earlier version of multi-function suggested adding a new library called rte_accelerator, as
> described here http://mails.dpdk.org/archives/dev/2020-February/157045.html
> We received some comments on the dev mailing list that we should not add yet another acceleration
> library to DPDK.
> And we also subsequently felt that the rawdev approach is better - that rationale is described below.
> 
> rte_accelerator was also built on top of crypto and compress devices which already existed e.g.
> drivers/crypto/aesni_mb, drivers/crypto/qat and drivers/compress/qat .
> We subsequently realized that this was somewhat confusing when performing multi-function type
> operations. For example, for combined Crypto-Compression operations in the future, it would use
> either an existing crypto or compress device, but neither really made sense when the operations are
> combined.
> What was needed was a raw device which allowed an application to configure any type of device and
> it's queue pairs and send any type of operation to that device.
> 
> For both of these reasons, we decided to go down the rawdev route, with a multi-function interface
> which can be used by several raw device drivers.
> 
> 3) rawdev is the ideal place to try out a new approach like this to accessing devices.
> Adding it here allows potential consumers of this such as VNF solution providers to study and try out
> this approach, and take advantage of the multi-function operations already supported in the IPSec
> Multi-Buffer library such as Crypto-CRC and Crypto-CRC-BIP, all without DPDK committing to a new
> library upfront.
> We would hope that the multi-function rawdev approach will mature over time (through feedback
> from customers, new use-cases arising etc.), at which point it could be potentially be moved into the
> main DPDK library set.
> 
[Fiona] agree with above, in particular item (2). Just to expand a bit more on this: To do a crypto+compression op
one would only send one op to one device. That device could have been either a crypto device which also
implemented multi-fn, by adding compression, crc, etc to its capabilities OR a compression device which added crypto, crc, bip capabilities.
Both were confusing and both raised questions about whether one could still do "normal" ops on the device, e.g. whether
a normal crypto op could be interleaved on same qp as a multi-fn op. And how the capabilities would reflect what the device could do.
It seems better to me to have a multifn device, which does explicitly just multifn ops.

Building this on top of rawdev is a good fit in my opinion, for the following reasons:
 * avoids duplication of device APIs, as rawdev configure, start, stop, qp_setup, etc are all there already, also a nice set of stats APIs
 * no impact or dependency added to rawdev lib
 * avoids breakages on cryptodev or compressdev APIs
 * avoids code duplication where functionality is already in a lib, e.g. re-uses cryptodev and compressdev headers. This adds a dependency, but I think that's ok as multi-function inherently depends on these functions.
 * allows easy extension to add new functionality not currently available in any lib (CRC and BIP)
 * allows evolution - range of useful chains which may emerge is not yet clear.

I do have some concerns, but these are resolvable in my opinion.
    (A)    as there's no rawdev capability APIs and capabilities are essentially opaque to the rawdev API, the application uses explicit device naming to create or find a device that it knows will fulfil the multifunction APIs. I can see how this works for rawdevs which expect to have only one PMD that will fulfil the service, however I'd expect multi-fn to have at least 2 driver types, probably more eventually. To be extensible I'd suggest a naming convention for a class of devices. E.g. all devices and drivers that implement multi-fn should create a rawdev named mfn_xxx, e.g. mfn_aesni_mb, mfn_qat. The "mfn_" string should be defined in the mfn hdr. This would allow creation of apis like rte_multi_fn_count() which could find rawdevs which implement mfn_ without hardcoding specific driver names.
    (B)    version control of the multi-function APIs. Putting the multifn API into the drivers/raw/common directory gives a lot of freedom while it's experimental. But can it benefit from API/ABI breakage infrastructure once the experimental tag is removed? Is there any reason not to move the common files to a lib/librte_multi_fn API?
    (C)    xstat name strings should be moved from aesni_mb PMD to common and maybe use same naming convention, so appl can query same stats from any device, e.g. "mfn_successful_enqueues" could be implemented by all PMDs. If PMDs want to add driver-specific stats they can add their own without the mfn_, instead create their own unique stat name.
    (D)    The unit test code is not extensible - again probably as based on previous rawdevs where there's only 1 implementation. For mfn I'd suggest replacing test_rawdev_selftest_aesni_mb() with a test_rawdev_selftest_multi_function(), which finds and/or creates all the raw PMDs implementing the mfn API and runs a test on each. And move the test files from the drivers/raw/aesni_mb dir to app/test and make generic so can run against any device named mfn_xxx
    (E)    the main reason to piggyback onto crypto_perf_tool is to get the benefit of parsing and of all the crypto setup.  However this code has been inflated a lot, in part due to name diffs like rte_cryptodev_enqueue_burst() vs rte_multi_fn_enqueue_burst(). Maybe could be a lot slimmer with macros like ENQUEUE_BURST(dev, qp, void *op, burst_size) ? would mean a compile time decision to do either multifn OR cryptodev API calls, but I think that may work and simplify it.
    (F)    ok, this is a bit pedantic, (sorry David!) but should the aesni_mb rawdev be renamed aesni_mb_mfn throughout (files, fns, dev and driver name). I mean it's implementing the mfn type of rawdev. I'm thinking ahead to QAT - it can implement a sym device, an asym device, a compression device and in future a multi-fn device. I'd propose to name it qat_multifn in case there'll be some other kind of rawdev device it could also implement in future. So the name qat_raw wouldn't be so helpful. (we made that mistake with qat_crypto, which should probably have been qat_sym_crypto - in my opinion more specific names are better)

I have a few minor comment- I'll reply on specific patches.
  

On 4/7/2020 12:27 PM, Coyle, David wrote:
> Hi Ferruh, see below
> 
>>>
>>> While DPDK's rte_cryptodev and rte_compressdev allow many
>>> cryptographic and compression algorithms to be chained together in one
>>> operation, there is no way to chain these with any error detection or
>>> checksum algorithms. And there is no way to chain crypto and
>>> compression algorithms together. The multi-function  interface will
>>> allow these chains to be created, and also allow any future type of
>> operation to be easily added.
>>
>> I was thinking if the cryptodev can be used instead but this paragraph already
>> seems explained it. But again can you please elaborate why rawdev is used?
> 
> [DC] There are a number of reasons the rawdev approach was ultimately chosen:
> 
> 1) As the paragraph above explains, our primary use-case was to chain a crypto operation with error detection algorithms such as CRC or BIP as this could leverage optimized multi-function implementations such as in the IPSec Multi-Buffer library and have a significant impact on performance of network access dataplane processing such as for vCMTS (DOCSIS MAC).
> However such error detection algorithms are not Crypto functions so some early advice we took was that it would not be suitable to add these to cryptodev.
> Also, with a view to the future, the multi-function rawdev approach allows crypto operations to be chained with compression operations.
> Again, neither cryptodev or compressdev allows this type chaining.
> 
> 2) An earlier version of multi-function suggested adding a new library called rte_accelerator, as described here http://mails.dpdk.org/archives/dev/2020-February/157045.html
> We received some comments on the dev mailing list that we should not add yet another acceleration library to DPDK.
> And we also subsequently felt that the rawdev approach is better - that rationale is described below.
> 
> rte_accelerator was also built on top of crypto and compress devices which already existed e.g. drivers/crypto/aesni_mb, drivers/crypto/qat and drivers/compress/qat .
> We subsequently realized that this was somewhat confusing when performing multi-function type operations. For example, for combined Crypto-Compression operations in the future, it would use either an existing crypto or compress device, but neither really made sense when the operations are combined.
> What was needed was a raw device which allowed an application to configure any type of device and it's queue pairs and send any type of operation to that device.
> 
> For both of these reasons, we decided to go down the rawdev route, with a multi-function interface which can be used by several raw device drivers.
> 
> 3) rawdev is the ideal place to try out a new approach like this to accessing devices.
> Adding it here allows potential consumers of this such as VNF solution providers to study and try out this approach, and take advantage of the multi-function operations already supported in the IPSec Multi-Buffer library such as Crypto-CRC and Crypto-CRC-BIP, all without DPDK committing to a new library upfront.
> We would hope that the multi-function rawdev approach will mature over time (through feedback from customers, new use-cases arising etc.), at which point it could be potentially be moved into the main DPDK library set.
> 

Sounds good to me, thank you for detailed explanation.
  

Thanks for the detailed review Fiona.

Based on your feedback, we will reduce the scope of our plans for multi-function processing support in DPDK.

We will focus on implementing a rawdev-based AESNI-MB PMD for Crypto-CRC and Crypto-CRC-BIP processing and we will add QAT Crypto-CRC support in a later release.
This functionality is specific to accelerated dataplane processing for DOCSIS and PON MAC workloads.

We also note that there hasn't been much community engagement in the broader scope, so these simpler rawdev PMDs should be sufficient.
If the DPDK community is interested in expanding this concept later, then this can be explored, but it would not seem necessary for now.

We will also remove crypto-perf-tester updates to test rawdev multi-function processing as this would seem like too much code churn on that test tool.


> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Tuesday, April 7, 2020 7:06 PM
> 
> Hi David, Ferruh,
> 
> > -----Original Message-----
> > From: Coyle, David <david.coyle@intel.com>
> > Sent: Tuesday, April 7, 2020 12:28 PM
> > To: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> >
> > Hi Ferruh, see below
> >
> > > >
> > > > While DPDK's rte_cryptodev and rte_compressdev allow many
> > > > cryptographic and compression algorithms to be chained together in
> > > > one operation, there is no way to chain these with any error
> > > > detection or checksum algorithms. And there is no way to chain
> > > > crypto and compression algorithms together. The multi-function
> > > > interface will allow these chains to be created, and also allow
> > > > any future type of
> > > operation to be easily added.
> > >
> > > I was thinking if the cryptodev can be used instead but this
> > > paragraph already seems explained it. But again can you please elaborate
> why rawdev is used?
> >
> > [DC] There are a number of reasons the rawdev approach was ultimately
> chosen:
> >
> > 1) As the paragraph above explains, our primary use-case was to chain
> > a crypto operation with error detection algorithms such as CRC or BIP
> > as this could leverage optimized multi-function implementations such
> > as in the IPSec Multi-Buffer library and have a significant impact on
> performance of network access dataplane processing such as for vCMTS
> (DOCSIS MAC).
> > However such error detection algorithms are not Crypto functions so
> > some early advice we took was that it would not be suitable to add these to
> cryptodev.
> > Also, with a view to the future, the multi-function rawdev approach
> > allows crypto operations to be chained with compression operations.
> > Again, neither cryptodev or compressdev allows this type chaining.
> >
> > 2) An earlier version of multi-function suggested adding a new library
> > called rte_accelerator, as described here
> > http://mails.dpdk.org/archives/dev/2020-February/157045.html
> > We received some comments on the dev mailing list that we should not
> > add yet another acceleration library to DPDK.
> > And we also subsequently felt that the rawdev approach is better - that
> rationale is described below.
> >
> > rte_accelerator was also built on top of crypto and compress devices which
> already existed e.g.
> > drivers/crypto/aesni_mb, drivers/crypto/qat and drivers/compress/qat .
> > We subsequently realized that this was somewhat confusing when
> > performing multi-function type operations. For example, for combined
> > Crypto-Compression operations in the future, it would use either an
> > existing crypto or compress device, but neither really made sense when
> the operations are combined.
> > What was needed was a raw device which allowed an application to
> > configure any type of device and it's queue pairs and send any type of
> operation to that device.
> >
> > For both of these reasons, we decided to go down the rawdev route,
> > with a multi-function interface which can be used by several raw device
> drivers.
> >
> > 3) rawdev is the ideal place to try out a new approach like this to accessing
> devices.
> > Adding it here allows potential consumers of this such as VNF solution
> > providers to study and try out this approach, and take advantage of
> > the multi-function operations already supported in the IPSec
> > Multi-Buffer library such as Crypto-CRC and Crypto-CRC-BIP, all without
> DPDK committing to a new library upfront.
> > We would hope that the multi-function rawdev approach will mature over
> > time (through feedback from customers, new use-cases arising etc.), at
> > which point it could be potentially be moved into the main DPDK library set.
> >
> [Fiona] agree with above, in particular item (2). Just to expand a bit more on
> this: To do a crypto+compression op one would only send one op to one
> device. That device could have been either a crypto device which also
> implemented multi-fn, by adding compression, crc, etc to its capabilities OR a
> compression device which added crypto, crc, bip capabilities.
> Both were confusing and both raised questions about whether one could still
> do "normal" ops on the device, e.g. whether a normal crypto op could be
> interleaved on same qp as a multi-fn op. And how the capabilities would
> reflect what the device could do.
> It seems better to me to have a multifn device, which does explicitly just
> multifn ops.
> 
> Building this on top of rawdev is a good fit in my opinion, for the following
> reasons:
>  * avoids duplication of device APIs, as rawdev configure, start, stop,
> qp_setup, etc are all there already, also a nice set of stats APIs
>  * no impact or dependency added to rawdev lib
>  * avoids breakages on cryptodev or compressdev APIs
>  * avoids code duplication where functionality is already in a lib, e.g. re-uses
> cryptodev and compressdev headers. This adds a dependency, but I think
> that's ok as multi-function inherently depends on these functions.
>  * allows easy extension to add new functionality not currently available in
> any lib (CRC and BIP)
>  * allows evolution - range of useful chains which may emerge is not yet
> clear.
> 
> I do have some concerns, but these are resolvable in my opinion.
>     (A)    as there's no rawdev capability APIs and capabilities are essentially
> opaque to the rawdev API, the application uses explicit device naming to
> create or find a device that it knows will fulfil the multifunction APIs. I can see
> how this works for rawdevs which expect to have only one PMD that will
> fulfil the service, however I'd expect multi-fn to have at least 2 driver types,
> probably more eventually. To be extensible I'd suggest a naming convention
> for a class of devices. E.g. all devices and drivers that implement multi-fn
> should create a rawdev named mfn_xxx, e.g. mfn_aesni_mb, mfn_qat. The
> "mfn_" string should be defined in the mfn hdr. This would allow creation of
> apis like rte_multi_fn_count() which could find rawdevs which implement
> mfn_ without hardcoding specific driver names.
>     (B)    version control of the multi-function APIs. Putting the multifn API into
> the drivers/raw/common directory gives a lot of freedom while it's
> experimental. But can it benefit from API/ABI breakage infrastructure once
> the experimental tag is removed? Is there any reason not to move the
> common files to a lib/librte_multi_fn API?
>     (C)    xstat name strings should be moved from aesni_mb PMD to common
> and maybe use same naming convention, so appl can query same stats from
> any device, e.g. "mfn_successful_enqueues" could be implemented by all
> PMDs. If PMDs want to add driver-specific stats they can add their own
> without the mfn_, instead create their own unique stat name.
>     (D)    The unit test code is not extensible - again probably as based on
> previous rawdevs where there's only 1 implementation. For mfn I'd suggest
> replacing test_rawdev_selftest_aesni_mb() with a
> test_rawdev_selftest_multi_function(), which finds and/or creates all the
> raw PMDs implementing the mfn API and runs a test on each. And move the
> test files from the drivers/raw/aesni_mb dir to app/test and make generic so
> can run against any device named mfn_xxx
>     (E)    the main reason to piggyback onto crypto_perf_tool is to get the
> benefit of parsing and of all the crypto setup.  However this code has been
> inflated a lot, in part due to name diffs like rte_cryptodev_enqueue_burst()
> vs rte_multi_fn_enqueue_burst(). Maybe could be a lot slimmer with
> macros like ENQUEUE_BURST(dev, qp, void *op, burst_size) ? would mean a
> compile time decision to do either multifn OR cryptodev API calls, but I think
> that may work and simplify it.
>     (F)    ok, this is a bit pedantic, (sorry David!) but should the aesni_mb
> rawdev be renamed aesni_mb_mfn throughout (files, fns, dev and driver
> name). I mean it's implementing the mfn type of rawdev. I'm thinking ahead
> to QAT - it can implement a sym device, an asym device, a compression
> device and in future a multi-fn device. I'd propose to name it qat_multifn in
> case there'll be some other kind of rawdev device it could also implement in
> future. So the name qat_raw wouldn't be so helpful. (we made that mistake
> with qat_crypto, which should probably have been qat_sym_crypto - in my
> opinion more specific names are better)
> 
> I have a few minor comment- I'll reply on specific patches.
  

Hi David,

Answer inline below

> -----Original Message-----
> From: Coyle, David <david.coyle@intel.com>
> Sent: Thursday, April 9, 2020 10:26 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Ryan, Brendan <brendan.ryan@intel.com>;
> shreyansh.jain@nxp.com; hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>;
> O'loingsigh, Mairtin <mairtin.oloingsigh@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/4] introduce multi-function processing support
> 
> Thanks for the detailed review Fiona.
> 
> Based on your feedback, we will reduce the scope of our plans for multi-function processing support
> in DPDK.
> 
> We will focus on implementing a rawdev-based AESNI-MB PMD for Crypto-CRC and Crypto-CRC-BIP
> processing and we will add QAT Crypto-CRC support in a later release.
> This functionality is specific to accelerated dataplane processing for DOCSIS and PON MAC workloads.
> 
> We also note that there hasn't been much community engagement in the broader scope, so these
> simpler rawdev PMDs should be sufficient.
> If the DPDK community is interested in expanding this concept later, then this can be explored, but it
> would not seem necessary for now.
> 
> We will also remove crypto-perf-tester updates to test rawdev multi-function processing as this would
> seem like too much code churn on that test tool.

[Fiona] That sounds like a good idea. In that case my comments B, D and E are not relevant as assuming a broader scope.
Comments A, C and F can still be considered, but are just suggestions, not blockers to this being 
applied in 20.05, they could easily be done in a later release.

///snip///

> > I do have some concerns, but these are resolvable in my opinion.
> >     (A)    as there's no rawdev capability APIs and capabilities are essentially
> > opaque to the rawdev API, the application uses explicit device naming to
> > create or find a device that it knows will fulfil the multifunction APIs. I can see
> > how this works for rawdevs which expect to have only one PMD that will
> > fulfil the service, however I'd expect multi-fn to have at least 2 driver types,
> > probably more eventually. To be extensible I'd suggest a naming convention
> > for a class of devices. E.g. all devices and drivers that implement multi-fn
> > should create a rawdev named mfn_xxx, e.g. mfn_aesni_mb, mfn_qat. The
> > "mfn_" string should be defined in the mfn hdr. This would allow creation of
> > apis like rte_multi_fn_count() which could find rawdevs which implement
> > mfn_ without hardcoding specific driver names.
> >     (B)    version control of the multi-function APIs. Putting the multifn API into
> > the drivers/raw/common directory gives a lot of freedom while it's
> > experimental. But can it benefit from API/ABI breakage infrastructure once
> > the experimental tag is removed? Is there any reason not to move the
> > common files to a lib/librte_multi_fn API?
> >     (C)    xstat name strings should be moved from aesni_mb PMD to common
> > and maybe use same naming convention, so appl can query same stats from
> > any device, e.g. "mfn_successful_enqueues" could be implemented by all
> > PMDs. If PMDs want to add driver-specific stats they can add their own
> > without the mfn_, instead create their own unique stat name.
> >     (D)    The unit test code is not extensible - again probably as based on
> > previous rawdevs where there's only 1 implementation. For mfn I'd suggest
> > replacing test_rawdev_selftest_aesni_mb() with a
> > test_rawdev_selftest_multi_function(), which finds and/or creates all the
> > raw PMDs implementing the mfn API and runs a test on each. And move the
> > test files from the drivers/raw/aesni_mb dir to app/test and make generic so
> > can run against any device named mfn_xxx
> >     (E)    the main reason to piggyback onto crypto_perf_tool is to get the
> > benefit of parsing and of all the crypto setup.  However this code has been
> > inflated a lot, in part due to name diffs like rte_cryptodev_enqueue_burst()
> > vs rte_multi_fn_enqueue_burst(). Maybe could be a lot slimmer with
> > macros like ENQUEUE_BURST(dev, qp, void *op, burst_size) ? would mean a
> > compile time decision to do either multifn OR cryptodev API calls, but I think
> > that may work and simplify it.
> >     (F)    ok, this is a bit pedantic, (sorry David!) but should the aesni_mb
> > rawdev be renamed aesni_mb_mfn throughout (files, fns, dev and driver
> > name). I mean it's implementing the mfn type of rawdev. I'm thinking ahead
> > to QAT - it can implement a sym device, an asym device, a compression
> > device and in future a multi-fn device. I'd propose to name it qat_multifn in
> > case there'll be some other kind of rawdev device it could also implement in
> > future. So the name qat_raw wouldn't be so helpful. (we made that mistake
> > with qat_crypto, which should probably have been qat_sym_crypto - in my
> > opinion more specific names are better)
> >
> > I have a few minor comment- I'll reply on specific patches.
  

Hi Fiona, see below

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Thursday, April 9, 2020 10:37 AM
> 
> Hi David,
> 
> Answer inline below
> 
> > -----Original Message-----
> > From: Coyle, David <david.coyle@intel.com>
> > Sent: Thursday, April 9, 2020 10:26 AM
> >
> > Thanks for the detailed review Fiona.
> >
> > Based on your feedback, we will reduce the scope of our plans for
> > multi-function processing support in DPDK.
> >
> > We will focus on implementing a rawdev-based AESNI-MB PMD for
> > Crypto-CRC and Crypto-CRC-BIP processing and we will add QAT Crypto-
> CRC support in a later release.
> > This functionality is specific to accelerated dataplane processing for DOCSIS
> and PON MAC workloads.
> >
> > We also note that there hasn't been much community engagement in the
> > broader scope, so these simpler rawdev PMDs should be sufficient.
> > If the DPDK community is interested in expanding this concept later,
> > then this can be explored, but it would not seem necessary for now.
> >
> > We will also remove crypto-perf-tester updates to test rawdev
> > multi-function processing as this would seem like too much code churn on
> that test tool.
> 
> [Fiona] That sounds like a good idea. In that case my comments B, D and E are
> not relevant as assuming a broader scope.
> Comments A, C and F can still be considered, but are just suggestions, not
> blockers to this being applied in 20.05, they could easily be done in a later
> release.

[DC] For 20.05, I plan to address A, C and F from below.
We will look to address D in a later release when we add QAT multi-function PMD to see if unit test extensibility can be improved.
And B and E are now no longer applicable due to reduced scope.

> 
> ///snip///
> 
> > > I do have some concerns, but these are resolvable in my opinion.
> > >     (A)    as there's no rawdev capability APIs and capabilities are essentially
> > > opaque to the rawdev API, the application uses explicit device
> > > naming to create or find a device that it knows will fulfil the
> > > multifunction APIs. I can see how this works for rawdevs which
> > > expect to have only one PMD that will fulfil the service, however
> > > I'd expect multi-fn to have at least 2 driver types, probably more
> > > eventually. To be extensible I'd suggest a naming convention for a
> > > class of devices. E.g. all devices and drivers that implement
> > > multi-fn should create a rawdev named mfn_xxx, e.g. mfn_aesni_mb,
> > > mfn_qat. The "mfn_" string should be defined in the mfn hdr. This
> > > would allow creation of apis like rte_multi_fn_count() which could find
> rawdevs which implement mfn_ without hardcoding specific driver names.

[DC] The AESNI-MB rawdev will be renamed to rawdev_mfn_aesni_mb.
Keeping "rawdev_" as first prefix keeps this consistent with other rawdevs
Adding "mfn_" allows rawdevs implementing multi-function be found as you suggested

> > >     (B)    version control of the multi-function APIs. Putting the multifn API
> into
> > > the drivers/raw/common directory gives a lot of freedom while it's
> > > experimental. But can it benefit from API/ABI breakage
> > > infrastructure once the experimental tag is removed? Is there any
> > > reason not to move the common files to a lib/librte_multi_fn API?

[DC] As stated above, this is no longer applicable due to reduced scope

> > >     (C)    xstat name strings should be moved from aesni_mb PMD to
> common
> > > and maybe use same naming convention, so appl can query same stats
> > > from any device, e.g. "mfn_successful_enqueues" could be
> implemented
> > > by all PMDs. If PMDs want to add driver-specific stats they can add
> > > their own without the mfn_, instead create their own unique stat name.

[DC] This is a good suggestion as these same stats will also be needed by the QAT PMD.
I will make this change.

> > >     (D)    The unit test code is not extensible - again probably as based on
> > > previous rawdevs where there's only 1 implementation. For mfn I'd
> > > suggest replacing test_rawdev_selftest_aesni_mb() with a
> > > test_rawdev_selftest_multi_function(), which finds and/or creates
> > > all the raw PMDs implementing the mfn API and runs a test on each.
> > > And move the test files from the drivers/raw/aesni_mb dir to
> > > app/test and make generic so can run against any device named mfn_xxx

[DC] As stated above we will look at making the unit tests more extensible when we add the QAT PMD
For now, keeping the tests in the drivers/raw/aesni_mb_mfn directory is consistent with existing rawdevs

> > >     (E)    the main reason to piggyback onto crypto_perf_tool is to get the
> > > benefit of parsing and of all the crypto setup.  However this code
> > > has been inflated a lot, in part due to name diffs like
> > > rte_cryptodev_enqueue_burst() vs rte_multi_fn_enqueue_burst().
> Maybe
> > > could be a lot slimmer with macros like ENQUEUE_BURST(dev, qp, void
> > > *op, burst_size) ? would mean a compile time decision to do either
> > > multifn OR cryptodev API calls, but I think that may work and simplify it.

[DC] We will remove support for multi-function from the crypto-perf too due to amount of code churn it required

> > >     (F)    ok, this is a bit pedantic, (sorry David!) but should the aesni_mb
> > > rawdev be renamed aesni_mb_mfn throughout (files, fns, dev and
> > > driver name). I mean it's implementing the mfn type of rawdev. I'm
> > > thinking ahead to QAT - it can implement a sym device, an asym
> > > device, a compression device and in future a multi-fn device. I'd
> > > propose to name it qat_multifn in case there'll be some other kind
> > > of rawdev device it could also implement in future. So the name
> > > qat_raw wouldn't be so helpful. (we made that mistake with
> > > qat_crypto, which should probably have been qat_sym_crypto - in my
> > > opinion more specific names are better)

[DC] To keep naming of files, directories, functions etc. consistent with the driver name which now includes the mfn tag,
the files, directories, functions etc. will be renamed to also include 'mfn' e.g. aesni_mb_mfn_rawdev.c/h, aesni_mb_mfn_probe()
A similar naming convention will be used for the QAT driver when we add that.

> > >
> > > I have a few minor comment- I'll reply on specific patches.
  

Thanks David,
Sounds good.
I look forward to the v3.
Fiona

> -----Original Message-----
> From: Coyle, David <david.coyle@intel.com>
> Sent: Thursday, April 9, 2020 12:56 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Ryan, Brendan <brendan.ryan@intel.com>;
> shreyansh.jain@nxp.com; hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>;
> O'loingsigh, Mairtin <mairtin.oloingsigh@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 0/4] introduce multi-function processing support
> 
> Hi Fiona, see below
> 
> > -----Original Message-----
> > From: Trahe, Fiona <fiona.trahe@intel.com>
> > Sent: Thursday, April 9, 2020 10:37 AM
> >
> > Hi David,
> >
> > Answer inline below
> >
> > > -----Original Message-----
> > > From: Coyle, David <david.coyle@intel.com>
> > > Sent: Thursday, April 9, 2020 10:26 AM
> > >
> > > Thanks for the detailed review Fiona.
> > >
> > > Based on your feedback, we will reduce the scope of our plans for
> > > multi-function processing support in DPDK.
> > >
> > > We will focus on implementing a rawdev-based AESNI-MB PMD for
> > > Crypto-CRC and Crypto-CRC-BIP processing and we will add QAT Crypto-
> > CRC support in a later release.
> > > This functionality is specific to accelerated dataplane processing for DOCSIS
> > and PON MAC workloads.
> > >
> > > We also note that there hasn't been much community engagement in the
> > > broader scope, so these simpler rawdev PMDs should be sufficient.
> > > If the DPDK community is interested in expanding this concept later,
> > > then this can be explored, but it would not seem necessary for now.
> > >
> > > We will also remove crypto-perf-tester updates to test rawdev
> > > multi-function processing as this would seem like too much code churn on
> > that test tool.
> >
> > [Fiona] That sounds like a good idea. In that case my comments B, D and E are
> > not relevant as assuming a broader scope.
> > Comments A, C and F can still be considered, but are just suggestions, not
> > blockers to this being applied in 20.05, they could easily be done in a later
> > release.
> 
> [DC] For 20.05, I plan to address A, C and F from below.
> We will look to address D in a later release when we add QAT multi-function PMD to see if unit test
> extensibility can be improved.
> And B and E are now no longer applicable due to reduced scope.
> 
> >
> > ///snip///
> >
> > > > I do have some concerns, but these are resolvable in my opinion.
> > > >     (A)    as there's no rawdev capability APIs and capabilities are essentially
> > > > opaque to the rawdev API, the application uses explicit device
> > > > naming to create or find a device that it knows will fulfil the
> > > > multifunction APIs. I can see how this works for rawdevs which
> > > > expect to have only one PMD that will fulfil the service, however
> > > > I'd expect multi-fn to have at least 2 driver types, probably more
> > > > eventually. To be extensible I'd suggest a naming convention for a
> > > > class of devices. E.g. all devices and drivers that implement
> > > > multi-fn should create a rawdev named mfn_xxx, e.g. mfn_aesni_mb,
> > > > mfn_qat. The "mfn_" string should be defined in the mfn hdr. This
> > > > would allow creation of apis like rte_multi_fn_count() which could find
> > rawdevs which implement mfn_ without hardcoding specific driver names.
> 
> [DC] The AESNI-MB rawdev will be renamed to rawdev_mfn_aesni_mb.
> Keeping "rawdev_" as first prefix keeps this consistent with other rawdevs
> Adding "mfn_" allows rawdevs implementing multi-function be found as you suggested
> 
> > > >     (B)    version control of the multi-function APIs. Putting the multifn API
> > into
> > > > the drivers/raw/common directory gives a lot of freedom while it's
> > > > experimental. But can it benefit from API/ABI breakage
> > > > infrastructure once the experimental tag is removed? Is there any
> > > > reason not to move the common files to a lib/librte_multi_fn API?
> 
> [DC] As stated above, this is no longer applicable due to reduced scope
> 
> > > >     (C)    xstat name strings should be moved from aesni_mb PMD to
> > common
> > > > and maybe use same naming convention, so appl can query same stats
> > > > from any device, e.g. "mfn_successful_enqueues" could be
> > implemented
> > > > by all PMDs. If PMDs want to add driver-specific stats they can add
> > > > their own without the mfn_, instead create their own unique stat name.
> 
> [DC] This is a good suggestion as these same stats will also be needed by the QAT PMD.
> I will make this change.
> 
> > > >     (D)    The unit test code is not extensible - again probably as based on
> > > > previous rawdevs where there's only 1 implementation. For mfn I'd
> > > > suggest replacing test_rawdev_selftest_aesni_mb() with a
> > > > test_rawdev_selftest_multi_function(), which finds and/or creates
> > > > all the raw PMDs implementing the mfn API and runs a test on each.
> > > > And move the test files from the drivers/raw/aesni_mb dir to
> > > > app/test and make generic so can run against any device named mfn_xxx
> 
> [DC] As stated above we will look at making the unit tests more extensible when we add the QAT PMD
> For now, keeping the tests in the drivers/raw/aesni_mb_mfn directory is consistent with existing
> rawdevs
> 
> > > >     (E)    the main reason to piggyback onto crypto_perf_tool is to get the
> > > > benefit of parsing and of all the crypto setup.  However this code
> > > > has been inflated a lot, in part due to name diffs like
> > > > rte_cryptodev_enqueue_burst() vs rte_multi_fn_enqueue_burst().
> > Maybe
> > > > could be a lot slimmer with macros like ENQUEUE_BURST(dev, qp, void
> > > > *op, burst_size) ? would mean a compile time decision to do either
> > > > multifn OR cryptodev API calls, but I think that may work and simplify it.
> 
> [DC] We will remove support for multi-function from the crypto-perf too due to amount of code
> churn it required
> 
> > > >     (F)    ok, this is a bit pedantic, (sorry David!) but should the aesni_mb
> > > > rawdev be renamed aesni_mb_mfn throughout (files, fns, dev and
> > > > driver name). I mean it's implementing the mfn type of rawdev. I'm
> > > > thinking ahead to QAT - it can implement a sym device, an asym
> > > > device, a compression device and in future a multi-fn device. I'd
> > > > propose to name it qat_multifn in case there'll be some other kind
> > > > of rawdev device it could also implement in future. So the name
> > > > qat_raw wouldn't be so helpful. (we made that mistake with
> > > > qat_crypto, which should probably have been qat_sym_crypto - in my
> > > > opinion more specific names are better)
> 
> [DC] To keep naming of files, directories, functions etc. consistent with the driver name which now
> includes the mfn tag,
> the files, directories, functions etc. will be renamed to also include 'mfn' e.g.
> aesni_mb_mfn_rawdev.c/h, aesni_mb_mfn_probe()
> A similar naming convention will be used for the QAT driver when we add that.
> 
> > > >
> > > > I have a few minor comment- I'll reply on specific patches.