Merge review feedback from mgsmith on upstream gerrit 33709 ps8..10

lcpng.c

@@ -70,7 +70,7 @@ int lcp_set_default_ns(u8 *ns) {
 }
 
 void
-lcp_set_lcp_sync (u8 is_auto)
+lcp_set_sync (u8 is_auto)
 {
   lcp_main_t *lcpm = &lcp_main;
 
@@ -82,7 +82,7 @@ lcp_set_lcp_sync (u8 is_auto)
 }
 
 int
-lcp_lcp_sync (void)
+lcp_sync (void)
 {
   lcp_main_t *lcpm = &lcp_main;
 
@@ -90,7 +90,7 @@ lcp_lcp_sync (void)
 }
 
 void
-lcp_set_lcp_auto_subint (u8 is_auto)
+lcp_set_auto_subint (u8 is_auto)
 {
   lcp_main_t *lcpm = &lcp_main;
 
@@ -98,7 +98,7 @@ lcp_set_lcp_auto_subint (u8 is_auto)
 }
 
 int
-lcp_lcp_auto_subint (void)
+lcp_auto_subint (void)
 {
   lcp_main_t *lcpm = &lcp_main;
 

lcpng_if_cli.c

@@ -112,7 +112,7 @@ VLIB_CLI_COMMAND(lcp_itf_pair_create_command, static) = {
 };
 
 static clib_error_t *
-lcp_lcp_sync_command_fn (vlib_main_t *vm, unformat_input_t *input,
+lcp_sync_command_fn (vlib_main_t *vm, unformat_input_t *input,
 		     vlib_cli_command_t *cmd)
 {
   unformat_input_t _line_input, *line_input = &_line_input;
@@ -123,10 +123,10 @@ lcp_lcp_sync_command_fn (vlib_main_t *vm, unformat_input_t *input,
   while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
     {
       if (unformat (line_input, "on") || unformat (line_input, "enable"))
-	lcp_set_lcp_sync (1);
+	lcp_set_sync (1);
       else if (unformat (line_input, "off") ||
 	       unformat (line_input, "disable"))
-	lcp_set_lcp_sync (0);
+	lcp_set_sync (0);
       else
 	return clib_error_return (0, "unknown input `%U'",
 				  format_unformat_error, line_input);
@@ -136,14 +136,14 @@ lcp_lcp_sync_command_fn (vlib_main_t *vm, unformat_input_t *input,
   return 0;
 }
 
-VLIB_CLI_COMMAND (lcp_lcp_sync_command, static) = {
+VLIB_CLI_COMMAND (lcp_sync_command, static) = {
   .path = "lcp lcp-sync",
   .short_help = "lcp lcp-sync [on|enable|off|disable]",
-  .function = lcp_lcp_sync_command_fn,
+  .function = lcp_sync_command_fn,
 };
 
 static clib_error_t *
-lcp_lcp_auto_subint_command_fn (vlib_main_t *vm, unformat_input_t *input,
+lcp_auto_subint_command_fn (vlib_main_t *vm, unformat_input_t *input,
 			    vlib_cli_command_t *cmd)
 {
   unformat_input_t _line_input, *line_input = &_line_input;
@@ -154,10 +154,10 @@ lcp_lcp_auto_subint_command_fn (vlib_main_t *vm, unformat_input_t *input,
   while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
     {
       if (unformat (line_input, "on") || unformat (line_input, "enable"))
-	lcp_set_lcp_auto_subint (1);
+	lcp_set_auto_subint (1);
       else if (unformat (line_input, "off") ||
 	       unformat (line_input, "disable"))
-	lcp_set_lcp_auto_subint (0);
+	lcp_set_auto_subint (0);
       else
 	return clib_error_return (0, "unknown input `%U'",
 				  format_unformat_error, line_input);
@@ -167,10 +167,10 @@ lcp_lcp_auto_subint_command_fn (vlib_main_t *vm, unformat_input_t *input,
   return 0;
 }
 
-VLIB_CLI_COMMAND (lcp_lcp_auto_subint_command, static) = {
+VLIB_CLI_COMMAND (lcp_auto_subint_command, static) = {
   .path = "lcp lcp-auto-subint",
   .short_help = "lcp lcp-auto-subint [on|enable|off|disable]",
-  .function = lcp_lcp_auto_subint_command_fn,
+  .function = lcp_auto_subint_command_fn,
 };
 
 static clib_error_t *lcp_default_netns_command_fn(vlib_main_t *vm,

lcpng_if_sync.c

@@ -15,27 +15,16 @@
  * limitations under the License.
  */
 
-#include <sys/socket.h>
+#include <linux/rtnetlink.h>
-#include <linux/if.h>
 
 #include <vnet/vnet.h>
 #include <vnet/plugin/plugin.h>
-
+#include <vnet/devices/netlink.h>
+#include <vnet/ip/ip.h>
 #include <vppinfra/linux/netns.h>
 
 #include <plugins/lcpng/lcpng_interface.h>
 
-#include <vlibapi/api.h>
-#include <vlibmemory/api.h>
-#include <vpp/app/version.h>
-#include <vnet/format_fns.h>
-#include <vnet/udp/udp.h>
-#include <vnet/tcp/tcp.h>
-#include <vnet/devices/tap/tap.h>
-#include <vnet/devices/netlink.h>
-#include <linux/netlink.h>
-#include <linux/rtnetlink.h>
-
 /* helper function to copy forward all sw interface link state flags
  * MTU, and IP addresses into their counterpart LIP interface.
  *
@@ -51,7 +40,7 @@ lcp_itf_pair_sync_state (lcp_itf_pair_t *lip)
   u32 mtu;
   u32 netlink_mtu;
 
-  if (!lcp_lcp_sync ())
+  if (!lcp_sync ())
     return;
 
   sw =
@@ -85,7 +74,7 @@ lcp_itf_pair_sync_state (lcp_itf_pair_t *lip)
 	"forcing state to sup-flags to satisfy netlink",
 	format_lcp_itf_pair, lip, sw->flags, sup_sw->flags,
 	sw->mtu[VNET_MTU_L3], sup_sw->mtu[VNET_MTU_L3]);
-      state = sup_sw->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP;
+      state = 0;
     }
   lcp_itf_set_link_state (lip, state);
 
@@ -151,15 +140,9 @@ static walk_rc_t
 lcp_itf_pair_walk_sync_state_hw_cb (vnet_main_t *vnm, u32 sw_if_index,
 				    void *arg)
 {
-  const vnet_sw_interface_t *sw;
   lcp_itf_pair_t *lip;
 
-  sw = vnet_get_sw_interface (vnm, sw_if_index);
+  lip = lcp_itf_pair_get (lcp_itf_pair_find_by_phy (sw_if_index));
-  if (!sw)
-    {
-      return WALK_CONTINUE;
-    }
-  lip = lcp_itf_pair_get (lcp_itf_pair_find_by_phy (sw->sw_if_index));
   if (!lip)
     {
       return WALK_CONTINUE;
@@ -194,7 +177,7 @@ lcp_itf_admin_state_change (vnet_main_t * vnm, u32 sw_if_index, u32 flags)
   vnet_hw_interface_t *hi;
   vnet_sw_interface_t *si;
 
-  if (!lcp_lcp_sync ())
+  if (!lcp_sync ())
     return 0;
 
   LCP_ITF_PAIR_DBG ("admin_state_change: sw %U %u",
@@ -240,7 +223,7 @@ lcp_itf_mtu_change (vnet_main_t *vnm, u32 sw_if_index, u32 flags)
 {
   vnet_sw_interface_t *si;
   vnet_hw_interface_t *hi;
-  if (!lcp_lcp_sync ())
+  if (!lcp_sync ())
     return NULL;
 
   LCP_ITF_PAIR_DBG ("mtu_change: sw %U %u", format_vnet_sw_if_index_name, vnm,
@@ -415,7 +398,7 @@ vnet_netlink_del_ip6_addr (int ifindex, void *addr, int pfx_len)
 }
 // TODO(pim) move previous block upstream
 
-void
+static void
 lcp_itf_ip4_add_del_interface_addr (ip4_main_t *im, uword opaque,
 				    u32 sw_if_index, ip4_address_t *address,
 				    u32 address_length, u32 if_address_index,
@@ -425,7 +408,7 @@ lcp_itf_ip4_add_del_interface_addr (ip4_main_t *im, uword opaque,
   int curr_ns_fd = -1;
   int vif_ns_fd = -1;
 
-  if (!lcp_lcp_sync ())
+  if (!lcp_sync ())
     return;
 
   LCP_ITF_PAIR_DBG ("ip4_addr_%s: si:%U %U/%u", is_del ? "del" : "add",
@@ -464,7 +447,7 @@ lcp_itf_ip4_add_del_interface_addr (ip4_main_t *im, uword opaque,
   return;
 }
 
-void
+static void
 lcp_itf_ip6_add_del_interface_addr (ip6_main_t *im, uword opaque,
 				    u32 sw_if_index, ip6_address_t *address,
 				    u32 address_length, u32 if_address_index,
@@ -474,7 +457,7 @@ lcp_itf_ip6_add_del_interface_addr (ip6_main_t *im, uword opaque,
   int curr_ns_fd = -1;
   int vif_ns_fd = -1;
 
-  if (!lcp_lcp_sync ())
+  if (!lcp_sync ())
     return;
 
   LCP_ITF_PAIR_DBG ("ip6_addr_%s: si:%U %U/%u", is_del ? "del" : "add",
@@ -516,18 +499,13 @@ lcp_itf_interface_add_del (vnet_main_t *vnm, u32 sw_if_index, u32 is_create)
   const vnet_sw_interface_t *sw;
   uword is_sub;
 
-  is_sub = vnet_sw_interface_is_sub (vnm, sw_if_index);
+  if (!lcp_auto_subint ())
-  LCP_ITF_PAIR_DBG ("interface_%s: [%u] sw %U is_sub %u lcp-auto-subint %u",
-		    is_create ? "add" : "del", sw_if_index,
-		    format_vnet_sw_if_index_name, vnet_get_main (),
-		    sw_if_index, is_sub, lcp_lcp_auto_subint ());
-
-  if (!lcp_lcp_auto_subint ())
     return NULL;
 
   sw = vnet_get_sw_interface_or_null (vnm, sw_if_index);
   if (!sw)
     return NULL;
+  is_sub = vnet_sw_interface_is_sub (vnm, sw_if_index);
   if (!is_sub)
     return NULL;
 
@@ -561,12 +539,6 @@ lcp_itf_interface_add_del (vnet_main_t *vnm, u32 sw_if_index, u32 is_create)
     }
   else
     {
-      const lcp_itf_pair_t *lip;
-
-      // If the interface has a LIP, auto-delete it
-      lip = lcp_itf_pair_get (lcp_itf_pair_find_by_phy (sw_if_index));
-      if (!lip)
-	return NULL;
       lcp_itf_pair_delete (sw_if_index);
     }
 
@@ -574,3 +546,35 @@ lcp_itf_interface_add_del (vnet_main_t *vnm, u32 sw_if_index, u32 is_create)
 }
 
 VNET_SW_INTERFACE_ADD_DEL_FUNCTION (lcp_itf_interface_add_del);
+
+static clib_error_t *
+lcp_itf_sync_init (vlib_main_t *vm)
+{
+  ip4_main_t *im4 = &ip4_main;
+  ip6_main_t *im6 = &ip6_main;
+
+  ip4_add_del_interface_address_callback_t cb4;
+  ip6_add_del_interface_address_callback_t cb6;
+
+  cb4.function = lcp_itf_ip4_add_del_interface_addr;
+  cb4.function_opaque = 0;
+  vec_add1 (im4->add_del_interface_address_callbacks, cb4);
+
+  cb6.function = lcp_itf_ip6_add_del_interface_addr;
+  cb6.function_opaque = 0;
+  vec_add1 (im6->add_del_interface_address_callbacks, cb6);
+
+  return NULL;
+}
+
+VLIB_INIT_FUNCTION (lcp_itf_sync_init) = {
+  .runs_after = VLIB_INITS ("vnet_interface_init", "tcp_init", "udp_init"),
+};
+
+/*
+ * fd.io coding-style-patch-verification: ON
+ *
+ * Local Variables:
+ * eval: (c-set-style "gnu")
+ * End:
+ */

lcpng_interface.c

@@ -131,8 +131,8 @@ lcp_itf_pair_show (u32 phy_sw_if_index)
   ns = lcp_get_default_ns();
   vlib_cli_output (vm, "lcp default netns %v\n", ns ? (char *) ns : "<unset>");
   vlib_cli_output (vm, "lcp lcp-auto-subint %s\n",
-		   lcp_lcp_auto_subint () ? "on" : "off");
+		   lcp_auto_subint () ? "on" : "off");
-  vlib_cli_output (vm, "lcp lcp-sync %s\n", lcp_lcp_sync () ? "on" : "off");
+  vlib_cli_output (vm, "lcp lcp-sync %s\n", lcp_sync () ? "on" : "off");
 
   if (phy_sw_if_index == ~0)
     {
@@ -545,9 +545,9 @@ lcp_itf_pair_config (vlib_main_t *vm, unformat_input_t *input)
 	    }
 	}
       else if (unformat (input, "lcp-auto-subint"))
-	lcp_set_lcp_auto_subint (1 /* is_auto */);
+	lcp_set_auto_subint (1 /* is_auto */);
       else if (unformat (input, "lcp-sync"))
-	lcp_set_lcp_sync (1 /* is_auto */);
+	lcp_set_sync (1 /* is_auto */);
       else
 	return clib_error_return (0, "unknown input `%U'",
 				  format_unformat_error, input);
@@ -601,7 +601,6 @@ void
 lcp_itf_set_link_state (const lcp_itf_pair_t *lip, u8 state)
 {
   vnet_main_t *vnm = vnet_get_main ();
-  vnet_sw_interface_t *si;
   int curr_ns_fd, vif_ns_fd;
 
   if (!lip) return;
@@ -620,21 +619,14 @@ lcp_itf_set_link_state (const lcp_itf_pair_t *lip, u8 state)
    */
   if (state)
     {
-      vnet_sw_interface_admin_up (vnm, lip->lip_host_sw_if_index);
       vnet_sw_interface_admin_up (vnm, lip->lip_phy_sw_if_index);
     }
   else
     {
       vnet_sw_interface_admin_down (vnm, lip->lip_phy_sw_if_index);
-      vnet_sw_interface_admin_down (vnm, lip->lip_host_sw_if_index);
     }
   vnet_netlink_set_link_state (lip->lip_vif_index, state);
 
-  /* Set carrier (oper link) on the TAP
-   */
-  si = vnet_get_sw_interface_or_null (vnm, lip->lip_host_sw_if_index);
-  tap_set_carrier (si->hw_if_index, state);
-
   if (vif_ns_fd != -1)
     close (vif_ns_fd);
 
@@ -1041,7 +1033,7 @@ lcp_itf_pair_create (u32 phy_sw_if_index, u8 *host_if_name,
    * The TAP is shared by many interfaces, always keep it up.
    */
   vnet_sw_interface_admin_up (vnm, host_sw_if_index);
-  if (lcp_lcp_sync ())
+  if (lcp_sync ())
     {
       lip = lcp_itf_pair_get (lcp_itf_pair_find_by_vif (vif_index));
       lcp_itf_pair_sync_state (lip);
@@ -1109,14 +1101,46 @@ lcp_itf_pair_replace_end (void)
   return (0);
 }
 
+static clib_error_t *
+lcp_itf_pair_link_up_down (vnet_main_t *vnm, u32 hw_if_index, u32 flags)
+{
+  vnet_hw_interface_t *hi;
+  vnet_sw_interface_t *si;
+  index_t lipi;
+  lcp_itf_pair_t *lip;
+
+  hi = vnet_get_hw_interface_or_null (vnm, hw_if_index);
+  if (!hi)
+    return 0;
+
+  lipi = lcp_itf_pair_find_by_phy (hi->sw_if_index);
+  if (lipi == INDEX_INVALID)
+    return 0;
+
+  lip = lcp_itf_pair_get (lipi);
+  si = vnet_get_sw_interface_or_null (vnm, lip->lip_host_sw_if_index);
+  if (!si)
+    return 0;
+
+  if (!lcp_main.test_mode)
+    {
+      tap_set_carrier (si->hw_if_index,
+		       (flags & VNET_HW_INTERFACE_FLAG_LINK_UP));
+
+      if (flags & VNET_HW_INTERFACE_FLAG_LINK_UP)
+	{
+	  tap_set_speed (si->hw_if_index, hi->link_speed / 1000);
+	}
+    }
+
+  return 0;
+}
+
+VNET_HW_INTERFACE_LINK_UP_DOWN_FUNCTION (lcp_itf_pair_link_up_down);
+
 static clib_error_t *
 lcp_itf_pair_init (vlib_main_t *vm)
 {
-  ip4_main_t *im4 = &ip4_main;
-  ip6_main_t *im6 = &ip6_main;
-  ip4_add_del_interface_address_callback_t cb4;
-  ip6_add_del_interface_address_callback_t cb6;
-
   vlib_punt_hdl_t punt_hdl = vlib_punt_client_register("linux-cp");
   lcp_itf_pair_logger = vlib_log_register_class ("linux-cp", "if");
 
@@ -1130,14 +1154,6 @@ lcp_itf_pair_init (vlib_main_t *vm)
   tcp_punt_unknown (vm, 0, 1);
   tcp_punt_unknown (vm, 1, 1);
 
-  cb4.function = lcp_itf_ip4_add_del_interface_addr;
-  cb4.function_opaque = 0;
-  vec_add1 (im4->add_del_interface_address_callbacks, cb4);
-
-  cb6.function = lcp_itf_ip6_add_del_interface_addr;
-  cb6.function_opaque = 0;
-  vec_add1 (im6->add_del_interface_address_callbacks, cb6);
-
   return NULL;
 }
 

lcpng_interface.h

@@ -18,8 +18,6 @@
 #include <vnet/dpo/dpo.h>
 #include <vnet/adj/adj.h>
 #include <vnet/ip/ip_types.h>
-#include <vnet/udp/udp.h>
-#include <vnet/tcp/tcp.h>
 
 #include <plugins/lcpng/lcpng.h>
 
@@ -165,14 +163,14 @@ lcp_itf_pair_find_by_host (u32 host_sw_if_index)
 /**
  * sub-interface auto creation/deletion for LCP
  */
-void lcp_set_lcp_auto_subint (u8 is_auto);
+void lcp_set_auto_subint (u8 is_auto);
-int lcp_lcp_auto_subint (void);
+int lcp_auto_subint (void);
 
 /**
  * sync state changes from VPP into LCP
  */
-void lcp_set_lcp_sync (u8 is_auto);
+void lcp_set_sync (u8 is_auto);
-int lcp_lcp_sync (void);
+int lcp_sync (void);
 
 typedef void (*lcp_itf_pair_add_cb_t) (lcp_itf_pair_t *);
 typedef void (*lcp_itf_pair_del_cb_t) (lcp_itf_pair_t *);
@@ -191,18 +189,6 @@ void lcp_itf_set_link_state (const lcp_itf_pair_t *lip, u8 state);
 /* Set any VPP L3 addresses on Linux host device */
 void lcp_itf_set_interface_addr (const lcp_itf_pair_t *lip);
 
-/* Sync IPv4 and IPv6 address from VPP to Linux device */
-void lcp_itf_ip4_add_del_interface_addr (ip4_main_t *im, uword opaque,
-					 u32 sw_if_index,
-					 ip4_address_t *address,
-					 u32 address_length,
-					 u32 if_address_index, u32 is_del);
-void lcp_itf_ip6_add_del_interface_addr (ip6_main_t *im, uword opaque,
-					 u32 sw_if_index,
-					 ip6_address_t *address,
-					 u32 address_length,
-					 u32 if_address_index, u32 is_del);
-
 /* Sync all state from VPP to a specific Linux device, all sub-interfaces
  * of a hardware interface, or all interfaces in the system.
  *

lcpng_nl_sync.c

@@ -795,7 +795,6 @@ lcp_nl_link_add (struct rtnl_link *rl, void *ctx)
 {
   vnet_main_t *vnm = vnet_get_main ();
   lcp_itf_pair_t *lip;
-  vnet_sw_interface_t *si;
   int admin_state;
 
   NL_DBG ("link_add: netlink %U", format_nl_object, rl);
@@ -816,18 +815,12 @@ lcp_nl_link_add (struct rtnl_link *rl, void *ctx)
   // by sending a netlink message.
   if (admin_state)
     {
-      vnet_sw_interface_admin_up (vnm, lip->lip_host_sw_if_index);
       vnet_sw_interface_admin_up (vnm, lip->lip_phy_sw_if_index);
     }
   else
     {
       vnet_sw_interface_admin_down (vnm, lip->lip_phy_sw_if_index);
-      vnet_sw_interface_admin_down (vnm, lip->lip_host_sw_if_index);
     }
-  /* Set carrier (oper link) on the TAP
-   */
-  si = vnet_get_sw_interface_or_null (vnm, lip->lip_host_sw_if_index);
-  tap_set_carrier (si->hw_if_index, admin_state);
 
   lcp_nl_link_set_mtu (rl, lip);
   lcp_nl_link_set_lladdr (rl, lip);