[v3,0/4] add AESNI-MB rawdev for multi-function processing
mbox series

Message ID 20200410142757.31508-1-david.coyle@intel.com
Headers show
Series
  • add AESNI-MB rawdev for multi-function processing
Related show

Message

David Coyle April 10, 2020, 2:27 p.m. UTC
Introduction
============

This patchset adds a new AESNI-MB Multi-Function raw device PMD for
utilizing multi-function capabilities of the Intel IPSec Multi Buffer
library.

The aim of this rawdev PMD is to provide a way of combining one or more
common packet-processing functions into a single operation, focused on
DOCSIS and GPON MAC workloads. This allows these functions to be performed
in parallel by the Intel IPSec Multi Buffer library. These functions
include cryptography and CRC/BIP calculations. 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 used across many
access network data-plane pipelines, such as Cipher, CRC and
Bit-Interleaved-Parity (BIP). Some prototyping has been done at Intel as
part of 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 DOCSIS MAC data-plane and
GPON MAC data-plane pipelines based on DPDK.

The original prototypes on 01.org used some protocol-specific modifications
to the DPDK cryptodev library. In order to make this performance
optimization consumable for network access equipment vendors, a better
solution was required as CRC and BIP cannot be regarded as cryptographic
algorithms.

Hence, the introduction of an AESNI-MB Multi-Function rawdev PMD. This
PMD uses a new multi-function interface which allows different types of
operations be combined together. Initially, only symmetric crypto and error
detection (i.e. CRC and BIP) operations can be combined.

NOTE: In a future DPDK release, a QAT Multi-Function 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 a DOCSIS or GPON MAC application to access
multiple devices using the same interface.


Use Cases
=========

The primary use cases for the AESNI-MB Multi-Function interface 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]

- GPON 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.


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

The following diagram shows where the AESNI-MB Multi-Function rawdev PMD
fits in an overall application architecture.

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

v2:
* moved unit tests under from test app to under aesni_mb rawdev.
* Added support to crypto-perf tool for multi-function processing.
* fixed checkpatch errors.
* general tidy-up and improvements.

v3:
* removed support from crypto-perf tool for multi-function processing.
* renamed driver to rawdev_mfn_aesni_mb.
* renamed files/directories/functions/etc. of new driver to have
  aesni_mb_mfn_ prefix.
* resolved review comments.
* updated documentation.

David Coyle (4):
  raw/common: add multi-function interface
  raw/aesni_mb_mfn: add aesni_mb_mfn raw device PMD
  test/rawdev: add aesni_mb_mfn raw device tests
  doc: update docs for aesni_mb_mfn raw device PMD

 MAINTAINERS                                   |    7 +
 app/test/test_rawdev.c                        |   18 +
 config/common_base                            |   11 +
 doc/api/doxy-api-index.md                     |    3 +-
 doc/api/doxy-api.conf.in                      |    1 +
 doc/guides/rawdevs/aesni_mb_mfn.rst           |  219 +++
 doc/guides/rawdevs/index.rst                  |    1 +
 doc/guides/rel_notes/release_20_05.rst        |    6 +
 drivers/meson.build                           |    5 +
 drivers/raw/Makefile                          |    3 +
 drivers/raw/aesni_mb_mfn/Makefile             |   50 +
 .../raw/aesni_mb_mfn/aesni_mb_mfn_rawdev.c    | 1508 +++++++++++++++++
 .../raw/aesni_mb_mfn/aesni_mb_mfn_rawdev.h    |  118 ++
 .../aesni_mb_mfn/aesni_mb_mfn_rawdev_test.c   | 1123 ++++++++++++
 .../aesni_mb_mfn_rawdev_test_vectors.h        | 1184 +++++++++++++
 drivers/raw/aesni_mb_mfn/meson.build          |   26 +
 .../rte_rawdev_aesni_mb_mfn_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  |   12 +
 drivers/raw/common/multi_fn/rte_multi_fn.c    |  148 ++
 drivers/raw/common/multi_fn/rte_multi_fn.h    |  438 +++++
 .../raw/common/multi_fn/rte_multi_fn_driver.h |   97 ++
 drivers/raw/meson.build                       |    3 +-
 meson.build                                   |    4 +
 mk/rte.app.mk                                 |    3 +
 28 files changed, 5040 insertions(+), 2 deletions(-)
 create mode 100644 doc/guides/rawdevs/aesni_mb_mfn.rst
 create mode 100644 drivers/raw/aesni_mb_mfn/Makefile
 create mode 100644 drivers/raw/aesni_mb_mfn/aesni_mb_mfn_rawdev.c
 create mode 100644 drivers/raw/aesni_mb_mfn/aesni_mb_mfn_rawdev.h
 create mode 100644 drivers/raw/aesni_mb_mfn/aesni_mb_mfn_rawdev_test.c
 create mode 100644 drivers/raw/aesni_mb_mfn/aesni_mb_mfn_rawdev_test_vectors.h
 create mode 100644 drivers/raw/aesni_mb_mfn/meson.build
 create mode 100644 drivers/raw/aesni_mb_mfn/rte_rawdev_aesni_mb_mfn_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

Comments

Thomas Monjalon April 10, 2020, 10:55 p.m. UTC | #1
Hi,

Adding more people (crypto PMD maintainers) as Cc.

10/04/2020 16:27, David Coyle:
> Introduction
> ============
> 
> This patchset adds a new AESNI-MB Multi-Function raw device PMD for
> utilizing multi-function capabilities of the Intel IPSec Multi Buffer
> library.
> 
> The aim of this rawdev PMD is to provide a way of combining one or more
> common packet-processing functions into a single operation, focused on
> DOCSIS and GPON MAC workloads. This allows these functions to be performed
> in parallel by the Intel IPSec Multi Buffer library. These functions
> include cryptography and CRC/BIP calculations. Performing these functions
> in parallel as a single operation can enable a significant performance
> improvement.

I don't know crypto but I don't think using rawdev for crypto operations
is an API improvement.

Repeating the initial comments from v1 (because got no reply):
"
As a first impression, I feel it is not the right API.
DPDK is based on classes: ethdev, crypto, compress, baseband, regex
I want to understand why your features cannot fit in a class.

I feel we will need a lot of time to discuss the design.
If you don't see any consensus on the design in the mailing list,
you should request an opinion from the Technical Board.

This feature is not a priority for 20.05 release.
By the way, it has not been announced in any roadmap.
"

> Background
> ==========
> 
> There are a number of byte-wise operations which are used across many
> access network data-plane pipelines, such as Cipher, CRC and
> Bit-Interleaved-Parity (BIP). Some prototyping has been done at Intel as
> part of 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 DOCSIS MAC data-plane and
> GPON MAC data-plane pipelines based on DPDK.
> 
> The original prototypes on 01.org used some protocol-specific modifications
> to the DPDK cryptodev library. In order to make this performance
> optimization consumable for network access equipment vendors, a better
> solution was required as CRC and BIP cannot be regarded as cryptographic
> algorithms.

Why not part of crypto?
If not crypto, is it a new API class? Which one?
Please do not say rawdev.


> Hence, the introduction of an AESNI-MB Multi-Function rawdev PMD. This
> PMD uses a new multi-function interface which allows different types of
> operations be combined together. Initially, only symmetric crypto and error
> detection (i.e. CRC and BIP) operations can be combined.
> 
> NOTE: In a future DPDK release, a QAT Multi-Function 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 a DOCSIS or GPON MAC application to access
> multiple devices using the same interface.
> 
> 
> Use Cases
> =========
> 
> The primary use cases for the AESNI-MB Multi-Function interface 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]
> 
> - GPON 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.
> 
> 
> Architecture
> ============
> 
> The following diagram shows where the AESNI-MB Multi-Function rawdev PMD
> fits in an overall application architecture.
> 
>   +------------------------------------------------+
>   |                                                |
>   |                  Application                   |
>   |    (e.g. vCMTS (DOCSIS), vOLT (GPON), etc.)    |
>   |                                                |
>   +------------------------------------------------+
>                           |
>   +-----------------------|------------------------+
>   |                       |                  DPDK  |
>   |                       |                        |
>   |             +---------------------+            |
>   |             |                     |            |
>   |             |     rte_rawdev      |            |
>   |             |                     |            |            NOTE:
>   |             +---------------------+        ____|______ 'MULTI-FUNCTION
>   |                    /      \              /     |          INTERFACE'
>   |                   /        \            /      |         is opaque to
>   |                  /          \          /       |          rte_rawdev
>   |       +--------------------------------+       |
>   |       |    MULTI-FUNCTION INTERFACE    |       |
>   |       +--------------------------------+       |
>   |       +------------+      +------------+       |
>   |       |  AESNI-MB  |      |    QAT     |       |
>   |       |  MULTI-FN  |      |  MULTI-FN  |       |
>   |       |   RAWDEV   |      |   RAWDEV   |       |
>   |       |    PMD     |      |    PMD     |       |
>   |       +------------+      +------------+       |           NOTE:
>   |              |                  |     \________|______ 'QAT MULTI-FN
>   |              |                  |              |         RAWDEV PMD'
>   +--------------|------------------|--------------+      will be added in
>                  |                  |                       later release
>           +------------+      +------------+
>           |  AESNI-MB  |      |   QAT HW   |
>           |   SW LIB   |      |            |
>           +------------+      +------------+
[...]
>  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  |   12 +
>  drivers/raw/common/multi_fn/rte_multi_fn.c    |  148 ++
>  drivers/raw/common/multi_fn/rte_multi_fn.h    |  438 +++++

From the explanations above, I don't understand what is the exact role
of the multi_fn interface. The comments in the patch 1 don't help either.
It just reminds me rte_security, which was, in my opinion, a bad design.
Ferruh Yigit April 14, 2020, 10:21 a.m. UTC | #2
On 4/10/2020 11:55 PM, Thomas Monjalon wrote:
> Hi,
> 
> Adding more people (crypto PMD maintainers) as Cc.
> 
> 10/04/2020 16:27, David Coyle:
>> Introduction
>> ============
>>
>> This patchset adds a new AESNI-MB Multi-Function raw device PMD for
>> utilizing multi-function capabilities of the Intel IPSec Multi Buffer
>> library.
>>
>> The aim of this rawdev PMD is to provide a way of combining one or more
>> common packet-processing functions into a single operation, focused on
>> DOCSIS and GPON MAC workloads. This allows these functions to be performed
>> in parallel by the Intel IPSec Multi Buffer library. These functions
>> include cryptography and CRC/BIP calculations. Performing these functions
>> in parallel as a single operation can enable a significant performance
>> improvement.
> 
> I don't know crypto but I don't think using rawdev for crypto operations
> is an API improvement.
> 
> Repeating the initial comments from v1 (because got no reply):
> "
> As a first impression, I feel it is not the right API.
> DPDK is based on classes: ethdev, crypto, compress, baseband, regex
> I want to understand why your features cannot fit in a class.

Hi Thomas,

I asked similar question, and there is already a detailed answer with some
background of the issue:
http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.namprd11.prod.outlook.com/

> 
> I feel we will need a lot of time to discuss the design.
> If you don't see any consensus on the design in the mailing list,
> you should request an opinion from the Technical Board.
> 
> This feature is not a priority for 20.05 release.
> By the way, it has not been announced in any roadmap.
> "

Is it an issue to not have it in the roadmap?

