UDP-Encap: name counters for the stats segment

change the ADD api so that is returns the 'ID' that can be used
by the client to read the stats from the stats segment and to
delete the object. Previously a similar value used required
to be chosen by the client, now VPP allocates one (like it
does e.g. for interfaces)

Change-Id: I563cf6092276eb990c52d5457c86e72546bcf69e
Signed-off-by: Neale Ranns <nranns@cisco.com>

4 files changed, 131 insertions, 206 deletions

diff --git a/src/vnet/udp/udp.api b/src/vnet/udp/udp.api
index 8788fba162d..193b577ad3b 100644
--- a/src/vnet/udp/udp.api
+++ b/src/vnet/udp/udp.api
@@ -33,16 +33,16 @@ import "vnet/ip/ip_types.api";
  * @param src_ip - Encap source address
  * @param dst_port - Encap destination port
  * @param src_port - Encap source port
- * @param id - The ID assigned to this encap object by the caller
+ * @param id - VPP assigned id; ignored in add message, set in dump
  */
 typedef udp_encap
 {
-  u32 id;
   u32 table_id;
   u16 src_port;
   u16 dst_port;
   vl_api_address_t src_ip;
   vl_api_address_t dst_ip;
+  u32 id;
 };
 
 /**
@@ -51,7 +51,7 @@ typedef udp_encap
  * @param context - sender context, to match reply w/ request
  * @param udp_encap - UDP encap description
  */
-autoreply define udp_encap_add
+define udp_encap_add
 {
   u32 client_index;
   u32 context;
@@ -59,6 +59,20 @@ autoreply define udp_encap_add
 };
 
 /**
+ * @brief Add UDP encap reply
+ * @param context - sender context, to match reply w/ request
+ * @param retval - success/failure return value
+ * @param id - The ID of the encap object that should be used on delete
+ *             and when reading stats from the stats segment.
+ */
+define udp_encap_add_reply
+{
+  u32 context;
+  i32 retval;
+  u32 id;
+};
+
+/**
  * @brief Del UDP encap
  * @param client_index - opaque cookie to identify the sender
  * @param context - sender context, to match reply w/ request
diff --git a/src/vnet/udp/udp_api.c b/src/vnet/udp/udp_api.c
index 43a39b82ef0..d00d74e27d5 100644
--- a/src/vnet/udp/udp_api.c
+++ b/src/vnet/udp/udp_api.c
@@ -86,7 +86,7 @@ send_udp_encap_details (const udp_encap_t * ue, vl_api_registration_t * reg,
 
   mp->udp_encap.table_id =
     htonl (fib_table_get_table_id (ue->ue_fib_index, ue->ue_ip_proto));
-  mp->udp_encap.id = htonl (ue->ue_id);
+  mp->udp_encap.id = htonl (ue - udp_encap_pool);
 
   vl_api_send_msg (reg, (u8 *) mp);
 }
@@ -115,12 +115,12 @@ vl_api_udp_encap_add_t_handler (vl_api_udp_encap_add_t * mp, vlib_main_t * vm)
 {
   vl_api_udp_encap_add_reply_t *rmp;
   ip46_address_t src_ip, dst_ip;
-  u32 fib_index, table_id, ue_id;
+  u32 fib_index, table_id;
   fib_protocol_t fproto;
   ip46_type_t itype;
+  index_t uei;
   int rv = 0;
 
-  ue_id = ntohl (mp->udp_encap.id);
   table_id = ntohl (mp->udp_encap.table_id);
 
   itype = ip_address_decode (&mp->udp_encap.src_ip, &src_ip);
@@ -134,14 +134,20 @@ vl_api_udp_encap_add_t_handler (vl_api_udp_encap_add_t * mp, vlib_main_t * vm)
       goto done;
     }
 
-  udp_encap_add_and_lock (ue_id, fproto, fib_index,
-			  &src_ip, &dst_ip,
-			  ntohs (mp->udp_encap.src_port),
-			  ntohs (mp->udp_encap.dst_port),
-			  UDP_ENCAP_FIXUP_NONE);
+  uei = udp_encap_add_and_lock (fproto, fib_index,
+				&src_ip, &dst_ip,
+				ntohs (mp->udp_encap.src_port),
+				ntohs (mp->udp_encap.dst_port),
+				UDP_ENCAP_FIXUP_NONE);
 
 done:
-  REPLY_MACRO (VL_API_UDP_ENCAP_ADD_REPLY);
+  /* *INDENT-OFF* */
+  REPLY_MACRO2 (VL_API_UDP_ENCAP_ADD_REPLY,
+  ({
+    rmp->id = ntohl (uei);
+  }));
+  /* *INDENT-ON* */
+
 }
 
 static void
