nat: prevent usage of ED nodes in EI code

Type: fix
Change-Id: I84d511c30eb5878a1867f5e9d2207a39d4f0926c
Signed-off-by: Klement Sekera <ksekera@cisco.com>

2 files changed, 118 insertions, 44 deletions

diff --git a/src/plugins/nat/nat44-ei/nat44_ei.c b/src/plugins/nat/nat44-ei/nat44_ei.c
index d7e74bbc725..9ee03626882 100644
--- a/src/plugins/nat/nat44-ei/nat44_ei.c
+++ b/src/plugins/nat/nat44-ei/nat44_ei.c
@@ -86,6 +86,12 @@ VNET_FEATURE_INIT (ip4_nat_classify, static) = {
   .runs_after = VNET_FEATURES ("acl-plugin-in-ip4-fa",
 			       "ip4-sv-reassembly-feature"),
 };
+VNET_FEATURE_INIT (ip4_nat_handoff_classify, static) = {
+  .arc_name = "ip4-unicast",
+  .node_name = "nat44-ei-handoff-classify",
+  .runs_after = VNET_FEATURES ("acl-plugin-in-ip4-fa",
+			       "ip4-sv-reassembly-feature"),
+};
 VNET_FEATURE_INIT (ip4_nat44_ei_in2out, static) = {
   .arc_name = "ip4-unicast",
   .node_name = "nat44-ei-in2out",
@@ -575,7 +581,9 @@ feature_set:
 
 		  if (nm->num_workers > 1)
 		    {
-		      del_feature_name = "nat44-handoff-classify";
+		      del_feature_name = "nat44-ei-handoff-classify";
+		      clib_warning (
+			"del_feature_name = nat44-ei-handoff-classify");
 		      feature_name = !is_inside ?
 				       "nat44-ei-in2out-worker-handoff" :
 				       "nat44-ei-out2in-worker-handoff";
@@ -583,6 +591,7 @@ feature_set:
 		  else
 		    {
 		      del_feature_name = "nat44-ei-classify";
+		      clib_warning ("del_feature_name = nat44-ei-classify");
 		      feature_name =
 			!is_inside ? "nat44-ei-in2out" : "nat44-ei-out2in";
 		    }
@@ -591,15 +600,21 @@ feature_set:
 		    ip4_sv_reass_enable_disable_with_refcnt (sw_if_index, 0);
 		  if (rv)
 		    return rv;
-		  vnet_feature_enable_disable ("ip4-unicast", del_feature_name,
-					       sw_if_index, 0, 0, 0);
-		  vnet_feature_enable_disable ("ip4-unicast", feature_name,
-					       sw_if_index, 1, 0, 0);
+		  rv = vnet_feature_enable_disable (
+		    "ip4-unicast", del_feature_name, sw_if_index, 0, 0, 0);
+		  if (rv)
+		    return rv;
+		  rv = vnet_feature_enable_disable (
+		    "ip4-unicast", feature_name, sw_if_index, 1, 0, 0);
+		  if (rv)
+		    return rv;
 		  if (!is_inside)
 		    {
-		      vnet_feature_enable_disable ("ip4-local",
-						   "nat44-ei-hairpinning",
-						   sw_if_index, 1, 0, 0);
+		      rv = vnet_feature_enable_disable ("ip4-local",
+							"nat44-ei-hairpinning",
+							sw_if_index, 1, 0, 0);
+		      if (rv)
+			return rv;
 		    }
 		}
 	      else
@@ -608,14 +623,18 @@ feature_set:
 		    ip4_sv_reass_enable_disable_with_refcnt (sw_if_index, 0);
 		  if (rv)
 		    return rv;
-		  vnet_feature_enable_disable ("ip4-unicast", feature_name,
-					       sw_if_index, 0, 0, 0);
+		  rv = vnet_feature_enable_disable (
+		    "ip4-unicast", feature_name, sw_if_index, 0, 0, 0);
+		  if (rv)
+		    return rv;
 		  pool_put (nm->interfaces, i);
 		  if (is_inside)
 		    {
-		      vnet_feature_enable_disable ("ip4-local",
-						   "nat44-ei-hairpinning",
-						   sw_if_index, 0, 0, 0);
+		      rv = vnet_feature_enable_disable ("ip4-local",
+							"nat44-ei-hairpinning",
+							sw_if_index, 0, 0, 0);
+		      if (rv)
+			return rv;
 		    }
 		}
 	    }
