Re: [dpdk-dev] [PATCH 1/3] security: support pdcp protocol

["Joseph, Anoob" <Anoob.Joseph@caviumnetworks.com>]

Hi Akhil,

Please see inline.

Thanks,
Anoob

On 28-08-2018 18:31, akhil.goyal@nxp.com wrote:
> External Email
>
> From: Akhil Goyal <akhil.goyal@nxp.com>
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> ---
>   doc/guides/prog_guide/rte_security.rst | 90 ++++++++++++++++++++++++--
>   lib/librte_security/rte_security.c     |  4 ++
>   lib/librte_security/rte_security.h     | 62 ++++++++++++++++++
>   3 files changed, 149 insertions(+), 7 deletions(-)
>
> diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
> index 0812abe77..412fff016 100644
> --- a/doc/guides/prog_guide/rte_security.rst
> +++ b/doc/guides/prog_guide/rte_security.rst
> @@ -10,8 +10,8 @@ The security library provides a framework for management and provisioning
>   of security protocol operations offloaded to hardware based devices. The
>   library defines generic APIs to create and free security sessions which can
>   support full protocol offload as well as inline crypto operation with
> -NIC or crypto devices. The framework currently only supports the IPSec protocol
> -and associated operations, other protocols will be added in future.
> +NIC or crypto devices. The framework currently only supports the IPSec and PDCP
> +protocol and associated operations, other protocols will be added in future.
>
>   Design Principles
>   -----------------
> @@ -253,6 +253,46 @@ for any protocol header addition.
>           +--------|--------+
>                    V
>
> +PDCP Flow Diagram
> +~~~~~~~~~~~~~~~~~
> +
> +.. code-block:: c
> +
> +        Transmitting PDCP Entity          Receiving PDCP Entity
> +                  |                                   ^
> +                  |                       +-----------|-----------+
> +                  V                       | In order delivery and |
> +        +---------|----------+            | Duplicate detection   |
> +        | Sequence Numbering |            |  (Data Plane only)    |
> +        +---------|----------+            +-----------|-----------+
> +                  |                                   |
> +        +---------|----------+            +-----------|----------+
> +        | Header Compression*|            | Header Decompression*|
> +        | (Data-Plane only)  |            |   (Data Plane only)  |
> +        +---------|----------+            +-----------|----------+
> +                  |                                   |
> +        +---------|-----------+           +-----------|----------+
> +        | Integrity Protection|           |Integrity Verification|
> +        | (Control Plane only)|           | (Control Plane only) |
> +        +---------|-----------+           +-----------|----------+
> +        +---------|-----------+            +----------|----------+
> +        |     Ciphering       |            |     Deciphering     |
> +        +---------|-----------+            +----------|----------+
> +        +---------|-----------+            +----------|----------+
> +        |   Add PDCP header   |            | Remove PDCP Header  |
> +        +---------|-----------+            +----------|----------+
> +                  |                                   |
> +                  +----------------->>----------------+
> +
[Anoob] Which PDCP specification revision is this based on? In the 5G 
specification, even data-plane may undergo integrity protection.
> +
> +.. note::
> +
> +    * Header Compression and decompression are not supported currently.
> +
> +Just like IPSec, in case of PDCP also header addition/deletion, cipher/
> +de-cipher, integrity protection/verification is done based on the action
> +type chosen.
> +
>   Device Features and Capabilities
>   ---------------------------------
>
> @@ -271,7 +311,7 @@ structure in the *DPDK API Reference*.
>
>   Each driver (crypto or ethernet) defines its own private array of capabilities
>   for the operations it supports. Below is an example of the capabilities for a
> -PMD which supports the IPSec protocol.
> +PMD which supports the IPSec and PDCP protocol.
>
>   .. code-block:: c
>
> @@ -298,6 +338,22 @@ PMD which supports the IPSec protocol.
>                   },
>                   .crypto_capabilities = pmd_capabilities
>           },
> +        { /* PDCP Lookaside Protocol offload Data Plane */
> +                .action = RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL,
> +                .protocol = RTE_SECURITY_PROTOCOL_PDCP,
> +                .pdcp = {
> +                        .domain = RTE_SECURITY_PDCP_MODE_DATA,
> +                },
> +                .crypto_capabilities = pmd_capabilities
> +        },
> +        { /* PDCP Lookaside Protocol offload Control */
> +                .action = RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL,
> +                .protocol = RTE_SECURITY_PROTOCOL_PDCP,
> +                .pdcp = {
> +                        .domain = RTE_SECURITY_PDCP_MODE_CONTROL,
> +                },
> +                .crypto_capabilities = pmd_capabilities
> +        },
>           {
>                   .action = RTE_SECURITY_ACTION_TYPE_NONE
>           }
> @@ -429,6 +485,7 @@ Security Session configuration structure is defined as ``rte_security_session_co
>           union {
>                   struct rte_security_ipsec_xform ipsec;
>                   struct rte_security_macsec_xform macsec;
> +                struct rte_security_pdcp_xform pdcp;
>           };
>           /**< Configuration parameters for security session */
>           struct rte_crypto_sym_xform *crypto_xform;
> @@ -463,15 +520,17 @@ The ``rte_security_session_protocol`` is defined as
>   .. code-block:: c
>
>       enum rte_security_session_protocol {
> -        RTE_SECURITY_PROTOCOL_IPSEC,
> +        RTE_SECURITY_PROTOCOL_IPSEC = 1,
>           /**< IPsec Protocol */
>           RTE_SECURITY_PROTOCOL_MACSEC,
>           /**< MACSec Protocol */
> +        RTE_SECURITY_PROTOCOL_PDCP,
> +        /**< PDCP Protocol */
>       };
>
> -Currently the library defines configuration parameters for IPSec only. For other
> -protocols like MACSec, structures and enums are defined as place holders which
> -will be updated in the future.
> +Currently the library defines configuration parameters for IPSec and PDCP only.
> +For other protocols like MACSec, structures and enums are defined as place holders
> +which will be updated in the future.
>
>   IPsec related configuration parameters are defined in ``rte_security_ipsec_xform``
>
> @@ -494,6 +553,23 @@ IPsec related configuration parameters are defined in ``rte_security_ipsec_xform
>           /**< Tunnel parameters, NULL for transport mode */
>       };
>
> +PDCP related configuration parameters are defined in ``rte_security_pdcp_xform``
> +
> +.. code-block:: c
> +
> +    struct rte_security_pdcp_xform {
> +        int8_t bearer; /**< PDCP bearer ID */
> +        enum rte_security_pdcp_domain domain;
> +        /** < PDCP mode of operation: Control or data */
> +        enum rte_security_pdcp_direction pkt_dir;
> +        /**< PDCP Frame Direction 0:UL 1:DL */
> +        enum rte_security_pdcp_sn_size sn_size;
> +        /**< Sequence number size, 5/7/12/15 */
> +        int8_t hfn_ovd; /**< Overwrite HFN per operation */
> +        uint32_t hfn;  /**< Hyper Frame Number */
> +        uint32_t hfn_threshold;        /**< HFN Threashold for key renegotiation */
> +    };
> +
[Anoob] PDCP packet ordering should be both a capability and a setting.
HFN will be incremented overtime and starts at 0. So why is it part of 
the xform?