diff --git a/src/vnet/udp/udp_encap.c b/src/vnet/udp/udp_encap.c
index 914680f98a2..072bc79c8af 100644
--- a/src/vnet/udp/udp_encap.c
+++ b/src/vnet/udp/udp_encap.c
@@ -24,11 +24,6 @@
 dpo_type_t udp_encap_dpo_types[FIB_PROTOCOL_MAX];
 
 /**
- * Hash DB to map from client ID to VPP index.
- */
-uword *udp_encap_db;
-
-/**
  * Pool of encaps
  */
 udp_encap_t *udp_encap_pool;
@@ -36,23 +31,16 @@ udp_encap_t *udp_encap_pool;
 /**
  * Stats for each UDP encap object
  */
-vlib_combined_counter_main_t udp_encap_counters;
-
-static udp_encap_t *
-udp_encap_get_w_id (u32 id)
-{
-  udp_encap_t *ue = NULL;
-  index_t uei;
-
-  uei = udp_encap_find (id);
-
-  if (INDEX_INVALID != uei)
-    {
-      ue = udp_encap_get (uei);
-    }
-
-  return (ue);
-}
+vlib_combined_counter_main_t udp_encap_counters = {
+  /**
+   * The counter collection's name.
+   */
+  .name = "udp-encap",
+  /**
+   * Name in stat segment directory
+   */
+  .stat_segment_name = "/net/udp-encap",
+};
 
 static void
 udp_encap_restack (udp_encap_t * ue)
@@ -64,8 +52,7 @@ udp_encap_restack (udp_encap_t * ue)
 }
 
 index_t
