[ovs-dev] [PATCH ovn v12 3/7] ovn-controller: Handle runtime data changes in flow output engine
numans at ovn.org
numans at ovn.org
Thu Jun 11 12:44:00 UTC 2020
From: Numan Siddique <numans at ovn.org>
In order to handle runtime data changes incrementally, the flow outut
runtime data handle should know the changed runtime data.
Runtime data now tracks the changed data for any OVS interface
and SB port binding changes. The tracked data contains a hmap
of tracked datapaths (which changed during runtime data processing.
The flow outout runtime_data handler in this patch doesn't do much
with the tracked data. It returns false if there is tracked data available
so that flow_output run is called. If no tracked data is available
then there is no need for flow computation and the handler returns true.
Next patch in the series processes the tracked data incrementally.
Co-Authored-by: Venkata Anil <anilvenkata at redhat.com>
Signed-off-by: Venkata Anil <anilvenkata at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
---
controller/binding.c | 198 ++++++++++++++++++++++++++++++------
controller/binding.h | 23 +++++
controller/ovn-controller.c | 143 +++++++++++++++++++++++++-
tests/ovn-performance.at | 20 ++--
4 files changed, 339 insertions(+), 45 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index 67fd2777f..77ed7441a 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -69,13 +69,25 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type);
}
+static struct tracked_binding_datapath *tracked_binding_datapath_create(
+ const struct sbrec_datapath_binding *,
+ bool is_new, struct hmap *tracked_dps);
+static struct tracked_binding_datapath *tracked_binding_datapath_find(
+ struct hmap *, const struct sbrec_datapath_binding *);
+static void tracked_binding_datapath_lport_add(
+ const struct sbrec_port_binding *, struct hmap *tracked_datapaths);
+static void update_lport_tracking(const struct sbrec_port_binding *pb,
+ bool old_claim, bool new_claim,
+ struct hmap *tracked_dp_bindings);
+
static void
add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
struct ovsdb_idl_index *sbrec_port_binding_by_name,
const struct sbrec_datapath_binding *datapath,
bool has_local_l3gateway, int depth,
- struct hmap *local_datapaths)
+ struct hmap *local_datapaths,
+ struct hmap *tracked_datapaths)
{
uint32_t dp_key = datapath->tunnel_key;
struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key);
@@ -92,6 +104,11 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
ld->localnet_port = NULL;
ld->has_local_l3gateway = has_local_l3gateway;
+ if (tracked_datapaths &&
+ !tracked_binding_datapath_find(tracked_datapaths, datapath)) {
+ tracked_binding_datapath_create(datapath, true, tracked_datapaths);
+ }
+
if (depth >= 100) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "datapaths nested too deep");
@@ -124,7 +141,8 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
sbrec_port_binding_by_datapath,
sbrec_port_binding_by_name,
peer->datapath, false,
- depth + 1, local_datapaths);
+ depth + 1, local_datapaths,
+ tracked_datapaths);
}
ld->n_peer_ports++;
if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
@@ -147,12 +165,14 @@ add_local_datapath(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
struct ovsdb_idl_index *sbrec_port_binding_by_name,
const struct sbrec_datapath_binding *datapath,
- bool has_local_l3gateway, struct hmap *local_datapaths)
+ bool has_local_l3gateway, struct hmap *local_datapaths,
+ struct hmap *tracked_datapaths)
{
add_local_datapath__(sbrec_datapath_binding_by_key,
sbrec_port_binding_by_datapath,
sbrec_port_binding_by_name,
- datapath, has_local_l3gateway, 0, local_datapaths);
+ datapath, has_local_l3gateway, 0, local_datapaths,
+ tracked_datapaths);
}
static void
@@ -506,6 +526,11 @@ update_local_lport_ids(struct binding_ctx_out *b_ctx,
pb->datapath->tunnel_key, pb->tunnel_key);
if (sset_add(b_ctx->local_lport_ids, buf) != NULL) {
b_ctx->local_lport_ids_changed = true;
+
+ if (b_ctx->tracked_dp_bindings) {
+ /* Add the 'pb' to the tracked_datapaths. */
+ tracked_binding_datapath_lport_add(pb, b_ctx->tracked_dp_bindings);
+ }
}
}
@@ -521,6 +546,11 @@ remove_local_lport_ids(struct binding_ctx_out *b_ctx,
pb->datapath->tunnel_key, pb->tunnel_key);
if (sset_find_and_delete(b_ctx->local_lport_ids, buf)) {
b_ctx->local_lport_ids_changed = true;
+
+ if (b_ctx->tracked_dp_bindings) {
+ /* Add the 'pb' to the tracked_datapaths. */
+ tracked_binding_datapath_lport_add(pb, b_ctx->tracked_dp_bindings);
+ }
}
}
@@ -669,6 +699,75 @@ is_lport_container(const struct sbrec_port_binding *pb)
return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0];
}
+static struct tracked_binding_datapath *
+tracked_binding_datapath_create(const struct sbrec_datapath_binding *dp,
+ bool is_new,
+ struct hmap *tracked_datapaths)
+{
+ struct tracked_binding_datapath *t_dp = xzalloc(sizeof *t_dp);
+ t_dp->dp = dp;
+ t_dp->is_new = is_new;
+ shash_init(&t_dp->lports);
+ hmap_insert(tracked_datapaths, &t_dp->node, uuid_hash(&dp->header_.uuid));
+ return t_dp;
+}
+
+static struct tracked_binding_datapath *
+tracked_binding_datapath_find(struct hmap *tracked_datapaths,
+ const struct sbrec_datapath_binding *dp)
+{
+ struct tracked_binding_datapath *t_dp;
+ size_t hash = uuid_hash(&dp->header_.uuid);
+ HMAP_FOR_EACH_WITH_HASH (t_dp, node, hash, tracked_datapaths) {
+ if (uuid_equals(&t_dp->dp->header_.uuid, &dp->header_.uuid)) {
+ return t_dp;
+ }
+ }
+
+ return NULL;
+}
+
+static void
+tracked_binding_datapath_lport_add(const struct sbrec_port_binding *pb,
+ struct hmap *tracked_datapaths)
+{
+ if (!tracked_datapaths) {
+ return;
+ }
+
+ struct tracked_binding_datapath *tracked_dp =
+ tracked_binding_datapath_find(tracked_datapaths, pb->datapath);
+ if (!tracked_dp) {
+ tracked_dp = tracked_binding_datapath_create(pb->datapath, false,
+ tracked_datapaths);
+ }
+
+ /* Check if the lport is already present or not.
+ * If it is already present, then just update the 'pb' and 'deleted'
+ * fields. */
+ struct tracked_binding_lport *lport =
+ shash_find_data(&tracked_dp->lports, pb->logical_port);
+
+ if (!lport) {
+ lport = xmalloc(sizeof *lport);
+ shash_add(&tracked_dp->lports, pb->logical_port, lport);
+ }
+
+ lport->pb = pb;
+}
+
+void
+binding_tracked_dp_destroy(struct hmap *tracked_datapaths)
+{
+ struct tracked_binding_datapath *t_dp;
+ HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) {
+ shash_destroy_free_data(&t_dp->lports);
+ free(t_dp);
+ }
+
+ hmap_destroy(tracked_datapaths);
+}
+
/* Corresponds to each Port_Binding.type. */
enum en_lport_type {
LP_UNKNOWN,
@@ -722,7 +821,7 @@ static bool
claim_lport(const struct sbrec_port_binding *pb,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface *iface_rec,
- bool sb_readonly)
+ bool sb_readonly, struct hmap *tracked_datapaths)
{
if (pb->chassis != chassis_rec) {
if (sb_readonly) {
@@ -741,6 +840,10 @@ claim_lport(const struct sbrec_port_binding *pb,
}
sbrec_port_binding_set_chassis(pb, chassis_rec);
+
+ if (tracked_datapaths) {
+ update_lport_tracking(pb, false, true, tracked_datapaths);
+ }
}
/* Check if the port encap binding, if any, has changed */
@@ -758,9 +861,14 @@ claim_lport(const struct sbrec_port_binding *pb,
/* Returns false if lport is not released due to 'sb_readonly'.
* Returns true otherwise.
+ *
+ * This function assumes that the the 'pb' was claimed
+ * earlier i.e port binding's chassis is set to this chassis.
+ * Caller should make sure that this is the case.
*/
static bool
-release_lport(const struct sbrec_port_binding *pb, bool sb_readonly)
+release_lport(const struct sbrec_port_binding *pb, bool sb_readonly,
+ struct hmap *tracked_datapaths)
{
if (pb->encap) {
if (sb_readonly) {
@@ -783,6 +891,7 @@ release_lport(const struct sbrec_port_binding *pb, bool sb_readonly)
sbrec_port_binding_set_virtual_parent(pb, NULL);
}
+ update_lport_tracking(pb, true, false, tracked_datapaths);
VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port);
return true;
}
@@ -828,13 +937,14 @@ is_lbinding_container_parent(struct local_binding *lbinding)
static bool
release_local_binding_children(const struct sbrec_chassis *chassis_rec,
struct local_binding *lbinding,
- bool sb_readonly)
+ bool sb_readonly,
+ struct hmap *tracked_dp_bindings)
{
struct shash_node *node;
SHASH_FOR_EACH (node, &lbinding->children) {
struct local_binding *l = node->data;
if (is_lbinding_this_chassis(l, chassis_rec)) {
- if (!release_lport(l->pb, sb_readonly)) {
+ if (!release_lport(l->pb, sb_readonly, tracked_dp_bindings)) {
return false;
}
}
@@ -849,16 +959,17 @@ release_local_binding_children(const struct sbrec_chassis *chassis_rec,
static bool
release_local_binding(const struct sbrec_chassis *chassis_rec,
- struct local_binding *lbinding, bool sb_readonly)
+ struct local_binding *lbinding, bool sb_readonly,
+ struct hmap *tracked_dp_bindings)
{
if (!release_local_binding_children(chassis_rec, lbinding,
- sb_readonly)) {
+ sb_readonly, tracked_dp_bindings)) {
return false;
}
bool retval = true;
if (is_lbinding_this_chassis(lbinding, chassis_rec)) {
- retval = release_lport(lbinding->pb, sb_readonly);
+ retval = release_lport(lbinding->pb, sb_readonly, tracked_dp_bindings);
}
lbinding->pb = NULL;
@@ -879,7 +990,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
if (can_bind) {
/* We can claim the lport. */
if (!claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface,
- !b_ctx_in->ovnsb_idl_txn)){
+ !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings)){
return false;
}
@@ -887,7 +999,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
b_ctx_in->sbrec_port_binding_by_datapath,
b_ctx_in->sbrec_port_binding_by_name,
pb->datapath, false,
- b_ctx_out->local_datapaths);
+ b_ctx_out->local_datapaths,
+ b_ctx_out->tracked_dp_bindings);
update_local_lport_ids(b_ctx_out, pb);
if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) {
get_qos_params(pb, qos_map);
@@ -907,7 +1020,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
if (pb->chassis == b_ctx_in->chassis_rec) {
/* Release the lport if there is no lbinding. */
if (!lbinding_set || !can_bind) {
- return release_lport(pb, !b_ctx_in->ovnsb_idl_txn);
+ return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings);
}
}
@@ -994,7 +1108,8 @@ consider_container_lport(const struct sbrec_port_binding *pb,
* release the container lport, if it was bound earlier. */
if (is_lbinding_this_chassis(container_lbinding,
b_ctx_in->chassis_rec)) {
- return release_lport(pb, !b_ctx_in->ovnsb_idl_txn);
+ return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings);
}
return true;
@@ -1074,13 +1189,16 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb,
b_ctx_in->sbrec_port_binding_by_datapath,
b_ctx_in->sbrec_port_binding_by_name,
pb->datapath, has_local_l3gateway,
- b_ctx_out->local_datapaths);
+ b_ctx_out->local_datapaths,
+ b_ctx_out->tracked_dp_bindings);
update_local_lport_ids(b_ctx_out, pb);
return claim_lport(pb, b_ctx_in->chassis_rec, NULL,
- !b_ctx_in->ovnsb_idl_txn);
+ !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings);
} else if (pb->chassis == b_ctx_in->chassis_rec) {
- return release_lport(pb, !b_ctx_in->ovnsb_idl_txn);
+ return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings);
}
return true;
@@ -1116,7 +1234,7 @@ consider_localnet_lport(const struct sbrec_port_binding *pb,
struct binding_ctx_out *b_ctx_out,
struct hmap *qos_map)
{
- /* Add all localnet ports to local_lports so that we allocate ct zones
+ /* Add all localnet ports to local_ifaces so that we allocate ct zones
* for them. */
update_local_lports(b_ctx_out, pb->logical_port);
@@ -1152,7 +1270,9 @@ consider_ha_lport(const struct sbrec_port_binding *pb,
b_ctx_in->sbrec_port_binding_by_datapath,
b_ctx_in->sbrec_port_binding_by_name,
pb->datapath, false,
- b_ctx_out->local_datapaths);
+ b_ctx_out->local_datapaths,
+ b_ctx_out->tracked_dp_bindings);
+ update_local_lport_ids(b_ctx_out, pb);
}
return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out);
@@ -1442,7 +1562,8 @@ add_local_datapath_peer_port(const struct sbrec_port_binding *pb,
b_ctx_in->sbrec_port_binding_by_datapath,
b_ctx_in->sbrec_port_binding_by_name,
peer->datapath, false,
- 1, b_ctx_out->local_datapaths);
+ 1, b_ctx_out->local_datapaths,
+ b_ctx_out->tracked_dp_bindings);
return;
}
@@ -1522,6 +1643,18 @@ remove_pb_from_local_datapath(const struct sbrec_port_binding *pb,
}
}
+static void
+update_lport_tracking(const struct sbrec_port_binding *pb,
+ bool old_claim, bool new_claim,
+ struct hmap *tracked_dp_bindings)
+{
+ if (!tracked_dp_bindings || (old_claim == new_claim)) {
+ return;
+ }
+
+ tracked_binding_datapath_lport_add(pb, tracked_dp_bindings);
+}
+
/* Considers the ovs iface 'iface_rec' for claiming.
* This function should be called if the external_ids:iface-id
* and 'ofport' are set for the 'iface_rec'.
@@ -1618,7 +1751,8 @@ consider_iface_release(const struct ovsrec_interface *iface_rec,
* lbinding->iface.
* Cannot access these members of lbinding after this call. */
if (!release_local_binding(b_ctx_in->chassis_rec, lbinding,
- !b_ctx_in->ovnsb_idl_txn)) {
+ !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings)) {
return false;
}
}
@@ -1831,9 +1965,10 @@ handle_deleted_vif_lport(const struct sbrec_port_binding *pb,
* clear the 'chassis' column of 'pb'. But we need to do
* for the local_binding's children. */
if (lbinding->type == BT_VIF &&
- !release_local_binding_children(b_ctx_in->chassis_rec,
- lbinding,
- !b_ctx_in->ovnsb_idl_txn)) {
+ !release_local_binding_children(
+ b_ctx_in->chassis_rec, lbinding,
+ !b_ctx_in->ovnsb_idl_txn,
+ b_ctx_out->tracked_dp_bindings)) {
return false;
}
}
@@ -1906,7 +2041,6 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
struct binding_ctx_out *b_ctx_out)
{
bool handled = true;
-
const struct sbrec_port_binding *pb;
/* Run the tracked port binding loop twice. One to handle deleted
@@ -1963,7 +2097,6 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
case LP_PATCH:
case LP_LOCALPORT:
case LP_VTEP:
- b_ctx_out->non_vif_ports_changed = true;
update_local_lport_ids(b_ctx_out, pb);
if (lport_type == LP_PATCH) {
/* Add the peer datapath to the local datapaths if it's
@@ -1973,30 +2106,31 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld);
}
}
+
+ if (lport_type == LP_VTEP) {
+ /* VTEP lports are claimed/released by ovn-controller-vteps.
+ * We are not sure what changed. */
+ b_ctx_out->non_vif_ports_changed = true;
+ }
break;
case LP_L2GATEWAY:
- b_ctx_out->non_vif_ports_changed = true;
handled = consider_l2gw_lport(pb, b_ctx_in, b_ctx_out);
break;
case LP_L3GATEWAY:
- b_ctx_out->non_vif_ports_changed = true;
handled = consider_l3gw_lport(pb, b_ctx_in, b_ctx_out);
break;
case LP_CHASSISREDIRECT:
- b_ctx_out->non_vif_ports_changed = true;
handled = consider_cr_lport(pb, b_ctx_in, b_ctx_out);
break;
case LP_EXTERNAL:
- b_ctx_out->non_vif_ports_changed = true;
handled = consider_external_lport(pb, b_ctx_in, b_ctx_out);
break;
case LP_LOCALNET: {
- b_ctx_out->non_vif_ports_changed = true;
consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr);
struct shash bridge_mappings =
diff --git a/controller/binding.h b/controller/binding.h
index 161766d2f..c9740560f 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -19,6 +19,9 @@
#include <stdbool.h>
#include "openvswitch/shash.h"
+#include "openvswitch/hmap.h"
+#include "openvswitch/uuid.h"
+#include "openvswitch/list.h"
struct hmap;
struct ovsdb_idl;
@@ -75,6 +78,12 @@ struct binding_ctx_out {
/* smap of OVS interface name as key and
* OVS interface external_ids:iface-id as value. */
struct smap *local_iface_ids;
+
+ /* hmap of 'struct tracked_binding_datapath' which the
+ * callee (binding_handle_ovs_interface_changes and
+ * binding_handle_port_binding_changes) fills in for
+ * the changed datapaths and port bindings. */
+ struct hmap *tracked_dp_bindings;
};
enum local_binding_type {
@@ -99,6 +108,19 @@ local_binding_find(struct shash *local_bindings, const char *name)
return shash_find_data(local_bindings, name);
}
+/* Represents a tracked binding logical port. */
+struct tracked_binding_lport {
+ const struct sbrec_port_binding *pb;
+};
+
+/* Represent a tracked binding datapath. */
+struct tracked_binding_datapath {
+ struct hmap_node node;
+ const struct sbrec_datapath_binding *dp;
+ bool is_new;
+ struct shash lports; /* shash of struct tracked_binding_lport. */
+};
+
void binding_register_ovs_idl(struct ovsdb_idl *);
void binding_run(struct binding_ctx_in *, struct binding_ctx_out *);
bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
@@ -111,4 +133,5 @@ bool binding_handle_ovs_interface_changes(struct binding_ctx_in *,
struct binding_ctx_out *);
bool binding_handle_port_binding_changes(struct binding_ctx_in *,
struct binding_ctx_out *);
+void binding_tracked_dp_destroy(struct hmap *tracked_datapaths);
#endif /* controller/binding.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6e66705da..351fa32db 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -980,10 +980,88 @@ struct ed_type_runtime_data {
struct sset local_lport_ids;
struct sset active_tunnels;
+ /* runtime data engine private data. */
struct sset egress_ifaces;
struct smap local_iface_ids;
+
+ /* Tracked data. See below for more details and comments. */
+ bool tracked;
+ bool local_lports_changed;
+ struct hmap tracked_dp_bindings;
};
+/* struct ed_type_runtime_data has the below members for tracking the
+ * changes done to the runtime_data engine by the runtime_data engine
+ * handlers. Since this engine is an input to the flow_output engine,
+ * the flow output runtime data handler will make use of this tracked data.
+ *
+ * ------------------------------------------------------------------------
+ * | | This is a hmap of |
+ * | | 'struct tracked_binding_datapath' defined in |
+ * | | binding.h. Runtime data handlers for OVS |
+ * | | Interface and Port Binding changes store the |
+ * | @tracked_dp_bindings | changed datapaths (datapaths added/removed from |
+ * | | local_datapaths) and changed port bindings |
+ * | | (added/updated/deleted in 'local_bindings'). |
+ * | | So any changes to the runtime data - |
+ * | | local_datapaths and local_bindings is captured |
+ * | | here. |
+ * ------------------------------------------------------------------------
+ * | | This is a bool which represents if the runtime |
+ * | | data 'local_lports' changed by the runtime data |
+ * | | handlers for OVS Interface and Port Binding |
+ * | | changes. If 'local_lports' is updated and also |
+ * | | results in any port binding updates, it is |
+ * |@local_lports_changed | captured in the @tracked_dp_bindings. So there |
+ * | | is no need to capture the changes in the |
+ * | | local_lports. If @local_lports_changed is true |
+ * | | but without anydata in the @tracked_dp_bindings,|
+ * | | it means we needto only update the SB monitor |
+ * | | clauses and there isno need for any flow |
+ * | | (re)computations. |
+ * ------------------------------------------------------------------------
+ * | | This represents if the data was tracked or not |
+ * | | by the runtime data handlers during the engine |
+ * | @tracked | run. If the runtime data recompute is |
+ * | | triggered, it means there is no tracked data. |
+ * ------------------------------------------------------------------------
+ *
+ *
+ * The changes to the following runtime_data variables are not tracked.
+ *
+ * ---------------------------------------------------------------------
+ * | local_datapaths | The changes to these runtime data is captured in |
+ * | local_bindings | the @tracked_dp_bindings indirectly and hence it |
+ * | local_lport_ids | is not tracked explicitly. |
+ * ---------------------------------------------------------------------
+ * | local_iface_ids | This is used internally within the runtime data |
+ * | egress_ifaces | engine (used only in binding.c) and hence there |
+ * | | there is no need to track. |
+ * ---------------------------------------------------------------------
+ * | | Active tunnels is built in the |
+ * | | bfd_calculate_active_tunnels() for the tunnel |
+ * | | OVS interfaces. Any changes to non VIF OVS |
+ * | | interfaces results in triggering the full |
+ * | active_tunnels | recompute of runtime data engine and hence there |
+ * | | the tracked data doesn't track it. When we |
+ * | | support handling changes to non VIF OVS |
+ * | | interfaces we need to track the changes to the |
+ * | | active tunnels. |
+ * ---------------------------------------------------------------------
+ *
+ */
+
+static void
+en_runtime_data_clear_tracked_data(void *data_)
+{
+ struct ed_type_runtime_data *data = data_;
+
+ binding_tracked_dp_destroy(&data->tracked_dp_bindings);
+ hmap_init(&data->tracked_dp_bindings);
+ data->local_lports_changed = false;
+ data->tracked = false;
+}
+
static void *
en_runtime_data_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
@@ -997,6 +1075,10 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED,
sset_init(&data->egress_ifaces);
smap_init(&data->local_iface_ids);
local_bindings_init(&data->local_bindings);
+
+ /* init the tracked data. */
+ hmap_init(&data->tracked_dp_bindings);
+
return data;
}
@@ -1019,6 +1101,8 @@ en_runtime_data_cleanup(void *data)
}
hmap_destroy(&rt_data->local_datapaths);
local_bindings_destroy(&rt_data->local_bindings);
+
+ en_runtime_data_clear_tracked_data(data);
}
static void
@@ -1102,6 +1186,8 @@ init_binding_ctx(struct engine_node *node,
b_ctx_out->egress_ifaces = &rt_data->egress_ifaces;
b_ctx_out->local_bindings = &rt_data->local_bindings;
b_ctx_out->local_iface_ids = &rt_data->local_iface_ids;
+ b_ctx_out->tracked_dp_bindings = NULL;
+ b_ctx_out->local_lports_changed = NULL;
}
static void
@@ -1113,6 +1199,23 @@ en_runtime_data_run(struct engine_node *node, void *data)
struct sset *local_lport_ids = &rt_data->local_lport_ids;
struct sset *active_tunnels = &rt_data->active_tunnels;
+ /* Clear the (stale) tracked data if any. Even though the tracked data
+ * gets cleared in the beginning of engine_init_run(),
+ * any of the runtime data handler might have set some tracked
+ * data and later another runtime data handler might return false
+ * resulting in full recompute of runtime engine and rendering the tracked
+ * data stale.
+ *
+ * It's possible that engine framework can be enhanced to indicate
+ * the node handlers (in this case flow_output_runtime_data_handler)
+ * that its input node had a full recompute. However we would still
+ * need to clear the tracked data, because we don't want the
+ * stale tracked data to be accessed outside of the engine, since the
+ * tracked data is cleared in the engine_init_run() and not at the
+ * end of the engine run.
+ * */
+ en_runtime_data_clear_tracked_data(data);
+
static bool first_run = true;
if (first_run) {
/* don't cleanup since there is no data yet */
@@ -1168,6 +1271,8 @@ runtime_data_ovs_interface_handler(struct engine_node *node, void *data)
struct binding_ctx_in b_ctx_in;
struct binding_ctx_out b_ctx_out;
init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
+ rt_data->tracked = true;
+ b_ctx_out.tracked_dp_bindings = &rt_data->tracked_dp_bindings;
if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out)) {
return false;
@@ -1175,6 +1280,7 @@ runtime_data_ovs_interface_handler(struct engine_node *node, void *data)
if (b_ctx_out.local_lports_changed) {
engine_set_node_state(node, EN_UPDATED);
+ rt_data->local_lports_changed = b_ctx_out.local_lports_changed;
}
return true;
@@ -1191,12 +1297,16 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
return false;
}
+ rt_data->tracked = true;
+ b_ctx_out.tracked_dp_bindings = &rt_data->tracked_dp_bindings;
+
if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out)) {
return false;
}
if (b_ctx_out.local_lport_ids_changed ||
- b_ctx_out.non_vif_ports_changed) {
+ b_ctx_out.non_vif_ports_changed ||
+ !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
engine_set_node_state(node, EN_UPDATED);
}
@@ -1796,6 +1906,32 @@ flow_output_port_groups_handler(struct engine_node *node, void *data)
return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP);
}
+static bool
+flow_output_runtime_data_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+ struct ed_type_runtime_data *rt_data =
+ engine_get_input_data("runtime_data", node);
+
+ /* There is no tracked data. Fall back to full recompute of
+ * flow_output. */
+ if (!rt_data->tracked) {
+ return false;
+ }
+
+ if (!hmap_is_empty(&rt_data->tracked_dp_bindings)) {
+ /* We are not yet handling the tracked datapath binding
+ * changes. Return false to trigger full recompute. */
+ return false;
+ }
+
+ if (rt_data->local_lports_changed) {
+ engine_set_node_state(node, EN_UPDATED);
+ }
+
+ return true;
+}
+
/* Engine node en_physical_flow_changes indicates whether
* there is a need to
* - recompute only physical flows or
@@ -2029,7 +2165,7 @@ main(int argc, char *argv[])
/* Define inc-proc-engine nodes. */
ENGINE_NODE_CUSTOM_DATA(ct_zones, "ct_zones");
- ENGINE_NODE(runtime_data, "runtime_data");
+ ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data");
ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve");
ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected");
ENGINE_NODE_WITH_CLEAR_TRACK_DATA(physical_flow_changes,
@@ -2066,7 +2202,8 @@ main(int argc, char *argv[])
flow_output_addr_sets_handler);
engine_add_input(&en_flow_output, &en_port_groups,
flow_output_port_groups_handler);
- engine_add_input(&en_flow_output, &en_runtime_data, NULL);
+ engine_add_input(&en_flow_output, &en_runtime_data,
+ flow_output_runtime_data_handler);
engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL);
engine_add_input(&en_flow_output, &en_physical_flow_changes,
flow_output_physical_flow_changes_handler);
diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at
index 5cc1960b6..08acd3bae 100644
--- a/tests/ovn-performance.at
+++ b/tests/ovn-performance.at
@@ -286,11 +286,11 @@ for i in 1 2; do
[hv1 hv2], [lflow_run],
[ovn-nbctl --wait=hv lsp-set-type $lsp router]
)
- OVN_CONTROLLER_EXPECT_HIT(
+ OVN_CONTROLLER_EXPECT_NO_HIT(
[hv1 hv2], [lflow_run],
[ovn-nbctl --wait=hv lsp-set-options $lsp router-port=$lrp]
)
- OVN_CONTROLLER_EXPECT_HIT(
+ OVN_CONTROLLER_EXPECT_NO_HIT(
[hv1 hv2], [lflow_run],
[ovn-nbctl --wait=hv lsp-set-addresses $lsp router]
)
@@ -416,15 +416,15 @@ OVN_CONTROLLER_EXPECT_NO_HIT(
[ovn-nbctl --wait=hv destroy Port_Group pg1]
)
-for i in 1 2; do
- ls=ls$i
+OVN_CONTROLLER_EXPECT_HIT(
+ [hv1 hv2], [lflow_run],
+ [ovn-nbctl --wait=hv ls-del ls1]
+)
- # Delete switch $ls
- OVN_CONTROLLER_EXPECT_HIT(
- [hv1 hv2], [lflow_run],
- [ovn-nbctl --wait=hv ls-del $ls]
- )
-done
+OVN_CONTROLLER_EXPECT_NO_HIT(
+ [hv1 hv2], [lflow_run],
+ [ovn-nbctl --wait=hv ls-del ls2]
+)
# Delete router lr1
OVN_CONTROLLER_EXPECT_NO_HIT(
--
2.26.2
More information about the dev
mailing list