npt66: ensure feature is not configured multiple times

If the control agent enabled a binding on an interface multiple times,
we would add the node in the feature arc multiple times.

Type: fix
Change-Id: I2ca247db0a0211f5fa3974a18ca4fcae8485cb12
Signed-off-by: Ole Troan <otroan@employees.org>

2 files changed, 23 insertions, 12 deletions

diff --git a/src/plugins/npt66/npt66.c b/src/plugins/npt66/npt66.c
index e3cbbbd1a7b..277fce496fc 100644
--- a/src/plugins/npt66/npt66.c
+++ b/src/plugins/npt66/npt66.c
@@ -49,17 +49,24 @@ npt66_binding_add_del (u32 sw_if_index, ip6_address_t *internal,
 		       int external_plen, bool is_add)
 {
   npt66_main_t *nm = &npt66_main;
+  int rv = 0;
+
+  /* Currently limited to a single binding per interface */
+  npt66_binding_t *b = npt66_interface_by_sw_if_index (sw_if_index);
 
   if (is_add)
     {
-
+      bool configure_feature = false;
       /* Ensure prefix lengths are less than or equal to a /64 */
       if (internal_plen > 64 || external_plen > 64)
 	return VNET_API_ERROR_INVALID_VALUE;
 
-      /* Create a binding entry */
-      npt66_binding_t *b;
-      pool_get_zero (nm->bindings, b);
+      /* Create a binding entry (or update existing) */
+      if (!b)
+	{
+	  pool_get_zero (nm->bindings, b);
+	  configure_feature = true;
+	}
       b->internal = *internal;
       b->internal_plen = internal_plen;
       b->external = *external;
@@ -78,6 +85,9 @@ npt66_binding_add_del (u32 sw_if_index, ip6_address_t *internal,
       delta = ip_csum_sub_even (delta, b->internal.as_u64[1]);
       delta = ip_csum_fold (delta);
       b->delta = delta;
+
+      if (configure_feature)
+	rv = npt66_feature_enable_disable (sw_if_index, is_add);
     }
   else
     {
@@ -87,11 +97,9 @@ npt66_binding_add_del (u32 sw_if_index, ip6_address_t *internal,
 	return VNET_API_ERROR_NO_SUCH_ENTRY;
       nm->interface_by_sw_if_index[sw_if_index] = ~0;
       pool_put (nm->bindings, b);
+      rv = npt66_feature_enable_disable (sw_if_index, is_add);
     }
 
-  /* Enable feature on interface */
-  int rv = npt66_feature_enable_disable (sw_if_index, is_add);
-
   return rv;
 }
 
diff --git a/src/plugins/npt66/npt66_node.c b/src/plugins/npt66/npt66_node.c
index 21dddee3951..960e4d446dd 100644
--- a/src/plugins/npt66/npt66_node.c
+++ b/src/plugins/npt66/npt66_node.c
@@ -126,7 +126,10 @@ npt66_translate (ip6_header_t *ip, npt66_binding_t *binding, int dir)
       if (!ip6_prefix_cmp (ip->src_address, binding->internal,
 			   binding->internal_plen))
 	{
-	  clib_warning ("npt66_translate: src address is not internal");
+	  clib_warning (
+	    "npt66_translate: src address is not internal (%U -> %U)",
+	    format_ip6_address, &ip->src_address, format_ip6_address,
+	    &ip->dst_address);
 	  goto done;
 	}
       ip->src_address = ip6_prefix_copy (ip->src_address, binding->external,
@@ -140,7 +143,10 @@ npt66_translate (ip6_header_t *ip, npt66_binding_t *binding, int dir)
       if (!ip6_prefix_cmp (ip->dst_address, binding->external,
 			   binding->external_plen))
 	{
-	  clib_warning ("npt66_translate: dst address is not external");
+	  clib_warning (
+	    "npt66_translate: dst address is not external (%U -> %U)",
+	    format_ip6_address, &ip->src_address, format_ip6_address,
+	    &ip->dst_address);
 	  goto done;
 	}
       ip->dst_address = ip6_prefix_copy (ip->dst_address, binding->internal,
@@ -177,7 +183,6 @@ npt66_node_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
   /* Stage 1: build vector of flow hash (based on lookup mask) */
   while (n_left_from > 0)
     {
-      clib_warning ("DIRECTION: %u", dir);
       u32 sw_if_index = vnet_buffer (b[0])->sw_if_index[dir];
       u32 iph_offset =
 	dir == VLIB_TX ? vnet_buffer (b[0])->ip.save_rewrite_length : 0;
@@ -278,10 +283,8 @@ VLIB_REGISTER_NODE (npt66_output_node) = {
 VNET_FEATURE_INIT (npt66_input, static) = {
   .arc_name = "ip6-unicast",
   .node_name = "npt66-input",
-  .runs_after = VNET_FEATURES ("ip4-sv-reassembly-feature"),
 };
 VNET_FEATURE_INIT (npt66_output, static) = {
   .arc_name = "ip6-output",
   .node_name = "npt66-output",
-  .runs_after = VNET_FEATURES ("ip4-sv-reassembly-output-feature"),
 };