ip ipsec: Remove IPSec SPI-0 punt reason 74/20574/3
authorNeale Ranns <nranns@cisco.com>
Wed, 10 Jul 2019 07:10:25 +0000 (07:10 +0000)
committerNeale Ranns <nranns@cisco.com>
Fri, 12 Jul 2019 07:38:47 +0000 (07:38 +0000)
Type: fix

There's no call for an SPI-0 punt reason with UDP encap, since
it's only with UDP encap that the ambiguity between IKE or IPSEC
occurs (and SPI=0 determines IKE).

Enhance the punt API to dum ponly the reason requested, so a client
can use this as a get-ID API

Change-Id: I5c6d72b03885e88c489117677e72f1ef5da90dfc
Signed-off-by: Neale Ranns <nranns@cisco.com>
src/vnet/ip/punt.api
src/vnet/ip/punt_api.c
src/vnet/ipsec/ipsec_if_in.c
src/vnet/ipsec/ipsec_punt.h
src/vnet/ipsec/ipsec_tun_in.c
test/test_punt.py

index 6cb2731..72efc7a 100644 (file)
@@ -131,18 +131,21 @@ autoreply define punt_socket_deregister {
     vl_api_punt_t punt;
 };
 
-/** \brief Dump all of the excpetion punt reasons
+typedef punt_reason
+{
+    u32 id;
+    string name;
+};
+
+/** \brief Dump all or one of the excpetion punt reasons
+*   @param - If the string is not set punt dump all reasons
+*            else dump only the one specified
 */
 define punt_reason_dump
 {
     u32 client_index;
     u32 context;
-};
-
-typedef punt_reason
-{
-    u32 id;
-    string name;
+    vl_api_punt_reason_t reason;
 };
 
 define punt_reason_details
index b356886..8bb0f7f 100644 (file)
@@ -321,6 +321,7 @@ typedef struct punt_reason_dump_walk_ctx_t_
 {
   vl_api_registration_t *reg;
   u32 context;
+  u8 *name;
 } punt_reason_dump_walk_ctx_t;
 
 static int
@@ -329,6 +330,14 @@ punt_reason_dump_walk_cb (vlib_punt_reason_t id, const u8 * name, void *args)
   punt_reason_dump_walk_ctx_t *ctx = args;
   vl_api_punt_reason_details_t *mp;
 
+  if (ctx->name)
+    {
+      /* user requested a specific punt-reason */
+      if (vec_cmp (name, ctx->name))
+       /* not the reasonn we're lookgin for */
+       return 1;
+    }
+
   mp = vl_msg_api_alloc (sizeof (*mp) + vec_len (name));
   if (!mp)
     return (0);
@@ -357,9 +366,12 @@ vl_api_punt_reason_dump_t_handler (vl_api_punt_reason_dump_t * mp)
   punt_reason_dump_walk_ctx_t ctx = {
     .reg = reg,
     .context = mp->context,
+    .name = vl_api_from_api_to_vec (&mp->reason.name),
   };
 
   punt_reason_walk (punt_reason_dump_walk_cb, &ctx);
+
+  vec_free (ctx.name);
 }
 
 #define vl_msg_name_crc_list