> 
>> Background
>> ==========
>>
>> There are a number of byte-wise operations which are used across many
>> access network data-plane pipelines, such as Cipher, CRC and
>> Bit-Interleaved-Parity (BIP). Some prototyping has been done at Intel as
>> part of 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 DOCSIS MAC data-plane and
>> GPON MAC data-plane pipelines based on DPDK.
>>
>> The original prototypes on 01.org used some protocol-specific modifications
>> to the DPDK cryptodev library. In order to make this performance
>> optimization consumable for network access equipment vendors, a better
>> solution was required as CRC and BIP cannot be regarded as cryptographic
>> algorithms.
> 
> Why not part of crypto?
> If not crypto, is it a new API class? Which one?
> Please do not say rawdev.
> 
> 
>> Hence, the introduction of an AESNI-MB Multi-Function rawdev PMD. This
>> PMD uses a new multi-function interface which allows different types of
>> operations be combined together. Initially, only symmetric crypto and error
>> detection (i.e. CRC and BIP) operations can be combined.
>>
>> NOTE: In a future DPDK release, a QAT Multi-Function 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 a DOCSIS or GPON MAC application to access
>> multiple devices using the same interface.
>>
>>
>> Use Cases
>> =========
>>
>> The primary use cases for the AESNI-MB Multi-Function interface 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]
>>
>> - GPON 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.
>>
>>
>> Architecture
>> ============
>>
>> The following diagram shows where the AESNI-MB Multi-Function rawdev PMD
>> fits in an overall application architecture.
>>
>>   +------------------------------------------------+
>>   |                                                |
>>   |                  Application                   |
>>   |    (e.g. vCMTS (DOCSIS), vOLT (GPON), etc.)    |
>>   |                                                |
>>   +------------------------------------------------+
>>                           |
>>   +-----------------------|------------------------+
>>   |                       |                  DPDK  |
>>   |                       |                        |
>>   |             +---------------------+            |
>>   |             |                     |            |
>>   |             |     rte_rawdev      |            |
>>   |             |                     |            |            NOTE:
>>   |             +---------------------+        ____|______ 'MULTI-FUNCTION
>>   |                    /      \              /     |          INTERFACE'
>>   |                   /        \            /      |         is opaque to
>>   |                  /          \          /       |          rte_rawdev
>>   |       +--------------------------------+       |
>>   |       |    MULTI-FUNCTION INTERFACE    |       |
>>   |       +--------------------------------+       |
>>   |       +------------+      +------------+       |
>>   |       |  AESNI-MB  |      |    QAT     |       |
>>   |       |  MULTI-FN  |      |  MULTI-FN  |       |
>>   |       |   RAWDEV   |      |   RAWDEV   |       |
>>   |       |    PMD     |      |    PMD     |       |
>>   |       +------------+      +------------+       |           NOTE:
>>   |              |                  |     \________|______ 'QAT MULTI-FN
>>   |              |                  |              |         RAWDEV PMD'
>>   +--------------|------------------|--------------+      will be added in
>>                  |                  |                       later release
>>           +------------+      +------------+
>>           |  AESNI-MB  |      |   QAT HW   |
>>           |   SW LIB   |      |            |
>>           +------------+      +------------+
> [...]
>>  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  |   12 +
>>  drivers/raw/common/multi_fn/rte_multi_fn.c    |  148 ++
>>  drivers/raw/common/multi_fn/rte_multi_fn.h    |  438 +++++
> 
> From the explanations above, I don't understand what is the exact role
> of the multi_fn interface. The comments in the patch 1 don't help either.
> It just reminds me rte_security, which was, in my opinion, a bad design.
> 
>
Thomas Monjalon April 14, 2020, 10:32 a.m. UTC | #3
14/04/2020 12:21, Ferruh Yigit:
> On 4/10/2020 11:55 PM, Thomas Monjalon wrote:
> > Hi,
> > 
> > Adding more people (crypto PMD maintainers) as Cc.
> > 
> > 10/04/2020 16:27, David Coyle:
> >> Introduction
> >> ============
> >>
> >> This patchset adds a new AESNI-MB Multi-Function raw device PMD for
> >> utilizing multi-function capabilities of the Intel IPSec Multi Buffer
> >> library.
> >>
> >> The aim of this rawdev PMD is to provide a way of combining one or more
> >> common packet-processing functions into a single operation, focused on
> >> DOCSIS and GPON MAC workloads. This allows these functions to be performed
> >> in parallel by the Intel IPSec Multi Buffer library. These functions
> >> include cryptography and CRC/BIP calculations. Performing these functions
> >> in parallel as a single operation can enable a significant performance
> >> improvement.
> > 
> > I don't know crypto but I don't think using rawdev for crypto operations
> > is an API improvement.
> > 
> > Repeating the initial comments from v1 (because got no reply):
> > "
> > As a first impression, I feel it is not the right API.
> > DPDK is based on classes: ethdev, crypto, compress, baseband, regex
> > I want to understand why your features cannot fit in a class.
> 
> Hi Thomas,
> 
> I asked similar question, and there is already a detailed answer with some
> background of the issue:

Good to see that you get some reply, Ferruh.

> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.namprd11.prod.outlook.com/

I am not convinced.
I don't like rawdev in general.
Rawdev is good only for hardware support which cannot be generic
like SoC, FPGA management or DMA engine.

Here the intent is to use rawdev because we don't find a good API.
API defeat is a no-go.


> > I feel we will need a lot of time to discuss the design.
> > If you don't see any consensus on the design in the mailing list,
> > you should request an opinion from the Technical Board.
> > 
> > This feature is not a priority for 20.05 release.
> > By the way, it has not been announced in any roadmap.
> > "
> 
> Is it an issue to not have it in the roadmap?

No roadmap is not mandatory.
But having it in the roadmap helps to remind something new will
require long review effort.
Trahe, Fiona April 14, 2020, 1:04 p.m. UTC | #4
Hi Thomas, David, Ferruh,

