Fix handling of ping to SNAT out interface

Change-Id: I322bfb3469b3d0d5b0cac39a6c2dba1c6f83ce3d
Signed-off-by: Juraj Sloboda <jsloboda@cisco.com>

2 files changed, 37 insertions, 8 deletions

diff --git a/src/plugins/snat/out2in.c b/src/plugins/snat/out2in.c
index 3c1bb885..328f5ba4 100644
--- a/src/plugins/snat/out2in.c
+++ b/src/plugins/snat/out2in.c
@@ -270,12 +270,6 @@ static inline u32 icmp_out2in_slow_path (snat_main_t *sm,
 
   if (!is_error_message)
     {
-      if (PREDICT_FALSE(icmp0->type != ICMP4_echo_reply))
-        {
-          b0->error = node->errors[SNAT_OUT2IN_ERROR_BAD_ICMP_TYPE];
-          next0 = SNAT_OUT2IN_NEXT_DROP;
-          goto out;
-        }
       key0.protocol = SNAT_PROTOCOL_ICMP;
       key0.port = echo0->identifier;
     }
@@ -353,6 +347,13 @@ static inline u32 icmp_out2in_slow_path (snat_main_t *sm,
     s0 = pool_elt_at_index (sm->per_thread_data[cpu_index].sessions,
                             value0.value);
 
+  if (PREDICT_FALSE(icmp0->type != ICMP4_echo_reply && !is_error_message))
+    {
+      b0->error = node->errors[SNAT_OUT2IN_ERROR_BAD_ICMP_TYPE];
+      next0 = SNAT_OUT2IN_NEXT_DROP;
+      goto out;
+    }
+
   sum0 = ip_incremental_checksum (0, icmp0,
                                   ntohs(ip0->length) - ip4_header_bytes (ip0));
   checksum0 = ~ip_csum_fold (sum0);
diff --git a/test/test_snat.py b/test/test_snat.py
index 500d629d..01bbe10a 100644
--- a/test/test_snat.py
+++ b/test/test_snat.py
@@ -485,7 +485,7 @@ class TestSNAT(VppTestCase):
                                                  src_ip=self.pg1.local_ip4)
 
     def test_dynamic_icmp_errors_in2out_ttl_2(self):
-        """ SNAT handling of error respones to client packets with TTL=2 """
+        """ SNAT handling of error responses to client packets with TTL=2 """
 
         self.snat_add_address(self.snat_addr)
         self.vapi.snat_interface_add_del_feature(self.pg0.sw_if_index)
@@ -512,7 +512,7 @@ class TestSNAT(VppTestCase):
         self.verify_capture_in_with_icmp_errors(capture, self.pg0)
 
     def test_dynamic_icmp_errors_out2in_ttl_2(self):
-        """ SNAT handling of error respones to server packets with TTL=2 """
+        """ SNAT handling of error responses to server packets with TTL=2 """
 
         self.snat_add_address(self.snat_addr)
         self.vapi.snat_interface_add_del_feature(self.pg0.sw_if_index)
@@ -546,6 +546,34 @@ class TestSNAT(VppTestCase):
         capture = self.pg1.get_capture(len(pkts))
         self.verify_capture_out_with_icmp_errors(capture)
 
+    def test_ping_out_interface_from_outside(self):
+        """ Ping SNAT out interface from outside """
+
+        self.snat_add_address(self.snat_addr)
+        self.vapi.snat_interface_add_del_feature(self.pg0.sw_if_index)
+        self.vapi.snat_interface_add_del_feature(self.pg1.sw_if_index,
+                                                 is_inside=0)
+
+        p = (Ether(dst=self.pg1.local_mac, src=self.pg1.remote_mac) /
+             IP(src=self.pg1.remote_ip4, dst=self.pg1.local_ip4) /
+             ICMP(id=self.icmp_id_out, type='echo-request'))
+        pkts = [p]
+        self.pg1.add_stream(pkts)
+        self.pg_enable_capture(self.pg_interfaces)
+        self.pg_start()
+        capture = self.pg1.get_capture(len(pkts))
+        self.assertEqual(1, len(capture))
+        packet = capture[0]
+        try:
+            self.assertEqual(packet[IP].src, self.pg1.local_ip4)
+            self.assertEqual(packet[IP].dst, self.pg1.remote_ip4)
+            self.assertEqual(packet[ICMP].id, self.icmp_id_in)
+            self.assertEqual(packet[ICMP].type, 0)  # echo reply
+        except:
+            self.logger.error(ppp("Unexpected or invalid packet "
+                                  "(outside network):", packet))
+            raise
+
     def test_static_in(self):
         """ SNAT 1:1 NAT initialized from inside network """