index d1f9d5c..4e93725 100644 (file)
@@ -74,7 +74,8 @@ ipsec_ip4_if_no_tunnel (vlib_node_runtime_t * node,
       b->error = node->errors[IPSEC_IF_INPUT_ERROR_SPI_0];
       b->punt_reason =
        ipsec_punt_reason[(ip4->protocol == IP_PROTOCOL_UDP ?
-                          IPSEC_PUNT_IP4_SPI_UDP_0 : IPSEC_PUNT_IP4_SPI_0)];
+                          IPSEC_PUNT_IP4_SPI_UDP_0 :
+                          IPSEC_PUNT_IP4_NO_SUCH_TUNNEL)];
     }
   else
     {
@@ -90,16 +91,9 @@ ipsec_ip6_if_no_tunnel (vlib_node_runtime_t * node,
                        vlib_buffer_t * b,
                        const esp_header_t * esp, u16 offset)
 {
-  if (PREDICT_FALSE (0 == esp->spi))
-    {
-      b->error = node->errors[IPSEC_IF_INPUT_ERROR_NO_TUNNEL];
-      b->punt_reason = ipsec_punt_reason[IPSEC_PUNT_IP6_SPI_0];
-    }
-  else
-    {
-      b->error = node->errors[IPSEC_IF_INPUT_ERROR_NO_TUNNEL];
-      b->punt_reason = ipsec_punt_reason[IPSEC_PUNT_IP6_NO_SUCH_TUNNEL];
-    }
+  b->error = node->errors[IPSEC_IF_INPUT_ERROR_NO_TUNNEL];
+  b->punt_reason = ipsec_punt_reason[IPSEC_PUNT_IP6_NO_SUCH_TUNNEL];
+
   vlib_buffer_advance (b, -offset);
   return (IPSEC_INPUT_NEXT_PUNT);
 }
index 4400ec9..f95e1da 100644 (file)
@@ -18,8 +18,6 @@
 #include <vlib/vlib.h>
 
 #define foreach_ipsec_punt_reason                             \
-  _(IP4_SPI_0, "ipsec4-spi-0")                                \
-  _(IP6_SPI_0, "ipsec6-spi-0")                                \
   _(IP4_SPI_UDP_0, "ipsec4-spi-o-udp-0")                      \
   _(IP4_NO_SUCH_TUNNEL, "ipsec4-no-such-tunnel")              \
   _(IP6_NO_SUCH_TUNNEL, "ipsec6-no-such-tunnel")
index 2ce1691..df6d927 100644 (file)
@@ -85,7 +85,7 @@ ipsec_ip4_if_no_tunnel (vlib_node_runtime_t * node,
       b->error = node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_SPI_0];
       b->punt_reason = ipsec_punt_reason[(ip4->protocol == IP_PROTOCOL_UDP ?
                                          IPSEC_PUNT_IP4_SPI_UDP_0 :
-                                         IPSEC_PUNT_IP4_SPI_0)];
+                                         IPSEC_PUNT_IP4_NO_SUCH_TUNNEL)];
     }
   else
     {
@@ -99,16 +99,9 @@ always_inline u16
 ipsec_ip6_if_no_tunnel (vlib_node_runtime_t * node,
                        vlib_buffer_t * b, const esp_header_t * esp)
 {
-  if (PREDICT_FALSE (0 == esp->spi))
-    {
-      b->error = node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_NO_TUNNEL];
-      b->punt_reason = ipsec_punt_reason[IPSEC_PUNT_IP6_SPI_0];
-    }
-  else
-    {
-      b->error = node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_NO_TUNNEL];
-      b->punt_reason = ipsec_punt_reason[IPSEC_PUNT_IP6_NO_SUCH_TUNNEL];
-    }
+  b->error = node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_NO_TUNNEL];
+  b->punt_reason = ipsec_punt_reason[IPSEC_PUNT_IP6_NO_SUCH_TUNNEL];
+
   return (IPSEC_INPUT_NEXT_PUNT);
 }
 
index b305e08..3ba1be4 100644 (file)
@@ -823,7 +823,6 @@ class TestExceptionPuntSocket(TestPuntSocket):
         #
         cfgs = dict()
         cfgs['ipsec4-no-such-tunnel'] = {'spi': 99, 'udp': False}
-        cfgs['ipsec4-spi-0'] = {'spi': 0, 'udp': False}
         cfgs['ipsec4-spi-o-udp-0'] = {'spi': 0, 'udp': True}
 
         #
@@ -1067,6 +1066,24 @@ class TestPunt(VppTestCase):
     def test_punt(self):
         """ Exception Path testing """
 
