CSIT-928 dpdk/ipsec: performance improvement 82/11082/22
authorRadu Nicolau <radu.nicolau@intel.com>
Mon, 12 Mar 2018 13:52:41 +0000 (13:52 +0000)
committerDamjan Marion <dmarion.lists@gmail.com>
Tue, 22 May 2018 12:09:23 +0000 (12:09 +0000)
Replace hash with a vector to improve performance.
Plus other minor performance improvements.

Change-Id: I3f0ebd909782ce3727f6360ce5ff5ddd131f8574
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
src/plugins/dpdk/ipsec/crypto_node.c
src/plugins/dpdk/ipsec/ipsec.c
src/plugins/dpdk/ipsec/ipsec.h
src/vnet/ipsec/ipsec_output.c

index 89c5068..6b9ff58 100644 (file)
@@ -102,74 +102,72 @@ dpdk_crypto_dequeue (vlib_main_t * vm, vlib_node_runtime_t * node,
 
   next_index = node->cached_next_index;
 
-  do
-    {
-      ops = cwm->ops;
-      n_ops = rte_cryptodev_dequeue_burst (res->dev_id,
-                                          res->qp_id + outbound,
-                                          ops, VLIB_FRAME_SIZE);
-      res->inflights[outbound] -= n_ops;
-      ASSERT (res->inflights >= 0);
-
-      n_deq = n_ops;
-      total_n_deq += n_ops;
-
-      while (n_ops > 0)
-       {
-         u32 n_left_to_next;
-
-         vlib_get_next_frame (vm, node, next_index, to_next, n_left_to_next);
-
-         while (n_ops > 0 && n_left_to_next > 0)
-           {
-             u32 bi0, next0;
-             vlib_buffer_t *b0 = 0;
-             struct rte_crypto_op *op;
-
-             op = ops[0];
-             ops += 1;
-             n_ops -= 1;
-             n_left_to_next -= 1;
-
-             dpdk_op_priv_t *priv = crypto_op_get_priv (op);
-             next0 = priv->next;
-
-             if (PREDICT_FALSE (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS))
-               {
-                 next0 = DPDK_CRYPTO_INPUT_NEXT_DROP;
-                 vlib_node_increment_counter (vm,
-                                              dpdk_crypto_input_node.index,
-                                              DPDK_CRYPTO_INPUT_ERROR_STATUS,
-                                              1);
-               }
-
-             /* XXX store bi0 and next0 in op private? */
-
-             b0 = vlib_buffer_from_rte_mbuf (op->sym[0].m_src);
-             bi0 = vlib_get_buffer_index (vm, b0);
-
-             to_next[0] = bi0;
-             to_next += 1;
-
-             if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED))
-               {
-                 vlib_trace_next_frame (vm, node, next0);
-                 dpdk_crypto_input_trace_t *tr =
-                   vlib_add_trace (vm, node, b0, sizeof (*tr));
-                 tr->status = op->status;
-               }
-
-             op->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
-
-             vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next,
-                                              n_left_to_next, bi0, next0);
-           }
-         vlib_put_next_frame (vm, node, next_index, n_left_to_next);
-       }
-
-      crypto_free_ops (numa, cwm->ops, n_deq);
-    }
-  while (n_deq == VLIB_FRAME_SIZE && res->inflights[outbound]);
+  {
+    ops = cwm->ops;
+    n_ops = rte_cryptodev_dequeue_burst (res->dev_id,
+                                        res->qp_id + outbound,
+                                        ops, VLIB_FRAME_SIZE);
+    res->inflights[outbound] -= n_ops;
+    ASSERT (res->inflights >= 0);
+
+    n_deq = n_ops;
+    total_n_deq += n_ops;
+
+    while (n_ops > 0)
+      {
+       u32 n_left_to_next;
+
+       vlib_get_next_frame (vm, node, next_index, to_next, n_left_to_next);
+
+       while (n_ops > 0 && n_left_to_next > 0)
+         {
+           u32 bi0, next0;
+           vlib_buffer_t *b0 = 0;
+           struct rte_crypto_op *op;
+
+           op = ops[0];
+           ops += 1;
+           n_ops -= 1;
+           n_left_to_next -= 1;
+
+           dpdk_op_priv_t *priv = crypto_op_get_priv (op);
+           next0 = priv->next;
+
+           if (PREDICT_FALSE (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS))
+             {
+               next0 = DPDK_CRYPTO_INPUT_NEXT_DROP;
+               vlib_node_increment_counter (vm,
+                                            dpdk_crypto_input_node.index,
+                                            DPDK_CRYPTO_INPUT_ERROR_STATUS,
+                                            1);
+             }
+
+           /* XXX store bi0 and next0 in op private? */
+
+           b0 = vlib_buffer_from_rte_mbuf (op->sym[0].m_src);
+           bi0 = vlib_get_buffer_index (vm, b0);
+
+           to_next[0] = bi0;
+           to_next += 1;
+
+           if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED))
+             {
+               vlib_trace_next_frame (vm, node, next0);
+               dpdk_crypto_input_trace_t *tr =
+                 vlib_add_trace (vm, node, b0, sizeof (*tr));
+               tr->status = op->status;
+             }
+
+           op->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
+
+           vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next,
+                                            n_left_to_next, bi0, next0);
+         }
+       vlib_put_next_frame (vm, node, next_index, n_left_to_next);
+      }
+
+    crypto_free_ops (numa, cwm->ops, n_deq);
+  }
 
   vlib_node_increment_counter (vm, dpdk_crypto_input_node.index,
                               DPDK_CRYPTO_INPUT_ERROR_DQ_COPS, total_n_deq);
