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 <fivarga@cisco.com>

diff --git a/src/plugins/nat/nat44-ed/nat44_ed_api.c b/src/plugins/nat/nat44-ed/nat44_ed_api.c
index 1505975..6123544 100644 (file)
--- 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 694bc6b..23f5a47 100644 (file)
--- 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 0134c78..5f42b83 100644 (file)
--- 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 c6d17c9..b67bc7d 100644 (file)
--- 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 36d98a4..a18c71b 100644 (file)
--- 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 4c112d3..de1e1b3 100644 (file)
--- 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 6dcda8c..fd01123 100644 (file)
--- 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,