L2FWD:fix seq_num overwritten + validate l2fib entries when forwarding

l2_classify memeber table_index was overlaid over l2.l2fib_seq_num
which over written when table_index gets initialized in l2_input_classify

solved by overlaying both table_index and opaque_index as only one is used

seperated l2fib seq num from l2_input configs
for better handling of theoretical ABA issue where an entry for a deleted
interface is considered valid by the ager because a different interface with
same sw_if_index and seq_num was created before the ager got a chance to delete

Change-Id: I7b0eeded971627406f1c80834d7e02c0ebe62136
Signed-off-by: Eyal Bari <ebari@cisco.com>

diff --git a/src/vnet/buffer.h b/src/vnet/buffer.h
index ec5e2f7..795bbd9 100644 (file)
--- a/src/vnet/buffer.h
+++ b/src/vnet/buffer.h
@@ -195,9 +195,13 @@ typedef struct
     /* L2 classify */
     struct
     {
-      u64 pad;
-      u32 table_index;
-      u32 opaque_index;
+      u64 pad;                 /* paddind for l2 */
+      u16 pad1;
+      union
+      {
+       u32 table_index;
+       u32 opaque_index;
+      };
       u64 hash;
     } l2_classify;
 
@@ -296,6 +300,11 @@ typedef struct
 STATIC_ASSERT (sizeof (vnet_buffer_opaque_t) <= STRUCT_SIZE_OF (vlib_buffer_t,
                                                                opaque),
               "VNET buffer meta-data too large for vlib_buffer");
+STATIC_ASSERT (STRUCT_OFFSET_OF
+              (vnet_buffer_opaque_t,
+               l2_classify.table_index) >=
+              STRUCT_SIZE_OF (vnet_buffer_opaque_t, l2),
+              "l2_classify padding smaller than l2");
 
 #define vnet_buffer(b) ((vnet_buffer_opaque_t *) (b)->opaque)
 