> -----Original Message-----
> Subject: Re: [dpdk-dev] [PATCH v3 0/4] add AESNI-MB rawdev for multi-function processing
> 
> 14/04/2020 12:21, Ferruh Yigit:
> > On 4/10/2020 11:55 PM, Thomas Monjalon wrote:
> > > Hi,
> > >
> > > Adding more people (crypto PMD maintainers) as Cc.
> > >
> > > 10/04/2020 16:27, David Coyle:
> > >> Introduction
> > >> ============
> > >>
> > >> This patchset adds a new AESNI-MB Multi-Function raw device PMD for
> > >> utilizing multi-function capabilities of the Intel IPSec Multi Buffer
> > >> library.
> > >>
> > >> The aim of this rawdev PMD is to provide a way of combining one or more
> > >> common packet-processing functions into a single operation, focused on
> > >> DOCSIS and GPON MAC workloads. This allows these functions to be performed
> > >> in parallel by the Intel IPSec Multi Buffer library. These functions
> > >> include cryptography and CRC/BIP calculations. Performing these functions
> > >> in parallel as a single operation can enable a significant performance
> > >> improvement.
> > >
> > > I don't know crypto but I don't think using rawdev for crypto operations
> > > is an API improvement.
> > >
> > > Repeating the initial comments from v1 (because got no reply):
> > > "
> > > As a first impression, I feel it is not the right API.
> > > DPDK is based on classes: ethdev, crypto, compress, baseband, regex
> > > I want to understand why your features cannot fit in a class.
> >
> > Hi Thomas,
> >
> > I asked similar question, and there is already a detailed answer with some
> > background of the issue:
> 
> Good to see that you get some reply, Ferruh.
> 
> >
> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
> mprd11.prod.outlook.com/
> 
> I am not convinced.
> I don't like rawdev in general.
> Rawdev is good only for hardware support which cannot be generic
> like SoC, FPGA management or DMA engine.
[Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
So there is no class in DPDK that these readily fit into.
There was resistance to adding another xxxddev, and even if one had been added
for error_detection_dev, there would still have been another layer needed
to couple this with cryptodev. Various proposals for this have been discussed on the ML
in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API. 
So it seems that only Intel has software and hardware engines that provide this
specialised feature coupling. In that case rawdev seems like the most
appropriate vehicle to expose this.


> Here the intent is to use rawdev because we don't find a good API.
> API defeat is a no-go.
[Fiona] It's not that we haven't found a good API, but that there doesn't seem
to be a general requirement for such a specialised API
Thomas Monjalon April 14, 2020, 1:24 p.m. UTC | #5
14/04/2020 15:04, Trahe, Fiona:
> > 14/04/2020 12:21, Ferruh Yigit:
> > http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
> > mprd11.prod.outlook.com/
> > 
> > I am not convinced.
> > I don't like rawdev in general.
> > Rawdev is good only for hardware support which cannot be generic
> > like SoC, FPGA management or DMA engine.
> 
> [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
> So there is no class in DPDK that these readily fit into.
> There was resistance to adding another xxxddev, and even if one had been added
> for error_detection_dev, there would still have been another layer needed
> to couple this with cryptodev. Various proposals for this have been discussed on the ML
> in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API. 
> So it seems that only Intel has software and hardware engines that provide this
> specialised feature coupling. In that case rawdev seems like the most
> appropriate vehicle to expose this.

Adding some vendor-specific API is not a good answer.
It will work in some cases, but it won't make DPDK better.
What's the purpose of DPDK if it's not solving a common problem
for different hardware?

> > Here the intent is to use rawdev because we don't find a good API.
> > API defeat is a no-go.
> 
> [Fiona] It's not that we haven't found a good API, but that there doesn't seem
> to be a general requirement for such a specialised API

There is a requirement to combine features of different classes.
In the past, rte_security was an "answer" to this issue with crypto and ethdev.
This is a real topic, please let's find a generic elegant solution.
Trahe, Fiona April 14, 2020, 2:02 p.m. UTC | #6
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, April 14, 2020 2:24 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> Cc: Coyle, David <david.coyle@intel.com>; dev@dpdk.org; 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@nxp.com; Anoob Joseph <anoobj@marvell.com>; Ruifeng Wang
> <ruifeng.wang@arm.com>; Liron Himi <lironh@marvell.com>; Nagadheeraj Rottela
> <rnagadheeraj@marvell.com>; Srikanth Jampala <jsrikanth@marvell.com>; Gagandeep Singh
> <g.singh@nxp.com>; Jay Zhou <jianjay.zhou@huawei.com>; Ravi Kumar <ravi1.kumar@amd.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 0/4] add AESNI-MB rawdev for multi-function processing
> 
> 14/04/2020 15:04, Trahe, Fiona:
> > > 14/04/2020 12:21, Ferruh Yigit:
> > >
> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
> > > mprd11.prod.outlook.com/
> > >
> > > I am not convinced.
> > > I don't like rawdev in general.
> > > Rawdev is good only for hardware support which cannot be generic
> > > like SoC, FPGA management or DMA engine.
> >
> > [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
> > So there is no class in DPDK that these readily fit into.
> > There was resistance to adding another xxxddev, and even if one had been added
> > for error_detection_dev, there would still have been another layer needed
> > to couple this with cryptodev. Various proposals for this have been discussed on the ML
> > in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API.
> > So it seems that only Intel has software and hardware engines that provide this
> > specialised feature coupling. In that case rawdev seems like the most
> > appropriate vehicle to expose this.
> 
> Adding some vendor-specific API is not a good answer.
> It will work in some cases, but it won't make DPDK better.
> What's the purpose of DPDK if it's not solving a common problem
> for different hardware?
[Fiona] Based on that logic rawdev should be deprecated.
But the community has agreed that it has a place.
And the common problem here is device exposure.
With a specialised service on top.


> > > Here the intent is to use rawdev because we don't find a good API.
> > > API defeat is a no-go.
> >
> > [Fiona] It's not that we haven't found a good API, but that there doesn't seem
> > to be a general requirement for such a specialised API
> 
> There is a requirement to combine features of different classes.
[Fiona] Can you point me to that requirement please?
We suggested it, but did not get community engagement and have 
dropped our generic API requirement, instead focussing on this specialised case.


> In the past, rte_security was an "answer" to this issue with crypto and ethdev.
> This is a real topic, please let's find a generic elegant solution.
Thomas Monjalon April 14, 2020, 2:44 p.m. UTC | #7
14/04/2020 16:02, Trahe, Fiona:
> Hi Thomas,
> 
> From: Thomas Monjalon <thomas@monjalon.net>
> > 14/04/2020 15:04, Trahe, Fiona:
> > > > 14/04/2020 12:21, Ferruh Yigit:
> > > >
> > http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
> > > > mprd11.prod.outlook.com/
> > > >
> > > > I am not convinced.
> > > > I don't like rawdev in general.
> > > > Rawdev is good only for hardware support which cannot be generic
> > > > like SoC, FPGA management or DMA engine.
> > >
> > > [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
> > > So there is no class in DPDK that these readily fit into.
> > > There was resistance to adding another xxxddev, and even if one had been added
> > > for error_detection_dev, there would still have been another layer needed
> > > to couple this with cryptodev. Various proposals for this have been discussed on the ML
> > > in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API.
> > > So it seems that only Intel has software and hardware engines that provide this
> > > specialised feature coupling. In that case rawdev seems like the most
> > > appropriate vehicle to expose this.
> > 
> > Adding some vendor-specific API is not a good answer.
> > It will work in some cases, but it won't make DPDK better.
> > What's the purpose of DPDK if it's not solving a common problem
> > for different hardware?
> 
> [Fiona] Based on that logic rawdev should be deprecated.
> But the community has agreed that it has a place.

No, as I said above, rawdev is good for SoC, FPGA management or DMA engine.

> And the common problem here is device exposure.
> With a specialised service on top.
> 
> 
> > > > Here the intent is to use rawdev because we don't find a good API.
> > > > API defeat is a no-go.
> > >
> > > [Fiona] It's not that we haven't found a good API, but that there doesn't seem
> > > to be a general requirement for such a specialised API
> > 
> > There is a requirement to combine features of different classes.
> 
> [Fiona] Can you point me to that requirement please?

Yes, rte_security is trying to address this exact issue.

> We suggested it, but did not get community engagement and have 
> dropped our generic API requirement, instead focussing on this specialised case.

I did not see such generic proposal, sorry.

> > In the past, rte_security was an "answer" to this issue with crypto and ethdev.
> > This is a real topic, please let's find a generic elegant solution.
Doherty, Declan April 15, 2020, 10:19 p.m. UTC | #8
On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
> 14/04/2020 16:02, Trahe, Fiona:
>> Hi Thomas,
>>
>> From: Thomas Monjalon <thomas@monjalon.net>
>>> 14/04/2020 15:04, Trahe, Fiona:
>>>>> 14/04/2020 12:21, Ferruh Yigit:
>>>>>
>>> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
>>>>> mprd11.prod.outlook.com/
>>>>>
>>>>> I am not convinced.
>>>>> I don't like rawdev in general.
>>>>> Rawdev is good only for hardware support which cannot be generic
>>>>> like SoC, FPGA management or DMA engine.
>>>>
>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
>>>> So there is no class in DPDK that these readily fit into.
>>>> There was resistance to adding another xxxddev, and even if one had been added
>>>> for error_detection_dev, there would still have been another layer needed
>>>> to couple this with cryptodev. Various proposals for this have been discussed on the ML
>>>> in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API.
>>>> So it seems that only Intel has software and hardware engines that provide this
>>>> specialised feature coupling. In that case rawdev seems like the most
>>>> appropriate vehicle to expose this.
>>>
>>> Adding some vendor-specific API is not a good answer.
>>> It will work in some cases, but it won't make DPDK better.
>>> What's the purpose of DPDK if it's not solving a common problem
>>> for different hardware?
>>
The current proposal in rawdev could easily be supported by any hardware 
which supports chaining multiple functions/services into a single 
operation, in this case symmetric crypto and error detection, but it 
could conceivably support chaining symmetric/asymmetric crypto 
operations or chaining symmetric crypto and compression operations.

>> [Fiona] Based on that logic rawdev should be deprecated.
>> But the community has agreed that it has a place.
> 
> No, as I said above, rawdev is good for SoC, FPGA management or DMA engine.

I distinctly remember when rawdev was being proposed one of the uses 
cases proposed was that a new classes of APIs could be prototyped and 
developed under rawdev and when a solid consensus was reached then 
migrated to a mainstream DPDK library. I think every effort has been 
made here to engage the community to develop a generic approach. As 
Fiona notes there hasn't really been much of an appetite for this.

Therefore I think the option to use rawdev makes sense, it allows an 
initial proposal to be deployed,  without a generic solution agreement, 
it will also give others in the community to see how this approach can 
work and hopefully lead to more engagement on a generic solution. Also 
as APIs in rawdev are essentially treated as private APIs the onus is on 
Intel to support this going forward.

> 
>> And the common problem here is device exposure.
>> With a specialised service on top.
>>
>>
>>>>> Here the intent is to use rawdev because we don't find a good API.
>>>>> API defeat is a no-go.
>>>>
>>>> [Fiona] It's not that we haven't found a good API, but that there doesn't seem
>>>> to be a general requirement for such a specialised API
>>>
>>> There is a requirement to combine features of different classes.
>>
>> [Fiona] Can you point me to that requirement please?
> 
> Yes, rte_security is trying to address this exact issue.
> 

I don't agree rte_security addresses the problem of different device 
types supporting the same services. The problem being addressed here is 
a single device which supports the chaining of multiple services (sym 
crypto & error detection)

>> We suggested it, but did not get community engagement and have
>> dropped our generic API requirement, instead focussing on this specialised case.
> 
> I did not see such generic proposal, sorry.
> 
>>> In the past, rte_security was an "answer" to this issue with crypto and ethdev.
>>> This is a real topic, please let's find a generic elegant solution.
> 
> 
>
Thomas Monjalon April 15, 2020, 10:33 p.m. UTC | #9
16/04/2020 00:19, Doherty, Declan:
> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
> > 14/04/2020 16:02, Trahe, Fiona:
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >>> 14/04/2020 15:04, Trahe, Fiona:
> >>>>> 14/04/2020 12:21, Ferruh Yigit:
> >>>>>
> >>> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
> >>>>> mprd11.prod.outlook.com/
> >>>>>
> >>>>> I am not convinced.
> >>>>> I don't like rawdev in general.
> >>>>> Rawdev is good only for hardware support which cannot be generic
> >>>>> like SoC, FPGA management or DMA engine.
> >>>>
> >>>> [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
> >>>> So there is no class in DPDK that these readily fit into.
> >>>> There was resistance to adding another xxxddev, and even if one had been added
> >>>> for error_detection_dev, there would still have been another layer needed
> >>>> to couple this with cryptodev. Various proposals for this have been discussed on the ML
> >>>> in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API.
> >>>> So it seems that only Intel has software and hardware engines that provide this
> >>>> specialised feature coupling. In that case rawdev seems like the most
> >>>> appropriate vehicle to expose this.
> >>>
> >>> Adding some vendor-specific API is not a good answer.
> >>> It will work in some cases, but it won't make DPDK better.
> >>> What's the purpose of DPDK if it's not solving a common problem
> >>> for different hardware?
> >>
> The current proposal in rawdev could easily be supported by any hardware 
> which supports chaining multiple functions/services into a single 
> operation, in this case symmetric crypto and error detection, but it 
> could conceivably support chaining symmetric/asymmetric crypto 
> operations or chaining symmetric crypto and compression operations.
> 
> >> [Fiona] Based on that logic rawdev should be deprecated.
> >> But the community has agreed that it has a place.
> > 
> > No, as I said above, rawdev is good for SoC, FPGA management or DMA engine.
> 
> I distinctly remember when rawdev was being proposed one of the uses 
> cases proposed was that a new classes of APIs could be prototyped and 
> developed under rawdev and when a solid consensus was reached then 
> migrated to a mainstream DPDK library. I think every effort has been 
> made here to engage the community to develop a generic approach. As 
> Fiona notes there hasn't really been much of an appetite for this.
> 
> Therefore I think the option to use rawdev makes sense, it allows an 
> initial proposal to be deployed,  without a generic solution agreement, 
> it will also give others in the community to see how this approach can 
> work and hopefully lead to more engagement on a generic solution. Also 
> as APIs in rawdev are essentially treated as private APIs the onus is on 
> Intel to support this going forward.

Because hardware support is pending,
we should accept an Intel-only "temporary" solution,
opening the door to more vendor-specific APIs?

What is the benefit for the DPDK project?


> >> And the common problem here is device exposure.
> >> With a specialised service on top.
> >>
> >>
> >>>>> Here the intent is to use rawdev because we don't find a good API.
> >>>>> API defeat is a no-go.
> >>>>
> >>>> [Fiona] It's not that we haven't found a good API, but that there doesn't seem
> >>>> to be a general requirement for such a specialised API
> >>>
> >>> There is a requirement to combine features of different classes.
> >>
> >> [Fiona] Can you point me to that requirement please?
> > 
> > Yes, rte_security is trying to address this exact issue.
> > 
> 
> I don't agree rte_security addresses the problem of different device 
> types supporting the same services. The problem being addressed here is 
> a single device which supports the chaining of multiple services (sym 
> crypto & error detection)

Doing IPsec processing in Rx or Tx of a NIC is not chaining?


> >> We suggested it, but did not get community engagement and have
> >> dropped our generic API requirement, instead focussing on this specialised case.
> > 
> > I did not see such generic proposal, sorry.
> > 
> >>> In the past, rte_security was an "answer" to this issue with crypto and ethdev.
> >>> This is a real topic, please let's find a generic elegant solution.
Doherty, Declan April 21, 2020, 4:46 p.m. UTC | #10
On 15/04/2020 11:33 PM, Thomas Monjalon wrote:
> 16/04/2020 00:19, Doherty, Declan:
>> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
>>> 14/04/2020 16:02, Trahe, Fiona:
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>> 14/04/2020 15:04, Trahe, Fiona:
>>>>>>> 14/04/2020 12:21, Ferruh Yigit:
>>>>>>>
>>>>> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
>>>>>>> mprd11.prod.outlook.com/
>>>>>>>
>>>>>>> I am not convinced.
>>>>>>> I don't like rawdev in general.
>>>>>>> Rawdev is good only for hardware support which cannot be generic
>>>>>>> like SoC, FPGA management or DMA engine.
>>>>>>
>>>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
>>>>>> So there is no class in DPDK that these readily fit into.
>>>>>> There was resistance to adding another xxxddev, and even if one had been added
>>>>>> for error_detection_dev, there would still have been another layer needed
>>>>>> to couple this with cryptodev. Various proposals for this have been discussed on the ML
>>>>>> in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API.
>>>>>> So it seems that only Intel has software and hardware engines that provide this
>>>>>> specialised feature coupling. In that case rawdev seems like the most
>>>>>> appropriate vehicle to expose this.
>>>>>
>>>>> Adding some vendor-specific API is not a good answer.
>>>>> It will work in some cases, but it won't make DPDK better.
>>>>> What's the purpose of DPDK if it's not solving a common problem
>>>>> for different hardware?
>>>>
>> The current proposal in rawdev could easily be supported by any hardware
>> which supports chaining multiple functions/services into a single
>> operation, in this case symmetric crypto and error detection, but it
>> could conceivably support chaining symmetric/asymmetric crypto
>> operations or chaining symmetric crypto and compression operations.
>>
>>>> [Fiona] Based on that logic rawdev should be deprecated.
>>>> But the community has agreed that it has a place.
>>>
>>> No, as I said above, rawdev is good for SoC, FPGA management or DMA engine.
>>
>> I distinctly remember when rawdev was being proposed one of the uses
>> cases proposed was that a new classes of APIs could be prototyped and
>> developed under rawdev and when a solid consensus was reached then
>> migrated to a mainstream DPDK library. I think every effort has been
>> made here to engage the community to develop a generic approach. As
>> Fiona notes there hasn't really been much of an appetite for this.
>>
>> Therefore I think the option to use rawdev makes sense, it allows an
>> initial proposal to be deployed,  without a generic solution agreement,
>> it will also give others in the community to see how this approach can
>> work and hopefully lead to more engagement on a generic solution. Also
>> as APIs in rawdev are essentially treated as private APIs the onus is on
>> Intel to support this going forward.
> 
> Because hardware support is pending,
> we should accept an Intel-only "temporary" solution,
> opening the door to more vendor-specific APIs?
> 
> What is the benefit for the DPDK project?
> 
> 

Sorry I don't agree with this sentiment, David has made every attempt to 
solicit feedback an to engage the community in this.

I also don't agree in classifying this as a "temporary solution" as this 
is a solid proposal for an approach to chaining multiple operations 
together, but I guess the fact remains that we only currently have a 
single use-case, but it is difficult to generate a generic solution in 
this case.

While there is only a single use case it is targeting two devices so 
that drove the need for a common interface withing rawdev.

The advantage of using rawdev is that it allows this to be consumed 
through DPDK, which enables DPDK project consumers, but also leaves the 
door open to other contributors to have their say on how this should 
evolve. For example this exact process seems to be occurring with DMA 
engines in rawdev today, with a critical mass of implementations which 
now is giving the impetus to create a generic solution, as we would hope 
can occur here too in the future.


>>>> And the common problem here is device exposure.
>>>> With a specialised service on top.
>>>>
>>>>
>>>>>>> Here the intent is to use rawdev because we don't find a good API.
>>>>>>> API defeat is a no-go.
>>>>>>
>>>>>> [Fiona] It's not that we haven't found a good API, but that there doesn't seem
>>>>>> to be a general requirement for such a specialised API
>>>>>
>>>>> There is a requirement to combine features of different classes.
>>>>
>>>> [Fiona] Can you point me to that requirement please?
>>>
>>> Yes, rte_security is trying to address this exact issue.
>>>
>>
>> I don't agree rte_security addresses the problem of different device
>> types supporting the same services. The problem being addressed here is
>> a single device which supports the chaining of multiple services (sym
>> crypto & error detection)
> 
> Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> 
> 

I wouldn't consider an inline crypto offload or full IPsec offload a 
chained operation in the vein being proposed here where completely 
independent services (in the view of DPDK which are currently on 
independent devices and APIs) are linked together.

We did look at using rte_security here but it wasn't considered suitable 
for a chaining of non-crypto operations such as CRC or possibly 
compression in the future, as it would still run into the issue of 
having to use the cryptodev enq/deq API in the lookaside offload case.


>>>> We suggested it, but did not get community engagement and have
>>>> dropped our generic API requirement, instead focussing on this specialised case.
>>>
>>> I did not see such generic proposal, sorry.
>>>
>>>>> In the past, rte_security was an "answer" to this issue with crypto and ethdev.
>>>>> This is a real topic, please let's find a generic elegant solution.
> 
> 
>
David Coyle April 21, 2020, 5:23 p.m. UTC | #11
Thank you Thomas for your input.

We would like to request that the Tech-Board (CC'ed) also review the proposal to help us reach a consensus.
 
If the current proposal is not acceptable, we would welcome feedback from the board on how to rework our
proposal to something that would be acceptable.
 
For the benefit of the Tech-Board here is the back-ground to our proposal for Rawdev-based multi-function
processing:
- The primary objective is to support the AESNI MB combined Crypto-CRC processing capability in DPDK and
   in future to add support for combined Crypto-CRC support in QAT.
- The cryptodev API was considered unsuitable because CRC is not a cryptographic operation, and this would
   also preclude other non-crypto operations in the future such as compression.
- The rte_security API was also not considered suitable for chaining of non-crypto operations such as CRC,
   as Declan pointed out below.
- A new Accelerator API was proposed as an RFC but was not pursued due to community feedback that a
   new API would not be welcome for a single use-case.
- Using Rawdev for multi-function processing was then proposed and, initially, as there was no opposition
   we implemented a patch-set for this approach.
 
It was considered that a Rawdev-based multi-function approach would be suitable for the following reasons:
1) Multi-function processing for Crypto-CRC cases is not a good fit for any of the existing DPDK classes.
2) Rawdev was intended for such specialized acceleration processing that are not a good fit for existing DPDK
     classes.
3) Rawdev was also intended as somewhere that new use-cases like this could be prototyped and developed,
     such as Declan mentions below
4) The Rawdev-based multi-function proposal is extensible and we would hope that it can evolve to support
     new use-cases and target new devices in the future with the communities involvement.


> -----Original Message-----
> From: Doherty, Declan <declan.doherty@intel.com>
> Sent: Tuesday, April 21, 2020 5:46 PM
> 
> On 15/04/2020 11:33 PM, Thomas Monjalon wrote:
> > 16/04/2020 00:19, Doherty, Declan:
> >> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
> >>> 14/04/2020 16:02, Trahe, Fiona:
> >>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>> 14/04/2020 15:04, Trahe, Fiona:
> >>>>>>> 14/04/2020 12:21, Ferruh Yigit:
> >>>>>>>
> >>>>>
> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30
> @M
> >>>>> N2PR11MB3550.na
> >>>>>>> mprd11.prod.outlook.com/
> >>>>>>>
> >>>>>>> I am not convinced.
> >>>>>>> I don't like rawdev in general.
> >>>>>>> Rawdev is good only for hardware support which cannot be generic
> >>>>>>> like SoC, FPGA management or DMA engine.
> >>>>>>
> >>>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error
> detection processes.
> >>>>>> So there is no class in DPDK that these readily fit into.
> >>>>>> There was resistance to adding another xxxddev, and even if one
> >>>>>> had been added for error_detection_dev, there would still have
> >>>>>> been another layer needed to couple this with cryptodev. Various
> >>>>>> proposals for this have been discussed on the ML in RFC and recent
> patches, there doesn't seem to be an appetite for this as a generic API.
> >>>>>> So it seems that only Intel has software and hardware engines
> >>>>>> that provide this specialised feature coupling. In that case
> >>>>>> rawdev seems like the most appropriate vehicle to expose this.
> >>>>>
> >>>>> Adding some vendor-specific API is not a good answer.
> >>>>> It will work in some cases, but it won't make DPDK better.
> >>>>> What's the purpose of DPDK if it's not solving a common problem
> >>>>> for different hardware?
> >>>>
> >> The current proposal in rawdev could easily be supported by any
> >> hardware which supports chaining multiple functions/services into a
> >> single operation, in this case symmetric crypto and error detection,
> >> but it could conceivably support chaining symmetric/asymmetric crypto
> >> operations or chaining symmetric crypto and compression operations.
> >>
> >>>> [Fiona] Based on that logic rawdev should be deprecated.
> >>>> But the community has agreed that it has a place.
> >>>
> >>> No, as I said above, rawdev is good for SoC, FPGA management or DMA
> engine.
> >>
> >> I distinctly remember when rawdev was being proposed one of the uses
> >> cases proposed was that a new classes of APIs could be prototyped and
> >> developed under rawdev and when a solid consensus was reached then
> >> migrated to a mainstream DPDK library. I think every effort has been
> >> made here to engage the community to develop a generic approach. As
> >> Fiona notes there hasn't really been much of an appetite for this.
> >>
> >> Therefore I think the option to use rawdev makes sense, it allows an
> >> initial proposal to be deployed,  without a generic solution
> >> agreement, it will also give others in the community to see how this
> >> approach can work and hopefully lead to more engagement on a generic
> >> solution. Also as APIs in rawdev are essentially treated as private
> >> APIs the onus is on Intel to support this going forward.
> >
> > Because hardware support is pending,
> > we should accept an Intel-only "temporary" solution, opening the door
> > to more vendor-specific APIs?
> >
> > What is the benefit for the DPDK project?
> >
> Sorry I don't agree with this sentiment, David has made every attempt to
> solicit feedback and to engage the community in this.
> 
> I also don't agree in classifying this as a "temporary solution" as this is a solid
> proposal for an approach to chaining multiple operations together, but I
> guess the fact remains that we only currently have a single use-case, but it is
> difficult to generate a generic solution in this case.
> 
> While there is only a single use case it is targeting two devices so that drove
> the need for a common interface within rawdev.
> 
> The advantage of using rawdev is that it allows this to be consumed through
> DPDK, which enables DPDK project consumers, but also leaves the door open
> to other contributors to have their say on how this should evolve. For
> example this exact process seems to be occurring with DMA engines in
> rawdev today, with a critical mass of implementations which now is giving the
> impetus to create a generic solution, as we would hope can occur here too in
> the future.
> 
> 
> >>>> And the common problem here is device exposure.
> >>>> With a specialised service on top.
> >>>>
> >>>>
> >>>>>>> Here the intent is to use rawdev because we don't find a good API.
> >>>>>>> API defeat is a no-go.
> >>>>>>
> >>>>>> [Fiona] It's not that we haven't found a good API, but that there
> >>>>>> doesn't seem to be a general requirement for such a specialised
> >>>>>> API
> >>>>>
> >>>>> There is a requirement to combine features of different classes.
> >>>>
> >>>> [Fiona] Can you point me to that requirement please?
> >>>
> >>> Yes, rte_security is trying to address this exact issue.
> >>>
> >>
> >> I don't agree rte_security addresses the problem of different device
> >> types supporting the same services. The problem being addressed here
> >> is a single device which supports the chaining of multiple services
> >> (sym crypto & error detection)
> >
> > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> >
> I wouldn't consider an inline crypto offload or full IPsec offload a chained
> operation in the vein being proposed here where completely independent
> services (in the view of DPDK which are currently on independent devices
> and APIs) are linked together.
> 
> We did look at using rte_security here but it wasn't considered suitable for a
> chaining of non-crypto operations such as CRC or possibly compression in the
> future, as it would still run into the issue of having to use the cryptodev
> enq/deq API in the lookaside offload case.
> 
> 
> >>>> We suggested it, but did not get community engagement and have
> >>>> dropped our generic API requirement, instead focussing on this
> specialised case.
> >>>
> >>> I did not see such generic proposal, sorry.
> >>>
> >>>>> In the past, rte_security was an "answer" to this issue with crypto and
> ethdev.
> >>>>> This is a real topic, please let's find a generic elegant solution.
> >
> >
> >
Thomas Monjalon April 21, 2020, 5:25 p.m. UTC | #12
21/04/2020 18:46, Doherty, Declan:
> On 15/04/2020 11:33 PM, Thomas Monjalon wrote:
> > 16/04/2020 00:19, Doherty, Declan:
> >> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
> >>> 14/04/2020 16:02, Trahe, Fiona:
> >>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>> 14/04/2020 15:04, Trahe, Fiona:
> >>>>>>> 14/04/2020 12:21, Ferruh Yigit:
> >>>>>>>
> >>>>> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30@MN2PR11MB3550.na
> >>>>>>> mprd11.prod.outlook.com/
> >>>>>>>
> >>>>>>> I am not convinced.
> >>>>>>> I don't like rawdev in general.
> >>>>>>> Rawdev is good only for hardware support which cannot be generic
> >>>>>>> like SoC, FPGA management or DMA engine.
> >>>>>>
> >>>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error detection processes.
> >>>>>> So there is no class in DPDK that these readily fit into.
> >>>>>> There was resistance to adding another xxxddev, and even if one had been added
> >>>>>> for error_detection_dev, there would still have been another layer needed
> >>>>>> to couple this with cryptodev. Various proposals for this have been discussed on the ML
> >>>>>> in RFC and recent patches, there doesn't seem to be an appetite for this as a generic API.
> >>>>>> So it seems that only Intel has software and hardware engines that provide this
> >>>>>> specialised feature coupling. In that case rawdev seems like the most
> >>>>>> appropriate vehicle to expose this.
> >>>>>
> >>>>> Adding some vendor-specific API is not a good answer.
> >>>>> It will work in some cases, but it won't make DPDK better.
> >>>>> What's the purpose of DPDK if it's not solving a common problem
> >>>>> for different hardware?
> >>>>
> >> The current proposal in rawdev could easily be supported by any hardware
> >> which supports chaining multiple functions/services into a single
> >> operation, in this case symmetric crypto and error detection, but it
> >> could conceivably support chaining symmetric/asymmetric crypto
> >> operations or chaining symmetric crypto and compression operations.
> >>
> >>>> [Fiona] Based on that logic rawdev should be deprecated.
> >>>> But the community has agreed that it has a place.
> >>>
> >>> No, as I said above, rawdev is good for SoC, FPGA management or DMA engine.
> >>
> >> I distinctly remember when rawdev was being proposed one of the uses
> >> cases proposed was that a new classes of APIs could be prototyped and
> >> developed under rawdev and when a solid consensus was reached then
> >> migrated to a mainstream DPDK library. I think every effort has been
> >> made here to engage the community to develop a generic approach. As
> >> Fiona notes there hasn't really been much of an appetite for this.
> >>
> >> Therefore I think the option to use rawdev makes sense, it allows an
> >> initial proposal to be deployed,  without a generic solution agreement,
> >> it will also give others in the community to see how this approach can
> >> work and hopefully lead to more engagement on a generic solution. Also
> >> as APIs in rawdev are essentially treated as private APIs the onus is on
> >> Intel to support this going forward.
> > 
> > Because hardware support is pending,
> > we should accept an Intel-only "temporary" solution,
> > opening the door to more vendor-specific APIs?
> > 
> > What is the benefit for the DPDK project?
> 
> Sorry I don't agree with this sentiment, David has made every attempt to 
> solicit feedback an to engage the community in this.

Really?

These are the recipients of the first patch:
	dev@dpdk.org, declan.doherty@intel.com, fiona.trahe@intel.com
In next patches, only Intel and NXP are Cc'ed.
Stephen and Jerin, who gave good comments on first patch,
were not Cc'ed in next versions.

Was it presented in an event?
Was it brought to the techboard?
Please don't exagerate and admit you are trying to push something
which is specific and convenient for Intel QuickAssist.


> I also don't agree in classifying this as a "temporary solution" as this 
> is a solid proposal for an approach to chaining multiple operations 
> together, but I guess the fact remains that we only currently have a 
> single use-case, but it is difficult to generate a generic solution in 
> this case.
> 
> While there is only a single use case it is targeting two devices so 
> that drove the need for a common interface withing rawdev.
> 
> The advantage of using rawdev is that it allows this to be consumed 
> through DPDK, which enables DPDK project consumers, but also leaves the 
> door open to other contributors to have their say on how this should 
> evolve. For example this exact process seems to be occurring with DMA 
> engines in rawdev today, with a critical mass of implementations which 
> now is giving the impetus to create a generic solution, as we would hope 
> can occur here too in the future.
> 
> 
> >>>> And the common problem here is device exposure.
> >>>> With a specialised service on top.
> >>>>
> >>>>
> >>>>>>> Here the intent is to use rawdev because we don't find a good API.
> >>>>>>> API defeat is a no-go.
> >>>>>>
> >>>>>> [Fiona] It's not that we haven't found a good API, but that there doesn't seem
> >>>>>> to be a general requirement for such a specialised API
> >>>>>
> >>>>> There is a requirement to combine features of different classes.
> >>>>
> >>>> [Fiona] Can you point me to that requirement please?
> >>>
> >>> Yes, rte_security is trying to address this exact issue.
> >>>
> >>
> >> I don't agree rte_security addresses the problem of different device
> >> types supporting the same services. The problem being addressed here is
> >> a single device which supports the chaining of multiple services (sym
> >> crypto & error detection)
> > 
> > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> 
> I wouldn't consider an inline crypto offload or full IPsec offload a 
> chained operation in the vein being proposed here where completely 
> independent services (in the view of DPDK which are currently on 
> independent devices and APIs) are linked together.
> 
> We did look at using rte_security here but it wasn't considered suitable 
> for a chaining of non-crypto operations such as CRC or possibly 
> compression in the future, as it would still run into the issue of 
> having to use the cryptodev enq/deq API in the lookaside offload case.

Because rte_security is not a generic solution (that's why I don't like it).
I think a good approach would be to check how to offload in HW
the chaining done in frameworks like rte_pipeline or rte_graph.
Stephen and Jerin already talked about it, but it was rejected by David,
because harder to implement I think.
Even worst, the team working on this patch did zero review of rte_graph.

I think the chaining requirement is a real problem to solve,
and it deserves a good architecture and API.
Maybe this future API should be based on rte_graph.
David Coyle April 21, 2020, 6:37 p.m. UTC | #13
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, April 21, 2020 6:25 PM
 [PATCH v3 0/4] add AESNI-MB rawdev for multi-
> function processing
> 
> 21/04/2020 18:46, Doherty, Declan:
> > On 15/04/2020 11:33 PM, Thomas Monjalon wrote:
> > > 16/04/2020 00:19, Doherty, Declan:
> > >> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
> > >>> 14/04/2020 16:02, Trahe, Fiona:
> > >>>> From: Thomas Monjalon <thomas@monjalon.net>
> > >>>>> 14/04/2020 15:04, Trahe, Fiona:
> > >>>>>>> 14/04/2020 12:21, Ferruh Yigit:
> > >>>>>>>
> > >>>>>
> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30
> > >>>>> @MN2PR11MB3550.na
> > >>>>>>> mprd11.prod.outlook.com/
> > >>>>>>>
> > >>>>>>> I am not convinced.
> > >>>>>>> I don't like rawdev in general.
> > >>>>>>> Rawdev is good only for hardware support which cannot be
> > >>>>>>> generic like SoC, FPGA management or DMA engine.
> > >>>>>>
> > >>>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error
> detection processes.
> > >>>>>> So there is no class in DPDK that these readily fit into.
> > >>>>>> There was resistance to adding another xxxddev, and even if one
> > >>>>>> had been added for error_detection_dev, there would still have
> > >>>>>> been another layer needed to couple this with cryptodev.
> > >>>>>> Various proposals for this have been discussed on the ML in RFC
> and recent patches, there doesn't seem to be an appetite for this as a
> generic API.
> > >>>>>> So it seems that only Intel has software and hardware engines
> > >>>>>> that provide this specialised feature coupling. In that case
> > >>>>>> rawdev seems like the most appropriate vehicle to expose this.
> > >>>>>
> > >>>>> Adding some vendor-specific API is not a good answer.
> > >>>>> It will work in some cases, but it won't make DPDK better.
> > >>>>> What's the purpose of DPDK if it's not solving a common problem
> > >>>>> for different hardware?
> > >>>>
> > >> The current proposal in rawdev could easily be supported by any
> > >> hardware which supports chaining multiple functions/services into a
> > >> single operation, in this case symmetric crypto and error
> > >> detection, but it could conceivably support chaining
> > >> symmetric/asymmetric crypto operations or chaining symmetric crypto
> and compression operations.
> > >>
> > >>>> [Fiona] Based on that logic rawdev should be deprecated.
> > >>>> But the community has agreed that it has a place.
> > >>>
> > >>> No, as I said above, rawdev is good for SoC, FPGA management or
> DMA engine.
> > >>
> > >> I distinctly remember when rawdev was being proposed one of the
> > >> uses cases proposed was that a new classes of APIs could be
> > >> prototyped and developed under rawdev and when a solid consensus
> > >> was reached then migrated to a mainstream DPDK library. I think
> > >> every effort has been made here to engage the community to develop
> > >> a generic approach. As Fiona notes there hasn't really been much of an
> appetite for this.
> > >>
> > >> Therefore I think the option to use rawdev makes sense, it allows
> > >> an initial proposal to be deployed,  without a generic solution
> > >> agreement, it will also give others in the community to see how
> > >> this approach can work and hopefully lead to more engagement on a
> > >> generic solution. Also as APIs in rawdev are essentially treated as
> > >> private APIs the onus is on Intel to support this going forward.
> > >
> > > Because hardware support is pending, we should accept an Intel-only
> > > "temporary" solution, opening the door to more vendor-specific APIs?
> > >
> > > What is the benefit for the DPDK project?
> >
> > Sorry I don't agree with this sentiment, David has made every attempt
> > to solicit feedback an to engage the community in this.
> 
> Really?
> 
> These are the recipients of the first patch:
> 	dev@dpdk.org, declan.doherty@intel.com, fiona.trahe@intel.com In
> next patches, only Intel and NXP are Cc'ed.
> Stephen and Jerin, who gave good comments on first patch, were not Cc'ed
> in next versions.
> 
> Was it presented in an event?
> Was it brought to the techboard?
> Please don't exagerate and admit you are trying to push something which is
> specific and convenient for Intel QuickAssist.

[DC] This is being brought to the TechBoard tomorrow (22/04)

> 
> 
> > I also don't agree in classifying this as a "temporary solution" as
> > this is a solid proposal for an approach to chaining multiple
> > operations together, but I guess the fact remains that we only
> > currently have a single use-case, but it is difficult to generate a
> > generic solution in this case.
> >
> > While there is only a single use case it is targeting two devices so
> > that drove the need for a common interface withing rawdev.
> >
> > The advantage of using rawdev is that it allows this to be consumed
> > through DPDK, which enables DPDK project consumers, but also leaves
> > the door open to other contributors to have their say on how this
> > should evolve. For example this exact process seems to be occurring
> > with DMA engines in rawdev today, with a critical mass of
> > implementations which now is giving the impetus to create a generic
> > solution, as we would hope can occur here too in the future.
> >
> >
> > >>>> And the common problem here is device exposure.
> > >>>> With a specialised service on top.
> > >>>>
> > >>>>
> > >>>>>>> Here the intent is to use rawdev because we don't find a good
> API.
> > >>>>>>> API defeat is a no-go.
> > >>>>>>
> > >>>>>> [Fiona] It's not that we haven't found a good API, but that
> > >>>>>> there doesn't seem to be a general requirement for such a
> > >>>>>> specialised API
> > >>>>>
> > >>>>> There is a requirement to combine features of different classes.
> > >>>>
> > >>>> [Fiona] Can you point me to that requirement please?
> > >>>
> > >>> Yes, rte_security is trying to address this exact issue.
> > >>>
> > >>
> > >> I don't agree rte_security addresses the problem of different
> > >> device types supporting the same services. The problem being
> > >> addressed here is a single device which supports the chaining of
> > >> multiple services (sym crypto & error detection)
> > >
> > > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> >
> > I wouldn't consider an inline crypto offload or full IPsec offload a
> > chained operation in the vein being proposed here where completely
> > independent services (in the view of DPDK which are currently on
> > independent devices and APIs) are linked together.
> >
> > We did look at using rte_security here but it wasn't considered
> > suitable for a chaining of non-crypto operations such as CRC or
> > possibly compression in the future, as it would still run into the
> > issue of having to use the cryptodev enq/deq API in the lookaside offload
> case.
> 
> Because rte_security is not a generic solution (that's why I don't like it).
> I think a good approach would be to check how to offload in HW the chaining
> done in frameworks like rte_pipeline or rte_graph.
> Stephen and Jerin already talked about it, but it was rejected by David,
> because harder to implement I think.
> Even worst, the team working on this patch did zero review of rte_graph.

[DC] The team working on this patch did review rte_graph and explained our reasoning
to Jerin as to why we felt it was not suitable.

While Jerin explained it would be possible to combine 2 nodes as a single optimized node at runtime,
he also agreed that it did NOT make sense to abstract what we are trying to do as a graph.

Please see http://mails.dpdk.org/archives/dev/2020-March/159238.html

> 
> I think the chaining requirement is a real problem to solve, and it deserves a
> good architecture and API.
> Maybe this future API should be based on rte_graph.
>
Thomas Monjalon April 21, 2020, 9:51 p.m. UTC | #14
21/04/2020 20:37, Coyle, David:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 21/04/2020 18:46, Doherty, Declan:
> > > On 15/04/2020 11:33 PM, Thomas Monjalon wrote:
> > > > 16/04/2020 00:19, Doherty, Declan:
> > > >> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
> > > >>> 14/04/2020 16:02, Trahe, Fiona:
> > > >>>> From: Thomas Monjalon <thomas@monjalon.net>
> > > >>>>> 14/04/2020 15:04, Trahe, Fiona:
> > > >>>>>>> 14/04/2020 12:21, Ferruh Yigit:
> > > >>>>>>>
> > > >>>>>
> > http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30
> > > >>>>> @MN2PR11MB3550.na
> > > >>>>>>> mprd11.prod.outlook.com/
> > > >>>>>>>
> > > >>>>>>> I am not convinced.
> > > >>>>>>> I don't like rawdev in general.
> > > >>>>>>> Rawdev is good only for hardware support which cannot be
> > > >>>>>>> generic like SoC, FPGA management or DMA engine.
> > > >>>>>>
> > > >>>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error
> > detection processes.
> > > >>>>>> So there is no class in DPDK that these readily fit into.
> > > >>>>>> There was resistance to adding another xxxddev, and even if one
> > > >>>>>> had been added for error_detection_dev, there would still have
> > > >>>>>> been another layer needed to couple this with cryptodev.
> > > >>>>>> Various proposals for this have been discussed on the ML in RFC
> > and recent patches, there doesn't seem to be an appetite for this as a
> > generic API.
> > > >>>>>> So it seems that only Intel has software and hardware engines
> > > >>>>>> that provide this specialised feature coupling. In that case
> > > >>>>>> rawdev seems like the most appropriate vehicle to expose this.
> > > >>>>>
> > > >>>>> Adding some vendor-specific API is not a good answer.
> > > >>>>> It will work in some cases, but it won't make DPDK better.
> > > >>>>> What's the purpose of DPDK if it's not solving a common problem
> > > >>>>> for different hardware?
> > > >>>>
> > > >> The current proposal in rawdev could easily be supported by any
> > > >> hardware which supports chaining multiple functions/services into a
> > > >> single operation, in this case symmetric crypto and error
> > > >> detection, but it could conceivably support chaining
> > > >> symmetric/asymmetric crypto operations or chaining symmetric crypto
> > and compression operations.
> > > >>
> > > >>>> [Fiona] Based on that logic rawdev should be deprecated.
> > > >>>> But the community has agreed that it has a place.
> > > >>>
> > > >>> No, as I said above, rawdev is good for SoC, FPGA management or
> > DMA engine.
> > > >>
> > > >> I distinctly remember when rawdev was being proposed one of the
> > > >> uses cases proposed was that a new classes of APIs could be
> > > >> prototyped and developed under rawdev and when a solid consensus
> > > >> was reached then migrated to a mainstream DPDK library. I think
> > > >> every effort has been made here to engage the community to develop
> > > >> a generic approach. As Fiona notes there hasn't really been much of an
> > appetite for this.
> > > >>
> > > >> Therefore I think the option to use rawdev makes sense, it allows
> > > >> an initial proposal to be deployed,  without a generic solution
> > > >> agreement, it will also give others in the community to see how
> > > >> this approach can work and hopefully lead to more engagement on a
> > > >> generic solution. Also as APIs in rawdev are essentially treated as
> > > >> private APIs the onus is on Intel to support this going forward.
> > > >
> > > > Because hardware support is pending, we should accept an Intel-only
> > > > "temporary" solution, opening the door to more vendor-specific APIs?
> > > >
> > > > What is the benefit for the DPDK project?
> > >
> > > Sorry I don't agree with this sentiment, David has made every attempt
> > > to solicit feedback an to engage the community in this.
> > 
> > Really?
> > 
> > These are the recipients of the first patch:
> > 	dev@dpdk.org, declan.doherty@intel.com, fiona.trahe@intel.com In
> > next patches, only Intel and NXP are Cc'ed.
> > Stephen and Jerin, who gave good comments on first patch, were not Cc'ed
> > in next versions.
> > 
> > Was it presented in an event?
> > Was it brought to the techboard?
> > Please don't exagerate and admit you are trying to push something which is
> > specific and convenient for Intel QuickAssist.
> 
> [DC] This is being brought to the TechBoard tomorrow (22/04)
> 
> > 
> > 
> > > I also don't agree in classifying this as a "temporary solution" as
> > > this is a solid proposal for an approach to chaining multiple
> > > operations together, but I guess the fact remains that we only
> > > currently have a single use-case, but it is difficult to generate a
> > > generic solution in this case.
> > >
> > > While there is only a single use case it is targeting two devices so
> > > that drove the need for a common interface withing rawdev.
> > >
> > > The advantage of using rawdev is that it allows this to be consumed
> > > through DPDK, which enables DPDK project consumers, but also leaves
> > > the door open to other contributors to have their say on how this
> > > should evolve. For example this exact process seems to be occurring
> > > with DMA engines in rawdev today, with a critical mass of
> > > implementations which now is giving the impetus to create a generic
> > > solution, as we would hope can occur here too in the future.
> > >
> > >
> > > >>>> And the common problem here is device exposure.
> > > >>>> With a specialised service on top.
> > > >>>>
> > > >>>>
> > > >>>>>>> Here the intent is to use rawdev because we don't find a good
> > API.
> > > >>>>>>> API defeat is a no-go.
> > > >>>>>>
> > > >>>>>> [Fiona] It's not that we haven't found a good API, but that
> > > >>>>>> there doesn't seem to be a general requirement for such a
> > > >>>>>> specialised API
> > > >>>>>
> > > >>>>> There is a requirement to combine features of different classes.
> > > >>>>
> > > >>>> [Fiona] Can you point me to that requirement please?
> > > >>>
> > > >>> Yes, rte_security is trying to address this exact issue.
> > > >>>
> > > >>
> > > >> I don't agree rte_security addresses the problem of different
> > > >> device types supporting the same services. The problem being
> > > >> addressed here is a single device which supports the chaining of
> > > >> multiple services (sym crypto & error detection)
> > > >
> > > > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> > >
> > > I wouldn't consider an inline crypto offload or full IPsec offload a
> > > chained operation in the vein being proposed here where completely
> > > independent services (in the view of DPDK which are currently on
> > > independent devices and APIs) are linked together.
> > >
> > > We did look at using rte_security here but it wasn't considered
> > > suitable for a chaining of non-crypto operations such as CRC or
> > > possibly compression in the future, as it would still run into the
> > > issue of having to use the cryptodev enq/deq API in the lookaside offload
> > case.
> > 
> > Because rte_security is not a generic solution (that's why I don't like it).
> > I think a good approach would be to check how to offload in HW the chaining
> > done in frameworks like rte_pipeline or rte_graph.
> > Stephen and Jerin already talked about it, but it was rejected by David,
> > because harder to implement I think.
> > Even worst, the team working on this patch did zero review of rte_graph.
> 
> [DC] The team working on this patch did review rte_graph and explained our reasoning
> to Jerin as to why we felt it was not suitable.

No, by review, I mean doing some comments in the rte_graph series
to help making the code, design or doc better.
You are complaining about the lack of attention to your patch,
that's why I point out that you didn't help other patches to progress.

> While Jerin explained it would be possible to combine 2 nodes as a single optimized node at runtime,
> he also agreed that it did NOT make sense to abstract what we are trying to do as a graph.

I think this exact point requires more discussion.

> Please see http://mails.dpdk.org/archives/dev/2020-March/159238.html

He did not say "it does not make sense" but
"In that way, it makes sense to not abstract as a graph."
This is slightly different.
Anyway, we should re-consider using a graph abstraction for chaining.

> > I think the chaining requirement is a real problem to solve, and it deserves a
> > good architecture and API.
> > Maybe this future API should be based on rte_graph.
Akhil Goyal April 22, 2020, 10:51 a.m. UTC | #15
Hi David,
> > >>
> > >> I don't agree rte_security addresses the problem of different device
> > >> types supporting the same services. The problem being addressed here
> > >> is a single device which supports the chaining of multiple services
> > >> (sym crypto & error detection)
> > >
> > > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> > >
> > I wouldn't consider an inline crypto offload or full IPsec offload a chained
> > operation in the vein being proposed here where completely independent
> > services (in the view of DPDK which are currently on independent devices
> > and APIs) are linked together.
> >
> > We did look at using rte_security here but it wasn't considered suitable for a
> > chaining of non-crypto operations such as CRC or possibly compression in the
> > future, as it would still run into the issue of having to use the cryptodev
> > enq/deq API in the lookaside offload case.
> >
> >
I did not look at your patches completely, but looking at the ops that you have added
For rawdev are pretty much same as that of a crypto device.

I see that there are 2 types of ops that you need
- session create/destroy
- enq/deq

On the first impression of your patchset, I see that you want to enq to driver only once for both
The operations - CRC and crypto.

So what is the issue in using the cryptodev_enqueue for processing in the existing AESNI-MB driver.
For session creation, the cryptodev layer will not give flexibility to add CRC+crypto kind of sessions.
But in case of rte_security, you can define your new session xform based on your requirement.

And while doing the cryptodev enq/deq, based on the session type, you can process the packet
Specific to your usecase in your aesni-mb PMD

Now if you want to add compression also along with crypto, then you can define another xform which
Will be combination of crypto+compression and the aesni-mb PMD can have another mode which
Can make sessions based on the new xform and the enq and deq can be done using the cryptodev enq/deq.
For all your cases you will be having only one action type - lookaside protocol and can define different
Protocols (that may not be standard).

So to conclude, your AESNI-MB will have 3 types of operations
- plain crypto
- crc+crypto
- compression+crypto

I believe this is doable or did I miss something very obvious?

Regards,
Akhil
David Coyle April 22, 2020, 1:17 p.m. UTC | #16
Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Wednesday, April 22, 2020 11:51 AM 
> Hi David,
> > > >>
> > > >> I don't agree rte_security addresses the problem of different
> > > >> device types supporting the same services. The problem being
> > > >> addressed here is a single device which supports the chaining of
> > > >> multiple services (sym crypto & error detection)
> > > >
> > > > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> > > >
> > > I wouldn't consider an inline crypto offload or full IPsec offload a
> > > chained operation in the vein being proposed here where completely
> > > independent services (in the view of DPDK which are currently on
> > > independent devices and APIs) are linked together.
> > >
> > > We did look at using rte_security here but it wasn't considered
> > > suitable for a chaining of non-crypto operations such as CRC or
> > > possibly compression in the future, as it would still run into the
> > > issue of having to use the cryptodev enq/deq API in the lookaside offload
> case.
> > >
> > >
> I did not look at your patches completely, but looking at the ops that you
> have added For rawdev are pretty much same as that of a crypto device.
> 
> I see that there are 2 types of ops that you need
> - session create/destroy
> - enq/deq
> 
> On the first impression of your patchset, I see that you want to enq to driver
> only once for both The operations - CRC and crypto.
> 
> So what is the issue in using the cryptodev_enqueue for processing in the
> existing AESNI-MB driver.
> For session creation, the cryptodev layer will not give flexibility to add
> CRC+crypto kind of sessions.
> But in case of rte_security, you can define your new session xform based on
> your requirement.
> 
> And while doing the cryptodev enq/deq, based on the session type, you can
> process the packet Specific to your usecase in your aesni-mb PMD
> 
> Now if you want to add compression also along with crypto, then you can
> define another xform which Will be combination of crypto+compression and
> the aesni-mb PMD can have another mode which Can make sessions based
> on the new xform and the enq and deq can be done using the cryptodev
> enq/deq.
> For all your cases you will be having only one action type - lookaside protocol
> and can define different Protocols (that may not be standard).
> 
> So to conclude, your AESNI-MB will have 3 types of operations
> - plain crypto
> - crc+crypto
> - compression+crypto
> 
> I believe this is doable or did I miss something very obvious?

[DC] Thank you for this feedback

I have done this exact same analysis on rte_security and how we could use it.

The main issue of this approach (and it may be possible to easily overcome) is that ultimately crypto_op's need
to be enqueued into cryptodev. This means we can't easily control the CRC (or compression in the future) at the
operation level - application developers using this API would create a Crypto+CRC security xform session  for a
particular flow but may want to turn off the CRC part for some packets in that flow.

There are a number of ways this issue could possibly be overcome:
1) the auth offset/length fields in a rte_crypto_op could be overloaded to control the CRC part of the combined operation
    - this is not the cleanest approach