@@ -185,7 +183,6 @@ dpdk_crypto_input_fn (vlib_main_t * vm, vlib_node_runtime_t * node,
   crypto_worker_main_t *cwm = &dcm->workers_main[thread_index];
   crypto_resource_t *res;
   u32 n_deq = 0;
-  u8 outbound;
   u16 *remove = NULL, *res_idx;
   word i;
 
@@ -194,13 +191,11 @@ dpdk_crypto_input_fn (vlib_main_t * vm, vlib_node_runtime_t * node,
     {
       res = vec_elt_at_index (dcm->resource, res_idx[0]);
 
-      outbound = 0;
-      if (res->inflights[outbound])
-       n_deq += dpdk_crypto_dequeue (vm, node, res, outbound);
+      if (res->inflights[0])
+       n_deq += dpdk_crypto_dequeue (vm, node, res, 0);
 
-      outbound = 1;
-      if (res->inflights[outbound])
-       n_deq += dpdk_crypto_dequeue (vm, node, res, outbound);
+      if (res->inflights[1])
+       n_deq += dpdk_crypto_dequeue (vm, node, res, 1);
 
       if (unlikely(res->remove && !(res->inflights[0] || res->inflights[1])))
        vec_add1 (remove, res_idx[0]);
index 5933cf9..b8cfc7e 100644 (file)
@@ -329,11 +329,8 @@ create_sym_session (struct rte_cryptodev_sym_session **session,
   struct rte_crypto_sym_xform auth_xform = { 0 };
   struct rte_crypto_sym_xform *xfs;
   struct rte_cryptodev_sym_session **s;
-  crypto_session_key_t key = { 0 };
   clib_error_t *erorr = 0;
 
-  key.drv_id = res->drv_id;
-  key.sa_idx = sa_idx;
 
   sa = pool_elt_at_index (im->sad, sa_idx);
 
@@ -399,7 +396,7 @@ create_sym_session (struct rte_cryptodev_sym_session **session,
       goto done;
     }
 
-  hash_set (data->session_by_drv_id_and_sa_index, key.val, session[0]);
+  add_session_by_drv_and_sa_idx (session[0], data, res->drv_id, sa_idx);
 
 done:
   clib_spinlock_unlock_if_init (&data->lockp);
@@ -486,7 +483,6 @@ add_del_sa_session (u32 sa_index, u8 is_add)
   dpdk_crypto_main_t *dcm = &dpdk_crypto_main;
   crypto_data_t *data;
   struct rte_cryptodev_sym_session *s;
-  crypto_session_key_t key = { 0 };
   uword *val;
   u32 drv_id;
 
@@ -510,23 +506,19 @@ add_del_sa_session (u32 sa_index, u8 is_add)
       return 0;
     }
 
-  key.sa_idx = sa_index;
-
   /* *INDENT-OFF* */
   vec_foreach (data, dcm->data)
     {
       clib_spinlock_lock_if_init (&data->lockp);
       val = hash_get (data->session_by_sa_index, sa_index);
-      s = (struct rte_cryptodev_sym_session *) val;
-      if (s)
+      if (val)
         {
+          s = (struct rte_cryptodev_sym_session *) val[0];
           vec_foreach_index (drv_id, dcm->drv)
             {
-              key.drv_id = drv_id;
-              val = hash_get (data->session_by_drv_id_and_sa_index, key.val);
-              s = (struct rte_cryptodev_sym_session *) val;
-              if (s)
-                hash_unset (data->session_by_drv_id_and_sa_index, key.val);
+              val = (uword*) get_session_by_drv_and_sa_idx (data, drv_id, sa_index);
+              if (val)
+                add_session_by_drv_and_sa_idx(NULL, data, drv_id, sa_index);
             }
 
           hash_unset (data->session_by_sa_index, sa_index);
@@ -913,6 +905,8 @@ crypto_create_session_drv_pool (vlib_main_t * vm, crypto_dev_t * dev)
 
   vec_validate (data->session_drv, dev->drv_id);
   vec_validate (data->session_drv_failed, dev->drv_id);
+  vec_validate_aligned (data->session_by_drv_id_and_sa_index, 32,
+                       CLIB_CACHE_LINE_BYTES);
 
   if (data->session_drv[dev->drv_id])
     return NULL;
@@ -989,7 +983,6 @@ crypto_disable (void)
 
   vec_free (dcm->data);
   vec_free (dcm->workers_main);
-  vec_free (dcm->sa_session);
   vec_free (dcm->dev);
   vec_free (dcm->resource);
   vec_free (dcm->cipher_algs);
index 4287a2a..775e752 100644 (file)
@@ -125,6 +125,12 @@ typedef struct
   struct rte_cryptodev_sym_session *session;
 } crypto_session_disposal_t;
 
+typedef struct
+{
+  struct rte_cryptodev_sym_session *session;
+  u64 dev_mask;
+} crypto_session_by_drv_t;
+
 typedef struct
 {
   /* Required for vec_validate_aligned */
@@ -134,17 +140,16 @@ typedef struct
   struct rte_mempool **session_drv;
   crypto_session_disposal_t *session_disposal;
   uword *session_by_sa_index;
-  uword *session_by_drv_id_and_sa_index;
   u64 crypto_op_get_failed;
   u64 session_h_failed;
   u64 *session_drv_failed;
+  crypto_session_by_drv_t *session_by_drv_id_and_sa_index;
   clib_spinlock_t lockp;
 } crypto_data_t;
 
 typedef struct
 {
   crypto_worker_main_t *workers_main;
-  struct rte_cryptodev_sym_session **sa_session;
   crypto_dev_t *dev;
   crypto_resource_t *resource;
   crypto_alg_t *cipher_algs;
@@ -194,38 +199,47 @@ crypto_op_get_priv (struct rte_crypto_op * op)
   return (dpdk_op_priv_t *) (((u8 *) op) + crypto_op_get_priv_offset ());
 }
 
-/* XXX this requires 64 bit builds so hash_xxx macros use u64 key */
-typedef union
+
+static_always_inline void
+add_session_by_drv_and_sa_idx (struct rte_cryptodev_sym_session *session,
+                              crypto_data_t * data, u32 drv_id, u32 sa_idx)
 {
-  u64 val;
-  struct
-  {
-    u32 drv_id;
-    u32 sa_idx;
-  };
-} crypto_session_key_t;
+  crypto_session_by_drv_t *sbd;
+  vec_validate_aligned (data->session_by_drv_id_and_sa_index, sa_idx,
+                       CLIB_CACHE_LINE_BYTES);
+  sbd = vec_elt_at_index (data->session_by_drv_id_and_sa_index, sa_idx);
+  sbd->dev_mask |= 1L << drv_id;
+  sbd->session = session;
+}
+
+static_always_inline struct rte_cryptodev_sym_session *
+get_session_by_drv_and_sa_idx (crypto_data_t * data, u32 drv_id, u32 sa_idx)
+{
+  crypto_session_by_drv_t *sess_by_sa;
+  if (_vec_len (data->session_by_drv_id_and_sa_index) <= sa_idx)
+    return NULL;
+  sess_by_sa =
+    vec_elt_at_index (data->session_by_drv_id_and_sa_index, sa_idx);
+  return (sess_by_sa->dev_mask & (1L << drv_id)) ? sess_by_sa->session : NULL;
+}
 
 static_always_inline clib_error_t *
-crypto_get_session (struct rte_cryptodev_sym_session **session,
+crypto_get_session (struct rte_cryptodev_sym_session ** session,
                    u32 sa_idx,
                    crypto_resource_t * res,
                    crypto_worker_main_t * cwm, u8 is_outbound)
 {
   dpdk_crypto_main_t *dcm = &dpdk_crypto_main;
   crypto_data_t *data;
-  uword *val;
-  crypto_session_key_t key = { 0 };
-
-  key.drv_id = res->drv_id;
-  key.sa_idx = sa_idx;
+  struct rte_cryptodev_sym_session *sess;
 
   data = vec_elt_at_index (dcm->data, res->numa);
-  val = hash_get (data->session_by_drv_id_and_sa_index, key.val);
+  sess = get_session_by_drv_and_sa_idx (data, res->drv_id, sa_idx);
 
-  if (PREDICT_FALSE (!val))
+  if (PREDICT_FALSE (!sess))
     return create_sym_session (session, sa_idx, res, cwm, is_outbound);
 
-  session[0] = (struct rte_cryptodev_sym_session *) val[0];
+  session[0] = sess;
 
   return NULL;
 }
@@ -239,8 +253,8 @@ get_resource (crypto_worker_main_t * cwm, ipsec_sa_t * sa)
 
   /* Not allowed to setup SA with no-aead-cipher/NULL or NULL/NULL */
 
-  is_aead = ((sa->crypto_alg == IPSEC_CRYPTO_ALG_AES_GCM_128) |
-            (sa->crypto_alg == IPSEC_CRYPTO_ALG_AES_GCM_192) |
+  is_aead = ((sa->crypto_alg == IPSEC_CRYPTO_ALG_AES_GCM_128) ||
+            (sa->crypto_alg == IPSEC_CRYPTO_ALG_AES_GCM_192) ||
             (sa->crypto_alg == IPSEC_CRYPTO_ALG_AES_GCM_256));
 
   if (sa->crypto_alg == IPSEC_CRYPTO_ALG_NONE)
index d56b665..4bfbf60 100644 (file)
@@ -88,16 +88,16 @@ ipsec_output_policy_match (ipsec_spd_t * spd, u8 pr, u32 la, u32 ra, u16 lp,
     if (PREDICT_FALSE (p->protocol && (p->protocol != pr)))
       continue;
 
-    if (la < clib_net_to_host_u32 (p->laddr.start.ip4.as_u32))
+    if (ra < clib_net_to_host_u32 (p->raddr.start.ip4.as_u32))
       continue;
 
-    if (la > clib_net_to_host_u32 (p->laddr.stop.ip4.as_u32))
+    if (ra > clib_net_to_host_u32 (p->raddr.stop.ip4.as_u32))
       continue;
 
-    if (ra < clib_net_to_host_u32 (p->raddr.start.ip4.as_u32))
+    if (la < clib_net_to_host_u32 (p->laddr.start.ip4.as_u32))
       continue;
 
-    if (ra > clib_net_to_host_u32 (p->raddr.stop.ip4.as_u32))
+    if (la > clib_net_to_host_u32 (p->laddr.stop.ip4.as_u32))
       continue;
 
     if (PREDICT_FALSE
@@ -274,11 +274,9 @@ ipsec_output_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
        {
          if (p0->policy == IPSEC_POLICY_ACTION_PROTECT)
            {
-             u32 sa_index = 0;
              ipsec_sa_t *sa = 0;
              nc_protect++;
-             sa_index = ipsec_get_sa_index_by_sa_id (p0->sa_id);
-             sa = pool_elt_at_index (im->sad, sa_index);
+             sa = pool_elt_at_index (im->sad, p0->sa_index);
              if (sa->protocol == IPSEC_PROTOCOL_ESP)
                next_node_index = im->esp_encrypt_node_index;
              else