Fix potential L2 forwarding crash with stale L2FIB entry (VPP-516) 12/3612/2
authorJohn Lo <loj@cisco.com>
Thu, 27 Oct 2016 15:17:02 +0000 (11:17 -0400)
committerDave Barach <openvpp@barachs.net>
Thu, 27 Oct 2016 17:29:10 +0000 (17:29 +0000)
On moving interface between L2 and L3 forwarding modes, adjust the
l2-output node next index for the sw_if_index of the interface so
packet output via stale MAC entries in L2 FIB will be dropped.

Change-Id: I25afd7a617edb4ae35ce296bac33e62683edad9a
Signed-off-by: John Lo <loj@cisco.com>
vnet/vnet/l2/l2_input.c
vnet/vnet/l2/l2_output.c
vnet/vnet/l2/l2_output.h
vnet/vnet/vxlan/vxlan.c

index 171ba73..9515250 100644 (file)
@@ -606,22 +606,29 @@ set_int_l2_mode (vlib_main_t * vm, vnet_main_t * vnet_main, u32 mode, u32 sw_if_
       l2_if_adjust--;
     }
 
+  /*
+   * Directs the l2 output path to work out the interface
+   * output next-arc itself. Needed when recycling a sw_if_index.
+   */
+  vec_validate_init_empty (l2om->next_nodes.output_node_index_vec,
+                          sw_if_index, ~0);
+  l2om->next_nodes.output_node_index_vec[sw_if_index] = ~0;
+
   /* Initialize the l2-input configuration for the interface */
   if (mode == MODE_L3)
     {
+      /* Set L2 config to BD index 0 so that if any packet accidentally
+       * came in on L2 path, it will be dropped in BD 0 */
       config->xconnect = 0;
       config->bridge = 0;
       config->shg = 0;
       config->bd_index = 0;
       config->feature_bitmap = L2INPUT_FEAT_DROP;
-      /*
-       * Directs the l2 output path to work out the interface
-       * output next-arc itself. Needed when recycling a sw_if_index.
-       */
-      vec_validate_init_empty (l2om->next_nodes.output_node_index_vec,
-                              sw_if_index, ~0);
-      l2om->next_nodes.output_node_index_vec[sw_if_index] = ~0;
 
+      /* Make sure any L2-output packet to this interface now in L3 mode is
+       * dropped. This may happen if L2 FIB MAC entry is stale */
+      l2om->next_nodes.output_node_index_vec[sw_if_index] =
+       L2OUTPUT_NEXT_BAD_INTF;
     }
   else if (mode == MODE_L2_CLASSIFY)
     {
index b84501a..f8ebe14 100644 (file)
@@ -469,46 +469,46 @@ VLIB_REGISTER_NODE (l2output_node,static) = {
   /* edit / add dispositions here */
   .next_nodes = {
         [L2OUTPUT_NEXT_DROP] = "error-drop",
-        [L2OUTPUT_NEXT_DEL_TUNNEL] = "l2-output-del-tunnel",
+        [L2OUTPUT_NEXT_BAD_INTF] = "l2-output-bad-intf",
   },
 };
 /* *INDENT-ON* */
 
 
-#define foreach_l2output_del_tunnel_error      \
-_(DROP,     "L2 output to deleted tunnel")
+#define foreach_l2output_bad_intf_error        \
+_(DROP,     "L2 output to interface not in L2 mode or deleted")
 
-static char *l2output_del_tunnel_error_strings[] = {
+static char *l2output_bad_intf_error_strings[] = {
 #define _(sym,string) string,
-  foreach_l2output_del_tunnel_error
+  foreach_l2output_bad_intf_error
 #undef _
 };
 
 typedef enum
 {
-#define _(sym,str) L2OUTPUT_DEL_TUNNEL_ERROR_##sym,
-  foreach_l2output_del_tunnel_error
+#define _(sym,str) L2OUTPUT_BAD_INTF_ERROR_##sym,
+  foreach_l2output_bad_intf_error
 #undef _
-    L2OUTPUT_DEL_TUNNEL_N_ERROR,
-} l2output_del_tunnel_error_t;
+    L2OUTPUT_BAD_INTF_N_ERROR,
+} l2output_bad_intf_error_t;
 
 
 /**
- * Output node for tunnels which was in L2 BD's but were deleted.
- * On deletion of any tunnel which was on a L2 BD, its entry in
- * l2_output_main table next_nodes.output_node_index_vec[sw_if_index]
- * MUST be set to the value of L2OUTPUT_NEXT_DEL_TUNNEL. Thus, if there
- * are stale entries in the L2FIB for this tunnel sw_if_index, l2-output
- * will send packets for this sw_if_index to the l2-output-tunnel-del
- * node which just setup the proper drop reason before sending packets
- * to the error-drop node to drop the packet. Then, stale L2FIB entries
- * for delted tunnels won't cause possible packet or memory corrpution.
+ * Output node for interfaces/tunnels which was in L2 mode but were changed
+ * to L3 mode or possibly deleted thereafter. On changing forwarding mode
+ * of any tunnel/interface from L2 to L3, its entry in l2_output_main table
+ * next_nodes.output_node_index_vec[sw_if_index] MUST be set to the value of
+ * L2OUTPUT_NEXT_BAD_INTF. Thus, if there are stale entries in the L2FIB for
+ * this sw_if_index, l2-output will send packets for this sw_if_index to the
+ * l2-output-bad-intf node which just setup the proper drop reason before
+ * sending packets to the error-drop node to drop the packet. Then, stale L2FIB
+ * entries for delted tunnels won't cause possible packet or memory corrpution.
  */
-static vlib_node_registration_t l2output_del_tunnel_node;
+static vlib_node_registration_t l2output_bad_intf_node;
 
 static uword
-l2output_del_tunnel_node_fn (vlib_main_t * vm,
-                            vlib_node_runtime_t * node, vlib_frame_t * frame)
+l2output_bad_intf_node_fn (vlib_main_t * vm,
+                          vlib_node_runtime_t * node, vlib_frame_t * frame)
 {
   u32 n_left_from, *from, *to_next;
   l2output_next_t next_index = 0;
@@ -536,8 +536,8 @@ l2output_del_tunnel_node_fn (vlib_main_t * vm,
          n_left_to_next -= 2;
          b0 = vlib_get_buffer (vm, bi0);
          b1 = vlib_get_buffer (vm, bi1);
-         b0->error = node->errors[L2OUTPUT_DEL_TUNNEL_ERROR_DROP];
-         b1->error = node->errors[L2OUTPUT_DEL_TUNNEL_ERROR_DROP];
+         b0->error = node->errors[L2OUTPUT_BAD_INTF_ERROR_DROP];
+         b1->error = node->errors[L2OUTPUT_BAD_INTF_ERROR_DROP];
        }
 
       while (n_left_from > 0 && n_left_to_next > 0)
@@ -552,7 +552,7 @@ l2output_del_tunnel_node_fn (vlib_main_t * vm,
          n_left_from -= 1;
          n_left_to_next -= 1;
          b0 = vlib_get_buffer (vm, bi0);
-         b0->error = node->errors[L2OUTPUT_DEL_TUNNEL_ERROR_DROP];
+         b0->error = node->errors[L2OUTPUT_BAD_INTF_ERROR_DROP];
        }
 
       vlib_put_next_frame (vm, node, next_index, n_left_to_next);
@@ -562,14 +562,14 @@ l2output_del_tunnel_node_fn (vlib_main_t * vm,
 }
 
 /* *INDENT-OFF* */
-VLIB_REGISTER_NODE (l2output_del_tunnel_node,static) = {
-  .function = l2output_del_tunnel_node_fn,
-  .name = "l2-output-del-tunnel",
+VLIB_REGISTER_NODE (l2output_bad_intf_node,static) = {
+  .function = l2output_bad_intf_node_fn,
+  .name = "l2-output-bad-intf",
   .vector_size = sizeof (u32),
   .type = VLIB_NODE_TYPE_INTERNAL,
 
-  .n_errors =  ARRAY_LEN(l2output_del_tunnel_error_strings),
-  .error_strings = l2output_del_tunnel_error_strings,
+  .n_errors =  ARRAY_LEN(l2output_bad_intf_error_strings),
+  .error_strings = l2output_bad_intf_error_strings,
 
   .n_next_nodes = 1,
 
index 2e04914..c683b1a 100644 (file)
@@ -134,7 +134,7 @@ _(MAPPING_DROP, "L2 Output interface mapping in progress")
 typedef enum
 {
   L2OUTPUT_NEXT_DROP,
-  L2OUTPUT_NEXT_DEL_TUNNEL,
+  L2OUTPUT_NEXT_BAD_INTF,
   L2OUTPUT_N_NEXT,
 } l2output_next_t;
 
index b90704a..9ec4c74 100644 (file)
@@ -214,7 +214,6 @@ int vnet_vxlan_add_del_tunnel
   int rv;
   vxlan4_tunnel_key_t key4;
   vxlan6_tunnel_key_t key6;
-  l2output_main_t * l2om = &l2output_main;
 
   if (!a->is_ip6) {
     key4.src = a->dst.ip4.as_u32; /* decap src in key is encap dst in config */
@@ -327,14 +326,6 @@ int vnet_vxlan_add_del_tunnel
          l2im->configs[sw_if_index].bd_index = 0;
        }
       
-      /* 
-       * Directs the l2 output path to work out the interface
-       * output next-arc itself. Needed when recycling a tunnel.
-       */
-      vec_validate_init_empty(l2om->next_nodes.output_node_index_vec, 
-                              sw_if_index, ~0);
-      l2om->next_nodes.output_node_index_vec[t->sw_if_index] 
-        = ~0;
       vnet_sw_interface_set_flags (vnm, sw_if_index, 
                                    VNET_SW_INTERFACE_FLAG_ADMIN_UP);
       if (!a->is_ip6) {
@@ -362,10 +353,6 @@ int vnet_vxlan_add_del_tunnel
 
       vxm->tunnel_index_by_sw_if_index[t->sw_if_index] = ~0;
 
-      /* Directs the l2 path to turf packets sent to this sw_if_index */
-      l2om->next_nodes.output_node_index_vec[t->sw_if_index] 
-        = L2OUTPUT_NEXT_DEL_TUNNEL;
-
       if (!a->is_ip6)
         {
           hash_unset (vxm->vxlan4_tunnel_by_key, key4.as_u64);