From 987abe9eeb65a3950401073c770012a7898593b7 Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Wed, 27 Sep 2017 13:50:31 +0200 Subject: [PATCH] acl-plugin: take 2 at VPP-991 fix, this time with a test case which verifies it. The replacement of [] with pool_elt_at_index and subsequent fixing it was incorrect - it was equivalent to &[], since it returns a pointer to the element. I've added VPP-993 previously to create a testcase, so this commit partially fulfills that one as well. Change-Id: I5b15e3ce48316f0429232aacf885e8f7c63d9522 Signed-off-by: Andrew Yourtchenko --- src/plugins/acl/acl.c | 4 ++-- test/test_acl_plugin.py | 31 ++++++++++++++++++++++++++++++- test/vpp_papi_provider.py | 10 ++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index 9e029e62e46..efd506de805 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -259,13 +259,13 @@ acl_del_list (u32 acl_list_index) } if (acl_list_index < vec_len(am->input_sw_if_index_vec_by_acl)) { - if (vec_len(vec_elt_at_index(am->input_sw_if_index_vec_by_acl, acl_list_index)) > 0) { + if (vec_len(vec_elt(am->input_sw_if_index_vec_by_acl, acl_list_index)) > 0) { /* ACL is applied somewhere inbound. Refuse to delete */ return -1; } } if (acl_list_index < vec_len(am->output_sw_if_index_vec_by_acl)) { - if (vec_len(vec_elt_at_index(am->output_sw_if_index_vec_by_acl, acl_list_index)) > 0) { + if (vec_len(vec_elt(am->output_sw_if_index_vec_by_acl, acl_list_index)) > 0) { /* ACL is applied somewhere outbound. Refuse to delete */ return -1; } diff --git a/test/test_acl_plugin.py b/test/test_acl_plugin.py index 605efbd4ccb..97fca1a1bb3 100644 --- a/test/test_acl_plugin.py +++ b/test/test_acl_plugin.py @@ -497,7 +497,7 @@ class TestACLplugin(VppTestCase): # self.assertEqual(reply.minor, 0) def test_0001_acl_create(self): - """ ACL create test + """ ACL create/delete test """ self.logger.info("ACLP_TEST_START_0001") @@ -517,6 +517,7 @@ class TestACLplugin(VppTestCase): self.assertEqual(reply.retval, 0) # The very first ACL gets #0 self.assertEqual(reply.acl_index, 0) + first_acl = reply.acl_index rr = self.vapi.acl_dump(reply.acl_index) self.logger.info("Dumped ACL: " + str(rr)) self.assertEqual(len(rr), 1) @@ -555,6 +556,7 @@ class TestACLplugin(VppTestCase): self.assertEqual(reply.retval, 0) # The second ACL gets #1 self.assertEqual(reply.acl_index, 1) + second_acl = reply.acl_index # Test 2: try to modify a nonexistent ACL reply = self.vapi.acl_add_replace(acl_index=432, r=r, @@ -562,6 +564,33 @@ class TestACLplugin(VppTestCase): self.assertEqual(reply.retval, -1) # The ACL number should pass through self.assertEqual(reply.acl_index, 432) + # apply an ACL on an interface inbound, try to delete ACL, must fail + self.vapi.acl_interface_set_acl_list(sw_if_index=self.pg0.sw_if_index, + n_input=1, + acls=[first_acl]) + reply = self.vapi.acl_del(acl_index=first_acl, expected_retval=-1) + # Unapply an ACL and then try to delete it - must be ok + self.vapi.acl_interface_set_acl_list(sw_if_index=self.pg0.sw_if_index, + n_input=0, + acls=[]) + reply = self.vapi.acl_del(acl_index=first_acl, expected_retval=0) + + # apply an ACL on an interface outbound, try to delete ACL, must fail + self.vapi.acl_interface_set_acl_list(sw_if_index=self.pg0.sw_if_index, + n_input=0, + acls=[second_acl]) + reply = self.vapi.acl_del(acl_index=second_acl, expected_retval=-1) + # Unapply the ACL and then try to delete it - must be ok + self.vapi.acl_interface_set_acl_list(sw_if_index=self.pg0.sw_if_index, + n_input=0, + acls=[]) + reply = self.vapi.acl_del(acl_index=second_acl, expected_retval=0) + + # try to apply a nonexistent ACL - must fail + self.vapi.acl_interface_set_acl_list(sw_if_index=self.pg0.sw_if_index, + n_input=1, + acls=[first_acl], + expected_retval=-1) self.logger.info("ACLP_TEST_FINISH_0001") diff --git a/test/vpp_papi_provider.py b/test/vpp_papi_provider.py index 97f201d33a2..634dabeafda 100644 --- a/test/vpp_papi_provider.py +++ b/test/vpp_papi_provider.py @@ -2293,6 +2293,16 @@ class VppPapiProvider(object): 'tag': tag}, expected_retval=expected_retval) + def acl_del(self, acl_index, expected_retval=0): + """ + + :param acl_index: + :return: + """ + return self.api(self.papi.acl_del, + {'acl_index': acl_index}, + expected_retval=expected_retval) + def acl_interface_set_acl_list(self, sw_if_index, n_input, acls, expected_retval=0): return self.api(self.papi.acl_interface_set_acl_list, -- 2.16.6