ipsec: remove redundant policy array in fast path spd

Fast path spd was explicitely storing array of policy id vectors.
This information was redundand, as this inofrmation is already stored
in bihash table. This additional array was affecting performance
when adding and removing fast path policies.
The other place that needed refactoring after removing this array  was
cli command showing fast path policies.

Type: feature

Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
Change-Id: I78d45653f71539e7ba90ff5d2834451f83ead4be

3 files changed, 178 insertions, 90 deletions

diff --git a/src/vnet/ipsec/ipsec_format.c b/src/vnet/ipsec/ipsec_format.c
index 9204b1c090d..86ec3685edd 100644
--- a/src/vnet/ipsec/ipsec_format.c
+++ b/src/vnet/ipsec/ipsec_format.c
@@ -210,11 +210,145 @@ format_ipsec_policy (u8 *s, va_list *args)
 }
 
 u8 *
-format_ipsec_policy_fp (u8 *s, va_list *args)
+format_ipsec_fp_policy (u8 *s, va_list *args)
 {
   return format_ipsec_policy_with_suffix (s, args, (u8 *) "<fast-path>");
 }
 
+/**
+ * @brief Context when walking the fp bihash  table. We need to filter
+ * only those policies that are of given type as we walk the table.
+ */
+typedef struct ipsec_spd_policy_ctx_t_
+{
+  u32 *policies;
+  ipsec_spd_policy_type_t t;
+} ipsec_fp_walk_ctx_t;
+
+static int
+ipsec_fp_table_walk_ip4_cb (clib_bihash_kv_16_8_t *kvp, void *arg)
+{
+  ipsec_fp_walk_ctx_t *ctx = (ipsec_fp_walk_ctx_t *) arg;
+  ipsec_main_t *im = &ipsec_main;
+  ipsec_policy_t *p;
+
+  ipsec_fp_lookup_value_t *val = (ipsec_fp_lookup_value_t *) &kvp->value;
+
+  u32 *policy_id;
+
+  vec_foreach (policy_id, val->fp_policies_ids)
+    {
+      p = pool_elt_at_index (im->policies, *policy_id);
+      if (p->type == ctx->t)
+	vec_add1 (ctx->policies, *policy_id);
+    }
+
+  return BIHASH_WALK_CONTINUE;
+}
+
+static int
+ipsec_fp_table_walk_ip6_cb (clib_bihash_kv_40_8_t *kvp, void *arg)
+{
+  ipsec_fp_walk_ctx_t *ctx = (ipsec_fp_walk_ctx_t *) arg;
+  ipsec_main_t *im = &ipsec_main;
+  ipsec_policy_t *p;
+
+  ipsec_fp_lookup_value_t *val = (ipsec_fp_lookup_value_t *) &kvp->value;
+
+  u32 *policy_id;
+
+  vec_foreach (policy_id, val->fp_policies_ids)
+    {
+      p = pool_elt_at_index (im->policies, *policy_id);
+      if (p->type == ctx->t)
+	vec_add1 (ctx->policies, *policy_id);
+    }
+
+  return BIHASH_WALK_CONTINUE;
+}
+
+u8 *
+format_ipsec_fp_policies (u8 *s, va_list *args)
+{
+  ipsec_main_t *im = &ipsec_main;
+  ipsec_spd_t *spd = va_arg (*args, ipsec_spd_t *);
+  ipsec_spd_policy_type_t t = va_arg (*args, ipsec_spd_policy_type_t);
+  u32 *i;
+  ipsec_fp_walk_ctx_t ctx = {
+    .policies = 0,
+    .t = t,
+  };
+
+  u32 ip4_in_lookup_hash_idx = spd->fp_spd.ip4_in_lookup_hash_idx;
+  u32 ip4_out_lookup_hash_idx = spd->fp_spd.ip4_out_lookup_hash_idx;
+  u32 ip6_in_lookup_hash_idx = spd->fp_spd.ip6_in_lookup_hash_idx;
+  u32 ip6_out_lookup_hash_idx = spd->fp_spd.ip6_out_lookup_hash_idx;
+
+  switch (t)
+    {
+    case IPSEC_SPD_POLICY_IP4_INBOUND_PROTECT:
+    case IPSEC_SPD_POLICY_IP4_INBOUND_BYPASS:
+    case IPSEC_SPD_POLICY_IP4_INBOUND_DISCARD:
+      if (INDEX_INVALID != ip4_in_lookup_hash_idx)
+	{
+	  clib_bihash_16_8_t *bihash_table = pool_elt_at_index (
+	    im->fp_ip4_lookup_hashes_pool, ip4_in_lookup_hash_idx);
+
+	  clib_bihash_foreach_key_value_pair_16_8 (
+	    bihash_table, ipsec_fp_table_walk_ip4_cb, &ctx);
+	}
+
+      break;
+
+    case IPSEC_SPD_POLICY_IP6_INBOUND_PROTECT:
+    case IPSEC_SPD_POLICY_IP6_INBOUND_BYPASS:
+    case IPSEC_SPD_POLICY_IP6_INBOUND_DISCARD:
+      if (INDEX_INVALID != ip6_in_lookup_hash_idx)
+	{
+	  clib_bihash_40_8_t *bihash_table = pool_elt_at_index (
+	    im->fp_ip6_lookup_hashes_pool, ip6_in_lookup_hash_idx);
+
+	  clib_bihash_foreach_key_value_pair_40_8 (
+	    bihash_table, ipsec_fp_table_walk_ip6_cb, &ctx);
+	}
+
+      break;
+    case IPSEC_SPD_POLICY_IP4_OUTBOUND:
+      if (INDEX_INVALID != ip4_out_lookup_hash_idx)
+	{
+	  clib_bihash_16_8_t *bihash_table = pool_elt_at_index (
+	    im->fp_ip4_lookup_hashes_pool, ip4_out_lookup_hash_idx);
+
+	  clib_bihash_foreach_key_value_pair_16_8 (
+	    bihash_table, ipsec_fp_table_walk_ip4_cb, &ctx);
+	}
+
+      break;
+    case IPSEC_SPD_POLICY_IP6_OUTBOUND:
+      if (INDEX_INVALID != ip6_out_lookup_hash_idx)
+	{
+	  clib_bihash_40_8_t *bihash_table = pool_elt_at_index (
+	    im->fp_ip6_lookup_hashes_pool, ip6_out_lookup_hash_idx);
+
+	  clib_bihash_foreach_key_value_pair_40_8 (
+	    bihash_table, ipsec_fp_table_walk_ip6_cb, &ctx);
+	}
+
+      break;
+    default:
+      break;
+    }
+
+  vec_foreach (i, ctx.policies)
+    {
+      s = format (s, "\n %U", format_ipsec_fp_policy, *i);
+    }
+
+  vec_free (ctx.policies);
+
+  return s;
+}
+
 u8 *
 format_ipsec_spd (u8 * s, va_list * args)
 {
@@ -239,10 +373,7 @@ format_ipsec_spd (u8 * s, va_list * args)
     {                                                                         \
       s = format (s, "\n %U", format_ipsec_policy, *i);                       \
     }                                                                         \
-  vec_foreach (i, spd->fp_spd.fp_policies[IPSEC_SPD_POLICY_##v])              \
-    {                                                                         \
-      s = format (s, "\n %U", format_ipsec_policy_fp, *i);                    \
-    }
+  s = format (s, "\n %U", format_ipsec_fp_policies, spd, IPSEC_SPD_POLICY_##v);
   foreach_ipsec_spd_policy_type;
 #undef _
 
diff --git a/src/vnet/ipsec/ipsec_spd.h b/src/vnet/ipsec/ipsec_spd.h
index 3a4fd0ec91c..3b1e4b40747 100644
--- a/src/vnet/ipsec/ipsec_spd.h
+++ b/src/vnet/ipsec/ipsec_spd.h
@@ -55,8 +55,6 @@ typedef struct
  */
 typedef struct
 {
-  /** vectors for each of the fast path policy types */
-  u32 *fp_policies[IPSEC_SPD_POLICY_N_TYPES];
   ipsec_fp_mask_id_t *fp_mask_ids[IPSEC_SPD_POLICY_N_TYPES];
   /* names of bihash tables */
   u8 *name4_out;
diff --git a/src/vnet/ipsec/ipsec_spd_policy.c b/src/vnet/ipsec/ipsec_spd_policy.c
index d5310a61cbd..4a17062b80e 100644
--- a/src/vnet/ipsec/ipsec_spd_policy.c
+++ b/src/vnet/ipsec/ipsec_spd_policy.c
@@ -622,7 +622,6 @@ ipsec_fp_ip4_add_policy (ipsec_main_t *im, ipsec_spd_fp_t *fp_spd,
     (fp_spd->fp_mask_ids[policy->type] + searched_idx)->refcount++;
 
   mte->refcount++;
-  vec_add1 (fp_spd->fp_policies[policy->type], policy_index);
   clib_memcpy (vp, policy, sizeof (*vp));
 
   return 0;
@@ -727,7 +726,6 @@ ipsec_fp_ip6_add_policy (ipsec_main_t *im, ipsec_spd_fp_t *fp_spd,
     (fp_spd->fp_mask_ids[policy->type] + searched_idx)->refcount++;
 
   mte->refcount++;
-  vec_add1 (fp_spd->fp_policies[policy->type], policy_index);
   clib_memcpy (vp, policy, sizeof (*vp));
 
   return 0;
@@ -756,7 +754,7 @@ ipsec_fp_ip6_del_policy (ipsec_main_t *im, ipsec_spd_fp_t *fp_spd,
 				 fp_spd->ip6_out_lookup_hash_idx);
 
   ipsec_policy_t *vp;
-  u32 ii, iii, imt;
+  u32 ii, imt;
 
   ipsec_fp_ip6_get_policy_mask (policy, &mask, inbound);
   ipsec_fp_get_policy_5tuple (policy, &policy_5tuple, inbound);
@@ -765,57 +763,38 @@ ipsec_fp_ip6_del_policy (ipsec_main_t *im, ipsec_spd_fp_t *fp_spd,
   if (res != 0)
     return -1;
 
-  res = -1;
   vec_foreach_index (ii, result_val->fp_policies_ids)
     {
       vp =
 	pool_elt_at_index (im->policies, *(result_val->fp_policies_ids + ii));
       if (ipsec_policy_is_equal (vp, policy))
 	{
-	  vec_foreach_index (iii, fp_spd->fp_policies[policy->type])
+	  if (vec_len (result_val->fp_policies_ids) == 1)
 	    {
-	      if (*(fp_spd->fp_policies[policy->type] + iii) ==
-		  *(result_val->fp_policies_ids + ii))
+	      vec_free (result_val->fp_policies_ids);
+	      clib_bihash_add_del_40_8 (bihash_table, &result, 0);
+	    }
+	  else
+	    vec_del1 (result_val->fp_policies_ids, ii);
+
+	  vec_foreach_index (imt, fp_spd->fp_mask_ids[policy->type])
+	    {
+	      if ((fp_spd->fp_mask_ids[policy->type] + imt)->mask_type_idx ==
+		  vp->fp_mask_type_id)
 		{
-		  if (vec_len (result_val->fp_policies_ids) == 1)
-		    {
-		      vec_free (result_val->fp_policies_ids);
-		      clib_bihash_add_del_40_8 (bihash_table, &result, 0);
-		    }
-		  else
-		    {
-		      vec_del1 (result_val->fp_policies_ids, ii);
-		    }
-		  vec_del1 (fp_spd->fp_policies[policy->type], iii);
-
-		  vec_foreach_index (imt, fp_spd->fp_mask_ids[policy->type])
-		    {
-		      if ((fp_spd->fp_mask_ids[policy->type] + imt)
-			    ->mask_type_idx == vp->fp_mask_type_id)
-			{
-
-			  if ((fp_spd->fp_mask_ids[policy->type] + imt)
-				->refcount-- == 1)
-			    vec_del1 (fp_spd->fp_mask_ids[policy->type], imt);
-
-			  break;
-			}
-		    }
-
-		  res = 0;
+
+		  if ((fp_spd->fp_mask_ids[policy->type] + imt)->refcount-- ==
+		      1)
+		    vec_del1 (fp_spd->fp_mask_ids[policy->type], imt);
+
 		  break;
 		}
 	    }
 
-	  if (res != 0)
-	    continue;
-	  else
-	    {
-	      ipsec_fp_release_mask_type (im, vp->fp_mask_type_id);
-	      ipsec_sa_unlock (vp->sa_index);
-	      pool_put (im->policies, vp);
-	      return 0;
-	    }
+	  ipsec_fp_release_mask_type (im, vp->fp_mask_type_id);
+	  ipsec_sa_unlock (vp->sa_index);
+	  pool_put (im->policies, vp);
+	  return 0;
 	}
     }
   return -1;
@@ -833,7 +812,7 @@ ipsec_fp_ip4_del_policy (ipsec_main_t *im, ipsec_spd_fp_t *fp_spd,
     (ipsec_fp_lookup_value_t *) &result.value;
   bool inbound = ipsec_is_policy_inbound (policy);
   ipsec_policy_t *vp;
-  u32 ii, iii, imt;
+  u32 ii, imt;
   clib_bihash_16_8_t *bihash_table =
     inbound ? pool_elt_at_index (im->fp_ip4_lookup_hashes_pool,
 				 fp_spd->ip4_in_lookup_hash_idx) :
@@ -848,57 +827,37 @@ ipsec_fp_ip4_del_policy (ipsec_main_t *im, ipsec_spd_fp_t *fp_spd,
   if (res != 0)
     return -1;
 
-  res = -1;
   vec_foreach_index (ii, result_val->fp_policies_ids)
     {
       vp =
 	pool_elt_at_index (im->policies, *(result_val->fp_policies_ids + ii));
       if (ipsec_policy_is_equal (vp, policy))
 	{
-	  vec_foreach_index (iii, fp_spd->fp_policies[policy->type])
+	  if (vec_len (result_val->fp_policies_ids) == 1)
 	    {
-	      if (*(fp_spd->fp_policies[policy->type] + iii) ==
-		  *(result_val->fp_policies_ids + ii))
-		{
-		  if (vec_len (result_val->fp_policies_ids) == 1)
-		    {
-		      vec_free (result_val->fp_policies_ids);
-		      clib_bihash_add_del_16_8 (bihash_table, &result, 0);
-		    }
-		  else
-		    {
-		      vec_del1 (result_val->fp_policies_ids, ii);
-		    }
-		  vec_del1 (fp_spd->fp_policies[policy->type], iii);
-
-		  vec_foreach_index (imt, fp_spd->fp_mask_ids[policy->type])
-		    {
-		      if ((fp_spd->fp_mask_ids[policy->type] + imt)
-			    ->mask_type_idx == vp->fp_mask_type_id)
-			{
-
-			  if ((fp_spd->fp_mask_ids[policy->type] + imt)
-				->refcount-- == 1)
-			    vec_del1 (fp_spd->fp_mask_ids[policy->type], imt);
-
-			  break;
-			}
-		    }
-
-		  res = 0;
-		  break;
-		}
+	      vec_free (result_val->fp_policies_ids);
+	      clib_bihash_add_del_16_8 (bihash_table, &result, 0);
 	    }
-
-	  if (res != 0)
-	    continue;
 	  else
+	    vec_del1 (result_val->fp_policies_ids, ii);
+
+	  vec_foreach_index (imt, fp_spd->fp_mask_ids[policy->type])
 	    {
-	      ipsec_fp_release_mask_type (im, vp->fp_mask_type_id);
-	      ipsec_sa_unlock (vp->sa_index);
-	      pool_put (im->policies, vp);
-	      return 0;
+	      if ((fp_spd->fp_mask_ids[policy->type] + imt)->mask_type_idx ==
+		  vp->fp_mask_type_id)
+		{
+
+		  if ((fp_spd->fp_mask_ids[policy->type] + imt)->refcount-- ==
+		      1)
+		    vec_del1 (fp_spd->fp_mask_ids[policy->type], imt);
+
+		  break;
+		}
 	    }
+	  ipsec_fp_release_mask_type (im, vp->fp_mask_type_id);
+	  ipsec_sa_unlock (vp->sa_index);
+	  pool_put (im->policies, vp);
+	  return 0;
 	}
     }
   return -1;