From d78349109fdb98fa0ba5f5aff779be700ff78357 Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Sat, 9 Dec 2017 14:55:52 +0100 Subject: [PATCH] acl-plugin: unapply/reapply the classifier-based inacls when performing macip_acl_add_replace on an existing MACIP ACL The classifier tables layout might (and most always will) change during the MACIP ACL modification. Furthermore, vnet_set_input_acl_intfc() is quite a picky creature - it quietly does nothing if there is an existing inacl applied, even if the number is different, so a simple "reapply" does not work. So, cleanly remove inacl, then reapply when the new tables are ready. Also, fix the testcase which was supposed to test this exact behavior. Thanks to Jon Loeliger for spotting this issue. Change-Id: I7e4bd8023d9de7e914448bb4466c1b0ef6940f58 Signed-off-by: Andrew Yourtchenko --- src/plugins/acl/acl.c | 30 +++++++++++++++++++++++++++++- test/test_acl_plugin_macip.py | 34 +++++++++++++++------------------- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index dbb740abc55..286b2e96d1e 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -1502,6 +1502,27 @@ macip_destroy_classify_tables (acl_main_t * am, u32 macip_acl_index) } } +static int +macip_maybe_apply_unapply_classifier_tables (acl_main_t * am, u32 acl_index, + int is_apply) +{ + int rv = 0; + int rv0 = 0; + int i; + macip_acl_list_t *a = pool_elt_at_index (am->macip_acls, acl_index); + + for (i = 0; i < vec_len (am->macip_acl_by_sw_if_index); i++) + if (vec_elt (am->macip_acl_by_sw_if_index, i) == acl_index) + { + rv0 = vnet_set_input_acl_intfc (am->vlib_main, i, a->ip4_table_index, + a->ip6_table_index, a->l2_table_index, + is_apply); + /* return the first unhappy outcome but make try to plough through. */ + rv = rv || rv0; + } + return rv; +} + static int macip_acl_add_list (u32 count, vl_api_macip_acl_rule_t rules[], u32 * acl_list_index, u8 * tag) @@ -1511,6 +1532,7 @@ macip_acl_add_list (u32 count, vl_api_macip_acl_rule_t rules[], macip_acl_rule_t *r; macip_acl_rule_t *acl_new_rules = 0; int i; + int rv = 0; if (*acl_list_index != ~0) { @@ -1531,6 +1553,9 @@ macip_acl_add_list (u32 count, vl_api_macip_acl_rule_t rules[], ("acl-plugin-warning: Trying to create empty MACIP ACL (tag %s)", tag); } + /* if replacing the ACL, unapply the classifier tables first - they will be gone.. */ + if (~0 != *acl_list_index) + rv = macip_maybe_apply_unapply_classifier_tables (am, *acl_list_index, 0); void *oldheap = acl_set_heap (am); /* Create and populate the rules */ if (count > 0) @@ -1575,7 +1600,10 @@ macip_acl_add_list (u32 count, vl_api_macip_acl_rule_t rules[], /* Create and populate the classifer tables */ macip_create_classify_tables (am, *acl_list_index); clib_mem_set_heap (oldheap); - return 0; + /* If the ACL was already applied somewhere, reapply the newly created tables */ + rv = rv + || macip_maybe_apply_unapply_classifier_tables (am, *acl_list_index, 1); + return rv; } diff --git a/test/test_acl_plugin_macip.py b/test/test_acl_plugin_macip.py index d27458d7df0..bf87e02ecd9 100644 --- a/test/test_acl_plugin_macip.py +++ b/test/test_acl_plugin_macip.py @@ -588,7 +588,8 @@ class MethodHolder(VppTestCase): def run_traffic(self, mac_type, ip_type, traffic, is_ip6, packets, do_not_expected_capture=False, tags=None, - apply_rules=True, isMACIP=True, permit_tags=PERMIT_TAGS): + apply_rules=True, isMACIP=True, permit_tags=PERMIT_TAGS, + try_replace=False): self.reset_packet_infos() if tags is None: @@ -650,6 +651,15 @@ class MethodHolder(VppTestCase): self.vapi.macip_acl_interface_add_del( sw_if_index=tx_if.sw_if_index, acl_index=0) + if try_replace: + if isMACIP: + reply = self.vapi.macip_acl_add_replace( + test_dict['macip_rules'], + acl_index) + else: + reply = self.vapi.acl_add_replace(acl_index=acl_index, + r=test_dict['acl_rules']) + self.assertEqual(reply.retval, 0) if not isinstance(src_if, VppSubInterface): tx_if.add_stream(test_dict['stream']) @@ -812,16 +822,9 @@ class TestMACIP_IP4(MethodHolder): """ MACIP replace ACL with IP4 traffic """ self.run_traffic(self.OUI_MAC, self.SUBNET_IP, - self.BRIDGED, self.IS_IP4, 9) - - r = self.create_rules() - # replace acls #2, #3 with new - reply = self.vapi.macip_acl_add_replace(r[0], 0) - self.assertEqual(reply.retval, 0) - self.assertEqual(reply.acl_index, 0) - + self.BRIDGED, self.IS_IP4, 9, try_replace=True) self.run_traffic(self.EXACT_MAC, self.EXACT_IP, - self.BRIDGED, self.IS_IP4, 9, True) + self.BRIDGED, self.IS_IP4, 9, try_replace=True) class TestMACIP_IP6(MethodHolder): @@ -957,16 +960,9 @@ class TestMACIP_IP6(MethodHolder): """ MACIP replace ACL with IP6 traffic """ self.run_traffic(self.OUI_MAC, self.SUBNET_IP, - self.BRIDGED, self.IS_IP6, 9) - - r = self.create_rules() - # replace acls #2, #3 with new - reply = self.vapi.macip_acl_add_replace(r[0], 0) - self.assertEqual(reply.retval, 0) - self.assertEqual(reply.acl_index, 0) - + self.BRIDGED, self.IS_IP6, 9, try_replace=True) self.run_traffic(self.EXACT_MAC, self.EXACT_IP, - self.BRIDGED, self.IS_IP6, 9, True) + self.BRIDGED, self.IS_IP6, 9, try_replace=True) class TestMACIP(MethodHolder): -- 2.16.6