2) we add a "security" op struct of some type to the union at end of the rte_crypto_op
    - to avoid any circular dependencies, this would need to be opaque to rte_cryptodev
    - rte_cryptodev should not be aware of rte_security

Number 2 above is probably the cleaner and more preferable approach.

The other approach is that CRC is either on/off at the session level. That limitation would then need to be adhered
by application developers, which is something we would ideally like to avoid.

The rawdev multi-function approach did not have these issues which is one of the reasons we have pursued this
approach to date.

However, we think the rte_security approach is workable.
It still requires some deeper analysis but with your support, we think we can overcome the challenges.

> 
> Regards,
> Akhil
Akhil Goyal April 22, 2020, 1:44 p.m. UTC | #17
Hi David,
> Hi Akhil,
> 
> > -----Original Message-----
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > Sent: Wednesday, April 22, 2020 11:51 AM
> > Hi David,
> > > > >>
> > > > >> I don't agree rte_security addresses the problem of different
> > > > >> device types supporting the same services. The problem being
> > > > >> addressed here is a single device which supports the chaining of
> > > > >> multiple services (sym crypto & error detection)
> > > > >
> > > > > Doing IPsec processing in Rx or Tx of a NIC is not chaining?
> > > > >
> > > > I wouldn't consider an inline crypto offload or full IPsec offload a
> > > > chained operation in the vein being proposed here where completely
> > > > independent services (in the view of DPDK which are currently on
> > > > independent devices and APIs) are linked together.
> > > >
> > > > We did look at using rte_security here but it wasn't considered
> > > > suitable for a chaining of non-crypto operations such as CRC or
> > > > possibly compression in the future, as it would still run into the
> > > > issue of having to use the cryptodev enq/deq API in the lookaside offload
> > case.
> > > >
> > > >
> > I did not look at your patches completely, but looking at the ops that you
> > have added For rawdev are pretty much same as that of a crypto device.
> >
> > I see that there are 2 types of ops that you need
> > - session create/destroy
> > - enq/deq
> >
> > On the first impression of your patchset, I see that you want to enq to driver
> > only once for both The operations - CRC and crypto.
> >
> > So what is the issue in using the cryptodev_enqueue for processing in the
> > existing AESNI-MB driver.
> > For session creation, the cryptodev layer will not give flexibility to add
> > CRC+crypto kind of sessions.
> > But in case of rte_security, you can define your new session xform based on
> > your requirement.
> >
> > And while doing the cryptodev enq/deq, based on the session type, you can
> > process the packet Specific to your usecase in your aesni-mb PMD
> >
> > Now if you want to add compression also along with crypto, then you can
> > define another xform which Will be combination of crypto+compression and
> > the aesni-mb PMD can have another mode which Can make sessions based
> > on the new xform and the enq and deq can be done using the cryptodev
> > enq/deq.
> > For all your cases you will be having only one action type - lookaside protocol
> > and can define different Protocols (that may not be standard).
> >
> > So to conclude, your AESNI-MB will have 3 types of operations
> > - plain crypto
> > - crc+crypto
> > - compression+crypto
> >
> > I believe this is doable or did I miss something very obvious?
> 
> [DC] Thank you for this feedback
> 
> I have done this exact same analysis on rte_security and how we could use it.
> 
> The main issue of this approach (and it may be possible to easily overcome) is
> that ultimately crypto_op's need
> to be enqueued into cryptodev. This means we can't easily control the CRC (or
> compression in the future) at the
> operation level - application developers using this API would create a
> Crypto+CRC security xform session  for a
> particular flow but may want to turn off the CRC part for some packets in that
> flow.
> 
> There are a number of ways this issue could possibly be overcome:
> 1) the auth offset/length fields in a rte_crypto_op could be overloaded to
> control the CRC part of the combined operation
>     - this is not the cleanest approach
> 2) we add a "security" op struct of some type to the union at end of the
> rte_crypto_op
>     - to avoid any circular dependencies, this would need to be opaque to
> rte_cryptodev
>     - rte_cryptodev should not be aware of rte_security
> 
> Number 2 above is probably the cleaner and more preferable approach.

