Re: [RFC PATCH 1/4] rte_security: API definitions

On 8/15/2017 4:34 PM, Radu Nicolau wrote:

On 8/15/2017 7:35 AM, Akhil Goyal wrote:

quoted

Detailed description is added in the coverletter

Signed-off-by: Akhil Goyal <redacted>
---
  lib/librte_cryptodev/rte_security.c | 171 +++++++++++++++
  lib/librte_cryptodev/rte_security.h | 409 
++++++++++++++++++++++++++++++++++++
  2 files changed, 580 insertions(+)
  create mode 100644 lib/librte_cryptodev/rte_security.c
  create mode 100644 lib/librte_cryptodev/rte_security.h

quoted

+int
+rte_security_session_init(uint16_t dev_id,
+              struct rte_security_session *sess,
+              struct rte_security_sess_conf *conf,
+              struct rte_mempool *mp)
+{
+    struct rte_cryptodev *cdev = NULL;
+    struct rte_eth_dev *dev = NULL;
+    uint8_t index;
+    int ret;
+
+    if (sess == NULL || conf == NULL)
+        return -EINVAL;
+
+    switch (conf->action_type) {
+    case RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD:
+        if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
+            return -EINVAL;
+        cdev = rte_cryptodev_pmd_get_dev(dev_id);
+        index = cdev->driver_id;
+        if (sess->sess_private_data[index] == NULL) {
+            ret = cdev->sec_ops->session_configure(cdev, conf, sess, 
mp);
+            if (ret < 0) {
+                CDEV_LOG_ERR(
+                    "cdev_id %d failed to configure session details",
+                    dev_id);
+                return ret;
+            }
+        }
+        break;
+    case RTE_SECURITY_SESS_ETH_INLINE_CRYPTO:
+    case RTE_SECURITY_SESS_ETH_PROTO_OFFLOAD:
+        dev = &rte_eth_devices[dev_id];
+        index = dev->data->port_id;
+        if (sess->sess_private_data[index] == NULL) {
+//            ret = dev->sec_ops->session_configure(dev, conf, sess, 
mp);
+//            if (ret < 0) {
+//                CDEV_LOG_ERR(
+//                    "dev_id %d failed to configure session details",
+//                    dev_id);
+//                return ret;
+//            }

The commented lines above suggests that also eth devices will have a 
sec_ops field, (which makes sense). Is this correct?
Also, if the above is correct, session_configure and session_clear 
should accept both crypto and eth devices as first parameter.

Yes you are correct both these ops should accept void *dev and 
internally in the driver should typecast to respective device.
Please consider the following diff over this patch

diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h 

b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index 219fba6..ab3ecf7 100644

--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h

@@ -371,7 +371,7 @@ struct rte_cryptodev_ops {
   *  - Returns -ENOTSUP if crypto device does not support the crypto 

transform.
   *  - Returns -ENOMEM if the private session could not be allocated.
   */
-typedef int (*security_configure_session_t)(struct rte_cryptodev *dev,
+typedef int (*security_configure_session_t)(void *dev,
                 struct rte_security_sess_conf *conf,
                 struct rte_security_session *sess,
                 struct rte_mempool *mp);

@@ -382,7 +382,7 @@ typedef int (*security_configure_session_t)(struct 

rte_cryptodev *dev,
   * @param      dev             Crypto device pointer
   * @param      sess            Security session structure
   */
-typedef void (*security_free_session_t)(struct rte_cryptodev *dev,
+typedef void (*security_free_session_t)(void *dev,
                 struct rte_security_session *sess);

  /** Security operations function pointer table */

diff --git a/lib/librte_cryptodev/rte_security.c 

b/lib/librte_cryptodev/rte_security.c
index 7c73c93..a7558bb 100644

--- a/lib/librte_cryptodev/rte_security.c
+++ b/lib/librte_cryptodev/rte_security.c

@@ -87,7 +87,8 @@ rte_security_session_init(uint16_t dev_id,
                 cdev = rte_cryptodev_pmd_get_dev(dev_id);
                 index = cdev->driver_id;
                 if (sess->sess_private_data[index] == NULL) {
-                       ret = cdev->sec_ops->session_configure(cdev, 

conf, sess, mp);
+                       ret = cdev->sec_ops->session_configure((void *)cdev,
+                                                       conf, sess, mp);
                         if (ret < 0) {
                                 CDEV_LOG_ERR(
                                         "cdev_id %d failed to configure 
session details",

@@ -101,7 +102,8 @@ rte_security_session_init(uint16_t dev_id,
                 dev = &rte_eth_devices[dev_id];
                 index = dev->data->port_id;
                 if (sess->sess_private_data[index] == NULL) {
-//                     ret = dev->sec_ops->session_configure(dev, conf, 

sess, mp);
+//                     ret = dev->sec_ops->session_configure((void *)dev,
+//                                                     conf, sess, mp);

Thanks,
Akhil