@@ -630,27 +649,35 @@ feature_set:
 		  del_feature_name = !is_inside ?
 				       "nat44-ei-in2out-worker-handoff" :
 				       "nat44-ei-out2in-worker-handoff";
-		  feature_name = "nat44-handoff-classify";
+		  feature_name = "nat44-ei-handoff-classify";
+		  clib_warning ("feature_name = nat44-ei-handoff-classify");
 		}
 	      else
 		{
 		  del_feature_name =
 		    !is_inside ? "nat44-ei-in2out" : "nat44-ei-out2in";
 		  feature_name = "nat44-ei-classify";
+		  clib_warning ("feature_name = nat44-ei-classify");
 		}
 
 	      int rv =
 		ip4_sv_reass_enable_disable_with_refcnt (sw_if_index, 1);
 	      if (rv)
 		return rv;
-	      vnet_feature_enable_disable ("ip4-unicast", del_feature_name,
-					   sw_if_index, 0, 0, 0);
-	      vnet_feature_enable_disable ("ip4-unicast", feature_name,
-					   sw_if_index, 1, 0, 0);
+	      rv = vnet_feature_enable_disable (
+		"ip4-unicast", del_feature_name, sw_if_index, 0, 0, 0);
+	      if (rv)
+		return rv;
+	      rv = vnet_feature_enable_disable ("ip4-unicast", feature_name,
+						sw_if_index, 1, 0, 0);
+	      if (rv)
+		return rv;
 	      if (!is_inside)
 		{
-		  vnet_feature_enable_disable (
+		  rv = vnet_feature_enable_disable (
 		    "ip4-local", "nat44-ei-hairpinning", sw_if_index, 0, 0, 0);
+		  if (rv)
+		    return rv;
 		}
 	      goto set_flags;
 	    }
@@ -670,17 +697,21 @@ feature_set:
   i->flags = 0;
   nat_validate_interface_counters (nm, sw_if_index);
 
-  vnet_feature_enable_disable ("ip4-unicast", feature_name, sw_if_index, 1, 0,
-			       0);
+  int rv = vnet_feature_enable_disable ("ip4-unicast", feature_name,
+					sw_if_index, 1, 0, 0);
+  if (rv)
+    return rv;
 
-  int rv = ip4_sv_reass_enable_disable_with_refcnt (sw_if_index, 1);
+  rv = ip4_sv_reass_enable_disable_with_refcnt (sw_if_index, 1);
   if (rv)
     return rv;
 
   if (is_inside && !nm->out2in_dpo)
     {
-      vnet_feature_enable_disable ("ip4-local", "nat44-ei-hairpinning",
-				   sw_if_index, 1, 0, 0);
+      rv = vnet_feature_enable_disable ("ip4-local", "nat44-ei-hairpinning",
+					sw_if_index, 1, 0, 0);
+      if (rv)
+	return rv;
     }
 
 set_flags:
@@ -775,10 +806,14 @@ feature_set:
 	ip4_sv_reass_output_enable_disable_with_refcnt (sw_if_index, !is_del);
       if (rv)
 	return rv;
-      vnet_feature_enable_disable ("ip4-unicast", "nat44-ei-hairpin-dst",
-				   sw_if_index, !is_del, 0, 0);
-      vnet_feature_enable_disable ("ip4-output", "nat44-ei-hairpin-src",
-				   sw_if_index, !is_del, 0, 0);
+      rv = vnet_feature_enable_disable ("ip4-unicast", "nat44-ei-hairpin-dst",
+					sw_if_index, !is_del, 0, 0);
+      if (rv)
+	return rv;
+      rv = vnet_feature_enable_disable ("ip4-output", "nat44-ei-hairpin-src",
+					sw_if_index, !is_del, 0, 0);
+      if (rv)
+	return rv;
       goto fq;
     }
 
@@ -791,12 +826,16 @@ feature_set:
 	ip4_sv_reass_output_enable_disable_with_refcnt (sw_if_index, !is_del);
       if (rv)
 	return rv;
