nat: nat44 cli bug fix

Two similar CLI paths "nat44" and "nat44 add interface address"
caused unexpected behavior. If "nat44 add interface address"
command would fail the vlib cli processing function would
call "nat44" handler. This would also clean any previously
set errors from the first command and basically return
same error returned by "nat44" handler for every failed
command that starts with the same path string.

Fixes nat44-ed and nat44-ei plugin.

Change-Id: I1aac85c8ae2932da582a2b78243521d1bf8a0653
Ticket: VPP-2021
Type: fix
Signed-off-by: Filip Varga <fivarga@cisco.com>

diff --git a/src/plugins/nat/nat44-ed/nat44_ed.c b/src/plugins/nat/nat44-ed/nat44_ed.c
index 3b1c9a2..f2d2c71 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed.c
@@ -2341,8 +2341,6 @@ nat_init (vlib_main_t * vm)
 
   clib_memset (sm, 0, sizeof (*sm));
 
-  // required
-  sm->vnet_main = vnet_get_main ();
   // convenience
   sm->ip4_main = &ip4_main;
 
@@ -3500,6 +3498,7 @@ nat44_ed_add_interface_address (u32 sw_if_index, u8 twice_nat)
 
   if (!sm->enabled)
     {
+      nat_log_err ("nat44 is disabled");
       return VNET_API_ERROR_UNSUPPORTED;
     }
 
@@ -3537,6 +3536,7 @@ nat44_ed_del_interface_address (u32 sw_if_index, u8 twice_nat)
 
   if (!sm->enabled)
     {
+      nat_log_err ("nat44 is disabled");
       return VNET_API_ERROR_UNSUPPORTED;
     }
 

diff --git a/src/plugins/nat/nat44-ed/nat44_ed.h b/src/plugins/nat/nat44-ed/nat44_ed.h
index 1ca7118..05503a4 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed.h
+++ b/src/plugins/nat/nat44-ed/nat44_ed.h
@@ -650,8 +650,6 @@ typedef struct snat_main_s
   /* nat44 plugin enabled */
   u8 enabled;
 
