From: Filip Varga Date: Thu, 21 Oct 2021 11:00:27 +0000 (+0200) Subject: nat: api autoendian fix X-Git-Tag: v22.06-rc0~271 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=9c25eb1f4876a399919782c97e116732ea2ee628;p=vpp.git nat: api autoendian fix Fixed bad use of macros for autoendian API calls and updated tests for the new API. Removed sw_if_index check macro because of ntol conversion. Changed REPLY_MACRO to REPLY_MACRO_END to fix ntohl conversions. Type: fix Change-Id: I878a07b3f80fe03179feab60f0abc662f408a2c8 Signed-off-by: Filip Varga --- diff --git a/src/plugins/nat/nat44-ed/nat44_ed_api.c b/src/plugins/nat/nat44-ed/nat44_ed_api.c index 15059752ee7..6123544087b 100644 --- a/src/plugins/nat/nat44-ed/nat44_ed_api.c +++ b/src/plugins/nat/nat44-ed/nat44_ed_api.c @@ -552,24 +552,25 @@ vl_api_nat44_ed_add_del_output_interface_t_handler ( { vl_api_nat44_ed_add_del_output_interface_reply_t *rmp; snat_main_t *sm = &snat_main; - u32 sw_if_index; int rv = 0; - VALIDATE_SW_IF_INDEX (mp); - - sw_if_index = ntohl (mp->sw_if_index); + if (!vnet_sw_if_index_is_api_valid (mp->sw_if_index)) + { + rv = VNET_API_ERROR_INVALID_SW_IF_INDEX; + goto bad_sw_if_index; + } if (mp->is_add) { - rv = nat44_ed_add_output_interface (sw_if_index); + rv = nat44_ed_add_output_interface (mp->sw_if_index); } else { - rv = nat44_ed_del_output_interface (sw_if_index); + rv = nat44_ed_del_output_interface (mp->sw_if_index); } - BAD_SW_IF_INDEX_LABEL; - REPLY_MACRO (VL_API_NAT44_ED_ADD_DEL_OUTPUT_INTERFACE_REPLY); +bad_sw_if_index: + REPLY_MACRO_END (VL_API_NAT44_ED_ADD_DEL_OUTPUT_INTERFACE_REPLY); } #define vl_endianfun diff --git a/src/plugins/nat/nat44-ei/nat44_ei.c b/src/plugins/nat/nat44-ei/nat44_ei.c index 694bc6bec5a..23f5a47464c 100644 --- a/src/plugins/nat/nat44-ei/nat44_ei.c +++ b/src/plugins/nat/nat44-ei/nat44_ei.c @@ -610,7 +610,7 @@ nat44_ei_get_interface (nat44_ei_interface_t *interfaces, u32 sw_if_index) return 0; } -static_always_inline int +int nat44_ei_add_interface (u32 sw_if_index, u8 is_inside) { const char *feature_name, *del_feature_name; @@ -753,7 +753,7 @@ nat44_ei_add_interface (u32 sw_if_index, u8 is_inside) return 0; } -static_always_inline int +int nat44_ei_del_interface (u32 sw_if_index, u8 is_inside) { const char *feature_name, *del_feature_name; @@ -884,19 +884,6 @@ nat44_ei_del_interface (u32 sw_if_index, u8 is_inside) } int -nat44_ei_add_del_interface (u32 sw_if_index, u8 is_inside, int is_del) -{ - if (is_del) - { - return nat44_ei_del_interface (sw_if_index, is_inside); - } - else - { - return nat44_ei_add_interface (sw_if_index, is_inside); - } -} - -static_always_inline int nat44_ei_add_output_interface (u32 sw_if_index) { nat44_ei_main_t *nm = &nat44_ei_main; @@ -1000,7 +987,7 @@ nat44_ei_add_output_interface (u32 sw_if_index) return 0; } -static_always_inline int +int nat44_ei_del_output_interface (u32 sw_if_index) { nat44_ei_main_t *nm = &nat44_ei_main; @@ -1091,19 +1078,6 @@ nat44_ei_del_output_interface (u32 sw_if_index) return 0; } -int -nat44_ei_add_del_output_interface (u32 sw_if_index, int is_del) -{ - if (is_del) - { - return nat44_ei_del_output_interface (sw_if_index); - } - else - { - return nat44_ei_add_output_interface (sw_if_index); - } -} - int nat44_ei_plugin_disable () { diff --git a/src/plugins/nat/nat44-ei/nat44_ei.h b/src/plugins/nat/nat44-ei/nat44_ei.h index 0134c7882dd..5f42b834094 100644 --- a/src/plugins/nat/nat44-ei/nat44_ei.h +++ b/src/plugins/nat/nat44-ei/nat44_ei.h @@ -484,9 +484,10 @@ extern nat44_ei_main_t nat44_ei_main; int nat44_ei_plugin_enable (nat44_ei_config_t c); int nat44_ei_plugin_disable (); -int nat44_ei_add_del_interface (u32 sw_if_index, u8 is_inside, int is_del); -int nat44_ei_add_del_output_interface (u32 sw_if_index, int is_del); - +int nat44_ei_add_interface (u32 sw_if_index, u8 is_inside); +int nat44_ei_del_interface (u32 sw_if_index, u8 is_inside); +int nat44_ei_add_output_interface (u32 sw_if_index); +int nat44_ei_del_output_interface (u32 sw_if_index); int nat44_ei_add_address (ip4_address_t *addr, u32 vrf_id); int nat44_ei_del_address (ip4_address_t addr, u8 delete_sm); int nat44_ei_add_interface_address (u32 sw_if_index); diff --git a/src/plugins/nat/nat44-ei/nat44_ei_api.c b/src/plugins/nat/nat44-ei/nat44_ei_api.c index c6d17c94205..b67bc7dfb88 100644 --- a/src/plugins/nat/nat44-ei/nat44_ei_api.c +++ b/src/plugins/nat/nat44-ei/nat44_ei_api.c @@ -533,18 +533,22 @@ vl_api_nat44_ei_interface_add_del_feature_t_handler ( nat44_ei_main_t *nm = &nat44_ei_main; vl_api_nat44_ei_interface_add_del_feature_reply_t *rmp; u32 sw_if_index = ntohl (mp->sw_if_index); - u8 is_del; int rv = 0; - is_del = !mp->is_add; - VALIDATE_SW_IF_INDEX (mp); - rv = nat44_ei_add_del_interface (sw_if_index, mp->flags & NAT44_EI_IF_INSIDE, - is_del); + if (mp->is_add) + { + rv = + nat44_ei_add_interface (sw_if_index, mp->flags & NAT44_EI_IF_INSIDE); + } + else + { + rv = + nat44_ei_del_interface (sw_if_index, mp->flags & NAT44_EI_IF_INSIDE); + } BAD_SW_IF_INDEX_LABEL; - REPLY_MACRO (VL_API_NAT44_EI_INTERFACE_ADD_DEL_FEATURE_REPLY); } @@ -648,7 +652,14 @@ vl_api_nat44_ei_interface_add_del_output_feature_t_handler ( if (!(mp->flags & NAT44_EI_IF_INSIDE)) { - rv = nat44_ei_add_del_output_interface (sw_if_index, !mp->is_add); + if (mp->is_add) + { + rv = nat44_ei_add_output_interface (sw_if_index); + } + else + { + rv = nat44_ei_del_output_interface (sw_if_index); + } } BAD_SW_IF_INDEX_LABEL; @@ -702,17 +713,25 @@ vl_api_nat44_ei_add_del_output_interface_t_handler ( { vl_api_nat44_ei_add_del_output_interface_reply_t *rmp; nat44_ei_main_t *nm = &nat44_ei_main; - u32 sw_if_index; int rv = 0; - VALIDATE_SW_IF_INDEX (mp); - - sw_if_index = ntohl (mp->sw_if_index); + if (!vnet_sw_if_index_is_api_valid (mp->sw_if_index)) + { + rv = VNET_API_ERROR_INVALID_SW_IF_INDEX; + goto bad_sw_if_index; + } - rv = nat44_ei_add_del_output_interface (sw_if_index, !mp->is_add); + if (mp->is_add) + { + rv = nat44_ei_add_output_interface (mp->sw_if_index); + } + else + { + rv = nat44_ei_del_output_interface (mp->sw_if_index); + } - BAD_SW_IF_INDEX_LABEL; - REPLY_MACRO (VL_API_NAT44_EI_ADD_DEL_OUTPUT_INTERFACE_REPLY); +bad_sw_if_index: + REPLY_MACRO_END (VL_API_NAT44_EI_ADD_DEL_OUTPUT_INTERFACE_REPLY); } #define vl_endianfun diff --git a/src/plugins/nat/nat44-ei/nat44_ei_cli.c b/src/plugins/nat/nat44-ei/nat44_ei_cli.c index 36d98a46cb0..a18c71b5037 100644 --- a/src/plugins/nat/nat44-ei/nat44_ei_cli.c +++ b/src/plugins/nat/nat44-ei/nat44_ei_cli.c @@ -859,8 +859,7 @@ nat44_ei_feature_command_fn (vlib_main_t *vm, unformat_input_t *input, u32 *inside_sw_if_indices = 0; u32 *outside_sw_if_indices = 0; u8 is_output_feature = 0; - int is_del = 0; - int i; + int i, rv, is_del = 0; sw_if_index = ~0; @@ -894,7 +893,15 @@ nat44_ei_feature_command_fn (vlib_main_t *vm, unformat_input_t *input, sw_if_index = inside_sw_if_indices[i]; if (is_output_feature) { - if (nat44_ei_add_del_output_interface (sw_if_index, is_del)) + if (is_del) + { + rv = nat44_ei_del_output_interface (sw_if_index); + } + else + { + rv = nat44_ei_add_output_interface (sw_if_index); + } + if (rv) { error = clib_error_return ( 0, "%s %U failed", is_del ? "del" : "add", @@ -904,7 +911,15 @@ nat44_ei_feature_command_fn (vlib_main_t *vm, unformat_input_t *input, } else { - if (nat44_ei_add_del_interface (sw_if_index, 1, is_del)) + if (is_del) + { + rv = nat44_ei_del_interface (sw_if_index, 1); + } + else + { + rv = nat44_ei_add_interface (sw_if_index, 1); + } + if (rv) { error = clib_error_return ( 0, "%s %U failed", is_del ? "del" : "add", @@ -922,7 +937,15 @@ nat44_ei_feature_command_fn (vlib_main_t *vm, unformat_input_t *input, sw_if_index = outside_sw_if_indices[i]; if (is_output_feature) { - if (nat44_ei_add_del_output_interface (sw_if_index, is_del)) + if (is_del) + { + rv = nat44_ei_del_output_interface (sw_if_index); + } + else + { + rv = nat44_ei_add_output_interface (sw_if_index); + } + if (rv) { error = clib_error_return ( 0, "%s %U failed", is_del ? "del" : "add", @@ -932,7 +955,15 @@ nat44_ei_feature_command_fn (vlib_main_t *vm, unformat_input_t *input, } else { - if (nat44_ei_add_del_interface (sw_if_index, 0, is_del)) + if (is_del) + { + rv = nat44_ei_del_interface (sw_if_index, 0); + } + else + { + rv = nat44_ei_add_interface (sw_if_index, 0); + } + if (rv) { error = clib_error_return ( 0, "%s %U failed", is_del ? "del" : "add", diff --git a/test/test_nat44_ed.py b/test/test_nat44_ed.py index 4c112d3573e..de1e1b343ae 100644 --- a/test/test_nat44_ed.py +++ b/test/test_nat44_ed.py @@ -2742,7 +2742,7 @@ class TestNAT44EDMW(TestNAT44ED): self.nat_add_inside_interface(self.pg0) self.nat_add_outside_interface(self.pg0) - self.vapi.nat44_interface_add_del_output_feature( + self.vapi.nat44_ed_add_del_output_interface( sw_if_index=self.pg1.sw_if_index, is_add=1) # from client to service @@ -2820,7 +2820,7 @@ class TestNAT44EDMW(TestNAT44ED): self.nat_add_inside_interface(self.pg0) self.nat_add_outside_interface(self.pg0) - self.vapi.nat44_interface_add_del_output_feature( + self.vapi.nat44_ed_add_del_output_interface( sw_if_index=self.pg1.sw_if_index, is_add=1) p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) / @@ -3055,7 +3055,7 @@ class TestNAT44EDMW(TestNAT44ED): """ NAT44ED output feature works with stateful ACL """ self.nat_add_address(self.nat_addr) - self.vapi.nat44_interface_add_del_output_feature( + self.vapi.nat44_ed_add_del_output_interface( sw_if_index=self.pg1.sw_if_index, is_add=1) # First ensure that the NAT is working sans ACL @@ -3133,7 +3133,7 @@ class TestNAT44EDMW(TestNAT44ED): self.vapi.nat44_interface_add_del_feature( sw_if_index=self.pg0.sw_if_index, flags=flags, is_add=1) - self.vapi.nat44_interface_add_del_output_feature( + self.vapi.nat44_ed_add_del_output_interface( is_add=1, sw_if_index=self.pg1.sw_if_index) diff --git a/test/test_nat44_ei.py b/test/test_nat44_ei.py index 6dcda8c6ae4..fd011230b2b 100644 --- a/test/test_nat44_ei.py +++ b/test/test_nat44_ei.py @@ -2831,9 +2831,8 @@ class TestNAT44EI(MethodHolder): def test_output_feature(self): """ NAT44EI output feature (in2out postrouting) """ self.nat44_add_address(self.nat_addr) - self.vapi.nat44_ei_interface_add_del_output_feature( - is_add=1, - sw_if_index=self.pg3.sw_if_index) + self.vapi.nat44_ei_add_del_output_interface( + sw_if_index=self.pg3.sw_if_index, is_add=1) # in2out pkts = self.create_stream_in(self.pg0, self.pg3) @@ -2877,9 +2876,8 @@ class TestNAT44EI(MethodHolder): self.nat44_add_address(nat_ip_vrf10, vrf_id=10) self.nat44_add_address(nat_ip_vrf20, vrf_id=20) - self.vapi.nat44_ei_interface_add_del_output_feature( - is_add=1, - sw_if_index=self.pg3.sw_if_index) + self.vapi.nat44_ei_add_del_output_interface( + sw_if_index=self.pg3.sw_if_index, is_add=1) # in2out VRF 10 pkts = self.create_stream_in(self.pg4, self.pg3) @@ -2923,12 +2921,10 @@ class TestNAT44EI(MethodHolder): server_out_port = 8765 self.nat44_add_address(self.nat_addr) - self.vapi.nat44_ei_interface_add_del_output_feature( - is_add=1, - sw_if_index=self.pg0.sw_if_index) - self.vapi.nat44_ei_interface_add_del_output_feature( - is_add=1, - sw_if_index=self.pg1.sw_if_index) + self.vapi.nat44_ei_add_del_output_interface( + sw_if_index=self.pg0.sw_if_index, is_add=1) + self.vapi.nat44_ei_add_del_output_interface( + sw_if_index=self.pg1.sw_if_index, is_add=1) # add static mapping for server self.nat44_add_static_mapping(server.ip4, self.nat_addr,