-      vnet_feature_enable_disable ("ip4-unicast",
-				   "nat44-ei-out2in-worker-handoff",
-				   sw_if_index, !is_del, 0, 0);
-      vnet_feature_enable_disable ("ip4-output",
-				   "nat44-ei-in2out-output-worker-handoff",
-				   sw_if_index, !is_del, 0, 0);
+      rv = vnet_feature_enable_disable ("ip4-unicast",
+					"nat44-ei-out2in-worker-handoff",
+					sw_if_index, !is_del, 0, 0);
+      if (rv)
+	return rv;
+      rv = vnet_feature_enable_disable (
+	"ip4-output", "nat44-ei-in2out-output-worker-handoff", sw_if_index,
+	!is_del, 0, 0);
+      if (rv)
+	return rv;
     }
   else
     {
@@ -807,10 +846,14 @@ feature_set:
 	ip4_sv_reass_output_enable_disable_with_refcnt (sw_if_index, !is_del);
       if (rv)
 	return rv;
-      vnet_feature_enable_disable ("ip4-unicast", "nat44-ei-out2in",
-				   sw_if_index, !is_del, 0, 0);
-      vnet_feature_enable_disable ("ip4-output", "nat44-ei-in2out-output",
-				   sw_if_index, !is_del, 0, 0);
+      rv = vnet_feature_enable_disable ("ip4-unicast", "nat44-ei-out2in",
+					sw_if_index, !is_del, 0, 0);
+      if (rv)
+	return rv;
+      rv = vnet_feature_enable_disable ("ip4-output", "nat44-ei-in2out-output",
+					sw_if_index, !is_del, 0, 0);
+      if (rv)
+	return rv;
     }
 
 fq:
@@ -2900,8 +2943,9 @@ match:
 			"i4", rv);
 }
 
-VLIB_NODE_FN (nat44_ei_classify_node)
-(vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame)
+static_always_inline uword
+nat44_ei_classify_inline_fn (vlib_main_t *vm, vlib_node_runtime_t *node,
+			     vlib_frame_t *frame)
 {
   u32 n_left_from, *from, *to_next;
   nat44_ei_classify_next_t next_index;
@@ -3000,6 +3044,12 @@ VLIB_NODE_FN (nat44_ei_classify_node)
   return frame->n_vectors;
 }
 
+VLIB_NODE_FN (nat44_ei_classify_node)
+(vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame)
+{
+  return nat44_ei_classify_inline_fn (vm, node, frame);
+}
+
 VLIB_REGISTER_NODE (nat44_ei_classify_node) = {
   .name = "nat44-ei-classify",
   .vector_size = sizeof (u32),
@@ -3015,6 +3065,27 @@ VLIB_REGISTER_NODE (nat44_ei_classify_node) = {
   },
 };
 
+VLIB_NODE_FN (nat44_ei_handoff_classify_node)
+(vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame)
+{
+  return nat44_ei_classify_inline_fn (vm, node, frame);
+}
+
+VLIB_REGISTER_NODE (nat44_ei_handoff_classify_node) = {
+  .name = "nat44-ei-handoff-classify",
+  .vector_size = sizeof (u32),
+  .format_trace = format_nat44_ei_classify_trace,
+  .type = VLIB_NODE_TYPE_INTERNAL,
+  .n_errors = ARRAY_LEN(nat44_ei_classify_error_strings),
+  .error_strings = nat44_ei_classify_error_strings,
+  .n_next_nodes = NAT44_EI_CLASSIFY_N_NEXT,
+  .next_nodes = {
+    [NAT44_EI_CLASSIFY_NEXT_IN2OUT] = "nat44-ei-in2out-worker-handoff",
+    [NAT44_EI_CLASSIFY_NEXT_OUT2IN] = "nat44-ei-out2in-worker-handoff",
+    [NAT44_EI_CLASSIFY_NEXT_DROP] = "error-drop",
+  },
+};
+
 /*
  * fd.io coding-style-patch-verification: ON
  *
diff --git a/src/plugins/nat/test/test_nat44_ei.py b/src/plugins/nat/test/test_nat44_ei.py
index f5c5abefa9e..cd1941a7f12 100644
--- a/src/plugins/nat/test/test_nat44_ei.py
+++ b/src/plugins/nat/test/test_nat44_ei.py
@@ -3044,11 +3044,14 @@ class TestNAT44EI(MethodHolder):
             self.logger.error(ppp("Unexpected or invalid packet:", p))
             raise
 
-        err = self.statistics.get_err_counter(
-            '/err/nat44-ei-classify/next in2out')
+        if self.vpp_worker_count > 1:
+            node = "nat44-ei-handoff-classify"
+        else:
+            node = "nat44-ei-classify"
+
+        err = self.statistics.get_err_counter('/err/%s/next in2out' % node)
         self.assertEqual(err, 1)
-        err = self.statistics.get_err_counter(
-            '/err/nat44-ei-classify/next out2in')
+        err = self.statistics.get_err_counter('/err/%s/next out2in' % node)
         self.assertEqual(err, 1)
 
     def test_del_session(self):