From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id ECE20DE3 for ; Fri, 22 Sep 2017 13:42:30 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Sep 2017 04:42:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,427,1500966000"; d="scan'208";a="315045436" Received: from rnicolau-mobl.ger.corp.intel.com (HELO [10.237.221.79]) ([10.237.221.79]) by fmsmga004.fm.intel.com with ESMTP; 22 Sep 2017 04:42:27 -0700 To: Shahaf Shuler , Akhil Goyal , "dev@dpdk.org" Cc: "declan.doherty@intel.com" , "pablo.de.lara.guarch@intel.com" , "hemant.agrawal@nxp.com" , Boris Pismenny , Aviad Yehezkel , Thomas Monjalon , "sandeep.malik@nxp.com" , "jerin.jacob@caviumnetworks.com" References: <20170914082651.26232-1-akhil.goyal@nxp.com> <20170914082651.26232-7-akhil.goyal@nxp.com> From: Radu Nicolau Message-ID: <9168a738-da8a-f0a2-0b00-c6f2ae7a732b@intel.com> Date: Fri, 22 Sep 2017 12:42:26 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH 06/11] ethdev: extend ethdev to support security APIs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Sep 2017 11:42:31 -0000 Hi On 9/17/2017 2:45 PM, Shahaf Shuler wrote: > Hi Declan, > > Thursday, September 14, 2017 11:27 AM, Akhil Goyal: >> From: Declan Doherty >> >> rte_flow_action type and ethdev updated to support rte_security sessions >> for crypto offload to ethernet device. >> >> Signed-off-by: Boris Pismenny >> Signed-off-by: Aviad Yehezkel >> Signed-off-by: Radu Nicolau >> Signed-off-by: Declan Doherty >> --- >> 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 >> #include >> #include >> +#include >> + >> #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. Indeed. > >> }; >> >> /** >> @@ -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? I guess so, when the new API will be merged we can revisit the security offloads as well. > >> }; >> >> /** >> @@ -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. Will be updated, thanks! > > >> /**< 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: I'm not sure here, probably you're right, but also we can have more flexibility having a device flag for security capabilities in general and various security offloads in particular. > > 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/ > > >