ip: rate-limit the sending of ICMP error messages

Type: improvement

For error conditions, such as TTL expired, dest unreach, etc, Rate limit the sending of ICMP error messages.
The rate limiting is done based on src,dst IP address of the received packet.
the rate limit has been chosen, somewhat arbitrarily, to be 1e-3. This is the same limit as the ARP throttling.

Signed-off-by: Neale Ranns <neale@graphiant.com>
Change-Id: I4a0b791cde8c941a9bf37de6aa5da56779d3cef4

12 files changed, 107 insertions, 49 deletions

diff --git a/src/vnet/ip/icmp4.c b/src/vnet/ip/icmp4.c
index 5f9ffa3b2b7..857c3b1d2a5 100644
--- a/src/vnet/ip/icmp4.c
+++ b/src/vnet/ip/icmp4.c
@@ -41,6 +41,7 @@
 #include <vnet/ip/ip.h>
 #include <vnet/pg/pg.h>
 #include <vnet/ip/ip_sas.h>
+#include <vnet/util/throttle.h>
 
 static char *icmp_error_strings[] = {
 #define _(f,s) s,
@@ -48,6 +49,9 @@ static char *icmp_error_strings[] = {
 #undef _
 };
 
+/** ICMP throttling */
+static throttle_t icmp_throttle;
+
 static u8 *
 format_ip4_icmp_type_and_code (u8 * s, va_list * args)
 {
@@ -255,11 +259,14 @@ ip4_icmp_error (vlib_main_t * vm,
   u32 *from, *to_next;
   uword n_left_from, n_left_to_next;
   ip4_icmp_error_next_t next_index;
+  u32 thread_index = vm->thread_index;
 
   from = vlib_frame_vector_args (frame);
   n_left_from = frame->n_vectors;
   next_index = node->cached_next_index;
 
+  u64 seed = throttle_seed (&icmp_throttle, thread_index, vlib_time_now (vm));
+
   if (node->flags & VLIB_NODE_FLAG_TRACE)
     vlib_trace_frame_buffers_only (vm, node, from, frame->n_vectors,
 				   /* stride */ 1,
@@ -289,6 +296,21 @@ ip4_icmp_error (vlib_main_t * vm,
 	  ip_csum_t sum;
 
 	  org_p0 = vlib_get_buffer (vm, org_pi0);
+	  ip0 = vlib_buffer_get_current (org_p0);
+
+	  /* Rate limit based on the src,dst addresses in the original packet
+	   */
+	  u64 r0 =
+	    (u64) ip0->dst_address.as_u32 << 32 | ip0->src_address.as_u32;
+
+	  if (throttle_check (&icmp_throttle, thread_index, r0, seed))
+	    {
+	      vlib_error_count (vm, node->node_index, ICMP4_ERROR_DROP, 1);
+	      from += 1;
+	      n_left_from -= 1;
+	      continue;
+	    }
+
 	  p0 = vlib_buffer_copy_no_chain (vm, org_p0, &pi0);
 	  if (!p0 || pi0 == ~0)	/* Out of buffers */
 	    continue;
@@ -300,9 +322,10 @@ ip4_icmp_error (vlib_main_t * vm,
 	  n_left_from -= 1;
 	  n_left_to_next -= 1;
 
-	  ip0 = vlib_buffer_get_current (p0);
 	  sw_if_index0 = vnet_buffer (p0)->sw_if_index[VLIB_RX];
 
+	  vlib_buffer_copy_trace_flag (vm, p0, pi0);
+
 	  /* Add IP header and ICMPv4 header including a 4 byte data field */
 	  vlib_buffer_advance (p0,
 			       -sizeof (ip4_header_t) -
@@ -570,6 +593,11 @@ icmp4_init (vlib_main_t * vm)
 	       ICMP_INPUT_NEXT_ERROR,
 	       sizeof (cm->ip4_input_next_index_by_type));
 
+  vlib_thread_main_t *tm = &vlib_thread_main;
+  u32 n_vlib_mains = tm->n_vlib_mains;
+
+  throttle_init (&icmp_throttle, n_vlib_mains, 1e-3);
+
   return 0;
 }
 
diff --git a/src/vnet/ip/icmp6.c b/src/vnet/ip/icmp6.c
index 3c8300ff014..f92f31c05cb 100644
--- a/src/vnet/ip/icmp6.c
+++ b/src/vnet/ip/icmp6.c
@@ -41,6 +41,10 @@
 #include <vnet/ip/ip.h>
 #include <vnet/pg/pg.h>
 #include <vnet/ip/ip_sas.h>
+#include <vnet/util/throttle.h>
+
+/** ICMP throttling */
+static throttle_t icmp_throttle;
 
 static u8 *
 format_ip6_icmp_type_and_code (u8 * s, va_list * args)
@@ -296,11 +300,14 @@ ip6_icmp_error (vlib_main_t * vm,
   u32 *from, *to_next;
   uword n_left_from, n_left_to_next;
   ip6_icmp_error_next_t next_index;
+  u32 thread_index = vm->thread_index;
 
   from = vlib_frame_vector_args (frame);
   n_left_from = frame->n_vectors;
   next_index = node->cached_next_index;
 
+  u64 seed = throttle_seed (&icmp_throttle, thread_index, vlib_time_now (vm));
+
   if (node->flags & VLIB_NODE_FLAG_TRACE)
     vlib_trace_frame_buffers_only (vm, node, from, frame->n_vectors,
 				   /* stride */ 1,
@@ -330,6 +337,21 @@ ip6_icmp_error (vlib_main_t * vm,
 	  int bogus_length;
 
 	  org_p0 = vlib_get_buffer (vm, org_pi0);
+	  ip0 = vlib_buffer_get_current (org_p0);
+
+	  /* Rate limit based on the src,dst addresses in the original packet
+	   */
+	  u64 r0 = (ip6_address_hash_to_u64 (&ip0->dst_address) ^
+		    ip6_address_hash_to_u64 (&ip0->src_address));
+
+	  if (throttle_check (&icmp_throttle, thread_index, r0, seed))
+	    {
+	      vlib_error_count (vm, node->node_index, ICMP4_ERROR_DROP, 1);
+	      from += 1;
+	      n_left_from -= 1;
+	      continue;
+	    }
+
 	  p0 = vlib_buffer_copy_no_chain (vm, org_p0, &pi0);
 	  if (!p0 || pi0 == ~0)	/* Out of buffers */
 	    continue;
@@ -341,9 +363,10 @@ ip6_icmp_error (vlib_main_t * vm,
 	  n_left_from -= 1;
 	  n_left_to_next -= 1;
 
-	  ip0 = vlib_buffer_get_current (p0);
 	  sw_if_index0 = vnet_buffer (p0)->sw_if_index[VLIB_RX];
 
+	  vlib_buffer_copy_trace_flag (vm, p0, pi0);
+
 	  /* Add IP header and ICMPv6 header including a 4 byte data field */
 	  vlib_buffer_advance (p0,
 			       -(sizeof (ip6_header_t) +
@@ -624,6 +647,11 @@ icmp6_init (vlib_main_t * vm)
   cm->min_valid_length_by_type[ICMP6_redirect] =
     sizeof (icmp6_redirect_header_t);
 
+  vlib_thread_main_t *tm = &vlib_thread_main;
+  u32 n_vlib_mains = tm->n_vlib_mains;
+
+  throttle_init (&icmp_throttle, n_vlib_mains, 1e-3);
+
   return (NULL);
 }
 
diff --git a/test/framework.py b/test/framework.py
index 9266227f18e..257c85e9f5e 100644
--- a/test/framework.py
+++ b/test/framework.py
@@ -1352,6 +1352,17 @@ class VppTestCase(CPUInterface, unittest.TestCase):
             self.logger.debug(self.vapi.cli("show trace"))
         return rxs
 
+    def send_and_expect_some(self, intf, pkts, output,
+                             worker=None,
+                             trace=True):
+        self.pg_send(intf, pkts, worker=worker, trace=trace)
+        rx = output._get_capture(1)
+        if trace:
+            self.logger.debug(self.vapi.cli("show trace"))
+        self.assertTrue(len(rx) > 0)
+        self.assertTrue(len(rx) < len(pkts))
+        return rx
+
     def send_and_expect_only(self, intf, pkts, output, timeout=None,
                              stats_diff=None):
         if stats_diff:
diff --git a/test/test_gso.py b/test/test_gso.py
index 695ec5ef2a7..ee676a41c4a 100644
--- a/test/test_gso.py
+++ b/test/test_gso.py
@@ -279,7 +279,7 @@ class TestGSO(VppTestCase):
                TCP(sport=1234, dport=1234) /
                Raw(b'\xa5' * 65200))
 
-        rxs = self.send_and_expect(self.pg2, 5*[p63], self.pg2, 5)
+        rxs = self.send_and_expect_some(self.pg2, 5*[p63], self.pg2, 5)
         for rx in rxs:
             self.assertEqual(rx[Ether].src, self.pg2.local_mac)
             self.assertEqual(rx[Ether].dst, self.pg2.remote_mac)
diff --git a/test/test_ip4.py b/test/test_ip4.py
index de2cac0619e..873a38a22be 100644
--- a/test/test_ip4.py
+++ b/test/test_ip4.py
@@ -1947,16 +1947,15 @@ class TestIPInput(VppTestCase):
                  UDP(sport=1234, dport=1234) /
                  Raw(b'\xa5' * 100))
 
-        rx = self.send_and_expect(self.pg0, p_ttl * NUM_PKTS, self.pg0)
+        rxs = self.send_and_expect_some(self.pg0, p_ttl * NUM_PKTS, self.pg0)
 
-        rx = rx[0]
-        icmp = rx[ICMP]
-
-        self.assertEqual(icmptypes[icmp.type], "time-exceeded")
-        self.assertEqual(icmpcodes[icmp.type][icmp.code],
-                         "ttl-zero-during-transit")
-        self.assertEqual(icmp.src, self.pg0.remote_ip4)
-        self.assertEqual(icmp.dst, self.pg1.remote_ip4)
+        for rx in rxs:
+            icmp = rx[ICMP]
+            self.assertEqual(icmptypes[icmp.type], "time-exceeded")
+            self.assertEqual(icmpcodes[icmp.type][icmp.code],
+                             "ttl-zero-during-transit")
+            self.assertEqual(icmp.src, self.pg0.remote_ip4)
+            self.assertEqual(icmp.dst, self.pg1.remote_ip4)
 
         #
         # MTU exceeded
@@ -1971,15 +1970,15 @@ class TestIPInput(VppTestCase):
 
         self.vapi.sw_interface_set_mtu(self.pg1.sw_if_index, [1500, 0, 0, 0])
 
-        rx = self.send_and_expect(self.pg0, p_mtu * NUM_PKTS, self.pg0)
-        rx = rx[0]
-        icmp = rx[ICMP]
+        rxs = self.send_and_expect_some(self.pg0, p_mtu * NUM_PKTS, self.pg0)
 
-        self.assertEqual(icmptypes[icmp.type], "dest-unreach")
-        self.assertEqual(icmpcodes[icmp.type][icmp.code],
-                         "fragmentation-needed")
-        self.assertEqual(icmp.src, self.pg0.remote_ip4)
-        self.assertEqual(icmp.dst, self.pg1.remote_ip4)
+        for rx in rxs:
+            icmp = rx[ICMP]
+            self.assertEqual(icmptypes[icmp.type], "dest-unreach")
+            self.assertEqual(icmpcodes[icmp.type][icmp.code],
+                             "fragmentation-needed")
+            self.assertEqual(icmp.src, self.pg0.remote_ip4)
+            self.assertEqual(icmp.dst, self.pg1.remote_ip4)
 
         self.vapi.sw_interface_set_mtu(self.pg1.sw_if_index, [2500, 0, 0, 0])
         rx = self.send_and_expect(self.pg0, p_mtu * NUM_PKTS, self.pg1)
diff --git a/test/test_ip6.py b/test/test_ip6.py
index 2c318cdaf99..bac50a3df98 100644
--- a/test/test_ip6.py
+++ b/test/test_ip6.py
@@ -2599,12 +2599,14 @@ class TestIP6Input(VppTestCase):
                      inet6.UDP(sport=1234, dport=1234) /
                      Raw(b'\xa5' * 100))
 
-        rx = self.send_and_expect(self.pg0, p_version * NUM_PKTS, self.pg0)
-        rx = rx[0]
-        icmp = rx[ICMPv6TimeExceeded]
+        rxs = self.send_and_expect_some(self.pg0,
+                                        p_version * NUM_PKTS,
+                                        self.pg0)
 
-        # 0: "hop limit exceeded in transit",
-        self.assertEqual((icmp.type, icmp.code), (3, 0))
+        for rx in rxs:
+            icmp = rx[ICMPv6TimeExceeded]
+            # 0: "hop limit exceeded in transit",
+            self.assertEqual((icmp.type, icmp.code), (3, 0))
 
     icmpv6_data = '\x0a' * 18
     all_0s = "::"
diff --git a/test/test_mpls.py b/test/test_mpls.py
index 34645a9fce1..f7709d10cc1 100644
--- a/test/test_mpls.py
+++ b/test/test_mpls.py
@@ -366,7 +366,8 @@ class TestMPLS(VppTestCase):
 
     def verify_capture_ip6_icmp(self, src_if, capture, sent):
         try:
-            self.assertEqual(len(capture), len(sent))
+            # rate limited ICMP
+            self.assertTrue(len(capture) <= len(sent))
 
             for i in range(len(capture)):
                 tx = sent[i]
@@ -549,7 +550,7 @@ class TestMPLS(VppTestCase):
                                              [VppMplsLabel(333, ttl=64)],
                                              dst_ip=self.pg1.remote_ip6,
                                              hlim=1)
-        rx = self.send_and_expect(self.pg0, tx, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, tx, self.pg0)
         self.verify_capture_ip6_icmp(self.pg0, rx, tx)
 
         #
@@ -591,7 +592,7 @@ class TestMPLS(VppTestCase):
         tx = self.create_stream_labelled_ip6(self.pg0, [VppMplsLabel(334)],
                                              dst_ip=self.pg1.remote_ip6,
                                              hlim=0)
-        rx = self.send_and_expect(self.pg0, tx, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, tx, self.pg0)
         self.verify_capture_ip6_icmp(self.pg0, rx, tx)
 
         #
@@ -1500,7 +1501,7 @@ class TestMPLS(VppTestCase):
                                              [VppMplsLabel(34)],
                                              dst_ip="ff01::1",
                                              hlim=1)
-        rx = self.send_and_expect(self.pg0, tx, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, tx, self.pg0)
         self.verify_capture_ip6_icmp(self.pg0, rx, tx)
 
         #
diff --git a/test/test_mtu.py b/test/test_mtu.py
index 3c938a8cdce..27594e55727 100644
--- a/test/test_mtu.py
+++ b/test/test_mtu.py
@@ -104,7 +104,7 @@ class TestMTU(VppTestCase):
         n = icmp4_reply.__class__(icmp4_reply)
         s = bytes(icmp4_reply)
         icmp4_reply = s[0:576]
-        rx = self.send_and_expect(self.pg0, p4*11, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, p4*11, self.pg0)
         for p in rx:
             # p.show2()
             # n.show2()
@@ -185,7 +185,7 @@ class TestMTU(VppTestCase):
         s = bytes(icmp6_reply)
         icmp6_reply_str = s[0:1280]
 
-        rx = self.send_and_expect(self.pg0, p6*9, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, p6*9, self.pg0)
         for p in rx:
             self.validate_bytes(bytes(p[1]), icmp6_reply_str)
 
diff --git a/test/test_nat44_ed.py b/test/test_nat44_ed.py
index 764693d636a..2fa70fa8957 100644
--- a/test/test_nat44_ed.py
+++ b/test/test_nat44_ed.py
@@ -1310,10 +1310,7 @@ class TestNAT44ED(VppTestCase):
 
         # in2out
         pkts = self.create_stream_in(self.pg0, self.pg1, ttl=1)
-        self.pg0.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
-        capture = self.pg0.get_capture(len(pkts))
+        capture = self.send_and_expect_some(self.pg0, pkts, self.pg0)
         for p in capture:
             self.assertIn(ICMP, p)
             self.assertEqual(p[ICMP].type, 11)  # 11 == time-exceeded
diff --git a/test/test_nat44_ei.py b/test/test_nat44_ei.py
index c1b82ac2dc3..aafd345f43f 100644
--- a/test/test_nat44_ei.py
+++ b/test/test_nat44_ei.py
@@ -1056,12 +1056,9 @@ class TestNAT44EI(MethodHolder):
 
         # Client side - generate traffic
         pkts = self.create_stream_in(self.pg0, self.pg1, ttl=1)
-        self.pg0.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
+        capture = self.send_and_expect_some(self.pg0, pkts, self.pg0)
 
         # Client side - verify ICMP type 11 packets
-        capture = self.pg0.get_capture(len(pkts))
         self.verify_capture_in_with_icmp_errors(capture, self.pg0)
 
     def test_dynamic_icmp_errors_out2in_ttl_1(self):
@@ -1086,12 +1083,9 @@ class TestNAT44EI(MethodHolder):
         capture = self.pg1.get_capture(len(pkts))
         self.verify_capture_out(capture)
         pkts = self.create_stream_out(self.pg1, ttl=1)
-        self.pg1.add_stream(pkts)
-        self.pg_enable_capture(self.pg_interfaces)
-        self.pg_start()
+        capture = self.send_and_expect_some(self.pg1, pkts, self.pg1)
 
         # Server side - verify ICMP type 11 packets
-        capture = self.pg1.get_capture(len(pkts))
         self.verify_capture_out_with_icmp_errors(capture,
                                                  src_ip=self.pg1.local_ip4)
 
diff --git a/test/test_punt.py b/test/test_punt.py
index 3471a3f7dd5..ac059e904bd 100644
--- a/test/test_punt.py
+++ b/test/test_punt.py
@@ -305,7 +305,7 @@ class TestIP4PuntSocket(TestPuntSocket):
         #
         # expect ICMP - port unreachable for all packets
         #
-        rx = self.send_and_expect(self.pg0, pkts, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, pkts, self.pg0)
 
         for p in rx:
             self.assertEqual(int(p[IP].proto), 1)   # ICMP
@@ -334,7 +334,7 @@ class TestIP4PuntSocket(TestPuntSocket):
         punts = self.vapi.punt_socket_dump(type=pt_l4)
         self.assertEqual(len(punts), 0)
 
-        rx = self.send_and_expect(self.pg0, pkts, self.pg0)
+        rx = self.send_and_expect_some(self.pg0, pkts, self.pg0)
         for p in rx:
             self.assertEqual(int(p[IP].proto), 1)   # ICMP
             self.assertEqual(int(p[ICMP].code), 3)  # unreachable
diff --git a/test/test_reassembly.py b/test/test_reassembly.py
index 2291c933833..4c7a7cd320b 100644
--- a/test/test_reassembly.py
+++ b/test/test_reassembly.py
@@ -1326,8 +1326,7 @@ class TestIPv6Reassembly(VppTestCase):
         packets = self.dst_if.get_capture(
             len(self.pkt_infos) - len(dropped_packet_indexes))
         self.verify_capture(packets, dropped_packet_indexes)
-        pkts = self.src_if.get_capture(
-            expected_count=len(dropped_packet_indexes))
+        pkts = self.src_if._get_capture(1)
         for icmp in pkts:
             self.assertIn(ICMPv6TimeExceeded, icmp)
             self.assertIn(IPv6ExtHdrFragment, icmp)
@@ -1372,8 +1371,7 @@ class TestIPv6Reassembly(VppTestCase):
         packets = self.dst_if.get_capture(
             len(self.pkt_infos) - len(dropped_packet_indexes))
         self.verify_capture(packets, dropped_packet_indexes)
-        pkts = self.src_if.get_capture(
-            expected_count=len(dropped_packet_indexes))
+        pkts = self.src_if._get_capture(1)
         for icmp in pkts:
             self.assertIn(ICMPv6TimeExceeded, icmp)
             self.assertIn(IPv6ExtHdrFragment, icmp)