-udp_encap_add_and_lock (u32 id,
-			fib_protocol_t proto,
+udp_encap_add_and_lock (fib_protocol_t proto,
 			index_t fib_index,
 			const ip46_address_t * src_ip,
 			const ip46_address_t * dst_ip,
@@ -73,102 +60,80 @@ udp_encap_add_and_lock (u32 id,
 			u16 dst_port, udp_encap_fixup_flags_t flags)
 {
   udp_encap_t *ue;
+  u8 pfx_len = 0;
   index_t uei;
 
-  uei = udp_encap_find (id);
+  pool_get_aligned (udp_encap_pool, ue, CLIB_CACHE_LINE_BYTES);
+  uei = ue - udp_encap_pool;
 
-  if (INDEX_INVALID == uei)
-    {
-      u8 pfx_len = 0;
+  vlib_validate_combined_counter (&(udp_encap_counters), uei);
+  vlib_zero_combined_counter (&(udp_encap_counters), uei);
 
-      pool_get_aligned (udp_encap_pool, ue, CLIB_CACHE_LINE_BYTES);
-      uei = ue - udp_encap_pool;
+  fib_node_init (&ue->ue_fib_node, FIB_NODE_TYPE_UDP_ENCAP);
+  fib_node_lock (&ue->ue_fib_node);
+  ue->ue_fib_index = fib_index;
+  ue->ue_flags = flags;
+  ue->ue_ip_proto = proto;
 
-      vlib_validate_combined_counter (&(udp_encap_counters), uei);
-      vlib_zero_combined_counter (&(udp_encap_counters), uei);
+  switch (proto)
+    {
+    case FIB_PROTOCOL_IP4:
+      pfx_len = 32;
+      ue->ue_hdrs.ip4.ue_ip4.ip_version_and_header_length = 0x45;
+      ue->ue_hdrs.ip4.ue_ip4.ttl = 254;
+      ue->ue_hdrs.ip4.ue_ip4.protocol = IP_PROTOCOL_UDP;
+      ue->ue_hdrs.ip4.ue_ip4.src_address.as_u32 = src_ip->ip4.as_u32;
+      ue->ue_hdrs.ip4.ue_ip4.dst_address.as_u32 = dst_ip->ip4.as_u32;
+      ue->ue_hdrs.ip4.ue_ip4.checksum =
+	ip4_header_checksum (&ue->ue_hdrs.ip4.ue_ip4);
+      ue->ue_hdrs.ip4.ue_udp.src_port = clib_host_to_net_u16 (src_port);
+      ue->ue_hdrs.ip4.ue_udp.dst_port = clib_host_to_net_u16 (dst_port);
 
-      hash_set (udp_encap_db, id, uei);
+      break;
+    case FIB_PROTOCOL_IP6:
+      pfx_len = 128;
+      ue->ue_hdrs.ip6.ue_ip6.ip_version_traffic_class_and_flow_label =
+	clib_host_to_net_u32 (6 << 28);
+      ue->ue_hdrs.ip6.ue_ip6.hop_limit = 255;
+      ue->ue_hdrs.ip6.ue_ip6.protocol = IP_PROTOCOL_UDP;
+      ue->ue_hdrs.ip6.ue_ip6.src_address.as_u64[0] = src_ip->ip6.as_u64[0];
+      ue->ue_hdrs.ip6.ue_ip6.src_address.as_u64[1] = src_ip->ip6.as_u64[1];
+      ue->ue_hdrs.ip6.ue_ip6.dst_address.as_u64[0] = dst_ip->ip6.as_u64[0];
+      ue->ue_hdrs.ip6.ue_ip6.dst_address.as_u64[1] = dst_ip->ip6.as_u64[1];
+      ue->ue_hdrs.ip6.ue_udp.src_port = clib_host_to_net_u16 (src_port);
+      ue->ue_hdrs.ip6.ue_udp.dst_port = clib_host_to_net_u16 (dst_port);
 
-      fib_node_init (&ue->ue_fib_node, FIB_NODE_TYPE_UDP_ENCAP);
-      fib_node_lock (&ue->ue_fib_node);
-      ue->ue_fib_index = fib_index;
-      ue->ue_flags = flags;
-      ue->ue_id = id;
-      ue->ue_ip_proto = proto;
+      break;
+    default:
+      ASSERT (0);
+    }
 
-      switch (proto)
-	{
-	case FIB_PROTOCOL_IP4:
-	  pfx_len = 32;
-	  ue->ue_hdrs.ip4.ue_ip4.ip_version_and_header_length = 0x45;
-	  ue->ue_hdrs.ip4.ue_ip4.ttl = 254;
-	  ue->ue_hdrs.ip4.ue_ip4.protocol = IP_PROTOCOL_UDP;
-	  ue->ue_hdrs.ip4.ue_ip4.src_address.as_u32 = src_ip->ip4.as_u32;
-	  ue->ue_hdrs.ip4.ue_ip4.dst_address.as_u32 = dst_ip->ip4.as_u32;
-	  ue->ue_hdrs.ip4.ue_ip4.checksum =
-	    ip4_header_checksum (&ue->ue_hdrs.ip4.ue_ip4);
-	  ue->ue_hdrs.ip4.ue_udp.src_port = clib_host_to_net_u16 (src_port);
-	  ue->ue_hdrs.ip4.ue_udp.dst_port = clib_host_to_net_u16 (dst_port);
-
-	  break;
-	case FIB_PROTOCOL_IP6:
-	  pfx_len = 128;
-	  ue->ue_hdrs.ip6.ue_ip6.ip_version_traffic_class_and_flow_label =
-	    clib_host_to_net_u32 (6 << 28);
-	  ue->ue_hdrs.ip6.ue_ip6.hop_limit = 255;
-	  ue->ue_hdrs.ip6.ue_ip6.protocol = IP_PROTOCOL_UDP;
-	  ue->ue_hdrs.ip6.ue_ip6.src_address.as_u64[0] =
-	    src_ip->ip6.as_u64[0];
-	  ue->ue_hdrs.ip6.ue_ip6.src_address.as_u64[1] =
-	    src_ip->ip6.as_u64[1];
-	  ue->ue_hdrs.ip6.ue_ip6.dst_address.as_u64[0] =
-	    dst_ip->ip6.as_u64[0];
-	  ue->ue_hdrs.ip6.ue_ip6.dst_address.as_u64[1] =
-	    dst_ip->ip6.as_u64[1];
-	  ue->ue_hdrs.ip6.ue_udp.src_port = clib_host_to_net_u16 (src_port);
-	  ue->ue_hdrs.ip6.ue_udp.dst_port = clib_host_to_net_u16 (dst_port);
-
-	  break;
-	default:
-	  ASSERT (0);
-	}
+  /*
+   * track the destination address
+   */
+  fib_prefix_t dst_pfx = {
+    .fp_proto = proto,
+    .fp_len = pfx_len,
+    .fp_addr = *dst_ip,
+  };
+
+  ue->ue_fib_entry_index =
+    fib_table_entry_special_add (fib_index,
+				 &dst_pfx,
+				 FIB_SOURCE_RR, FIB_ENTRY_FLAG_NONE);
+  ue->ue_fib_sibling =
+    fib_entry_child_add (ue->ue_fib_entry_index,
+			 FIB_NODE_TYPE_UDP_ENCAP, uei);
+
+  udp_encap_restack (ue);
 
-      /*
-       * track the destination address
-       */
-      fib_prefix_t dst_pfx = {
-	.fp_proto = proto,
-	.fp_len = pfx_len,
-	.fp_addr = *dst_ip,
-      };
-
-      ue->ue_fib_entry_index =
-	fib_table_entry_special_add (fib_index,
-				     &dst_pfx,
-				     FIB_SOURCE_RR, FIB_ENTRY_FLAG_NONE);
-      ue->ue_fib_sibling =
-	fib_entry_child_add (ue->ue_fib_entry_index,
-			     FIB_NODE_TYPE_UDP_ENCAP, uei);
-
-      udp_encap_restack (ue);
-    }
-  else
-    {
-      /*
-       * existing entry. updates not supported yet
-       */
-      uei = INDEX_INVALID;
-    }
   return (uei);
 }
 
 void
-udp_encap_contribute_forwarding (u32 id, dpo_proto_t proto, dpo_id_t * dpo)
+udp_encap_contribute_forwarding (index_t uei, dpo_proto_t proto,
+				 dpo_id_t * dpo)
 {
-  index_t uei;
-
-  uei = udp_encap_find (id);
-
   if (INDEX_INVALID == uei)
     {
       dpo_copy (dpo, drop_dpo_get (proto));
@@ -183,25 +148,12 @@ udp_encap_contribute_forwarding (u32 id, dpo_proto_t proto, dpo_id_t * dpo)
     }
 }
 
-index_t
-udp_encap_find (u32 id)
-{
-  uword *p;
-
-  p = hash_get (udp_encap_db, id);
-
-  if (NULL != p)
-    return p[0];
-
-  return INDEX_INVALID;
-}
-
 void
-udp_encap_lock (u32 id)
+udp_encap_lock (index_t uei)
 {
   udp_encap_t *ue;
 
-  ue = udp_encap_get_w_id (id);
+  ue = udp_encap_get (uei);
 
   if (NULL != ue)
     {
@@ -210,7 +162,7 @@ udp_encap_lock (u32 id)
 }
 
 void
-udp_encap_unlock_w_index (index_t uei)
+udp_encap_unlock (index_t uei)
 {
   udp_encap_t *ue;
 
@@ -227,19 +179,6 @@ udp_encap_unlock_w_index (index_t uei)
     }
 }
 
-void
-udp_encap_unlock (u32 id)
-{
-  udp_encap_t *ue;
-
-  ue = udp_encap_get_w_id (id);
-
-  if (NULL != ue)
-    {
-      fib_node_unlock (&ue->ue_fib_node);
-    }
-}
-
 static void
 udp_encap_dpo_lock (dpo_id_t * dpo)
 {
@@ -272,8 +211,7 @@ format_udp_encap_i (u8 * s, va_list * args)
   ue = udp_encap_get (uei);
 
   // FIXME
-  s = format (s, "udp-ecap:[%d]: id:%d ip-fib-index:%d ",
-	      uei, ue->ue_id, ue->ue_fib_index);
+  s = format (s, "udp-ecap:[%d]: ip-fib-index:%d ", uei, ue->ue_fib_index);
   if (FIB_PROTOCOL_IP4 == ue->ue_ip_proto)
     {
       s = format (s, "ip:[src:%U, dst:%U] udp:[src:%d, dst:%d]",
@@ -331,16 +269,8 @@ format_udp_encap_dpo (u8 * s, va_list * args)
 u8 *
 format_udp_encap (u8 * s, va_list * args)
 {
-  u32 id = va_arg (*args, u32);
+  index_t uei = va_arg (*args, u32);
   u32 details = va_arg (*args, u32);
-  index_t uei;
-
-  uei = udp_encap_find (id);
-
-  if (INDEX_INVALID == uei)
-    {
-      return (format (s, "Invalid udp-encap ID: %d", id));
-    }
 
   return (format (s, "%U", format_udp_encap_i, uei, 0, details));
 }
@@ -391,7 +321,6 @@ udp_encap_fib_last_lock_gone (fib_node_t * node)
      * reset the stacked DPO to unlock it
      */
   dpo_reset (&ue->ue_dpo);
-  hash_unset (udp_encap_db, ue->ue_id);
 
   fib_entry_child_remove (ue->ue_fib_entry_index, ue->ue_fib_sibling);
   fib_table_entry_delete_index (ue->ue_fib_entry_index, FIB_SOURCE_RR);
@@ -473,8 +402,6 @@ const static dpo_vft_t udp_encap_dpo_vft = {
 clib_error_t *
 udp_encap_init (vlib_main_t * vm)
 {
-  udp_encap_db = hash_create (0, sizeof (index_t));
-
   fib_node_register_type (FIB_NODE_TYPE_UDP_ENCAP, &udp_encap_fib_vft);
 
   udp_encap_dpo_types[FIB_PROTOCOL_IP4] =
@@ -494,10 +421,10 @@ udp_encap_cli (vlib_main_t * vm,
   unformat_input_t _line_input, *line_input = &_line_input;
   clib_error_t *error = NULL;
   ip46_address_t src_ip, dst_ip;
-  u32 table_id, ue_id;
-  u32 src_port, dst_port;
+  u32 table_id, src_port, dst_port;
   udp_encap_fixup_flags_t flags;
   fib_protocol_t fproto;
+  index_t uei;
   u8 is_del;
 
   is_del = 0;
@@ -505,7 +432,7 @@ udp_encap_cli (vlib_main_t * vm,
   flags = UDP_ENCAP_FIXUP_NONE;
   fproto = FIB_PROTOCOL_MAX;
   dst_port = 0;
-  ue_id = ~0;
+  uei = ~0;
 
   /* Get a line of input. */
   if (!unformat_user (main_input, unformat_line_input, line_input))
@@ -513,7 +440,7 @@ udp_encap_cli (vlib_main_t * vm,
 
   while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
     {
-      if (unformat (line_input, "id %d", &ue_id))
+      if (unformat (line_input, "index %d", &uei))
 	;
       else if (unformat (line_input, "add"))
 	is_del = 0;
@@ -542,13 +469,6 @@ udp_encap_cli (vlib_main_t * vm,
 	}
     }
 
-  if (~0 == ue_id)
-    {
-      error =
-	clib_error_return (0, "An ID for the UDP encap instance is required");
-      goto done;
-    }
-
   if (!is_del && fproto != FIB_PROTOCOL_MAX)
     {
       u32 fib_index;
@@ -562,28 +482,24 @@ udp_encap_cli (vlib_main_t * vm,
 	  goto done;
 	}
 
-      uei = udp_encap_add_and_lock (ue_id, fproto, fib_index,
+      uei = udp_encap_add_and_lock (fproto, fib_index,
 				    &src_ip, &dst_ip,
 				    src_port, dst_port, flags);
 
+      vlib_cli_output (vm, "udp-encap: %d\n", uei);
+    }
+  else if (is_del)
+    {
       if (INDEX_INVALID == uei)
 	{
-	  error =
-	    clib_error_return (0, "update to existing encap not supported %d",
-			       ue_id);
+	  error = clib_error_return (0, "specify udp-encap object index");
 	  goto done;
 	}
-    }
-  else if (is_del)
-    {
-      udp_encap_unlock (ue_id);
+      udp_encap_unlock (uei);
     }
   else
     {
-      error =
-	clib_error_return (0,
-			   "Some IP addresses would be usefull, don't you think?",
-			   ue_id);
+      error = clib_error_return (0, "specify some IP addresses");
     }
 
 done:
@@ -609,34 +525,32 @@ clib_error_t *
 udp_encap_show (vlib_main_t * vm,
 		unformat_input_t * input, vlib_cli_command_t * cmd)
 {
-  u32 ue_id;
+  index_t uei;
 
-  ue_id = ~0;
+  uei = INDEX_INVALID;
 
   /* Get a line of input. */
   while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
     {
-      if (unformat (input, "%d", &ue_id))
+      if (unformat (input, "%d", &uei))
 	;
       else
 	return clib_error_return (0, "unknown input `%U'",
 				  format_unformat_error, input);
     }
 
-  if (~0 == ue_id)
+  if (INDEX_INVALID == uei)
     {
-      udp_encap_t *ue;
-
       /* *INDENT-OFF* */
-      pool_foreach(ue, udp_encap_pool,
+      pool_foreach_index(uei, udp_encap_pool,
       ({
-        vlib_cli_output(vm, "%U", format_udp_encap, ue->ue_id, 0);
+        vlib_cli_output(vm, "%U", format_udp_encap, uei, 0);
       }));
       /* *INDENT-ON* */
     }
   else
     {
-      vlib_cli_output (vm, "%U", format_udp_encap, ue_id, 1);
+      vlib_cli_output (vm, "%U", format_udp_encap, uei, 1);
     }
 
   return NULL;
diff --git a/src/vnet/udp/udp_encap.h b/src/vnet/udp/udp_encap.h
index e2d652775bc..ac8e88b9643 100644
--- a/src/vnet/udp/udp_encap.h
+++ b/src/vnet/udp/udp_encap.h
@@ -93,12 +93,6 @@ typedef struct udp_encap_t_
   fib_node_t ue_fib_node;
 
   /**
-   * The ID given by the user/client.
-   * This ID is used by the client for modifications.
-   */
-  u32 ue_id;
-
-  /**
    * Tracking information for the IP destination
    */
   fib_node_index_t ue_fib_entry_index;
@@ -110,8 +104,7 @@ typedef struct udp_encap_t_
   index_t ue_fib_index;
 } udp_encap_t;
 
-extern index_t udp_encap_add_and_lock (u32 id,
-				       fib_protocol_t proto,
+extern index_t udp_encap_add_and_lock (fib_protocol_t proto,
 				       index_t fib_index,
 				       const ip46_address_t * src_ip,
 				       const ip46_address_t * dst_ip,
@@ -119,12 +112,10 @@ extern index_t udp_encap_add_and_lock (u32 id,
 				       u16 dst_port,
 				       udp_encap_fixup_flags_t flags);
 
-extern index_t udp_encap_find (u32 id);
-extern void udp_encap_lock (u32 id);
-extern void udp_encap_unlock (u32 id);
+extern void udp_encap_lock (index_t uei);
+extern void udp_encap_unlock (index_t uei);
 extern u8 *format_udp_encap (u8 * s, va_list * args);
-extern void udp_encap_unlock_w_index (index_t uei);
-extern void udp_encap_contribute_forwarding (u32 id,
+extern void udp_encap_contribute_forwarding (index_t uei,
 					     dpo_proto_t proto,
 					     dpo_id_t * dpo);