diff --git a/src/vnet/l2/l2_bd.c b/src/vnet/l2/l2_bd.c
index f68b663..a87d02f 100644 (file)
--- a/src/vnet/l2/l2_bd.c
+++ b/src/vnet/l2/l2_bd.c
@@ -1019,8 +1019,7 @@ bd_show (vlib_main_t * vm, unformat_input_t * input, vlib_cli_command_t * cmd)
              {
                l2_flood_member_t *member =
                  vec_elt_at_index (bd_config->members, i);
-               l2_input_config_t *int_config =
-                 l2input_intf_config (member->sw_if_index);
+               u8 swif_seq_num = *l2fib_swif_seq_num (member->sw_if_index);
                u32 vtr_opr, dot1q, tag1, tag2;
                if (i == 0)
                  {
@@ -1033,7 +1032,7 @@ bd_show (vlib_main_t * vm, unformat_input_t * input, vlib_cli_command_t * cmd)
                vlib_cli_output (vm, "%=30U%=7d%=5d%=5d%=5s%=9s%=30U",
                                 format_vnet_sw_if_index_name, vnm,
                                 member->sw_if_index, member->sw_if_index,
-                                int_config->seq_num, member->shg,
+                                swif_seq_num, member->shg,
                                 member->flags & L2_FLOOD_MEMBER_BVI ? "*" :
                                 "-", i < bd_config->flood_count ? "*" : "-",
                                 format_vtr, vtr_opr, dot1q, tag1, tag2);

diff --git a/src/vnet/l2/l2_fib.c b/src/vnet/l2/l2_fib.c
index f17eee2..2bb6d10 100644 (file)
--- a/src/vnet/l2/l2_fib.c
+++ b/src/vnet/l2/l2_fib.c
@@ -41,17 +41,6 @@
  *
  */
 
-typedef struct
-{
-
-  /* hash table */
-  BVT (clib_bihash) mac_table;
-
-  /* convenience variables */
-  vlib_main_t *vlib_main;
-  vnet_main_t *vnet_main;
-} l2fib_main_t;
-
 l2fib_main_t l2fib_main;
 
 /** Format sw_if_index. If the value is ~0, use the text "N/A" */
@@ -65,7 +54,7 @@ format_vnet_sw_if_index_name_with_NA (u8 * s, va_list * args)
 
   vnet_sw_interface_t *swif = vnet_get_sw_interface_safe (vnm, sw_if_index);
   if (!swif)
-    return format (s, "Deleted");
+    return format (s, "Stale");
 
   return format (s, "%U", format_vnet_sw_interface_name, vnm,
                 vnet_get_sw_interface_safe (vnm, sw_if_index));
@@ -305,11 +294,10 @@ VLIB_CLI_COMMAND (clear_l2fib_cli, static) = {
 static inline l2fib_seq_num_t
 l2fib_cur_seq_num (u32 bd_index, u32 sw_if_index)
 {
-  l2_input_config_t *int_config = l2input_intf_config (sw_if_index);
   l2_bridge_domain_t *bd_config = l2input_bd_config (bd_index);
   /* *INDENT-OFF* */
   return (l2fib_seq_num_t) {
-    .swif = int_config->seq_num,
+    .swif = *l2fib_swif_seq_num (sw_if_index),
     .bd = bd_config->seq_num,
   };
   /* *INDENT-ON* */
@@ -748,8 +736,7 @@ l2fib_start_ager_scan (vlib_main_t * vm)
 void
 l2fib_flush_int_mac (vlib_main_t * vm, u32 sw_if_index)
 {
-  l2_input_config_t *int_config = l2input_intf_config (sw_if_index);
-  int_config->seq_num += 1;
+  *l2fib_swif_seq_num (sw_if_index) += 1;
   l2fib_start_ager_scan (vm);
 }
 

diff --git a/src/vnet/l2/l2_fib.h b/src/vnet/l2/l2_fib.h
index e571a21..0318450 100644 (file)
--- a/src/vnet/l2/l2_fib.h
+++ b/src/vnet/l2/l2_fib.h
@@ -27,6 +27,22 @@
 #define L2FIB_NUM_BUCKETS (64 * 1024)
 #define L2FIB_MEMORY_SIZE (256<<20)
 
+typedef struct
+{
+
+  /* hash table */
+  BVT (clib_bihash) mac_table;
+
+  /* per swif vector of sequence number for interface based flush of MACs */
+  u8 *swif_seq_num;
+
+  /* convenience variables */
+  vlib_main_t *vlib_main;
+  vnet_main_t *vnet_main;
+} l2fib_main_t;
+
+extern l2fib_main_t l2fib_main;
+
 /*
  * The L2fib key is the mac address and bridge domain ID
  */
@@ -350,6 +366,14 @@ l2fib_table_dump (u32 bd_index, l2fib_entry_key_t ** l2fe_key,
 
 u8 *format_vnet_sw_if_index_name_with_NA (u8 * s, va_list * args);
 
+static_always_inline u8 *
+l2fib_swif_seq_num (u32 sw_if_index)
+{
+  l2fib_main_t *mp = &l2fib_main;
+  vec_validate (mp->swif_seq_num, sw_if_index);
+  return vec_elt_at_index (mp->swif_seq_num, sw_if_index);
+}
+
 BVT (clib_bihash) * get_mac_table (void);
 
 #endif

diff --git a/src/vnet/l2/l2_fwd.c b/src/vnet/l2/l2_fwd.c
index f7e2ccb..8140728 100644 (file)
--- a/src/vnet/l2/l2_fwd.c
+++ b/src/vnet/l2/l2_fwd.c
@@ -89,7 +89,8 @@ _(HIT,           "L2 forward hits")                   \
 _(BVI_BAD_MAC,   "BVI L3 MAC mismatch")                \
 _(BVI_ETHERTYPE, "BVI packet with unhandled ethertype")        \
 _(FILTER_DROP,   "Filter Mac Drop")                    \
-_(REFLECT_DROP,  "Reflection Drop")
+_(REFLECT_DROP,  "Reflection Drop")                    \
+_(STALE_DROP,    "Stale entry Drop")
 
 typedef enum
 {
@@ -123,28 +124,15 @@ l2fwd_process (vlib_main_t * vm,
               vlib_buffer_t * b0,
               u32 sw_if_index0, l2fib_entry_result_t * result0, u32 * next0)
 {
-  if (PREDICT_FALSE (result0->raw == ~0))
-    {
-      /*
-       * lookup miss, so flood
-       * TODO:replicate packet to each intf in bridge-domain
-       * For now just drop
-       */
-      if (vnet_buffer (b0)->l2.feature_bitmap & L2INPUT_FEAT_UU_FLOOD)
-       {
-         *next0 = L2FWD_NEXT_FLOOD;
-       }
-      else
-       {
-         /* Flooding is disabled */
-         b0->error = node->errors[L2FWD_ERROR_FLOOD];
-         *next0 = L2FWD_NEXT_DROP;
-       }
+  int try_flood = result0->raw == ~0;
+  int flood_error;
 
+  if (PREDICT_FALSE (try_flood))
+    {
+      flood_error = L2FWD_ERROR_FLOOD;
     }
   else
     {
-
       /* lookup hit, forward packet  */
 #ifdef COUNTERS
       em->counters[node_counter_base_index + L2FWD_ERROR_HIT] += 1;
@@ -152,22 +140,37 @@ l2fwd_process (vlib_main_t * vm,
 
       vnet_buffer (b0)->sw_if_index[VLIB_TX] = result0->fields.sw_if_index;
       *next0 = L2FWD_NEXT_L2_OUTPUT;
+      int l2fib_seq_num_valid = 1;
+      /* check l2fib seq num for stale entries */
+      if (!result0->fields.static_mac)
+       {
+         l2fib_seq_num_t in_sn = {.as_u16 = vnet_buffer (b0)->l2.l2fib_sn };
+         l2fib_seq_num_t expected_sn = {
+           .bd = in_sn.bd,
+           .swif = *l2fib_swif_seq_num (result0->fields.sw_if_index),
+         };
+         l2fib_seq_num_valid =
+           expected_sn.as_u16 == result0->fields.sn.as_u16;
+       }
 
+      if (PREDICT_FALSE (!l2fib_seq_num_valid))
+       {
+         flood_error = L2FWD_ERROR_STALE_DROP;
+         try_flood = 1;
+       }
       /* perform reflection check */
-      if (PREDICT_FALSE (sw_if_index0 == result0->fields.sw_if_index))
+      else if (PREDICT_FALSE (sw_if_index0 == result0->fields.sw_if_index))
        {
          b0->error = node->errors[L2FWD_ERROR_REFLECT_DROP];
          *next0 = L2FWD_NEXT_DROP;
-
-         /* perform filter check */
        }
+      /* perform filter check */
       else if (PREDICT_FALSE (result0->fields.filter))
        {
          b0->error = node->errors[L2FWD_ERROR_FILTER_DROP];
          *next0 = L2FWD_NEXT_DROP;
-
-         /* perform BVI check */
        }
+      /* perform BVI check */
       else if (PREDICT_FALSE (result0->fields.bvi))
        {
          u32 rc;
@@ -192,6 +195,27 @@ l2fwd_process (vlib_main_t * vm,
            }
        }
     }
+
+  /* flood */
+  if (PREDICT_FALSE (try_flood))
+    {
+      /*
+       * lookup miss, so flood
+       * TODO:replicate packet to each intf in bridge-domain
+       * For now just drop
+       */
+      if (vnet_buffer (b0)->l2.feature_bitmap & L2INPUT_FEAT_UU_FLOOD)
+       {
+         *next0 = L2FWD_NEXT_FLOOD;
+       }
+      else
+       {
+         /* Flooding is disabled */
+         b0->error = node->errors[flood_error];
+         *next0 = L2FWD_NEXT_DROP;
+       }
+    }
+
 }
 
 

diff --git a/src/vnet/l2/l2_input.c b/src/vnet/l2/l2_input.c
index aca23fe..22fc2a9 100644 (file)
--- a/src/vnet/l2/l2_input.c
+++ b/src/vnet/l2/l2_input.c
@@ -205,7 +205,7 @@ classify_and_dispatch (vlib_main_t * vm,
       /* Save bridge domain and interface seq_num */
       /* *INDENT-OFF* */
       l2fib_seq_num_t sn = {
-        .swif = config->seq_num,
+        .swif = *l2fib_swif_seq_num(sw_if_index0),
        .bd = bd_config->seq_num,
       };
       /* *INDENT-ON* */
@@ -637,7 +637,7 @@ set_int_l2_mode (vlib_main_t * vm, vnet_main_t * vnet_main, /*           */
          config->xconnect = 0;
          config->bridge = 1;
          config->bd_index = bd_index;
-         config->seq_num += 1;
+         *l2fib_swif_seq_num (sw_if_index) += 1;
 
          /*
           * Enable forwarding, flooding, learning and ARP termination by default

diff --git a/src/vnet/l2/l2_input.h b/src/vnet/l2/l2_input.h
index cb67cb9..c1b669b 100644 (file)
--- a/src/vnet/l2/l2_input.h
+++ b/src/vnet/l2/l2_input.h
@@ -53,9 +53,6 @@ typedef struct
   /* split horizon group */
   u8 shg;
 
-  /* sequence number for interface based flush of MACs */
-  u8 seq_num;
-
 } l2_input_config_t;
 
 

diff --git a/src/vnet/l2/l2_learn.c b/src/vnet/l2/l2_learn.c
index adc5e70..3ff2e70 100644 (file)
--- a/src/vnet/l2/l2_learn.c
+++ b/src/vnet/l2/l2_learn.c
@@ -138,11 +138,14 @@ l2learn_process (vlib_node_runtime_t * node,
        * The entry was in the table, and the sw_if_index matched, the normal case
        */
       counter_base[L2LEARN_ERROR_HIT] += 1;
-      if (PREDICT_FALSE (result0->fields.timestamp != timestamp))
-       result0->fields.timestamp = timestamp;
-      if (PREDICT_FALSE
-         (result0->fields.sn.as_u16 != vnet_buffer (b0)->l2.l2fib_sn))
-       result0->fields.sn.as_u16 = vnet_buffer (b0)->l2.l2fib_sn;
+      if (!result0->fields.static_mac)
+       {
+         if (PREDICT_FALSE (result0->fields.timestamp != timestamp))
+           result0->fields.timestamp = timestamp;
+         if (PREDICT_FALSE
+             (result0->fields.sn.as_u16 != vnet_buffer (b0)->l2.l2fib_sn))
+           result0->fields.sn.as_u16 = vnet_buffer (b0)->l2.l2fib_sn;
+       }
     }
   else if (result0->raw == ~0)
     {

diff --git a/test/test_l2_fib.py b/test/test_l2_fib.py
index f9a78ef..9249a2c 100644 (file)
--- a/test/test_l2_fib.py
+++ b/test/test_l2_fib.py
@@ -490,7 +490,6 @@ class TestL2fib(VppTestCase):
         self.config_l2_fib_entries(bd_id=1, n_hosts_per_if=10)
         self.config_l2_fib_entries(bd_id=2, n_hosts_per_if=10)
         flushed = self.flush_int(self.pg_interfaces[0].sw_if_index)
-        self.sleep(1)
         self.run_verify_test(bd_id=1, dst_hosts=self.learned_hosts)
         self.run_verify_negat_test(bd_id=1, dst_hosts=flushed)
 
@@ -504,7 +503,6 @@ class TestL2fib(VppTestCase):
         self.config_l2_fib_entries(bd_id=1, n_hosts_per_if=10)
         self.config_l2_fib_entries(bd_id=2, n_hosts_per_if=10)
         flushed = self.flush_bd(bd_id=1)
-        self.sleep(1)
         self.run_verify_negat_test(bd_id=1, dst_hosts=flushed)
         self.run_verify_test(bd_id=2, dst_hosts=self.learned_hosts)
 
@@ -518,7 +516,6 @@ class TestL2fib(VppTestCase):
         self.config_l2_fib_entries(bd_id=1, n_hosts_per_if=10)
         self.config_l2_fib_entries(bd_id=2, n_hosts_per_if=10)
         flushed = self.flush_all()
-        self.sleep(2)
         self.run_verify_negat_test(bd_id=1, dst_hosts=flushed)
         self.run_verify_negat_test(bd_id=2, dst_hosts=flushed)