UDP-Local: fix unregistered ports

- if the port is unregistered then write ~0 into the sparse vec, this allows the DP to send packets to ICMP
- remove the v6 arcs from the v4 node and vice-versa (since they're never taken)
- i have tests for this in a pending change for the punt socket

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

1 files changed, 51 insertions, 64 deletions

diff --git a/src/vnet/udp/udp_local.c b/src/vnet/udp/udp_local.c
index a6a4867a86e..af54b73e2a2 100644
--- a/src/vnet/udp/udp_local.c
+++ b/src/vnet/udp/udp_local.c
@@ -21,25 +21,14 @@
 #include <vnet/udp/udp_packet.h>
 #include <vppinfra/sparse_vec.h>
 
-#define foreach_udp_local_next                  \
-  _ (PUNT4, "ip4-punt")                         \
-  _ (PUNT6, "ip6-punt")                         \
-  _ (DROP4, "ip4-drop")                         \
-  _ (DROP6, "ip6-drop")                         \
-  _ (ICMP4_ERROR, "ip4-icmp-error")             \
-  _ (ICMP6_ERROR, "ip6-icmp-error")
-
 typedef enum
 {
-#define _(s,n) UDP_LOCAL_NEXT_##s,
-  foreach_udp_local_next
-#undef _
-    UDP_LOCAL_N_NEXT,
+  UDP_LOCAL_NEXT_PUNT,
+  UDP_LOCAL_NEXT_DROP,
+  UDP_LOCAL_NEXT_ICMP,
+  UDP_LOCAL_N_NEXT
 } udp_local_next_t;
 
-#define udp_local_next_drop(is_ip4)      ((is_ip4) ? UDP_LOCAL_NEXT_DROP4 : UDP_LOCAL_NEXT_DROP6)
-#define udp_local_next_punt(is_ip4)      ((is_ip4) ? UDP_LOCAL_NEXT_PUNT4 : UDP_LOCAL_NEXT_PUNT6)
-
 typedef struct
 {
   u16 src_port;
@@ -47,6 +36,8 @@ typedef struct
   u8 bound;
 } udp_local_rx_trace_t;
 
+#define UDP_NO_NODE_SET ((u16) ~0)
+
 #ifndef CLIB_MARCH_VARIANT
 u8 *
 format_udp_rx_trace (u8 * s, va_list * args)
@@ -72,7 +63,8 @@ udp46_local_inline (vlib_main_t * vm,
   __attribute__ ((unused)) u32 n_left_from, next_index, *from, *to_next;
   word n_no_listener = 0;
   u8 punt_unknown = is_ip4 ? um->punt_unknown4 : um->punt_unknown6;
-
+  u16 *next_by_dst_port = (is_ip4 ?
+			   um->next_by_dst_port4 : um->next_by_dst_port6);
   from = vlib_frame_vector_args (from_frame);
   n_left_from = from_frame->n_vectors;
 
@@ -134,53 +126,51 @@ udp46_local_inline (vlib_main_t * vm,
 	  if (PREDICT_FALSE (b0->current_length < advance0 + sizeof (*h0)))
 	    {
 	      error0 = UDP_ERROR_LENGTH_ERROR;
-	      next0 = udp_local_next_drop (is_ip4);
+	      next0 = UDP_LOCAL_NEXT_DROP;
 	    }
 	  else
 	    {
 	      vlib_buffer_advance (b0, advance0);
 	      h0 = vlib_buffer_get_current (b0);
-	      error0 = next0 = 0;
+	      error0 = UDP_ERROR_NONE;
+	      next0 = UDP_LOCAL_NEXT_PUNT;
 	      if (PREDICT_FALSE (clib_net_to_host_u16 (h0->length) >
 				 vlib_buffer_length_in_chain (vm, b0)))
 		{
 		  error0 = UDP_ERROR_LENGTH_ERROR;
-		  next0 = udp_local_next_drop (is_ip4);
+		  next0 = UDP_LOCAL_NEXT_DROP;
 		}
 	    }
 
 	  if (PREDICT_FALSE (b1->current_length < advance1 + sizeof (*h1)))
 	    {
 	      error1 = UDP_ERROR_LENGTH_ERROR;
-	      next1 = udp_local_next_drop (is_ip4);
+	      next1 = UDP_LOCAL_NEXT_DROP;
 	    }
 	  else
 	    {
 	      vlib_buffer_advance (b1, advance1);
 	      h1 = vlib_buffer_get_current (b1);
-	      error1 = next1 = 0;
+	      error1 = UDP_ERROR_NONE;
+	      next1 = UDP_LOCAL_NEXT_PUNT;
 	      if (PREDICT_FALSE (clib_net_to_host_u16 (h1->length) >
 				 vlib_buffer_length_in_chain (vm, b1)))
 		{
 		  error1 = UDP_ERROR_LENGTH_ERROR;
-		  next1 = udp_local_next_drop (is_ip4);
+		  next1 = UDP_LOCAL_NEXT_DROP;
 		}
 	    }
 
 	  /* Index sparse array with network byte order. */
 	  dst_port0 = (error0 == 0) ? h0->dst_port : 0;
 	  dst_port1 = (error1 == 0) ? h1->dst_port : 0;
-	  sparse_vec_index2 (is_ip4 ? um->next_by_dst_port4 :
-			     um->next_by_dst_port6,
-			     dst_port0, dst_port1, &i0, &i1);
-	  next0 = (error0 == 0) ?
-	    vec_elt (is_ip4 ? um->next_by_dst_port4 : um->next_by_dst_port6,
-		     i0) : next0;
-	  next1 = (error1 == 0) ?
-	    vec_elt (is_ip4 ? um->next_by_dst_port4 : um->next_by_dst_port6,
-		     i1) : next1;
-
-	  if (PREDICT_FALSE (i0 == SPARSE_VEC_INVALID_INDEX))
+	  sparse_vec_index2 (next_by_dst_port, dst_port0, dst_port1, &i0,
+			     &i1);
+	  next0 = (error0 == 0) ? vec_elt (next_by_dst_port, i0) : next0;
+	  next1 = (error1 == 0) ? vec_elt (next_by_dst_port, i1) : next1;
+
+	  if (PREDICT_FALSE (i0 == SPARSE_VEC_INVALID_INDEX ||
+			     next0 == UDP_NO_NODE_SET))
 	    {
 	      // move the pointer back so icmp-error can find the
 	      // ip packet header
@@ -189,7 +179,7 @@ udp46_local_inline (vlib_main_t * vm,
 	      if (PREDICT_FALSE (punt_unknown))
 		{
 		  b0->error = node->errors[UDP_ERROR_PUNT];
-		  next0 = udp_local_next_punt (is_ip4);
+		  next0 = UDP_LOCAL_NEXT_PUNT;
 		}
 	      else if (is_ip4)
 		{
@@ -197,7 +187,7 @@ udp46_local_inline (vlib_main_t * vm,
 					       ICMP4_destination_unreachable,
 					       ICMP4_destination_unreachable_port_unreachable,
 					       0);
-		  next0 = UDP_LOCAL_NEXT_ICMP4_ERROR;
+		  next0 = UDP_LOCAL_NEXT_ICMP;
 		  n_no_listener++;
 		}
 	      else
@@ -206,7 +196,7 @@ udp46_local_inline (vlib_main_t * vm,
 					       ICMP6_destination_unreachable,
 					       ICMP6_destination_unreachable_port_unreachable,
 					       0);
-		  next0 = UDP_LOCAL_NEXT_ICMP6_ERROR;
+		  next0 = UDP_LOCAL_NEXT_ICMP;
 		  n_no_listener++;
 		}
 	    }
@@ -217,7 +207,8 @@ udp46_local_inline (vlib_main_t * vm,
 	      vlib_buffer_advance (b0, sizeof (*h0));
 	    }
 
-	  if (PREDICT_FALSE (i1 == SPARSE_VEC_INVALID_INDEX))
+	  if (PREDICT_FALSE (i1 == SPARSE_VEC_INVALID_INDEX ||
+			     next1 == UDP_NO_NODE_SET))
 	    {
 	      // move the pointer back so icmp-error can find the
 	      // ip packet header
@@ -226,7 +217,7 @@ udp46_local_inline (vlib_main_t * vm,
 	      if (PREDICT_FALSE (punt_unknown))
 		{
 		  b1->error = node->errors[UDP_ERROR_PUNT];
-		  next1 = udp_local_next_punt (is_ip4);
+		  next1 = UDP_LOCAL_NEXT_PUNT;
 		}
 	      else if (is_ip4)
 		{
@@ -234,7 +225,7 @@ udp46_local_inline (vlib_main_t * vm,
 					       ICMP4_destination_unreachable,
 					       ICMP4_destination_unreachable_port_unreachable,
 					       0);
-		  next1 = UDP_LOCAL_NEXT_ICMP4_ERROR;
+		  next1 = UDP_LOCAL_NEXT_ICMP;
 		  n_no_listener++;
 		}
 	      else
@@ -243,7 +234,7 @@ udp46_local_inline (vlib_main_t * vm,
 					       ICMP6_destination_unreachable,
 					       ICMP6_destination_unreachable_port_unreachable,
 					       0);
-		  next1 = UDP_LOCAL_NEXT_ICMP6_ERROR;
+		  next1 = UDP_LOCAL_NEXT_ICMP;
 		  n_no_listener++;
 		}
 	    }
@@ -262,8 +253,7 @@ udp46_local_inline (vlib_main_t * vm,
 		{
 		  tr->src_port = h0 ? h0->src_port : 0;
 		  tr->dst_port = h0 ? h0->dst_port : 0;
-		  tr->bound = (next0 != UDP_LOCAL_NEXT_ICMP4_ERROR &&
-			       next0 != UDP_LOCAL_NEXT_ICMP6_ERROR);
+		  tr->bound = (next0 != UDP_LOCAL_NEXT_ICMP);
 		}
 	    }
 	  if (PREDICT_FALSE (b1->flags & VLIB_BUFFER_IS_TRACED))
@@ -274,8 +264,7 @@ udp46_local_inline (vlib_main_t * vm,
 		{
 		  tr->src_port = h1 ? h1->src_port : 0;
 		  tr->dst_port = h1 ? h1->dst_port : 0;
-		  tr->bound = (next1 != UDP_LOCAL_NEXT_ICMP4_ERROR &&
-			       next1 != UDP_LOCAL_NEXT_ICMP6_ERROR);
+		  tr->bound = (next1 != UDP_LOCAL_NEXT_ICMP);
 		}
 	    }
 
@@ -310,7 +299,7 @@ udp46_local_inline (vlib_main_t * vm,
 	  if (PREDICT_FALSE (b0->current_length < advance0 + sizeof (*h0)))
 	    {
 	      b0->error = node->errors[UDP_ERROR_LENGTH_ERROR];
-	      next0 = udp_local_next_drop (is_ip4);
+	      next0 = UDP_LOCAL_NEXT_DROP;
 	      goto trace_x1;
 	    }
 
@@ -321,12 +310,11 @@ udp46_local_inline (vlib_main_t * vm,
 	  if (PREDICT_TRUE (clib_net_to_host_u16 (h0->length) <=
 			    vlib_buffer_length_in_chain (vm, b0)))
 	    {
-	      i0 = sparse_vec_index (is_ip4 ? um->next_by_dst_port4 :
-				     um->next_by_dst_port6, h0->dst_port);
-	      next0 = vec_elt (is_ip4 ? um->next_by_dst_port4 :
-			       um->next_by_dst_port6, i0);
+	      i0 = sparse_vec_index (next_by_dst_port, h0->dst_port);
+	      next0 = vec_elt (next_by_dst_port, i0);
 
-	      if (PREDICT_FALSE (i0 == SPARSE_VEC_INVALID_INDEX))
+	      if (PREDICT_FALSE ((i0 == SPARSE_VEC_INVALID_INDEX) ||
+				 next0 == UDP_NO_NODE_SET))
 		{
 		  // move the pointer back so icmp-error can find the
 		  // ip packet header
@@ -335,7 +323,7 @@ udp46_local_inline (vlib_main_t * vm,
 		  if (PREDICT_FALSE (punt_unknown))
 		    {
 		      b0->error = node->errors[UDP_ERROR_PUNT];
-		      next0 = udp_local_next_punt (is_ip4);
+		      next0 = UDP_LOCAL_NEXT_PUNT;
 		    }
 		  else if (is_ip4)
 		    {
@@ -343,7 +331,7 @@ udp46_local_inline (vlib_main_t * vm,
 						   ICMP4_destination_unreachable,
 						   ICMP4_destination_unreachable_port_unreachable,
 						   0);
-		      next0 = UDP_LOCAL_NEXT_ICMP4_ERROR;
+		      next0 = UDP_LOCAL_NEXT_ICMP;
 		      n_no_listener++;
 		    }
 		  else
@@ -352,7 +340,7 @@ udp46_local_inline (vlib_main_t * vm,
 						   ICMP6_destination_unreachable,
 						   ICMP6_destination_unreachable_port_unreachable,
 						   0);
-		      next0 = UDP_LOCAL_NEXT_ICMP6_ERROR;
+		      next0 = UDP_LOCAL_NEXT_ICMP;
 		      n_no_listener++;
 		    }
 		}
@@ -366,7 +354,7 @@ udp46_local_inline (vlib_main_t * vm,
 	  else
 	    {
 	      b0->error = node->errors[UDP_ERROR_LENGTH_ERROR];
-	      next0 = udp_local_next_drop (is_ip4);
+	      next0 = UDP_LOCAL_NEXT_DROP;
 	    }
 
 	trace_x1:
@@ -378,8 +366,7 @@ udp46_local_inline (vlib_main_t * vm,
 		{
 		  tr->src_port = h0->src_port;
 		  tr->dst_port = h0->dst_port;
-		  tr->bound = (next0 != UDP_LOCAL_NEXT_ICMP4_ERROR &&
-			       next0 != UDP_LOCAL_NEXT_ICMP6_ERROR);
+		  tr->bound = (next0 != UDP_LOCAL_NEXT_ICMP);
 		}
 	    }
 
@@ -426,9 +413,9 @@ VLIB_REGISTER_NODE (udp4_local_node) = {
 
   .n_next_nodes = UDP_LOCAL_N_NEXT,
   .next_nodes = {
-#define _(s,n) [UDP_LOCAL_NEXT_##s] = n,
-    foreach_udp_local_next
-#undef _
+    [UDP_LOCAL_NEXT_PUNT]= "ip4-punt",
+    [UDP_LOCAL_NEXT_DROP]= "ip4-drop",
+    [UDP_LOCAL_NEXT_ICMP]= "ip4-icmp-error",
   },
 
   .format_buffer = format_udp_header,
@@ -448,9 +435,9 @@ VLIB_REGISTER_NODE (udp6_local_node) = {
 
   .n_next_nodes = UDP_LOCAL_N_NEXT,
   .next_nodes = {
-#define _(s,n) [UDP_LOCAL_NEXT_##s] = n,
-    foreach_udp_local_next
-#undef _
+    [UDP_LOCAL_NEXT_PUNT]= "ip6-punt",
+    [UDP_LOCAL_NEXT_DROP]= "ip6-drop",
+    [UDP_LOCAL_NEXT_ICMP]= "ip6-icmp-error",
   },
 
   .format_buffer = format_udp_header,
@@ -538,7 +525,7 @@ udp_unregister_dst_port (vlib_main_t * vm, udp_dst_port_t dst_port, u8 is_ip4)
     n = sparse_vec_validate (um->next_by_dst_port6,
 			     clib_host_to_net_u16 (dst_port));
 
-  n[0] = SPARSE_VEC_INVALID_INDEX;
+  n[0] = UDP_NO_NODE_SET;
 }
 
 bool
@@ -554,7 +541,7 @@ udp_is_valid_dst_port (udp_dst_port_t dst_port, u8 is_ip4)
     n = sparse_vec_validate (um->next_by_dst_port6,
 			     clib_host_to_net_u16 (dst_port));
 
-  return (n[0] != SPARSE_VEC_INVALID_INDEX);
+  return (n[0] != SPARSE_VEC_INVALID_INDEX && n[0] != UDP_NO_NODE_SET);
 }
 
 void