From 719beb709818b70a1fd65f3c2a625d955678ceb6 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Wed, 10 Jul 2019 07:10:25 +0000 Subject: [PATCH] ip ipsec: Remove IPSec SPI-0 punt reason 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 --- src/vnet/ip/punt.api | 17 +++++++------ src/vnet/ip/punt_api.c | 12 +++++++++ src/vnet/ipsec/ipsec_if_in.c | 16 ++++-------- src/vnet/ipsec/ipsec_punt.h | 2 -- src/vnet/ipsec/ipsec_tun_in.c | 15 +++-------- test/test_punt.py | 58 ++++++++++++++++++++++++------------------- 6 files changed, 63 insertions(+), 57 deletions(-) diff --git a/src/vnet/ip/punt.api b/src/vnet/ip/punt.api index 6cb27311726..72efc7a9557 100644 --- a/src/vnet/ip/punt.api +++ b/src/vnet/ip/punt.api @@ -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 diff --git a/src/vnet/ip/punt_api.c b/src/vnet/ip/punt_api.c index b356886a56f..8bb0f7fbc82 100644 --- a/src/vnet/ip/punt_api.c +++ b/src/vnet/ip/punt_api.c @@ -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 diff --git a/src/vnet/ipsec/ipsec_if_in.c b/src/vnet/ipsec/ipsec_if_in.c index d1f9d5c3ad3..4e93725b97c 100644 --- a/src/vnet/ipsec/ipsec_if_in.c +++ b/src/vnet/ipsec/ipsec_if_in.c @@ -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); } diff --git a/src/vnet/ipsec/ipsec_punt.h b/src/vnet/ipsec/ipsec_punt.h index 4400ec9b4d2..f95e1da9133 100644 --- a/src/vnet/ipsec/ipsec_punt.h +++ b/src/vnet/ipsec/ipsec_punt.h @@ -18,8 +18,6 @@ #include #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") diff --git a/src/vnet/ipsec/ipsec_tun_in.c b/src/vnet/ipsec/ipsec_tun_in.c index 2ce1691b242..df6d9278303 100644 --- a/src/vnet/ipsec/ipsec_tun_in.c +++ b/src/vnet/ipsec/ipsec_tun_in.c @@ -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); } diff --git a/test/test_punt.py b/test/test_punt.py index b305e08bf5c..3ba1be4d5ed 100644 --- a/test/test_punt.py +++ b/test/test_punt.py @@ -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) -- 2.16.6