summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEyal Bari <ebari@cisco.com>2017-06-25 14:42:33 +0300
committerJohn Lo <loj@cisco.com>2017-06-27 12:41:56 +0000
commit25ff2ea3a31e422094f6d91eab46222a29a77c4b (patch)
treea8249ce452d6af4daa66e09851ae689612286e71
parente2547ab574a82c69482704ef27375f1391deb9af (diff)
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 <ebari@cisco.com>
-rw-r--r--src/vnet/l2/l2_api.c28
-rw-r--r--src/vnet/l2/l2_fib.c10
-rw-r--r--src/vnet/l2/l2_fib.h13
-rw-r--r--src/vnet/l2/l2_input.c2
-rw-r--r--src/vnet/l2/l2_learn.c98
-rw-r--r--test/test_l2bd_multi_instance.py58
6 files changed, 93 insertions, 116 deletions
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 */ );
}
diff --git a/test/test_l2bd_multi_instance.py b/test/test_l2bd_multi_instance.py
index 0bb9e5974d1..7dd27fb251e 100644
--- a/test/test_l2bd_multi_instance.py
+++ b/test/test_l2bd_multi_instance.py
@@ -403,7 +403,33 @@ class TestL2bdMultiInst(VppTestCase):
self.run_verify_test()
def test_l2bd_inst_02(self):
- """ L2BD Multi-instance test 2 - delete 2 BDs
+ """ L2BD Multi-instance test 2 - update data of 5 BDs
+ """
+ # Config 2
+ # Update data of 5 BDs (disable learn, forward, flood, uu-flood)
+ self.set_bd_flags(self.bd_list[0], learn=False, forward=False,
+ flood=False, uu_flood=False)
+ self.set_bd_flags(self.bd_list[1], forward=False)
+ self.set_bd_flags(self.bd_list[2], flood=False)
+ self.set_bd_flags(self.bd_list[3], uu_flood=False)
+ self.set_bd_flags(self.bd_list[4], learn=False)
+
+ # Verify 2
+ # Skipping check of uu_flood as it is not returned by
+ # bridge_domain_dump api command
+ self.verify_bd(self.bd_list[0], learn=False, forward=False,
+ flood=False, uu_flood=False)
+ self.verify_bd(self.bd_list[1], learn=True, forward=False,
+ flood=True, uu_flood=True)
+ self.verify_bd(self.bd_list[2], learn=True, forward=True,
+ flood=False, uu_flood=True)
+ self.verify_bd(self.bd_list[3], learn=True, forward=True,
+ flood=True, uu_flood=False)
+ self.verify_bd(self.bd_list[4], learn=False, forward=True,
+ flood=True, uu_flood=True)
+
+ def test_l2bd_inst_03(self):
+ """ L2BD Multi-instance test 3 - delete 2 BDs
"""
# Config 3
# Delete 2 BDs
@@ -418,8 +444,8 @@ class TestL2bdMultiInst(VppTestCase):
# Test 3
self.run_verify_test()
- def test_l2bd_inst_03(self):
- """ L2BD Multi-instance test 3 - add 2 BDs
+ def test_l2bd_inst_04(self):
+ """ L2BD Multi-instance test 4 - add 2 BDs
"""
# Config 4
# Create 5 BDs, put interfaces to these BDs and send MAC learning
@@ -434,32 +460,6 @@ class TestL2bdMultiInst(VppTestCase):
# self.vapi.cli("clear trace")
self.run_verify_test()
- def test_l2bd_inst_04(self):
- """ L2BD Multi-instance test 4 - update data of 5 BDs
- """
- # Config 2
- # Update data of 5 BDs (disable learn, forward, flood, uu-flood)
- self.set_bd_flags(self.bd_list[0], learn=False, forward=False,
- flood=False, uu_flood=False)
- self.set_bd_flags(self.bd_list[1], forward=False)
- self.set_bd_flags(self.bd_list[2], flood=False)
- self.set_bd_flags(self.bd_list[3], uu_flood=False)
- self.set_bd_flags(self.bd_list[4], learn=False)
-
- # Verify 2
- # Skipping check of uu_flood as it is not returned by
- # bridge_domain_dump api command
- self.verify_bd(self.bd_list[0], learn=False, forward=False,
- flood=False, uu_flood=False)
- self.verify_bd(self.bd_list[1], learn=True, forward=False,
- flood=True, uu_flood=True)
- self.verify_bd(self.bd_list[2], learn=True, forward=True,
- flood=False, uu_flood=True)
- self.verify_bd(self.bd_list[3], learn=True, forward=True,
- flood=True, uu_flood=False)
- self.verify_bd(self.bd_list[4], learn=False, forward=True,
- flood=True, uu_flood=True)
-
def test_l2bd_inst_05(self):
""" L2BD Multi-instance test 5 - delete 5 BDs
"""