Hi,
Comments inline, thanks for reviewing!
On 10/4/2017 11:52 AM, Shahaf Shuler wrote:
Tuesday, October 3, 2017 4:14 PM, Akhil Goyal:
From: Declan Doherty <declan.dohe...@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 <bor...@mellanox.com>
Signed-off-by: Aviad Yehezkel <avia...@mellanox.com>
Signed-off-by: Radu Nicolau <radu.nico...@intel.com>
Signed-off-by: Declan Doherty <declan.dohe...@intel.com>
---
lib/librte_ether/rte_ethdev.c | 11 +++++++++++
lib/librte_ether/rte_ethdev.h | 19 +++++++++++++++++--
lib/librte_ether/rte_ethdev_version.map | 7 +++++++
3 files changed, 35 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..193ad62 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 */
};
/**
@@ -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 */
Already comment on it in the previous version [1].
I don't think there is a justification to introduce new approach to set Tx
offloads given there is already patch set which provides such new API [2].
I think this patch should be on top of it.
I agree with you, that is if the new offload API will be merged we will
also change this one. But until then it makes testing and developing
more difficult.
};
/**
@@ -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,8 @@ 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
+#define DEV_TX_OFFLOAD_SEC_NEED_MDATA 0x00010000
Isn't it better to have the DEV_TX_OFFLOAD_SEC_NEED_MDATA as part of
rte_security_capability struct?
The "need metadata" should be per security operation indication.
For example It is possible that PMD will be able to do IPSEC without the need
in metadata and PDCP with the need in metadata.
IMO the better model is for the PMD to advertise it support all kind of
security offloads by setting the DEV_TX_OFFLOAD_SECURITY flag. Application will
be able to query more fine-grained capabilities per security operation using
rte_security APIs.
The DEV_TX_OFFLOAD_SEC_NEED_MDATA will be moved to the capabilities in
the v3.
/**< Multiple threads can invoke rte_eth_tx_burst() concurrently on the
same
* tx queue without SW lock.
*/
@@ -1651,6 +1659,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 +1722,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 +1734,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 still not understand why this flag is needed.
The PMD can advertise its supports rte_security by setting the corresponding
DEV_TX_OFFLOAD_SECURITY and DEV_RX_OFFLOAD_SECURITY flags.
Etherdev layer can validate the sec_id using those flags.
The various security offloads, as I mentioned above, should be part of
rte_security_capability query.
I think the reason to have this is to allow to advertise that a device
has security features without the need to check exactly which ones are
they...
/**
* @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/ml/archives/dev/2017-September/076382.html
[2] http://dpdk.org/ml/archives/dev/2017-October/077329.html
