[dpdk-dev] [PATCH 06/11] ethdev: extend ethdev to support security APIs

Shahaf Shuler shahafs at mellanox.com
Sun Sep 17 15:45:06 CEST 2017


Hi Declan,

Thursday, September 14, 2017 11:27 AM, Akhil Goyal:
> 
> From: Declan Doherty <declan.doherty at intel.com>
> 
> rte_flow_action type and ethdev updated to support rte_security sessions
> for crypto offload to ethernet device.
> 
> Signed-off-by: Boris Pismenny <borisp at mellanox.com>
> Signed-off-by: Aviad Yehezkel <aviadye at mellanox.com>
> Signed-off-by: Radu Nicolau <radu.nicolau at intel.com>
> Signed-off-by: Declan Doherty <declan.doherty at intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c           | 11 +++++++++++
>  lib/librte_ether/rte_ethdev.h           | 22 ++++++++++++++++++++--
>  lib/librte_ether/rte_ethdev_version.map |  7 +++++++
>  3 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 0597641..f51c5a5 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -302,6 +302,17 @@ rte_eth_dev_socket_id(uint8_t port_id)
>  	return rte_eth_devices[port_id].data->numa_node;
>  }
> 
> +uint16_t
> +rte_eth_dev_get_sec_id(uint8_t port_id) {
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
> +
> +	if (rte_eth_devices[port_id].data->dev_flags &
> RTE_ETH_DEV_SECURITY)
> +		return rte_eth_devices[port_id].data->sec_id;
> +
> +	return -1;
> +}
> +
>  uint8_t
>  rte_eth_dev_count(void)
>  {
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 0adf327..262ba47 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -180,6 +180,8 @@ extern "C" {
>  #include <rte_dev.h>
>  #include <rte_devargs.h>
>  #include <rte_errno.h>
> +#include <rte_common.h>
> +
>  #include "rte_ether.h"
>  #include "rte_eth_ctrl.h"
>  #include "rte_dev_info.h"
> @@ -357,7 +359,8 @@ struct rte_eth_rxmode {
>  		jumbo_frame      : 1, /**< Jumbo Frame Receipt enable. */
>  		hw_strip_crc     : 1, /**< Enable CRC stripping by hardware. */
>  		enable_scatter   : 1, /**< Enable scatter packets rx handler */
> -		enable_lro       : 1; /**< Enable LRO */
> +		enable_lro       : 1, /**< Enable LRO */
> +		enable_sec       : 1; /**< Enable security offload */

Based on the time of integration you may need to update the convert function [1] as well.

>  };
> 
>  /**
> @@ -679,8 +682,10 @@ struct rte_eth_txmode {
>  		/**< If set, reject sending out tagged pkts */
>  		hw_vlan_reject_untagged : 1,
>  		/**< If set, reject sending out untagged pkts */
> -		hw_vlan_insert_pvid : 1;
> +		hw_vlan_insert_pvid : 1,
>  		/**< If set, enable port based VLAN insertion */
> +		enable_sec       : 1;
> +		/**< Enable security offload */

Considering this enable_sec is an exception in compare to the regular Tx offloads which are always enabled and set per queue,
Wouldn't it be better to use the new offloads API [2] for that flag? 

>  };
> 
>  /**
> @@ -907,6 +912,7 @@ struct rte_eth_conf {  #define
> DEV_RX_OFFLOAD_QINQ_STRIP  0x00000020  #define
> DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
>  #define DEV_RX_OFFLOAD_MACSEC_STRIP     0x00000080
> +#define DEV_RX_OFFLOAD_SECURITY         0x00000100
> 
>  /**
>   * TX offload capabilities of a device.
> @@ -926,6 +932,11 @@ struct rte_eth_conf {
>  #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO   0x00001000    /**< Used for
> tunneling packet. */
>  #define DEV_TX_OFFLOAD_MACSEC_INSERT    0x00002000
>  #define DEV_TX_OFFLOAD_MT_LOCKFREE      0x00004000
> +#define DEV_TX_OFFLOAD_SECURITY         0x00008000

The above flag I understand. 

> +#define DEV_TX_OFFLOAD_SEC_NEED_MDATA   0x00010000
> +#define DEV_TX_OFFLOAD_IPSEC_CRYPTO_HW_TRAILER 0x00020000
> +#define DEV_TX_OFFLOAD_IPSEC_CRYPTO_TSO        0x00040000
> +#define DEV_TX_OFFLOAD_IPSEC_CRYPTO_CKSUM      0x00080000

The above 4 flags I don't. doc/comments are missing to describe what exactly each feature means. 
Also considering those caps may be protocol depended (e.g. PMD can do TSO for ipsec but cannot for macsec) isn't it better for those caps to be advertised as part of rte_security_capabilities ?
The PMD will report for ethdev layer it support the security offloads, then per protocol the above caps will be advertised. 


>  /**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the
> same
>   * tx queue without SW lock.
>   */
> @@ -1651,6 +1662,9 @@ struct rte_eth_dev {
>  	enum rte_eth_dev_state state; /**< Flag indicating the port state */
> } __rte_cache_aligned;
> 
> +uint16_t
> +rte_eth_dev_get_sec_id(uint8_t port_id);
> +
>  struct rte_eth_dev_sriov {
>  	uint8_t active;               /**< SRIOV is active with 16, 32 or 64 pools */
>  	uint8_t nb_q_per_pool;        /**< rx queue number per pool */
> @@ -1711,6 +1725,8 @@ struct rte_eth_dev_data {
>  	int numa_node;  /**< NUMA node connection */
>  	struct rte_vlan_filter_conf vlan_filter_conf;
>  	/**< VLAN filter configuration. */
> +	uint16_t sec_id;
> +	/**< security instance identifier */
>  };
> 
>  /** Device supports hotplug detach */
> @@ -1721,6 +1737,8 @@ struct rte_eth_dev_data {  #define
> RTE_ETH_DEV_BONDED_SLAVE 0x0004
>  /** Device supports device removal interrupt */
>  #define RTE_ETH_DEV_INTR_RMV     0x0008
> +/** Device supports inline security processing */
> +#define RTE_ETH_DEV_SECURITY    0x0010

I see you use this cap to protect in ethdev layer from returning invalid security id. However it seems to me kind of duplication with the  DEV_TX_OFFLOAD_SECURITY and DEV_RX_OFFLOAD_SECURITY.
The combination of the two flags means the PMD supports rte_security, can't we use it instead of the above flag? Meaning the function will be:

uint16_t                                                                    
rte_eth_dev_get_sec_id(uint8_t port_id)                                     
{                                                                           
       rte_eth_dev_info dev_info;
       unsigned int support_sec = 0;

        RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);                       
                                                                            
       rte_eth_dev_info_get(port_id, &dev_info);
       support_sec = !!(dev_info->rx_offloads_capa & DEV_RX_OFFLOAD_SECURITY) &&
		        (dev_info->tx_offloads_capa & DEV_TX_OFFLOAD_SECURITY))
        if (support_sec)
                return rte_eth_devices[port_id].data->sec_id;               
                                                                            
        return -1;                                                          
}                                                                           

> 
>  /**
>   * @internal
> diff --git a/lib/librte_ether/rte_ethdev_version.map
> b/lib/librte_ether/rte_ethdev_version.map
> index 4283728..24cbd7d 100644
> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> @@ -187,3 +187,10 @@ DPDK_17.08 {
>  	rte_tm_wred_profile_delete;
> 
>  } DPDK_17.05;
> +
> +DPDK_17.11 {
> +	global:
> +
> +	rte_eth_dev_get_sec_id;
> +
> +} DPDK_17.08;
> --
> 2.9.3

[1] http://dpdk.org/dev/patchwork/patch/28799/
[2] http://dpdk.org/dev/patchwork/patch/28800/





More information about the dev mailing list