Yes, it is preferred, but it should be a union to rte_crypto_sym_op/rte_crypto_asym_op.
Crypto_op->type as RTE_CRYPTO_OP_TYPE_SECURITY and sess_type as
RTE_CRYPTO_OP_SECURITY_SESSION
The size of rte_crypto_op will remain as is and there will be no ABI breakage I guess.

One more thing that can be looked into is the recently added CPU crypto process API
If that could of any use, we may extend that if need be.

> 
> The other approach is that CRC is either on/off at the session level. That
> limitation would then need to be adhered
> by application developers, which is something we would ideally like to avoid.

You mean that CRC can be on/off per session as well as per packet?
I think that can also be handled when you are defining your own security_op for per packet.

> 
> The rawdev multi-function approach did not have these issues which is one of
> the reasons we have pursued this
> approach to date.
> 
> However, we think the rte_security approach is workable.
> It still requires some deeper analysis but with your support, we think we can
> overcome the challenges.
> 
Yes, please let me know where ever my help is required.
Kevin Traynor April 22, 2020, 2:01 p.m. UTC | #18
Hi David,

On 21/04/2020 18:23, Coyle, David wrote:
> Thank you Thomas for your input.
> 
> We would like to request that the Tech-Board (CC'ed) also review the proposal to help us reach a consensus.
>  

The discussion on the mailing list still looks active and I think that's
where it should continue until there is no reasonable hope of consensus.
I'm not sure discussing over irc at TB will find a better technical
solution.

> If the current proposal is not acceptable, we would welcome feedback from the board on how to rework our
> proposal to something that would be acceptable.
>  
> For the benefit of the Tech-Board here is the back-ground to our proposal for Rawdev-based multi-function
> processing:
> - The primary objective is to support the AESNI MB combined Crypto-CRC processing capability in DPDK and
>    in future to add support for combined Crypto-CRC support in QAT.
> - The cryptodev API was considered unsuitable because CRC is not a cryptographic operation, and this would
>    also preclude other non-crypto operations in the future such as compression.
> - The rte_security API was also not considered suitable for chaining of non-crypto operations such as CRC,
>    as Declan pointed out below.
> - A new Accelerator API was proposed as an RFC but was not pursued due to community feedback that a
>    new API would not be welcome for a single use-case.
> - Using Rawdev for multi-function processing was then proposed and, initially, as there was no opposition
>    we implemented a patch-set for this approach.
>  
> It was considered that a Rawdev-based multi-function approach would be suitable for the following reasons:
> 1) Multi-function processing for Crypto-CRC cases is not a good fit for any of the existing DPDK classes.
> 2) Rawdev was intended for such specialized acceleration processing that are not a good fit for existing DPDK
>      classes.
> 3) Rawdev was also intended as somewhere that new use-cases like this could be prototyped and developed,
>      such as Declan mentions below
> 4) The Rawdev-based multi-function proposal is extensible and we would hope that it can evolve to support
>      new use-cases and target new devices in the future with the communities involvement.
> 

