From 0f360dc3aa40d0654198bd3f3850bd31a0d78f7e Mon Sep 17 00:00:00 2001 From: Eyal Bari Date: Wed, 14 Jun 2017 13:11:20 +0300 Subject: [PATCH] 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 --- src/vnet/buffer.h | 15 ++++++++--- src/vnet/l2/l2_bd.c | 5 ++-- src/vnet/l2/l2_fib.c | 19 +++---------- src/vnet/l2/l2_fib.h | 24 +++++++++++++++++ src/vnet/l2/l2_fwd.c | 72 +++++++++++++++++++++++++++++++++----------------- src/vnet/l2/l2_input.c | 4 +-- src/vnet/l2/l2_input.h | 3 --- src/vnet/l2/l2_learn.c | 13 +++++---- test/test_l2_fib.py | 3 --- 9 files changed, 99 insertions(+), 59 deletions(-) diff --git a/src/vnet/buffer.h b/src/vnet/buffer.h index ec5e2f7505f..795bbd9610b 100644 --- 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 f68b6638618..a87d02f2ffd 100644 --- 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 f17eee2acf9..2bb6d105335 100644 --- 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 e571a210975..031845029ac 100644 --- 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 f7e2ccb65b7..8140728b5e1 100644 --- 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 aca23fe03fc..22fc2a9894f 100644 --- 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 cb67cb9d20e..c1b669b4082 100644 --- 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 adc5e70f493..3ff2e704a6e 100644 --- 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 f9a78efce0f..9249a2ced23 100644 --- 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) -- 2.16.6