NAT44: fix ICMP virtual fragmentation reassembly (VPP-1466)

Change-Id: I8006bca02948d9121f474a3d14f0576747bb3c51
Signed-off-by: Matus Fabian <matfabia@cisco.com>

5 files changed, 144 insertions, 172 deletions

diff --git a/src/plugins/nat/in2out.c b/src/plugins/nat/in2out.c
index b99aef3944d..cb169814eac 100755
--- a/src/plugins/nat/in2out.c
+++ b/src/plugins/nat/in2out.c
@@ -952,8 +952,7 @@ snat_in2out_node_fn_inline (vlib_main_t * vm,
 	    }
 	  else
 	    {
-	      if (PREDICT_FALSE
-		  (proto0 == ~0 || proto0 == SNAT_PROTOCOL_ICMP))
+	      if (PREDICT_FALSE (proto0 == ~0))
 		{
 		  next0 = SNAT_IN2OUT_NEXT_SLOW_PATH;
 		  goto trace00;
@@ -964,6 +963,12 @@ snat_in2out_node_fn_inline (vlib_main_t * vm,
 		  next0 = SNAT_IN2OUT_NEXT_REASS;
 		  goto trace00;
 		}
+
+	      if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
+		{
+		  next0 = SNAT_IN2OUT_NEXT_SLOW_PATH;
+		  goto trace00;
+		}
 	    }
 
 	  key0.addr = ip0->src_address;
@@ -1131,8 +1136,7 @@ snat_in2out_node_fn_inline (vlib_main_t * vm,
 	    }
 	  else
 	    {
-	      if (PREDICT_FALSE
-		  (proto1 == ~0 || proto1 == SNAT_PROTOCOL_ICMP))
+	      if (PREDICT_FALSE (proto1 == ~0))
 		{
 		  next1 = SNAT_IN2OUT_NEXT_SLOW_PATH;
 		  goto trace01;
@@ -1143,6 +1147,12 @@ snat_in2out_node_fn_inline (vlib_main_t * vm,
 		  next1 = SNAT_IN2OUT_NEXT_REASS;
 		  goto trace01;
 		}
+
+	      if (PREDICT_FALSE (proto1 == SNAT_PROTOCOL_ICMP))
+		{
+		  next1 = SNAT_IN2OUT_NEXT_SLOW_PATH;
+		  goto trace01;
+		}
 	    }
 
 	  key1.addr = ip1->src_address;
@@ -1346,8 +1356,7 @@ snat_in2out_node_fn_inline (vlib_main_t * vm,
 	    }
 	  else
 	    {
-	      if (PREDICT_FALSE
-		  (proto0 == ~0 || proto0 == SNAT_PROTOCOL_ICMP))
+	      if (PREDICT_FALSE (proto0 == ~0))
 		{
 		  next0 = SNAT_IN2OUT_NEXT_SLOW_PATH;
 		  goto trace0;
@@ -1358,6 +1367,12 @@ snat_in2out_node_fn_inline (vlib_main_t * vm,
 		  next0 = SNAT_IN2OUT_NEXT_REASS;
 		  goto trace0;
 		}
+
+	      if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
+		{
+		  next0 = SNAT_IN2OUT_NEXT_SLOW_PATH;
+		  goto trace0;
+		}
 	    }
 
 	  key0.addr = ip0->src_address;
@@ -1672,6 +1687,7 @@ nat44_in2out_reass_node_fn (vlib_main_t * vm,
 	  nat_reass_ip4_t *reass0;
 	  udp_header_t *udp0;
 	  tcp_header_t *tcp0;
+	  icmp46_header_t *icmp0;
 	  snat_session_key_t key0;
 	  clib_bihash_kv_8_8_t kv0, value0;
 	  snat_session_t *s0 = 0;
@@ -1704,6 +1720,7 @@ nat44_in2out_reass_node_fn (vlib_main_t * vm,
 	  ip0 = (ip4_header_t *) vlib_buffer_get_current (b0);
 	  udp0 = ip4_next_header (ip0);
 	  tcp0 = (tcp_header_t *) udp0;
+	  icmp0 = (icmp46_header_t *) udp0;
 	  proto0 = ip_proto_to_snat_proto (ip0->protocol);
 
 	  reass0 = nat_ip4_reass_find_or_create (ip0->src_address,
@@ -1722,6 +1739,25 @@ nat44_in2out_reass_node_fn (vlib_main_t * vm,
 
 	  if (PREDICT_FALSE (ip4_is_first_fragment (ip0)))
 	    {
+	      if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
+		{
+		  next0 = icmp_in2out_slow_path
+		    (sm, b0, ip0, icmp0, sw_if_index0, rx_fib_index0, node,
+		     next0, now, thread_index, &s0);
+
+		  if (PREDICT_TRUE (next0 != SNAT_IN2OUT_NEXT_DROP))
+		    {
+		      if (s0)
+			reass0->sess_index = s0 - per_thread_data->sessions;
+		      else
+			reass0->flags |= NAT_REASS_FLAG_ED_DONT_TRANSLATE;
+		      nat_ip4_reass_get_frags (reass0,
+					       &fragments_to_loopback);
+		    }
+
+		  goto trace0;
+		}
+
 	      key0.addr = ip0->src_address;
 	      key0.port = udp0->src_port;
 	      key0.protocol = proto0;
diff --git a/src/plugins/nat/in2out_ed.c b/src/plugins/nat/in2out_ed.c
index 8db53c08142..f9f8d776eb4 100644
--- a/src/plugins/nat/in2out_ed.c
+++ b/src/plugins/nat/in2out_ed.c
@@ -1960,11 +1960,8 @@ nat44_ed_in2out_reass_node_fn_inline (vlib_main_t * vm,
 	    }
 
 	  /* Hairpinning */
-	  if (PREDICT_TRUE (proto0 != SNAT_PROTOCOL_ICMP))
-	    nat44_reass_hairpinning (sm, b0, ip0, s0->out2in.port,
-				     s0->ext_host_port, proto0, 1);
-	  else
-	    snat_icmp_hairpinning (sm, b0, ip0, icmp0, 1);
+	  nat44_reass_hairpinning (sm, b0, ip0, s0->out2in.port,
+				   s0->ext_host_port, proto0, 1);
 
 	  /* Accounting */
 	  nat44_session_update_counters (s0, now,
diff --git a/src/plugins/nat/nat44_hairpinning.c b/src/plugins/nat/nat44_hairpinning.c
index c07427d6bcb..09ea419e637 100644
--- a/src/plugins/nat/nat44_hairpinning.c
+++ b/src/plugins/nat/nat44_hairpinning.c
@@ -286,39 +286,6 @@ snat_icmp_hairpinning (snat_main_t * sm,
     }
   else
     {
-      if (!is_ed)
-	{
-	  icmp_echo_header_t *echo0 = (icmp_echo_header_t *) (icmp0 + 1);
-	  u16 icmp_id0 = echo0->identifier;
-	  key0.addr = ip0->dst_address;
-	  key0.port = icmp_id0;
-	  key0.protocol = SNAT_PROTOCOL_ICMP;
-	  key0.fib_index = sm->outside_fib_index;
-	  kv0.key = key0.as_u64;
-	  if (sm->num_workers > 1)
-	    ti =
-	      (clib_net_to_host_u16 (icmp_id0) - 1024) / sm->port_per_thread;
-	  else
-	    ti = sm->num_workers;
-	  int rv =
-	    clib_bihash_search_8_8 (&sm->per_thread_data[ti].out2in, &kv0,
-				    &value0);
-	  if (!rv)
-	    {
-	      si = value0.value;
-	      s0 = pool_elt_at_index (sm->per_thread_data[ti].sessions, si);
-	      new_dst_addr0 = s0->in2out.addr.as_u32;
-	      vnet_buffer (b0)->sw_if_index[VLIB_TX] = s0->in2out.fib_index;
-	      echo0->identifier = s0->in2out.port;
-	      sum0 = icmp0->checksum;
-	      sum0 = ip_csum_update (sum0, icmp_id0, s0->in2out.port,
-				     icmp_echo_header_t, identifier);
-	      icmp0->checksum = ip_csum_fold (sum0);
-	      goto change_addr;
-	    }
-	  ti = 0;
-	}
-
       key0.addr = ip0->dst_address;
       key0.port = 0;
       key0.protocol = 0;
@@ -327,7 +294,44 @@ snat_icmp_hairpinning (snat_main_t * sm,
 
       if (clib_bihash_search_8_8
 	  (&sm->static_mapping_by_external, &kv0, &value0))
-	return 1;
+	{
+	  if (!is_ed)
+	    {
+	      icmp_echo_header_t *echo0 = (icmp_echo_header_t *) (icmp0 + 1);
+	      u16 icmp_id0 = echo0->identifier;
+	      key0.addr = ip0->dst_address;
+	      key0.port = icmp_id0;
+	      key0.protocol = SNAT_PROTOCOL_ICMP;
+	      key0.fib_index = sm->outside_fib_index;
+	      kv0.key = key0.as_u64;
+	      if (sm->num_workers > 1)
+		ti =
+		  (clib_net_to_host_u16 (icmp_id0) -
+		   1024) / sm->port_per_thread;
+	      else
+		ti = sm->num_workers;
+	      int rv =
+		clib_bihash_search_8_8 (&sm->per_thread_data[ti].out2in, &kv0,
+					&value0);
+	      if (!rv)
+		{
+		  si = value0.value;
+		  s0 =
+		    pool_elt_at_index (sm->per_thread_data[ti].sessions, si);
+		  new_dst_addr0 = s0->in2out.addr.as_u32;
+		  vnet_buffer (b0)->sw_if_index[VLIB_TX] =
+		    s0->in2out.fib_index;
+		  echo0->identifier = s0->in2out.port;
+		  sum0 = icmp0->checksum;
+		  sum0 = ip_csum_update (sum0, icmp_id0, s0->in2out.port,
+					 icmp_echo_header_t, identifier);
+		  icmp0->checksum = ip_csum_fold (sum0);
+		  goto change_addr;
+		}
+	    }
+
+	  return 1;
+	}
 
       m0 = pool_elt_at_index (sm->static_mappings, value0.value);
 
diff --git a/src/plugins/nat/out2in.c b/src/plugins/nat/out2in.c
index eeecf165264..c4d1fbf6900 100755
--- a/src/plugins/nat/out2in.c
+++ b/src/plugins/nat/out2in.c
@@ -775,17 +775,17 @@ snat_out2in_node_fn (vlib_main_t * vm,
 	      goto trace0;
 	    }
 
-	  if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
+	  if (PREDICT_FALSE (ip4_is_fragment (ip0)))
 	    {
-	      next0 = icmp_out2in_slow_path
-		(sm, b0, ip0, icmp0, sw_if_index0, rx_fib_index0, node,
-		 next0, now, thread_index, &s0);
+	      next0 = SNAT_OUT2IN_NEXT_REASS;
 	      goto trace0;
 	    }
 
-	  if (PREDICT_FALSE (ip4_is_fragment (ip0)))
+	  if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
 	    {
-	      next0 = SNAT_OUT2IN_NEXT_REASS;
+	      next0 = icmp_out2in_slow_path
+		(sm, b0, ip0, icmp0, sw_if_index0, rx_fib_index0, node,
+		 next0, now, thread_index, &s0);
 	      goto trace0;
 	    }
 
@@ -936,17 +936,17 @@ snat_out2in_node_fn (vlib_main_t * vm,
 	      goto trace1;
 	    }
 
-	  if (PREDICT_FALSE (proto1 == SNAT_PROTOCOL_ICMP))
+	  if (PREDICT_FALSE (ip4_is_fragment (ip1)))
 	    {
-	      next1 = icmp_out2in_slow_path
-		(sm, b1, ip1, icmp1, sw_if_index1, rx_fib_index1, node,
-		 next1, now, thread_index, &s1);
+	      next1 = SNAT_OUT2IN_NEXT_REASS;
 	      goto trace1;
 	    }
 
-	  if (PREDICT_FALSE (ip4_is_fragment (ip1)))
+	  if (PREDICT_FALSE (proto1 == SNAT_PROTOCOL_ICMP))
 	    {
-	      next1 = SNAT_OUT2IN_NEXT_REASS;
+	      next1 = icmp_out2in_slow_path
+		(sm, b1, ip1, icmp1, sw_if_index1, rx_fib_index1, node,
+		 next1, now, thread_index, &s1);
 	      goto trace1;
 	    }
 
@@ -1134,17 +1134,17 @@ snat_out2in_node_fn (vlib_main_t * vm,
 	      goto trace00;
 	    }
 
-	  if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
+	  if (PREDICT_FALSE (ip4_is_fragment (ip0)))
 	    {
-	      next0 = icmp_out2in_slow_path
-		(sm, b0, ip0, icmp0, sw_if_index0, rx_fib_index0, node,
-		 next0, now, thread_index, &s0);
+	      next0 = SNAT_OUT2IN_NEXT_REASS;
 	      goto trace00;
 	    }
 
-	  if (PREDICT_FALSE (ip4_is_fragment (ip0)))
+	  if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
 	    {
-	      next0 = SNAT_OUT2IN_NEXT_REASS;
+	      next0 = icmp_out2in_slow_path
+		(sm, b0, ip0, icmp0, sw_if_index0, rx_fib_index0, node,
+		 next0, now, thread_index, &s0);
 	      goto trace00;
 	    }
 
@@ -1336,6 +1336,7 @@ nat44_out2in_reass_node_fn (vlib_main_t * vm,
 	  nat_reass_ip4_t *reass0;
 	  udp_header_t *udp0;
 	  tcp_header_t *tcp0;
+	  icmp46_header_t *icmp0;
 	  snat_session_key_t key0, sm0;
 	  clib_bihash_kv_8_8_t kv0, value0;
 	  snat_session_t *s0 = 0;
@@ -1369,6 +1370,7 @@ nat44_out2in_reass_node_fn (vlib_main_t * vm,
 	  ip0 = (ip4_header_t *) vlib_buffer_get_current (b0);
 	  udp0 = ip4_next_header (ip0);
 	  tcp0 = (tcp_header_t *) udp0;
+	  icmp0 = (icmp46_header_t *) udp0;
 	  proto0 = ip_proto_to_snat_proto (ip0->protocol);
 
 	  reass0 = nat_ip4_reass_find_or_create (ip0->src_address,
@@ -1387,6 +1389,26 @@ nat44_out2in_reass_node_fn (vlib_main_t * vm,
 
 	  if (PREDICT_FALSE (ip4_is_first_fragment (ip0)))
 	    {
+	      if (PREDICT_FALSE (proto0 == SNAT_PROTOCOL_ICMP))
+		{
+		  next0 = icmp_out2in_slow_path
+		    (sm, b0, ip0, icmp0, sw_if_index0, rx_fib_index0, node,
+		     next0, now, thread_index, &s0);
+
+		  if (PREDICT_TRUE (next0 != SNAT_OUT2IN_NEXT_DROP))
+		    {
+		      if (s0)
+			reass0->sess_index = s0 - per_thread_data->sessions;
+		      else
+			reass0->flags |= NAT_REASS_FLAG_ED_DONT_TRANSLATE;
+		      reass0->thread_index = thread_index;
+		      nat_ip4_reass_get_frags (reass0,
+					       &fragments_to_loopback);
+		    }
+
+		  goto trace0;
+		}
+
 	      key0.addr = ip0->dst_address;
 	      key0.port = udp0->dst_port;
 	      key0.protocol = proto0;
diff --git a/test/test_nat.py b/test/test_nat.py
index e9e7dfa3068..e26aa27ddbd 100644
--- a/test/test_nat.py
+++ b/test/test_nat.py
@@ -3335,86 +3335,36 @@ class TestNAT44(MethodHolder):
         self.vapi.nat44_interface_add_del_feature(self.pg1.sw_if_index,
                                                   is_inside=0)
 
-        data = "A" * 4 + "B" * 16 + "C" * 3
-        self.tcp_port_in = random.randint(1025, 65535)
-
-        reass = self.vapi.nat_reass_dump()
-        reass_n_start = len(reass)
-
-        # in2out
-        pkts = self.create_stream_frag(self.pg0,
-                                       self.pg1.remote_ip4,
-                                       self.tcp_port_in,
-                                       20,
-                                       data)
-        self.pg0.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        frags = self.pg1.get_capture(len(pkts))
-        p = self.reass_frags_and_verify(frags,
-                                        self.nat_addr,
-                                        self.pg1.remote_ip4)
-        self.assertEqual(p[TCP].dport, 20)
-        self.assertNotEqual(p[TCP].sport, self.tcp_port_in)
-        self.tcp_port_out = p[TCP].sport
-        self.assertEqual(data, p[Raw].load)
-
-        # out2in
-        pkts = self.create_stream_frag(self.pg1,
-                                       self.nat_addr,
-                                       20,
-                                       self.tcp_port_out,
-                                       data)
-        self.pg1.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        frags = self.pg0.get_capture(len(pkts))
-        p = self.reass_frags_and_verify(frags,
-                                        self.pg1.remote_ip4,
-                                        self.pg0.remote_ip4)
-        self.assertEqual(p[TCP].sport, 20)
-        self.assertEqual(p[TCP].dport, self.tcp_port_in)
-        self.assertEqual(data, p[Raw].load)
-
-        reass = self.vapi.nat_reass_dump()
-        reass_n_end = len(reass)
-
-        self.assertEqual(reass_n_end - reass_n_start, 2)
+        self.frag_in_order(proto=IP_PROTOS.tcp)
+        self.frag_in_order(proto=IP_PROTOS.udp)
+        self.frag_in_order(proto=IP_PROTOS.icmp)
 
     def test_reass_hairpinning(self):
         """ NAT44 fragments hairpinning """
 
-        server = self.pg0.remote_hosts[1]
-        host_in_port = random.randint(1025, 65535)
-        server_in_port = random.randint(1025, 65535)
-        server_out_port = random.randint(1025, 65535)
-        data = "A" * 4 + "B" * 16 + "C" * 3
+        self.server = self.pg0.remote_hosts[1]
+        self.host_in_port = random.randint(1025, 65535)
+        self.server_in_port = random.randint(1025, 65535)
+        self.server_out_port = random.randint(1025, 65535)
 
         self.nat44_add_address(self.nat_addr)
         self.vapi.nat44_interface_add_del_feature(self.pg0.sw_if_index)
         self.vapi.nat44_interface_add_del_feature(self.pg1.sw_if_index,
                                                   is_inside=0)
         # add static mapping for server
-        self.nat44_add_static_mapping(server.ip4, self.nat_addr,
-                                      server_in_port, server_out_port,
+        self.nat44_add_static_mapping(self.server.ip4, self.nat_addr,
+                                      self.server_in_port,
+                                      self.server_out_port,
                                       proto=IP_PROTOS.tcp)
+        self.nat44_add_static_mapping(self.server.ip4, self.nat_addr,
+                                      self.server_in_port,
+                                      self.server_out_port,
+                                      proto=IP_PROTOS.udp)
+        self.nat44_add_static_mapping(self.server.ip4, self.nat_addr)
 
-        # send packet from host to server
-        pkts = self.create_stream_frag(self.pg0,
-                                       self.nat_addr,
-                                       host_in_port,
-                                       server_out_port,
-                                       data)
-        self.pg0.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        frags = self.pg0.get_capture(len(pkts))
-        p = self.reass_frags_and_verify(frags,
-                                        self.nat_addr,
-                                        server.ip4)
-        self.assertNotEqual(p[TCP].sport, host_in_port)
-        self.assertEqual(p[TCP].dport, server_in_port)
-        self.assertEqual(data, p[Raw].load)
+        self.reass_hairpinning(proto=IP_PROTOS.tcp)
+        self.reass_hairpinning(proto=IP_PROTOS.udp)
+        self.reass_hairpinning(proto=IP_PROTOS.icmp)
 
     def test_frag_out_of_order(self):
         """ NAT44 translate fragments arriving out of order """
@@ -3424,45 +3374,9 @@ class TestNAT44(MethodHolder):
         self.vapi.nat44_interface_add_del_feature(self.pg1.sw_if_index,
                                                   is_inside=0)
 
-        data = "A" * 4 + "B" * 16 + "C" * 3
-        random.randint(1025, 65535)
-
-        # in2out
-        pkts = self.create_stream_frag(self.pg0,
-                                       self.pg1.remote_ip4,
-                                       self.tcp_port_in,
-                                       20,
-                                       data)
-        pkts.reverse()
-        self.pg0.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        frags = self.pg1.get_capture(len(pkts))
-        p = self.reass_frags_and_verify(frags,
-                                        self.nat_addr,
-                                        self.pg1.remote_ip4)
-        self.assertEqual(p[TCP].dport, 20)
-        self.assertNotEqual(p[TCP].sport, self.tcp_port_in)
-        self.tcp_port_out = p[TCP].sport
-        self.assertEqual(data, p[Raw].load)
-
-        # out2in
-        pkts = self.create_stream_frag(self.pg1,
-                                       self.nat_addr,
-                                       20,
-                                       self.tcp_port_out,
-                                       data)
-        pkts.reverse()
-        self.pg1.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        frags = self.pg0.get_capture(len(pkts))
-        p = self.reass_frags_and_verify(frags,
-                                        self.pg1.remote_ip4,
-                                        self.pg0.remote_ip4)
-        self.assertEqual(p[TCP].sport, 20)
-        self.assertEqual(p[TCP].dport, self.tcp_port_in)
-        self.assertEqual(data, p[Raw].load)
+        self.frag_out_of_order(proto=IP_PROTOS.tcp)
+        self.frag_out_of_order(proto=IP_PROTOS.udp)
+        self.frag_out_of_order(proto=IP_PROTOS.icmp)
 
     def test_port_restricted(self):
         """ Port restricted NAT44 (MAP-E CE) """
@@ -3971,8 +3885,7 @@ class TestNAT44EndpointDependent(MethodHolder):
                                       self.server_in_port,
                                       self.server_out_port,
                                       proto=IP_PROTOS.udp)
-        self.nat44_add_static_mapping(self.server.ip4, self.nat_addr,
-                                      proto=IP_PROTOS.icmp)
+        self.nat44_add_static_mapping(self.server.ip4, self.nat_addr)
 
         self.reass_hairpinning(proto=IP_PROTOS.tcp)
         self.reass_hairpinning(proto=IP_PROTOS.udp)