This is a useful summary and explaining your approach but it doesn't
mention the counter arguments, so it doesn't seem balanced. Of course
people can read that in the ML thread.

Kevin.

> 
>> -----Original Message-----
>> From: Doherty, Declan <declan.doherty@intel.com>
>> Sent: Tuesday, April 21, 2020 5:46 PM
>>
>> On 15/04/2020 11:33 PM, Thomas Monjalon wrote:
>>> 16/04/2020 00:19, Doherty, Declan:
>>>> On 14/04/2020 3:44 PM, Thomas Monjalon wrote:
>>>>> 14/04/2020 16:02, Trahe, Fiona:
>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>> 14/04/2020 15:04, Trahe, Fiona:
>>>>>>>>> 14/04/2020 12:21, Ferruh Yigit:
>>>>>>>>>
>>>>>>>
>> http://inbox.dpdk.org/dev/MN2PR11MB35507D4B96677A41E66440C5E3C30
>> @M
>>>>>>> N2PR11MB3550.na
>>>>>>>>> mprd11.prod.outlook.com/
>>>>>>>>>
>>>>>>>>> I am not convinced.
>>>>>>>>> I don't like rawdev in general.
>>>>>>>>> Rawdev is good only for hardware support which cannot be generic
>>>>>>>>> like SoC, FPGA management or DMA engine.
>>>>>>>>
>>>>>>>> [Fiona] CRC and BIP are not crypto algorithms, they are error
>> detection processes.
>>>>>>>> So there is no class in DPDK that these readily fit into.
>>>>>>>> There was resistance to adding another xxxddev, and even if one
>>>>>>>> had been added for error_detection_dev, there would still have
>>>>>>>> been another layer needed to couple this with cryptodev. Various
>>>>>>>> proposals for this have been discussed on the ML in RFC and recent
>> patches, there doesn't seem to be an appetite for this as a generic API.
>>>>>>>> So it seems that only Intel has software and hardware engines
>>>>>>>> that provide this specialised feature coupling. In that case
>>>>>>>> rawdev seems like the most appropriate vehicle to expose this.
>>>>>>>
>>>>>>> Adding some vendor-specific API is not a good answer.
>>>>>>> It will work in some cases, but it won't make DPDK better.
>>>>>>> What's the purpose of DPDK if it's not solving a common problem
>>>>>>> for different hardware?
>>>>>>
>>>> The current proposal in rawdev could easily be supported by any
>>>> hardware which supports chaining multiple functions/services into a
>>>> single operation, in this case symmetric crypto and error detection,
>>>> but it could conceivably support chaining symmetric/asymmetric crypto
>>>> operations or chaining symmetric crypto and compression operations.
>>>>
>>>>>> [Fiona] Based on that logic rawdev should be deprecated.
>>>>>> But the community has agreed that it has a place.
>>>>>
>>>>> No, as I said above, rawdev is good for SoC, FPGA management or DMA
>> engine.
>>>>
>>>> I distinctly remember when rawdev was being proposed one of the uses
>>>> cases proposed was that a new classes of APIs could be prototyped and
>>>> developed under rawdev and when a solid consensus was reached then
>>>> migrated to a mainstream DPDK library. I think every effort has been
>>>> made here to engage the community to develop a generic approach. As
>>>> Fiona notes there hasn't really been much of an appetite for this.
>>>>
>>>> Therefore I think the option to use rawdev makes sense, it allows an
>>>> initial proposal to be deployed,  without a generic solution
>>>> agreement, it will also give others in the community to see how this
>>>> approach can work and hopefully lead to more engagement on a generic
>>>> solution. Also as APIs in rawdev are essentially treated as private
>>>> APIs the onus is on Intel to support this going forward.
>>>
>>> Because hardware support is pending,
>>> we should accept an Intel-only "temporary" solution, opening the door
>>> to more vendor-specific APIs?
>>>
>>> What is the benefit for the DPDK project?
>>>
>> Sorry I don't agree with this sentiment, David has made every attempt to
>> solicit feedback and to engage the community in this.
>>
>> I also don't agree in classifying this as a "temporary solution" as this is a solid
>> proposal for an approach to chaining multiple operations together, but I
>> guess the fact remains that we only currently have a single use-case, but it is
>> difficult to generate a generic solution in this case.
>>
>> While there is only a single use case it is targeting two devices so that drove
>> the need for a common interface within rawdev.
>>
>> The advantage of using rawdev is that it allows this to be consumed through
>> DPDK, which enables DPDK project consumers, but also leaves the door open
>> to other contributors to have their say on how this should evolve. For
>> example this exact process seems to be occurring with DMA engines in
>> rawdev today, with a critical mass of implementations which now is giving the
>> impetus to create a generic solution, as we would hope can occur here too in
>> the future.
>>
>>
>>>>>> And the common problem here is device exposure.
>>>>>> With a specialised service on top.
>>>>>>
>>>>>>
>>>>>>>>> Here the intent is to use rawdev because we don't find a good API.
>>>>>>>>> API defeat is a no-go.
>>>>>>>>
>>>>>>>> [Fiona] It's not that we haven't found a good API, but that there
>>>>>>>> doesn't seem to be a general requirement for such a specialised
>>>>>>>> API
>>>>>>>
>>>>>>> There is a requirement to combine features of different classes.
>>>>>>
>>>>>> [Fiona] Can you point me to that requirement please?
>>>>>
>>>>> Yes, rte_security is trying to address this exact issue.
>>>>>
>>>>
>>>> I don't agree rte_security addresses the problem of different device
>>>> types supporting the same services. The problem being addressed here
>>>> is a single device which supports the chaining of multiple services
>>>> (sym crypto & error detection)
>>>
>>> Doing IPsec processing in Rx or Tx of a NIC is not chaining?
>>>
>> I wouldn't consider an inline crypto offload or full IPsec offload a chained
>> operation in the vein being proposed here where completely independent
>> services (in the view of DPDK which are currently on independent devices
>> and APIs) are linked together.
>>
>> We did look at using rte_security here but it wasn't considered suitable for a
>> chaining of non-crypto operations such as CRC or possibly compression in the
>> future, as it would still run into the issue of having to use the cryptodev
>> enq/deq API in the lookaside offload case.
>>
>>
>>>>>> We suggested it, but did not get community engagement and have
>>>>>> dropped our generic API requirement, instead focussing on this
>> specialised case.
>>>>>
>>>>> I did not see such generic proposal, sorry.
>>>>>
>>>>>>> In the past, rte_security was an "answer" to this issue with crypto and
>> ethdev.
>>>>>>> This is a real topic, please let's find a generic elegant solution.
>>>
>>>
>>>
David Coyle April 22, 2020, 2:21 p.m. UTC | #19
Hi Akhil

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Wednesday, April 22, 2020 2:44 PM
> 
> Hi David,
> > Hi Akhil,
> >
> > > > >
> > > I did not look at your patches completely, but looking at the ops
> > > that you have added For rawdev are pretty much same as that of a crypto
> device.
> > >
> > > I see that there are 2 types of ops that you need
> > > - session create/destroy
> > > - enq/deq
> > >
> > > On the first impression of your patchset, I see that you want to enq
> > > to driver only once for both The operations - CRC and crypto.
> > >
> > > So what is the issue in using the cryptodev_enqueue for processing
> > > in the existing AESNI-MB driver.
> > > For session creation, the cryptodev layer will not give flexibility
> > > to add
> > > CRC+crypto kind of sessions.
> > > But in case of rte_security, you can define your new session xform
> > > based on your requirement.
> > >
> > > And while doing the cryptodev enq/deq, based on the session type,
> > > you can process the packet Specific to your usecase in your aesni-mb
> > > PMD
> > >
> > > Now if you want to add compression also along with crypto, then you
> > > can define another xform which Will be combination of
> > > crypto+compression and the aesni-mb PMD can have another mode
> which
> > > Can make sessions based on the new xform and the enq and deq can be
> > > done using the cryptodev enq/deq.
> > > For all your cases you will be having only one action type -
> > > lookaside protocol and can define different Protocols (that may not be
> standard).
> > >
> > > So to conclude, your AESNI-MB will have 3 types of operations
> > > - plain crypto
> > > - crc+crypto
> > > - compression+crypto
> > >
> > > I believe this is doable or did I miss something very obvious?
> >
> > [DC] Thank you for this feedback
> >
> > I have done this exact same analysis on rte_security and how we could use
> it.
> >
> > The main issue of this approach (and it may be possible to easily
> > overcome) is that ultimately crypto_op's need to be enqueued into
> > cryptodev. This means we can't easily control the CRC (or compression
> > in the future) at the operation level - application developers using
> > this API would create a
> > Crypto+CRC security xform session  for a
> > particular flow but may want to turn off the CRC part for some packets
> > in that flow.
> >
> > There are a number of ways this issue could possibly be overcome:
> > 1) the auth offset/length fields in a rte_crypto_op could be
> > overloaded to control the CRC part of the combined operation
> >     - this is not the cleanest approach
> > 2) we add a "security" op struct of some type to the union at end of
> > the rte_crypto_op
> >     - to avoid any circular dependencies, this would need to be opaque
> > to rte_cryptodev
> >     - rte_cryptodev should not be aware of rte_security
> >
> > Number 2 above is probably the cleaner and more preferable approach.
> 
> Yes, it is preferred, but it should be a union to
> rte_crypto_sym_op/rte_crypto_asym_op.
> Crypto_op->type as RTE_CRYPTO_OP_TYPE_SECURITY and sess_type as
> RTE_CRYPTO_OP_SECURITY_SESSION The size of rte_crypto_op will remain
> as is and there will be no ABI breakage I guess.