-  vnet_main_t *vnet_main;
-
   /* TCP session state machine table:
    *   first dimension is possible states
    *   second dimension is direction (in2out/out2in)

diff --git a/src/plugins/nat/nat44-ed/nat44_ed_api.c b/src/plugins/nat/nat44-ed/nat44_ed_api.c
index 19e497e..c430429 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed_api.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed_api.c
@@ -1494,7 +1494,8 @@ send_nat44_user_session_v2_details (snat_session_t *s,
 {
   vl_api_nat44_user_session_v2_details_t *rmp;
   snat_main_t *sm = &snat_main;
-  u64 now = vlib_time_now (sm->vnet_main->vlib_main);
+  vnet_main_t *vnm = vnet_get_main ();
+  u64 now = vlib_time_now (vnm->vlib_main);
   u64 sess_timeout_time = 0;
 
   rmp = vl_msg_api_alloc (sizeof (*rmp));

diff --git a/src/plugins/nat/nat44-ed/nat44_ed_cli.c b/src/plugins/nat/nat44-ed/nat44_ed_cli.c
index bddd635..cde3d73 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed_cli.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed_cli.c
@@ -108,7 +108,6 @@ set_workers_command_fn (vlib_main_t * vm,
   int rv = 0;
   clib_error_t *error = 0;
 
-  /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
@@ -184,7 +183,6 @@ snat_set_log_level_command_fn (vlib_main_t * vm,
   u8 log_level = NAT_LOG_NONE;
   clib_error_t *error = 0;
 
-  /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
@@ -299,7 +297,6 @@ nat_set_mss_clamping_command_fn (vlib_main_t * vm, unformat_input_t * input,
   clib_error_t *error = 0;
   u32 mss;
 
-  /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
@@ -351,7 +348,6 @@ add_address_command_fn (vlib_main_t * vm,
   clib_error_t *error = 0;
   u8 twice_nat = 0;
 
-  /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
@@ -662,7 +658,6 @@ snat_feature_command_fn (vlib_main_t * vm,
 
   sw_if_index = ~0;
 
-  /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
@@ -1172,7 +1167,6 @@ add_lb_backend_command_fn (vlib_main_t * vm,
   ip_protocol_t proto;
   u8 proto_set = 0;
 
-  /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
@@ -1267,19 +1261,21 @@ snat_add_interface_address_command_fn (vlib_main_t * vm,
                                       vlib_cli_command_t * cmd)
 {
   unformat_input_t _line_input, *line_input = &_line_input;
-  snat_main_t *sm = &snat_main;
+  vnet_main_t *vnm = vnet_get_main ();
   clib_error_t *error = 0;
   int rv, is_del = 0;
   u8 twice_nat = 0;
   u32 sw_if_index;
 
+  sw_if_index = ~0;
+
   if (!unformat_user (input, unformat_line_input, line_input))
     return clib_error_return (0, NAT44_ED_EXPECTED_ARGUMENT);
 
   while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
     {
-      if (unformat (line_input, "%U", unformat_vnet_sw_interface,
-                   sm->vnet_main, &sw_if_index))
+      if (unformat (line_input, "%U", unformat_vnet_sw_interface, vnm,
+                   &sw_if_index))
        ;
       else if (unformat (line_input, "twice-nat"))
        {
@@ -1297,21 +1293,21 @@ snat_add_interface_address_command_fn (vlib_main_t * vm,
        }
     }
 
-  if (!is_del)
+  if (is_del)
     {
-      rv = nat44_ed_add_interface_address (sw_if_index, twice_nat);
-      if (rv)
-       {
-         error = clib_error_return (0, "add address returned %d", rv);
-       }
+      rv = nat44_ed_del_interface_address (sw_if_index, twice_nat);
     }
   else
     {
-      rv = nat44_ed_del_interface_address (sw_if_index, twice_nat);
-      if (rv)
-       {
-         error = clib_error_return (0, "del address returned %d", rv);
-       }
+      rv = nat44_ed_add_interface_address (sw_if_index, twice_nat);
+    }
+
+  if (0 != rv)
+    {
+      error =
+       clib_error_return (0, "%s %U address failed", is_del ? "del" : "add",
+                          format_vnet_sw_if_index_name, vnm, sw_if_index);
+      goto done;
     }
 
 done:
@@ -1847,18 +1843,19 @@ done:
  * @cliexstart{nat44}
  * Enable nat44 plugin
  * To enable nat44-ed, use:
- *  vpp# nat44 enable
+ *  vpp# nat44 plugin enable
  * To disable nat44-ed, use:
- *  vpp# nat44 disable
+ *  vpp# nat44 plugin disable
  * To set inside-vrf outside-vrf, use:
- *  vpp# nat44 enable inside-vrf <id> outside-vrf <id>
+ *  vpp# nat44 plugin enable inside-vrf <id> outside-vrf <id>
  * @cliexend
 ?*/
 VLIB_CLI_COMMAND (nat44_ed_enable_disable_command, static) = {
-  .path = "nat44",
-  .short_help = "nat44 <enable [sessions <max-number>] [inside-vrf <vrf-id>] "
-               "[outside-vrf <vrf-id>]>|disable",
+  .path = "nat44 plugin",
   .function = nat44_ed_enable_disable_command_fn,
+  .short_help =
+    "nat44 plugin <enable [sessions <max-number>] [inside-vrf <vrf-id>] "
+    "[outside-vrf <vrf-id>]>|disable",
 };
 
 /*?
@@ -2215,9 +2212,9 @@ VLIB_CLI_COMMAND (nat44_show_static_mappings_command, static) = {
  * @cliexend
 ?*/
 VLIB_CLI_COMMAND (snat_add_interface_address_command, static) = {
-    .path = "nat44 add interface address",
-    .short_help = "nat44 add interface address <interface> [twice-nat] [del]",
-    .function = snat_add_interface_address_command_fn,
+  .path = "nat44 add interface address",
+  .function = snat_add_interface_address_command_fn,
+  .short_help = "nat44 add interface address <interface> [twice-nat] [del]",
 };
 
 /*?

diff --git a/src/plugins/nat/nat44-ei/nat44_ei_cli.c b/src/plugins/nat/nat44-ei/nat44_ei_cli.c
index e30fce0..2fe01b0 100644 (file)
--- a/src/plugins/nat/nat44-ei/nat44_ei_cli.c
+++ b/src/plugins/nat/nat44-ei/nat44_ei_cli.c
@@ -1589,23 +1589,23 @@ nat_show_timeouts_command_fn (vlib_main_t *vm, unformat_input_t *input,
  * @cliexstart{nat44 ei}
  * Enable nat44 ei plugin
  * To enable nat44-ei, use:
- *  vpp# nat44 ei enable
+ *  vpp# nat44 ei plugin enable
  * To disable nat44-ei, use:
- *  vpp# nat44 ei disable
+ *  vpp# nat44 ei plugin disable
  * To enable nat44 ei static mapping only, use:
- *  vpp# nat44 ei enable static-mapping
+ *  vpp# nat44 ei plugin enable static-mapping
  * To enable nat44 ei static mapping with connection tracking, use:
- *  vpp# nat44 ei enable static-mapping connection-tracking
+ *  vpp# nat44 ei plugin enable static-mapping connection-tracking
  * To enable nat44 ei out2in dpo, use:
- *  vpp# nat44 ei enable out2in-dpo
+ *  vpp# nat44 ei plugin enable out2in-dpo
  * To set inside-vrf outside-vrf, use:
- *  vpp# nat44 ei enable inside-vrf <id> outside-vrf <id>
+ *  vpp# nat44 ei plugin enable inside-vrf <id> outside-vrf <id>
  * @cliexend
 ?*/
 VLIB_CLI_COMMAND (nat44_ei_enable_disable_command, static) = {
-  .path = "nat44 ei",
+  .path = "nat44 ei plugin",
   .short_help =
-    "nat44 ei <enable [sessions <max-number>] [users <max-number>] "
+    "nat44 ei plugin <enable [sessions <max-number>] [users <max-number>] "
     "[static-mappig-only [connection-tracking]|out2in-dpo] [inside-vrf "
     "<vrf-id>] [outside-vrf <vrf-id>] [user-sessions <max-number>]>|disable",
   .function = nat44_ei_enable_disable_command_fn,