Also the hfn_ovd is per operation. So why is it part of xform? Is it a 
boolean value? If so, where does the HFN for each operation come from?
>
>   Security API
>   ~~~~~~~~~~~~
> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
> index 1954960a5..c6355de95 100644
> --- a/lib/librte_security/rte_security.c
> +++ b/lib/librte_security/rte_security.c
> @@ -131,6 +131,10 @@ rte_security_capability_get(struct rte_security_ctx *instance,
>                                          capability->ipsec.direction ==
>                                                          idx->ipsec.direction)
>                                          return capability;
> +                       } else if (idx->protocol == RTE_SECURITY_PROTOCOL_PDCP) {
> +                               if (capability->pdcp.domain ==
> +                                                       idx->pdcp.domain)
> +                                       return capability;
>                          }
>                  }
>          }
> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
> index b0d1b97ee..e625bc656 100644
> --- a/lib/librte_security/rte_security.h
> +++ b/lib/librte_security/rte_security.h
> @@ -206,6 +206,52 @@ struct rte_security_macsec_xform {
>          int dummy;
>   };
>
> +/**
> + * PDCP Mode of session
> + */
> +enum rte_security_pdcp_domain {
> +       RTE_SECURITY_PDCP_MODE_CONTROL, /**< PDCP control plane */
> +       RTE_SECURITY_PDCP_MODE_DATA,    /**< PDCP data plane */
> +};
> +
> +/** PDCP Frame direction */
> +enum rte_security_pdcp_direction {
> +       RTE_SECURITY_PDCP_UPLINK,       /**< Uplink */
> +       RTE_SECURITY_PDCP_DOWNLINK,     /**< Downlink */
> +};
> +
> +/**
> + * PDCP Sequence Number Size selectors
> + * @PDCP_SN_SIZE_5: 5bit sequence number
> + * @PDCP_SN_SIZE_7: 7bit sequence number
> + * @PDCP_SN_SIZE_12: 12bit sequence number
> + * @PDCP_SN_SIZE_15: 15bit sequence number
> + */
> +enum rte_security_pdcp_sn_size {
> +       RTE_SECURITY_PDCP_SN_SIZE_5 = 5,
> +       RTE_SECURITY_PDCP_SN_SIZE_7 = 7,
> +       RTE_SECURITY_PDCP_SN_SIZE_12 = 12,
> +       RTE_SECURITY_PDCP_SN_SIZE_15 = 15
> +};
[Anoob] SN size 18 is also possible
> +
> +/**
> + * PDCP security association configuration data.
> + *
> + * This structure contains data required to create a PDCP security session.
> + */
> +struct rte_security_pdcp_xform {
> +       int8_t bearer;  /**< PDCP bearer ID */
> +       enum rte_security_pdcp_domain domain;
> +               /** < PDCP mode of operation: Control or data */
> +       enum rte_security_pdcp_direction pkt_dir;
> +               /**< PDCP Frame Direction 0:UL 1:DL */
> +       enum rte_security_pdcp_sn_size sn_size;
> +               /**< Sequence number size, 5/7/12/15 */
> +       int8_t hfn_ovd; /**< Overwrite HFN per operation */
> +       uint32_t hfn;   /**< Hyper Frame Number */
> +       uint32_t hfn_threshold; /**< HFN Threashold for key renegotiation */
> +};
> +
>   /**
>    * Security session action type.
>    */
> @@ -232,6 +278,8 @@ enum rte_security_session_protocol {
>          /**< IPsec Protocol */
>          RTE_SECURITY_PROTOCOL_MACSEC,
>          /**< MACSec Protocol */
> +       RTE_SECURITY_PROTOCOL_PDCP,
> +       /**< PDCP Protocol */
>   };
>
>   /**
> @@ -246,6 +294,7 @@ struct rte_security_session_conf {
>          union {
>                  struct rte_security_ipsec_xform ipsec;
>                  struct rte_security_macsec_xform macsec;
> +               struct rte_security_pdcp_xform pdcp;
>          };
>          /**< Configuration parameters for security session */
>          struct rte_crypto_sym_xform *crypto_xform;
> @@ -413,6 +462,10 @@ struct rte_security_ipsec_stats {
>
>   };
>
> +struct rte_security_pdcp_stats {
> +       uint64_t reserved;
> +};
> +
>   struct rte_security_stats {
>          enum rte_security_session_protocol protocol;
>          /**< Security protocol to be configured */
> @@ -421,6 +474,7 @@ struct rte_security_stats {
>          union {
>                  struct rte_security_macsec_stats macsec;
>                  struct rte_security_ipsec_stats ipsec;
> +               struct rte_security_pdcp_stats pdcp;
>          };
>   };
>
> @@ -465,6 +519,11 @@ struct rte_security_capability {
>                          int dummy;
>                  } macsec;
>                  /**< MACsec capability */
> +               struct {
> +                       enum rte_security_pdcp_domain domain;
> +                       /** < PDCP mode of operation: Control or data */
> +               } pdcp;
> +               /**< PDCP capability */
>          };
>
>          const struct rte_cryptodev_capabilities *crypto_capabilities;
> @@ -506,6 +565,9 @@ struct rte_security_capability_idx {
>                          enum rte_security_ipsec_sa_mode mode;
>                          enum rte_security_ipsec_sa_direction direction;
>                  } ipsec;
> +               struct {
> +                       enum rte_security_pdcp_domain domain;
> +               } pdcp;
>          };
>   };
>
> --
> 2.17.1
>