From 31a71ab497616940c105fa1719515fe7ae37f37a Mon Sep 17 00:00:00 2001 From: Eyal Bari Date: Sun, 25 Jun 2017 14:42:33 +0300 Subject: L2-LEARN:fix l2fib entry seq num not updated on hit (VPP-888) fixed instability in l2bd_multi_instnce test - sometimes failing with extra packets captured it appears l2-learn was not updating hit entries but rather a copy of them. if the ager did not have a chance to run before the test was running the learning cycle - entries were not updated with the packet's seq num - causing packets to flood when hitting the stale seq_num in l2-fwd - hence the extra packets fixed handling of filter entries revert workaround for instability in test Change-Id: I16d918e6310a5bf40bad5b7335b2140c2867cb71 Signed-off-by: Eyal Bari (cherry picked from commit 25ff2ea3a31e422094f6d91eab46222a29a77c4b) --- src/vnet/l2/l2_api.c | 28 ++++++--------- src/vnet/l2/l2_fib.c | 10 +++--- src/vnet/l2/l2_fib.h | 13 +++++++ src/vnet/l2/l2_input.c | 2 +- src/vnet/l2/l2_learn.c | 98 ++++++++++++++++++-------------------------------- 5 files changed, 64 insertions(+), 87 deletions(-) (limited to 'src/vnet/l2') diff --git a/src/vnet/l2/l2_api.c b/src/vnet/l2/l2_api.c index aa3dcb7e49e..a0b40d6d840 100644 --- a/src/vnet/l2/l2_api.c +++ b/src/vnet/l2/l2_api.c @@ -187,30 +187,24 @@ vl_api_l2fib_add_del_t_handler (vl_api_l2fib_add_del_t * mp) l2input_main_t *l2im = &l2input_main; vl_api_l2fib_add_del_reply_t *rmp; int rv = 0; - u64 mac = 0; - u32 sw_if_index = ntohl (mp->sw_if_index); u32 bd_id = ntohl (mp->bd_id); - u32 bd_index; - u32 static_mac; - u32 filter_mac; - u32 bvi_mac; - uword *p; - - mac = mp->mac; + uword *p = hash_get (bdm->bd_index_by_bd_id, bd_id); - p = hash_get (bdm->bd_index_by_bd_id, bd_id); if (!p) { rv = VNET_API_ERROR_NO_SUCH_ENTRY; goto bad_sw_if_index; } - bd_index = p[0]; + u32 bd_index = p[0]; + u64 mac = mp->mac; if (mp->is_add) { - filter_mac = mp->filter_mac ? 1 : 0; - if (filter_mac == 0) + if (mp->filter_mac) + l2fib_add_filter_entry (mac, bd_index); + else { + u32 sw_if_index = ntohl (mp->sw_if_index); VALIDATE_SW_IF_INDEX (mp); if (vec_len (l2im->configs) <= sw_if_index) { @@ -227,11 +221,11 @@ vl_api_l2fib_add_del_t_handler (vl_api_l2fib_add_del_t * mp) goto bad_sw_if_index; } } + u32 static_mac = mp->static_mac ? 1 : 0; + u32 bvi_mac = mp->bvi_mac ? 1 : 0; + l2fib_add_fwd_entry (mac, bd_index, sw_if_index, static_mac, + bvi_mac); } - static_mac = mp->static_mac ? 1 : 0; - bvi_mac = mp->bvi_mac ? 1 : 0; - l2fib_add_entry (mac, bd_index, sw_if_index, static_mac, filter_mac, - bvi_mac); } else { diff --git a/src/vnet/l2/l2_fib.c b/src/vnet/l2/l2_fib.c index 2bb6d105335..6f8f6e06330 100644 --- a/src/vnet/l2/l2_fib.c +++ b/src/vnet/l2/l2_fib.c @@ -413,8 +413,10 @@ l2fib_add (vlib_main_t * vm, } } - l2fib_add_entry (mac, bd_index, sw_if_index, static_mac, filter_mac, - bvi_mac); + if (filter_mac) + l2fib_add_filter_entry (mac, bd_index); + else + l2fib_add_fwd_entry (mac, bd_index, sw_if_index, static_mac, bvi_mac); done: return error; @@ -464,7 +466,6 @@ l2fib_test_command_fn (vlib_main_t * vm, u64 mac, save_mac; u32 bd_index = 0; u32 sw_if_index = 8; - u32 filter_mac = 0; u32 bvi_mac = 0; u32 is_add = 0; u32 is_del = 0; @@ -503,8 +504,7 @@ l2fib_test_command_fn (vlib_main_t * vm, for (i = 0; i < count; i++) { u64 tmp; - l2fib_add_entry (mac, bd_index, sw_if_index, mac, - filter_mac, bvi_mac); + l2fib_add_fwd_entry (mac, bd_index, sw_if_index, mac, bvi_mac); tmp = clib_net_to_host_u64 (mac); tmp >>= 16; tmp++; diff --git a/src/vnet/l2/l2_fib.h b/src/vnet/l2/l2_fib.h index 031845029ac..21dcc4513e0 100644 --- a/src/vnet/l2/l2_fib.h +++ b/src/vnet/l2/l2_fib.h @@ -350,6 +350,19 @@ l2fib_add_entry (u64 mac, u32 bd_index, u32 sw_if_index, u32 static_mac, u32 drop_mac, u32 bvi_mac); +static inline void +l2fib_add_fwd_entry (u64 mac, u32 bd_index, u32 sw_if_index, u32 static_mac, + u32 bvi_mac) +{ + l2fib_add_entry (mac, bd_index, sw_if_index, static_mac, 0, bvi_mac); +} + +static inline void +l2fib_add_filter_entry (u64 mac, u32 bd_index) +{ + l2fib_add_entry (mac, bd_index, ~0, 1, 1, 0); +} + u32 l2fib_del_entry (u64 mac, u32 bd_index); void l2fib_start_ager_scan (vlib_main_t * vm); diff --git a/src/vnet/l2/l2_input.c b/src/vnet/l2/l2_input.c index 22fc2a9894f..d536d15b8de 100644 --- a/src/vnet/l2/l2_input.c +++ b/src/vnet/l2/l2_input.c @@ -671,7 +671,7 @@ set_int_l2_mode (vlib_main_t * vm, vnet_main_t * vnet_main, /* */ /* create the l2fib entry for the bvi interface */ mac = *((u64 *) hi->hw_address); - l2fib_add_entry (mac, bd_index, sw_if_index, 1, 0, 1); /* static + bvi */ + l2fib_add_fwd_entry (mac, bd_index, sw_if_index, 1, 1); /* static + bvi */ /* Disable learning by default. no use since l2fib entry is static. */ config->feature_bitmap &= ~L2INPUT_FEAT_LEARN; diff --git a/src/vnet/l2/l2_learn.c b/src/vnet/l2/l2_learn.c index 3ff2e704a6e..b9904d3edf5 100644 --- a/src/vnet/l2/l2_learn.c +++ b/src/vnet/l2/l2_learn.c @@ -131,27 +131,22 @@ l2learn_process (vlib_node_runtime_t * node, feature_bitmap); /* Check mac table lookup result */ - if (PREDICT_TRUE (result0->fields.sw_if_index == sw_if_index0)) { /* * The entry was in the table, and the sw_if_index matched, the normal case */ counter_base[L2LEARN_ERROR_HIT] += 1; - 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; - } + int update = !result0->fields.static_mac && + (result0->fields.timestamp != timestamp || + result0->fields.sn.as_u16 != vnet_buffer (b0)->l2.l2fib_sn); + + if (PREDICT_TRUE (!update)) + return; } else if (result0->raw == ~0) { - /* The entry was not in table, so add it */ - counter_base[L2LEARN_ERROR_MISS] += 1; if (msm->global_learn_count == msm->global_learn_limit) @@ -161,32 +156,27 @@ l2learn_process (vlib_node_runtime_t * node, * In the future, limits could also be per-interface or bridge-domain. */ counter_base[L2LEARN_ERROR_LIMIT] += 1; - goto done; - - } - else - { - BVT (clib_bihash_kv) kv; - /* It is ok to learn */ - - result0->raw = 0; /* clear all fields */ - result0->fields.sw_if_index = sw_if_index0; - result0->fields.timestamp = timestamp; - result0->fields.sn.as_u16 = vnet_buffer (b0)->l2.l2fib_sn; - kv.key = key0->raw; - kv.value = result0->raw; - - BV (clib_bihash_add_del) (msm->mac_table, &kv, 1 /* is_add */ ); - - cached_key->raw = ~0; /* invalidate the cache */ - msm->global_learn_count++; + return; } + /* It is ok to learn */ + msm->global_learn_count++; + result0->raw = 0; /* clear all fields */ + result0->fields.sw_if_index = sw_if_index0; + cached_key->raw = ~0; /* invalidate the cache */ } else { - /* The entry was in the table, but with the wrong sw_if_index mapping (mac move) */ + if (result0->fields.filter) + { + ASSERT (result0->fields.sw_if_index == ~0); + /* drop packet because lookup matched a filter mac entry */ + b0->error = node->errors[L2LEARN_ERROR_FILTER_DROP]; + *next0 = L2LEARN_NEXT_DROP; + return; + } + counter_base[L2LEARN_ERROR_MAC_MOVE] += 1; if (result0->fields.static_mac) @@ -197,44 +187,24 @@ l2learn_process (vlib_node_runtime_t * node, */ b0->error = node->errors[L2LEARN_ERROR_MAC_MOVE_VIOLATE]; *next0 = L2LEARN_NEXT_DROP; + return; } - else - { - /* - * Update the entry - * TODO: may want to rate limit mac moves - * TODO: check global/bridge domain/interface learn limits - */ - BVT (clib_bihash_kv) kv; - - result0->raw = 0; /* clear all fields */ - result0->fields.sw_if_index = sw_if_index0; - result0->fields.timestamp = timestamp; - result0->fields.sn.as_u16 = vnet_buffer (b0)->l2.l2fib_sn; - kv.key = key0->raw; - kv.value = result0->raw; - - cached_key->raw = ~0; /* invalidate the cache */ - - BV (clib_bihash_add_del) (msm->mac_table, &kv, 1 /* is_add */ ); - } + /* + * TODO: may want to rate limit mac moves + * TODO: check global/bridge domain/interface learn limits + */ + result0->fields.sw_if_index = sw_if_index0; } - if (result0->fields.filter) - { - /* drop packet because lookup matched a filter mac entry */ + /* Update the entry */ + result0->fields.timestamp = timestamp; + result0->fields.sn.as_u16 = vnet_buffer (b0)->l2.l2fib_sn; - if (*next0 != L2LEARN_NEXT_DROP) - { - /* if we're not already dropping the packet, do it now */ - b0->error = node->errors[L2LEARN_ERROR_FILTER_DROP]; - *next0 = L2LEARN_NEXT_DROP; - } - } - -done: - return; + BVT (clib_bihash_kv) kv; + kv.key = key0->raw; + kv.value = result0->raw; + BV (clib_bihash_add_del) (msm->mac_table, &kv, 1 /* is_add */ ); } -- cgit 1.2.3-korg