+        #
+        # dump the punt registered reasons
+        #  search for a few we know should be there
+        #
+        rs = self.vapi.punt_reason_dump()
+
+        reasons = ["ipsec6-no-such-tunnel",
+                   "ipsec4-no-such-tunnel",
+                   "ipsec4-spi-o-udp-0"]
+
+        for reason in reasons:
+            found = False
+            for r in rs:
+                if r.reason.name == reason:
+                    found = True
+                    break
+            self.assertTrue(found)
+
         #
         # Using the test CLI we will hook in a exception path to
         # send ACL deny packets out of pg0 and pg1.
@@ -1100,6 +1117,14 @@ class TestPunt(VppTestCase):
         #
         self.vapi.cli("test punt pg2")
 
+        #
+        # dump the punt reasons to learn the IDs assigned
+        #
+        rs = self.vapi.punt_reason_dump(reason={'name': "reason-v4"})
+        r4 = rs[0].reason.id
+        rs = self.vapi.punt_reason_dump(reason={'name': "reason-v6"})
+        r6 = rs[0].reason.id
+
         #
         # pkts now dropped
         #
@@ -1117,8 +1142,8 @@ class TestPunt(VppTestCase):
         self.assertEqual(stats, 2*NUM_PKTS)
 
         stats = self.statistics.get_counter("/net/punt")
-        self.assertEqual(stats[0][7]['packets'], NUM_PKTS)
-        self.assertEqual(stats[0][8]['packets'], NUM_PKTS)
+        self.assertEqual(stats[0][r4]['packets'], NUM_PKTS)
+        self.assertEqual(stats[0][r6]['packets'], NUM_PKTS)
 
         #
         # use the test CLI to test a client that punts exception
@@ -1145,8 +1170,8 @@ class TestPunt(VppTestCase):
             self.assertEqual(p6[IPv6].hlim, rx[IPv6].hlim)
 
         stats = self.statistics.get_counter("/net/punt")
-        self.assertEqual(stats[0][7]['packets'], 2*NUM_PKTS)
-        self.assertEqual(stats[0][8]['packets'], 2*NUM_PKTS)
+        self.assertEqual(stats[0][r4]['packets'], 2*NUM_PKTS)
+        self.assertEqual(stats[0][r6]['packets'], 2*NUM_PKTS)
 
         #
         # add another registration for the same reason to send packets
@@ -1192,8 +1217,8 @@ class TestPunt(VppTestCase):
             self.assertEqual(p6[IPv6].hlim, rx[IPv6].hlim)
 
         stats = self.statistics.get_counter("/net/punt")
-        self.assertEqual(stats[0][7]['packets'], 3*NUM_PKTS)
-        self.assertEqual(stats[0][8]['packets'], 3*NUM_PKTS)
+        self.assertEqual(stats[0][r4]['packets'], 3*NUM_PKTS)
+        self.assertEqual(stats[0][r6]['packets'], 3*NUM_PKTS)
 
         self.logger.info(self.vapi.cli("show vlib graph punt-dispatch"))
         self.logger.info(self.vapi.cli("show punt client"))
@@ -1201,25 +1226,6 @@ class TestPunt(VppTestCase):
         self.logger.info(self.vapi.cli("show punt stats"))
         self.logger.info(self.vapi.cli("show punt db"))
 
-        #
-        # dump the punt registered reasons
-        #  search for a few we know should be there
-        #
-        rs = self.vapi.punt_reason_dump()
-
-        reasons = ["ipsec6-no-such-tunnel",
-                   "ipsec4-no-such-tunnel",
-                   "ipsec6-spi-0",
-                   "ipsec4-spi-0"]
-
-        for reason in reasons:
-            found = False
-            for r in rs:
-                if r.reason.name == reason:
-                    found = True
-                    break
-            self.assertTrue(found)
-
 
 if __name__ == '__main__':
     unittest.main(testRunner=VppTestRunner)