[DC]  Yes we would add to this union at the end of rte_crypto_op

        __extension__
        union {
                struct rte_crypto_sym_op sym[0];
                /**< Symmetric operation parameters */

                struct rte_crypto_asym_op asym[0];
                /**< Asymmetric operation parameters */

        }; /**< operation specific parameters */

I haven't figured out the finer details yet, but it should be straightforward to add some security element here.
As these are zero length arrays, we won't be affecting the size of rte_crypto_op if we add another zero length array.

We should not include rte_security.h and add something like struct rte_security_op sec[0] here though, as that would
cause a circular dependency between rte_cryptodev and rte_security.
This should be resolvable though

> 
> One more thing that can be looked into is the recently added CPU crypto
> process API If that could of any use, we may extend that if need be.

[DC] This is also being targeted at QAT and we would like to maintain the same
Interface for these use-cases for both AESNI-MB and QAT.

So I think the traditional enqueue/dequeue API is what we would initially use as it
means users of this API can easily switch between AESNI-MB and QAT. However, we
may look at the CPU crypto API for AESNI-MB in the future.

> 
> >
> > The other approach is that CRC is either on/off at the session level.
> > That limitation would then need to be adhered by application
> > developers, which is something we would ideally like to avoid.
> 
> You mean that CRC can be on/off per session as well as per packet?
> I think that can also be handled when you are defining your own security_op
> for per packet.

