@@ -16,6 +16,7 @@
#include <config.h>
#include <stdbool.h>
+#include "local_data.h"
#include "lport.h"
#include "mac-cache.h"
#include "openvswitch/hmap.h"
@@ -39,11 +40,8 @@ static uint32_t
fdb_data_hash(const struct fdb_data *fdb_data);
static inline bool
fdb_data_equals(const struct fdb_data *a, const struct fdb_data *b);
-static struct mac_cache_threshold *
-mac_cache_threshold_find(struct hmap *thresholds, const struct uuid *uuid);
static uint64_t
-mac_cache_threshold_get_value_ms(const struct sbrec_datapath_binding *dp,
- enum mac_cache_type type);
+mac_cache_threshold_get_value_ms(const struct sbrec_datapath_binding *dp);
static void
mac_cache_threshold_remove(struct hmap *thresholds,
struct mac_cache_threshold *threshold);
@@ -67,60 +65,82 @@ buffered_packets_db_lookup(struct buffered_packets *bp,
struct ovsdb_idl_index *sbrec_mb_by_lport_ip);
/* Thresholds. */
-bool
+void
mac_cache_threshold_add(struct mac_cache_data *data,
- const struct sbrec_datapath_binding *dp,
- enum mac_cache_type type)
+ const struct sbrec_datapath_binding *dp)
{
- struct hmap *thresholds = &data->thresholds[type];
struct mac_cache_threshold *threshold =
- mac_cache_threshold_find(thresholds, &dp->header_.uuid);
+ mac_cache_threshold_find(data, dp->tunnel_key);
if (threshold) {
- return true;
+ return;
}
- uint64_t value = mac_cache_threshold_get_value_ms(dp, type);
+ uint64_t value = mac_cache_threshold_get_value_ms(dp);
if (!value) {
- return false;
+ return;
}
threshold = xmalloc(sizeof *threshold);
- threshold->uuid = dp->header_.uuid;
+ threshold->dp_key = dp->tunnel_key;
threshold->value = value;
threshold->dump_period = (3 * value) / 4;
- hmap_insert(thresholds, &threshold->hmap_node,
- uuid_hash(&dp->header_.uuid));
-
- return true;
+ hmap_insert(&data->thresholds, &threshold->hmap_node, dp->tunnel_key);
}
-bool
+void
mac_cache_threshold_replace(struct mac_cache_data *data,
const struct sbrec_datapath_binding *dp,
- enum mac_cache_type type)
+ const struct hmap *local_datapaths)
{
- struct hmap *thresholds = &data->thresholds[type];
struct mac_cache_threshold *threshold =
- mac_cache_threshold_find(thresholds, &dp->header_.uuid);
+ mac_cache_threshold_find(data, dp->tunnel_key);
if (threshold) {
- mac_cache_threshold_remove(thresholds, threshold);
+ mac_cache_threshold_remove(&data->thresholds, threshold);
+ }
+
+ if (!get_local_datapath(local_datapaths, dp->tunnel_key)) {
+ return;
}
- return mac_cache_threshold_add(data, dp, type);
+ mac_cache_threshold_add(data, dp);
+}
+
+
+struct mac_cache_threshold *
+mac_cache_threshold_find(struct mac_cache_data *data, uint32_t dp_key)
+{
+ struct mac_cache_threshold *threshold;
+ HMAP_FOR_EACH_WITH_HASH (threshold, hmap_node, dp_key, &data->thresholds) {
+ if (threshold->dp_key == dp_key) {
+ return threshold;
+ }
+ }
+
+ return NULL;
}
void
-mac_cache_thresholds_clear(struct mac_cache_data *data)
+mac_cache_thresholds_sync(struct mac_cache_data *data,
+ const struct hmap *local_datapaths)
{
- for (size_t i = 0; i < MAC_CACHE_MAX; i++) {
- struct mac_cache_threshold *threshold;
- HMAP_FOR_EACH_POP (threshold, hmap_node, &data->thresholds[i]) {
- free(threshold);
+ struct mac_cache_threshold *threshold;
+ HMAP_FOR_EACH_SAFE (threshold, hmap_node, &data->thresholds) {
+ if (!get_local_datapath(local_datapaths, threshold->dp_key)) {
+ mac_cache_threshold_remove(&data->thresholds, threshold);
}
}
}
+void
+mac_cache_thresholds_clear(struct mac_cache_data *data)
+{
+ struct mac_cache_threshold *threshold;
+ HMAP_FOR_EACH_POP (threshold, hmap_node, &data->thresholds) {
+ free(threshold);
+ }
+}
+
/* MAC binding. */
struct mac_binding *
mac_binding_add(struct hmap *map, struct mac_binding_data mb_data,
@@ -231,7 +251,6 @@ fdb_add(struct hmap *map, struct fdb_data fdb_data) {
if (!fdb) {
fdb = xmalloc(sizeof *fdb);
fdb->sbrec_fdb = NULL;
- fdb->dp_uuid = UUID_ZERO;
hmap_insert(map, &fdb->hmap_node, fdb_data_hash(&fdb_data));
}
@@ -343,7 +362,6 @@ mac_binding_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
void *data)
{
struct mac_cache_data *cache_data = data;
- struct hmap *thresholds = &cache_data->thresholds[MAC_CACHE_MAC_BINDING];
long long timewall_now = time_wall_msec();
struct mac_cache_stats *stats;
@@ -355,9 +373,8 @@ mac_binding_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
continue;
}
- struct uuid *dp_uuid = &mb->sbrec_mb->datapath->header_.uuid;
struct mac_cache_threshold *threshold =
- mac_cache_threshold_find(thresholds, dp_uuid);
+ mac_cache_threshold_find(cache_data, mb->data.dp_key);
/* If "idle_age" is under threshold it means that the mac binding is
* used on this chassis. Also make sure that we don't update the
@@ -371,7 +388,7 @@ mac_binding_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
free(stats);
}
- mac_cache_update_req_delay(thresholds, req_delay);
+ mac_cache_update_req_delay(&cache_data->thresholds, req_delay);
}
/* FDB stat processing. */
@@ -396,7 +413,6 @@ fdb_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
void *data)
{
struct mac_cache_data *cache_data = data;
- struct hmap *thresholds = &cache_data->thresholds[MAC_CACHE_FDB];
long long timewall_now = time_wall_msec();
struct mac_cache_stats *stats;
@@ -409,7 +425,8 @@ fdb_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
}
struct mac_cache_threshold *threshold =
- mac_cache_threshold_find(thresholds, &fdb->dp_uuid);
+ mac_cache_threshold_find(cache_data, fdb->data.dp_key);
+
/* If "idle_age" is under threshold it means that the mac binding is
* used on this chassis. Also make sure that we don't update the
* timestamp more than once during the dump period. */
@@ -422,7 +439,7 @@ fdb_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
free(stats);
}
- mac_cache_update_req_delay(thresholds, req_delay);
+ mac_cache_update_req_delay(&cache_data->thresholds, req_delay);
}
/* Packet buffering. */
@@ -625,41 +642,22 @@ fdb_data_equals(const struct fdb_data *a, const struct fdb_data *b)
eth_addr_equals(a->mac, b->mac);
}
-
-static struct mac_cache_threshold *
-mac_cache_threshold_find(struct hmap *thresholds, const struct uuid *uuid)
+static uint64_t
+mac_cache_threshold_get_value_ms(const struct sbrec_datapath_binding *dp)
{
- uint32_t hash = uuid_hash(uuid);
+ uint64_t mb_value =
+ smap_get_uint(&dp->external_ids, "mac_binding_age_threshold", 0);
+ uint64_t fdb_value =
+ smap_get_uint(&dp->external_ids, "fdb_age_threshold", 0);
- struct mac_cache_threshold *threshold;
- HMAP_FOR_EACH_WITH_HASH (threshold, hmap_node, hash, thresholds) {
- if (uuid_equals(&threshold->uuid, uuid)) {
- return threshold;
- }
+ if (mb_value && fdb_value) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "Invalid aging threshold configuration for datapath:"
+ " "UUID_FMT, UUID_ARGS(&dp->header_.uuid));
+ return 0;
}
- return NULL;
-}
-
-static uint64_t
-mac_cache_threshold_get_value_ms(const struct sbrec_datapath_binding *dp,
- enum mac_cache_type type)
-{
- uint64_t value = 0;
- switch (type) {
- case MAC_CACHE_MAC_BINDING:
- value = smap_get_uint(&dp->external_ids, "mac_binding_age_threshold",
- 0);
- break;
- case MAC_CACHE_FDB:
- value = smap_get_uint(&dp->external_ids, "fdb_age_threshold", 0);
- break;
- case MAC_CACHE_MAX:
- default:
- break;
- }
-
- return value * 1000;
+ return mb_value ? mb_value * 1000 : fdb_value * 1000;
}
static void
@@ -29,15 +29,9 @@
struct ovsdb_idl_index;
-enum mac_cache_type {
- MAC_CACHE_MAC_BINDING,
- MAC_CACHE_FDB,
- MAC_CACHE_MAX
-};
-
struct mac_cache_data {
- /* 'struct mac_cache_threshold' by datapath UUID. */
- struct hmap thresholds[MAC_CACHE_MAX];
+ /* 'struct mac_cache_threshold' by datapath's tunnel_key. */
+ struct hmap thresholds;
/* 'struct mac_cache_mac_binding' by 'struct mac_cache_mb_data' that are
* local and have threshold > 0. */
struct hmap mac_bindings;
@@ -48,8 +42,8 @@ struct mac_cache_data {
struct mac_cache_threshold {
struct hmap_node hmap_node;
- /* Datapath UUID. */
- struct uuid uuid;
+ /* Datapath tunnel key. */
+ uint32_t dp_key;
/* Aging threshold in ms. */
uint64_t value;
/* Statistics dump period. */
@@ -89,8 +83,6 @@ struct fdb {
struct fdb_data data;
/* Reference to the SB FDB record. */
const struct sbrec_fdb *sbrec_fdb;
- /* UUID of datapath for this FDB record. */
- struct uuid dp_uuid;
};
struct bp_packet_data {
@@ -123,12 +115,15 @@ struct buffered_packets_ctx {
};
/* Thresholds. */
-bool mac_cache_threshold_add(struct mac_cache_data *data,
- const struct sbrec_datapath_binding *dp,
- enum mac_cache_type type);
-bool mac_cache_threshold_replace(struct mac_cache_data *data,
+void mac_cache_threshold_add(struct mac_cache_data *data,
+ const struct sbrec_datapath_binding *dp);
+void mac_cache_threshold_replace(struct mac_cache_data *data,
const struct sbrec_datapath_binding *dp,
- enum mac_cache_type type);
+ const struct hmap *local_datapaths);
+struct mac_cache_threshold *
+mac_cache_threshold_find(struct mac_cache_data *data, uint32_t dp_key);
+void mac_cache_thresholds_sync(struct mac_cache_data *data,
+ const struct hmap *local_datapaths);
void mac_cache_thresholds_clear(struct mac_cache_data *data);
/* MAC binding. */
@@ -3309,8 +3309,7 @@ mac_binding_remove_sb(struct mac_cache_data *data,
}
static void
-fdb_add_sb(struct mac_cache_data *data, const struct sbrec_fdb *sfdb,
- struct uuid dp_uuid)
+fdb_add_sb(struct mac_cache_data *data, const struct sbrec_fdb *sfdb)
{
struct fdb_data fdb_data;
if (!fdb_data_from_sbrec(&fdb_data, sfdb)) {
@@ -3320,7 +3319,6 @@ fdb_add_sb(struct mac_cache_data *data, const struct sbrec_fdb *sfdb,
struct fdb *fdb = fdb_add(&data->fdbs, fdb_data);
fdb->sbrec_fdb = sfdb;
- fdb->dp_uuid = dp_uuid;
}
static void
@@ -3345,8 +3343,7 @@ mac_cache_mb_handle_for_datapath(struct mac_cache_data *data,
struct ovsdb_idl_index *sbrec_mb_by_dp,
struct ovsdb_idl_index *sbrec_pb_by_name)
{
- bool has_threshold =
- mac_cache_threshold_replace(data, dp, MAC_CACHE_MAC_BINDING);
+ bool has_threshold = mac_cache_threshold_find(data, dp->tunnel_key);
struct sbrec_mac_binding *mb_index_row =
sbrec_mac_binding_index_init_row(sbrec_mb_by_dp);
@@ -3369,7 +3366,7 @@ mac_cache_fdb_handle_for_datapath(struct mac_cache_data *data,
const struct sbrec_datapath_binding *dp,
struct ovsdb_idl_index *sbrec_fdb_by_dp_key)
{
- bool has_threshold = mac_cache_threshold_replace(data, dp, MAC_CACHE_FDB);
+ bool has_threshold = mac_cache_threshold_find(data, dp->tunnel_key);
struct sbrec_fdb *fdb_index_row =
sbrec_fdb_index_init_row(sbrec_fdb_by_dp_key);
@@ -3378,7 +3375,7 @@ mac_cache_fdb_handle_for_datapath(struct mac_cache_data *data,
const struct sbrec_fdb *fdb;
SBREC_FDB_FOR_EACH_EQUAL (fdb, fdb_index_row, sbrec_fdb_by_dp_key) {
if (has_threshold) {
- fdb_add_sb(data, fdb, dp->header_.uuid);
+ fdb_add_sb(data, fdb);
} else {
fdb_remove_sb(data, fdb);
}
@@ -3393,9 +3390,7 @@ en_mac_cache_init(struct engine_node *node OVS_UNUSED,
{
struct mac_cache_data *cache_data = xzalloc(sizeof *cache_data);
- for (size_t i = 0; i < MAC_CACHE_MAX; i++) {
- hmap_init(&cache_data->thresholds[i]);
- }
+ hmap_init(&cache_data->thresholds);
hmap_init(&cache_data->mac_bindings);
hmap_init(&cache_data->fdbs);
@@ -3408,47 +3403,37 @@ en_mac_cache_run(struct engine_node *node, void *data)
struct mac_cache_data *cache_data = data;
struct ed_type_runtime_data *rt_data =
engine_get_input_data("runtime_data", node);
- const struct sbrec_mac_binding_table *mb_table =
- EN_OVSDB_GET(engine_get_input("SB_mac_binding", node));
- const struct sbrec_fdb_table *fdb_table =
- EN_OVSDB_GET(engine_get_input("SB_fdb", node));
+ const struct sbrec_datapath_binding_table *dp_table =
+ EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node));
+ struct ovsdb_idl_index *sbrec_mb_by_dp =
+ engine_ovsdb_node_get_index(
+ engine_get_input("SB_mac_binding", node),
+ "datapath");
struct ovsdb_idl_index *sbrec_pb_by_name =
engine_ovsdb_node_get_index(
engine_get_input("SB_port_binding", node),
"name");
+ struct ovsdb_idl_index *sbrec_fdb_by_dp_key =
+ engine_ovsdb_node_get_index(
+ engine_get_input("SB_fdb", node),
+ "dp_key");
mac_cache_thresholds_clear(cache_data);
mac_bindings_clear(&cache_data->mac_bindings);
fdbs_clear(&cache_data->fdbs);
- const struct sbrec_mac_binding *sbrec_mb;
- SBREC_MAC_BINDING_TABLE_FOR_EACH (sbrec_mb, mb_table) {
+ const struct sbrec_datapath_binding *sbrec_dp;
+ SBREC_DATAPATH_BINDING_TABLE_FOR_EACH (sbrec_dp, dp_table) {
if (!get_local_datapath(&rt_data->local_datapaths,
- sbrec_mb->datapath->tunnel_key)) {
- continue;
- }
-
- if (mac_cache_threshold_add(cache_data, sbrec_mb->datapath,
- MAC_CACHE_MAC_BINDING)) {
- mac_binding_add_sb(cache_data, sbrec_mb,
- sbrec_pb_by_name);
- }
- }
-
- struct local_datapath *local_dp;
- const struct sbrec_fdb *sbrec_fdb;
- SBREC_FDB_TABLE_FOR_EACH (sbrec_fdb, fdb_table) {
- local_dp = get_local_datapath(&rt_data->local_datapaths,
- sbrec_fdb->dp_key);
- if (!local_dp) {
+ sbrec_dp->tunnel_key)) {
continue;
}
- if (mac_cache_threshold_add(cache_data, local_dp->datapath,
- MAC_CACHE_FDB)) {
- fdb_add_sb(cache_data, sbrec_fdb,
- local_dp->datapath->header_.uuid);
- }
+ mac_cache_threshold_add(cache_data, sbrec_dp);
+ mac_cache_mb_handle_for_datapath(cache_data, sbrec_dp,
+ sbrec_mb_by_dp, sbrec_pb_by_name);
+ mac_cache_fdb_handle_for_datapath(cache_data, sbrec_dp,
+ sbrec_fdb_by_dp_key);
}
engine_set_node_state(node, EN_UPDATED);
@@ -3486,10 +3471,9 @@ mac_cache_sb_mac_binding_handler(struct engine_node *node, void *data)
continue;
}
- if (mac_cache_threshold_add(cache_data, sbrec_mb->datapath,
- MAC_CACHE_MAC_BINDING)) {
- mac_binding_add_sb(cache_data, sbrec_mb,
- sbrec_pb_by_name);
+ if (mac_cache_threshold_find(cache_data,
+ sbrec_mb->datapath->tunnel_key)) {
+ mac_binding_add_sb(cache_data, sbrec_mb, sbrec_pb_by_name);
}
}
@@ -3528,10 +3512,8 @@ mac_cache_sb_fdb_handler(struct engine_node *node, void *data)
continue;
}
- if (mac_cache_threshold_add(cache_data, local_dp->datapath,
- MAC_CACHE_FDB)) {
- fdb_add_sb(cache_data, sbrec_fdb,
- local_dp->datapath->header_.uuid);
+ if (mac_cache_threshold_find(cache_data, sbrec_fdb->dp_key)) {
+ fdb_add_sb(cache_data, sbrec_fdb);
}
}
@@ -3571,10 +3553,14 @@ mac_cache_runtime_data_handler(struct engine_node *node, void *data OVS_UNUSED)
struct tracked_datapath *tdp;
HMAP_FOR_EACH (tdp, node, &rt_data->tracked_dp_bindings) {
- if (tdp->tracked_type != TRACKED_RESOURCE_NEW) {
+ if (tdp->tracked_type == TRACKED_RESOURCE_UPDATED) {
continue;
}
+ mac_cache_threshold_replace(cache_data, tdp->dp,
+ &rt_data->local_datapaths);
+ }
+ HMAP_FOR_EACH (tdp, node, &rt_data->tracked_dp_bindings) {
mac_cache_mb_handle_for_datapath(cache_data, tdp->dp,
sbrec_mb_by_dp, sbrec_pb_by_name);
@@ -3613,16 +3599,25 @@ mac_cache_sb_datapath_binding_handler(struct engine_node *node, void *data)
size_t previous_mb_size = hmap_count(&cache_data->mac_bindings);
size_t previous_fdb_size = hmap_count(&cache_data->fdbs);
+ bool sync_needed = false;
const struct sbrec_datapath_binding *sbrec_dp;
- SBREC_DATAPATH_BINDING_TABLE_FOR_EACH (sbrec_dp, dp_table) {
- if (sbrec_datapath_binding_is_new(sbrec_dp) ||
- sbrec_datapath_binding_is_deleted(sbrec_dp) ||
- !get_local_datapath(&rt_data->local_datapaths,
- sbrec_dp->tunnel_key)) {
- continue;
+ SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (sbrec_dp, dp_table) {
+ if (!sbrec_datapath_binding_is_new(sbrec_dp) &&
+ sbrec_datapath_binding_is_updated(
+ sbrec_dp, SBREC_DATAPATH_BINDING_COL_TUNNEL_KEY)) {
+ sync_needed = true;
}
+ mac_cache_threshold_replace(cache_data, sbrec_dp,
+ &rt_data->local_datapaths);
+ }
+
+ if (sync_needed) {
+ mac_cache_thresholds_sync(cache_data, &rt_data->local_datapaths);
+ }
+
+ SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (sbrec_dp, dp_table) {
mac_cache_mb_handle_for_datapath(cache_data, sbrec_dp,
sbrec_mb_by_dp, sbrec_pb_by_name);
@@ -3645,9 +3640,7 @@ en_mac_cache_cleanup(void *data)
struct mac_cache_data *cache_data = data;
mac_cache_thresholds_clear(cache_data);
- for (size_t i = 0; i < MAC_CACHE_MAX; i++) {
- hmap_destroy(&cache_data->thresholds[i]);
- }
+ hmap_destroy(&cache_data->thresholds);
mac_bindings_clear(&cache_data->mac_bindings);
hmap_destroy(&cache_data->mac_bindings);
fdbs_clear(&cache_data->fdbs);
@@ -34490,7 +34490,7 @@ OVS_CTL_TIMEOUT=10
OVS_WAIT_UNTIL([
test "$timestamp" != "$(fetch_column mac_binding timestamp ip='192.168.10.20')"
])
-check $(test "$(fetch_column mac_binding timestamp ip='192.168.10.20')" != "")
+check test "$(fetch_column mac_binding timestamp ip='192.168.10.20')" != ""
# Check if the records are removed after some inactivity
OVS_WAIT_UNTIL([
@@ -36395,7 +36395,7 @@ OVS_WAIT_UNTIL([
test "$timestamp" != "$curr_timestamp"
])
timestamp=$(fetch_column fdb timestamp mac='"00:00:00:00:10:20"')
-check $(test "$timestamp" != "")
+check test "$timestamp" != ""
# Check if the records are removed after some inactivity
wait_row_count fdb 0 mac='"00:00:00:00:10:20"'
Use datapath tunnel key instead of UUID for the mac cache threshold handling. At the same time simplify the thresholds into single hmap. The tunnel key is unique so there shouldn't be any overlap. Having two thresholds per datapath is currently invalid configuration anyway. The switch to datapath's tunnel key requires somehow costly synchronization when the tunnel key changes. However this is fine as the key shouldn't change very often in some cases it won't change at all. Also fix wrong check in the aging tests that would ignore failure. Signed-off-by: Ales Musil <amusil@redhat.com> --- controller/mac-cache.c | 132 ++++++++++++++++++------------------ controller/mac-cache.h | 29 ++++---- controller/ovn-controller.c | 105 +++++++++++++--------------- tests/ovn.at | 4 +- 4 files changed, 128 insertions(+), 142 deletions(-)