[DC] I meant that if we didn't take the approach defining a security_op, then
we would have turn on/off CRC at the session level and impose that limit on
the app developers. But yes, by defining a security_op, we can probably turn
it on/off at both session and op level.

> 
> >
> > The rawdev multi-function approach did not have these issues which is
> > one of the reasons we have pursued this approach to date.
> >
> > However, we think the rte_security approach is workable.
> > It still requires some deeper analysis but with your support, we think
> > we can overcome the challenges.
> >
> Yes, please let me know where ever my help is required.
[DC] Thank you, appreciate that
David Coyle April 22, 2020, 2:41 p.m. UTC | #20
Hi Kevin,

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Wednesday, April 22, 2020 3:02 PM
> 
> Hi David,
> 
> On 21/04/2020 18:23, Coyle, David wrote:
> > Thank you Thomas for your input.
> >
> > We would like to request that the Tech-Board (CC'ed) also review the
> proposal to help us reach a consensus.
> >
> 
> The discussion on the mailing list still looks active and I think that's where it
> should continue until there is no reasonable hope of consensus.
> I'm not sure discussing over irc at TB will find a better technical solution.

[DC] Yes, there has been some further proposals and discussions today, mainly around the use
of rte_security. Although there are a few challenges to work around here, it is an approach we
had already considered and it would seem like a good compromise to the rawdev approach. 

With that in mind, we are happy to let this play out further on the mailing list for now.

If we can reach consensus on using rte_security, then we are happy to go with that and investigate
and develop it further.

> 
> > If the current proposal is not acceptable, we would welcome feedback
> > from the board on how to rework our proposal to something that would be
> acceptable.
> >
> > For the benefit of the Tech-Board here is the back-ground to our
> > proposal for Rawdev-based multi-function
> > processing:
> > - The primary objective is to support the AESNI MB combined Crypto-CRC
> processing capability in DPDK and
> >    in future to add support for combined Crypto-CRC support in QAT.
> > - The cryptodev API was considered unsuitable because CRC is not a
> cryptographic operation, and this would
> >    also preclude other non-crypto operations in the future such as
> compression.
> > - The rte_security API was also not considered suitable for chaining of non-
> crypto operations such as CRC,
> >    as Declan pointed out below.
> > - A new Accelerator API was proposed as an RFC but was not pursued due
> to community feedback that a
> >    new API would not be welcome for a single use-case.
> > - Using Rawdev for multi-function processing was then proposed and,
> initially, as there was no opposition
> >    we implemented a patch-set for this approach.
> >
> > It was considered that a Rawdev-based multi-function approach would be
> suitable for the following reasons:
> > 1) Multi-function processing for Crypto-CRC cases is not a good fit for any of
> the existing DPDK classes.
> > 2) Rawdev was intended for such specialized acceleration processing that
> are not a good fit for existing DPDK
> >      classes.
> > 3) Rawdev was also intended as somewhere that new use-cases like this
> could be prototyped and developed,
> >      such as Declan mentions below
> > 4) The Rawdev-based multi-function proposal is extensible and we would
> hope that it can evolve to support
> >      new use-cases and target new devices in the future with the
> communities involvement.
> >
> 
> This is a useful summary and explaining your approach but it doesn't mention
> the counter arguments, so it doesn't seem balanced. Of course people can
> read that in the ML thread.

[DC] That is a fair point, and something we will keep in mind for the future if
we need to come back to the tech-board.

> 
> Kevin.
>
Zhang, Roy Fan May 1, 2020, 1:18 p.m. UTC | #21
Hi Akhil,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Akhil Goyal
> Sent: Wednesday, April 22, 2020 2:44 PM
> To: Coyle, David <david.coyle@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Yigit, Ferruh <ferruh.yigit@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> Cc: techboard@dpdk.org; dev@dpdk.org; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Ryan, Brendan
> <brendan.ryan@intel.com>; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Anoob Joseph <anoobj@marvell.com>; Ruifeng Wang
> <ruifeng.wang@arm.com>; Liron Himi <lironh@marvell.com>; Nagadheeraj
> Rottela <rnagadheeraj@marvell.com>; Srikanth Jampala
> <jsrikanth@marvell.com>; Gagandeep Singh <G.Singh@nxp.com>; Jay Zhou
> <jianjay.zhou@huawei.com>; Ravi Kumar <ravi1.kumar@amd.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; olivier.matz@6wind.com;
> honnappa.nagarahalli@arm.com; Stephen Hemminger
> <stephen@networkplumber.org>; alexr@mellanox.com
> Subject: Re: [dpdk-dev] [PATCH v3 0/4] add AESNI-MB rawdev for multi-
> function processing
...
> Yes, it is preferred, but it should be a union to
> rte_crypto_sym_op/rte_crypto_asym_op.
> Crypto_op->type as RTE_CRYPTO_OP_TYPE_SECURITY and sess_type as
> RTE_CRYPTO_OP_SECURITY_SESSION
> The size of rte_crypto_op will remain as is and there will be no ABI breakage I
> guess.
> 
[Fan: with this way the PMD will have to do rte_crypto_op.type check, and then look into rte_security_op field, only when it find the security_op type is crypto_crc, it will process the security_op data. Would that being too many reads and checking for a single op? Can we create a new API for rte_security to process rte_security_ops for Crypto_CRC or future needs?]
...
 
Regards,
Fan
David Coyle May 12, 2020, 5:32 p.m. UTC | #22
Hi Fan & Akhil,

> -----Original Message-----
> From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Sent: Friday, May 1, 2020 2:18 PM
> 
> Hi Akhil,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Akhil Goyal
> > Sent: Wednesday, April 22, 2020 2:44 PM
> > To: Coyle, David <david.coyle@intel.com>; Doherty, Declan
> > <declan.doherty@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>;
> > Yigit, Ferruh <ferruh.yigit@intel.com>; Trahe, Fiona
> > <fiona.trahe@intel.com>
> > Cc: techboard@dpdk.org; dev@dpdk.org; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; Ryan, Brendan
> > <brendan.ryan@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>;
> > Anoob Joseph <anoobj@marvell.com>; Ruifeng Wang
> > <ruifeng.wang@arm.com>; Liron Himi <lironh@marvell.com>; Nagadheeraj
> > Rottela <rnagadheeraj@marvell.com>; Srikanth Jampala
> > <jsrikanth@marvell.com>; Gagandeep Singh <G.Singh@nxp.com>; Jay
> Zhou
> > <jianjay.zhou@huawei.com>; Ravi Kumar <ravi1.kumar@amd.com>;
> > Richardson, Bruce <bruce.richardson@intel.com>;
> > olivier.matz@6wind.com; honnappa.nagarahalli@arm.com; Stephen
> > Hemminger <stephen@networkplumber.org>; alexr@mellanox.com
> > Subject: Re: [dpdk-dev] [PATCH v3 0/4] add AESNI-MB rawdev for multi-
> > function processing
> ...
> > Yes, it is preferred, but it should be a union to
> > rte_crypto_sym_op/rte_crypto_asym_op.
> > Crypto_op->type as RTE_CRYPTO_OP_TYPE_SECURITY and sess_type as
> > RTE_CRYPTO_OP_SECURITY_SESSION The size of rte_crypto_op will remain
> > as is and there will be no ABI breakage I guess.
> >
> [Fan: with this way the PMD will have to do rte_crypto_op.type check, and
> then look into rte_security_op field, only when it find the security_op type is
> crypto_crc, it will process the security_op data. Would that being too many
> reads and checking for a single op? Can we create a new API for rte_security
> to process rte_security_ops for Crypto_CRC or future needs?] ...

[DC] If we were to add new enqueue/dequeue APIs to rte_security, then this may
cause extra churn and extra paths of code in a customer's application. For the
DOCSIS Crypto-CRC use-case which is currently supported by IPSecMB, only the
AES-DOCSISBPI cipher algorithm is supported. For these Crypto-CRC ops, they would
create rte_security sessions, attach these to rte_security_ops and enqueue/dequeue
using the new APIs in rte_security.

However, the customer may also be using the legacy DES-DOCSISBPI cipher algorithm
for some subscribers, and this algorithm is not supported in the chained Crypto-CRC
functionality in IPSecMB (and most likely never will be). So for these the customer
would need to create cryptodev sessions, attach these to rte_crypto_ops and enqueue/
dequeue with the cryptodev enq/deq APIs. That is 2 different paths of code now in
the application datapath, where some packets in a batch need to be enqueued through
rte_security and some need to be enqueued through cryptodev.

If rte_crypto_ops are always used and enqueued/dequeued through cryptodev, then the
only thing that changes is the type of session that is created and either the security session
or the cryptodev session gets attached to the crypto_op.

Now, we could add support to rte_security for DES-DOCSISBPI too, but it would not be a
combined operation with CRC - it would be a simple cipher operation going through
rte_security. But that, to me, does not seem like a good use of rte_security.

For DOCSIS Crypto-CRC, we may also want to take advantage of the
rte_cryptodev_sym_cpu_crypto_process() API which was added to cryptodev recently to
avoid the enqueue/dequeue overhead. A similar API would also then need to be added
to rte_security.

Taking all of the above into account, I feel keeping the normal cryptodev enqueue/dequeue
would be best. Having said all that, we do need to consider performance in the PMD of the
extra op type checks. Take aesni_mb PMD as an example. It would need to check
rte_crypto_op->type and if it's not RTE_CRYPTO_OP_TYPE_SECURITY, then it can assume
it's an RTE_CRYPTO_OP_TYPE_SYMMETRIC op and carry on as normal for existing symmetric
operations. Security ops will need some extra parsing but this is new functionality. The impact
on existing functionality of the extra checks would certainly need to be tested though, but as
all the op data will be in the same cache line, I don't see any major impact.

Akhil & Fan (& others), I would be interested to hear your feedback on this.

Regards,
David

> 
> Regards,
> Fan