[ovs-dev] [PATCH ovn v5 5/9] ovn-controller: I-P for ct zone and OVS interface changes in flow output stage.
numans at ovn.org
numans at ovn.org
Mon May 11 09:46:19 UTC 2020
From: Numan Siddique <numans at ovn.org>
This patch handles ct zone changes and OVS interface changes incrementally
in the flow output stage.
Any changes to ct zone can be handled by running physical_run() instead of running
flow_output_run(). And any changes to OVS interfaces can be either handled
incrementally (for OVS interfaces representing VIFs) or just running
physical_run() (for tunnel and other types of interfaces).
To better handle this, a new engine node 'physical_flow_changes' is added which
handles changes to ct zone and OVS interfaces.
Signed-off-by: Numan Siddique <numans at ovn.org>
---
controller/binding.c | 22 ---------
controller/lflow.c | 13 ++++++
controller/lflow.h | 2 +
controller/ovn-controller.c | 92 ++++++++++++++++++++++++++++++++++++-
controller/ovn-controller.h | 22 +++++++++
controller/physical.c | 51 ++++++++++++++++++++
controller/physical.h | 5 +-
7 files changed, 182 insertions(+), 25 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index 662424518..3821edc03 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -499,22 +499,6 @@ remove_local_lport_ids(const struct sbrec_port_binding *binding_rec,
sset_find_and_delete(local_lport_ids, buf);
}
-enum local_binding_type {
- BT_VIF,
- BT_CHILD,
- BT_VIRTUAL
-};
-
-struct local_binding {
- struct ovs_list node; /* In parent if any. */
- char *name;
- enum local_binding_type type;
- const struct ovsrec_interface *iface;
- const struct sbrec_port_binding *pb;
-
- struct ovs_list children;
-};
-
static struct local_binding *
local_binding_create(const char *name, const struct ovsrec_interface *iface,
const struct sbrec_port_binding *pb,
@@ -535,12 +519,6 @@ local_binding_add(struct shash *local_bindings, struct local_binding *lbinding)
shash_add(local_bindings, lbinding->name, lbinding);
}
-static struct local_binding *
-local_binding_find(struct shash *local_bindings, const char *name)
-{
- return shash_find_data(local_bindings, name);
-}
-
static void
local_binding_destroy(struct local_binding *lbinding)
{
diff --git a/controller/lflow.c b/controller/lflow.c
index 01214a3a6..512258cd3 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -847,3 +847,16 @@ lflow_destroy(void)
expr_symtab_destroy(&symtab);
shash_destroy(&symtab);
}
+
+bool
+lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *pb_table)
+{
+ const struct sbrec_port_binding *binding;
+ SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, pb_table) {
+ if (!strcmp(binding->type, "remote")) {
+ return false;
+ }
+ }
+
+ return true;
+}
diff --git a/controller/lflow.h b/controller/lflow.h
index f02f709d7..fa54d4de2 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -48,6 +48,7 @@ struct sbrec_dhcp_options_table;
struct sbrec_dhcpv6_options_table;
struct sbrec_logical_flow_table;
struct sbrec_mac_binding_table;
+struct sbrec_port_binding_table;
struct simap;
struct sset;
struct uuid;
@@ -153,6 +154,7 @@ void lflow_handle_changed_neighbors(
const struct hmap *local_datapaths,
struct ovn_desired_flow_table *);
+bool lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *);
void lflow_destroy(void);
void lflow_expr_destroy(struct hmap *lflow_expr_cache);
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index f94cd16d1..4eee9a2c9 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1369,8 +1369,13 @@ static void init_physical_ctx(struct engine_node *node,
ovs_assert(br_int && chassis);
+ struct ovsrec_interface_table *iface_table =
+ (struct ovsrec_interface_table *)EN_OVSDB_GET(
+ engine_get_input("OVS_interface",
+ engine_get_input("physical_flow_changes", node)));
struct ed_type_ct_zones *ct_zones_data =
- engine_get_input_data("ct_zones", node);
+ engine_get_input_data("ct_zones",
+ engine_get_input("physical_flow_changes", node));
struct simap *ct_zones = &ct_zones_data->current;
p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name;
@@ -1378,12 +1383,14 @@ static void init_physical_ctx(struct engine_node *node,
p_ctx->mc_group_table = multicast_group_table;
p_ctx->br_int = br_int;
p_ctx->chassis_table = chassis_table;
+ p_ctx->iface_table = iface_table;
p_ctx->chassis = chassis;
p_ctx->active_tunnels = &rt_data->active_tunnels;
p_ctx->local_datapaths = &rt_data->local_datapaths;
p_ctx->local_lports = &rt_data->local_lports;
p_ctx->ct_zones = ct_zones;
p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve;
+ p_ctx->local_bindings = &rt_data->local_bindings;
}
static void init_lflow_ctx(struct engine_node *node,
@@ -1646,6 +1653,8 @@ flow_output_sb_port_binding_handler(struct engine_node *node, void *data)
* always logical router port, and any change to logical router port
* would just trigger recompute.
*
+ * - When a port binding of type 'remote' is updated by ovn-ic.
+ *
* Although there is no correctness issue so far (except the unusual ACL
* use case, which doesn't seem to be a real problem), it might be better
* to handle this more gracefully, without the need to consider these
@@ -1656,6 +1665,14 @@ flow_output_sb_port_binding_handler(struct engine_node *node, void *data)
struct physical_ctx p_ctx;
init_physical_ctx(node, rt_data, &p_ctx);
+ if (!lflow_evaluate_pb_changes(p_ctx.port_binding_table)) {
+ /* If this returns false, it means there is an impact on
+ * the logical flow processing because of some changes in
+ * port bindings. Return false so that recompute is triggered
+ * for this stage. */
+ return false;
+ }
+
physical_handle_port_binding_changes(&p_ctx, flow_table);
engine_set_node_state(node, EN_UPDATED);
@@ -1791,6 +1808,70 @@ flow_output_port_groups_handler(struct engine_node *node, void *data)
return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP);
}
+struct ed_type_physical_flow_changes {
+ bool need_physical_run;
+ bool ovs_ifaces_changed;
+};
+
+static bool
+flow_output_physical_flow_changes_handler(struct engine_node *node, void *data)
+{
+ struct ed_type_physical_flow_changes *pfc =
+ engine_get_input_data("physical_flow_changes", node);
+ struct ed_type_runtime_data *rt_data =
+ engine_get_input_data("runtime_data", node);
+
+ struct ed_type_flow_output *fo = data;
+ struct physical_ctx p_ctx;
+ init_physical_ctx(node, rt_data, &p_ctx);
+
+ engine_set_node_state(node, EN_UPDATED);
+ if (pfc->need_physical_run) {
+ pfc->need_physical_run = false;
+ physical_run(&p_ctx, &fo->flow_table);
+ return true;
+ }
+
+ if (pfc->ovs_ifaces_changed) {
+ pfc->ovs_ifaces_changed = false;
+ return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table);
+ }
+
+ return true;
+}
+
+static void *
+en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct ed_type_physical_flow_changes *data = xzalloc(sizeof *data);
+ return data;
+}
+
+static void
+en_physical_flow_changes_cleanup(void *data OVS_UNUSED)
+{
+
+}
+
+static void
+en_physical_flow_changes_run(struct engine_node *node, void *data OVS_UNUSED)
+{
+ struct ed_type_physical_flow_changes *pfc = data;
+ pfc->need_physical_run = true;
+ engine_set_node_state(node, EN_UPDATED);
+}
+
+static bool
+physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED,
+ void *data OVS_UNUSED)
+{
+ struct ed_type_physical_flow_changes *pfc = data;
+ pfc->ovs_ifaces_changed = true;
+ engine_set_node_state(node, EN_UPDATED);
+ return true;
+}
+
struct ovn_controller_exit_args {
bool *exiting;
bool *restart;
@@ -1913,6 +1994,7 @@ main(int argc, char *argv[])
ENGINE_NODE(runtime_data, "runtime_data");
ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve");
ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected");
+ ENGINE_NODE(physical_flow_changes, "physical_flow_changes");
ENGINE_NODE(flow_output, "flow_output");
ENGINE_NODE(addr_sets, "addr_sets");
ENGINE_NODE(port_groups, "port_groups");
@@ -1932,13 +2014,19 @@ main(int argc, char *argv[])
engine_add_input(&en_port_groups, &en_sb_port_group,
port_groups_sb_port_group_handler);
+ engine_add_input(&en_physical_flow_changes, &en_ct_zones,
+ NULL);
+ engine_add_input(&en_physical_flow_changes, &en_ovs_interface,
+ physical_flow_changes_ovs_iface_handler);
+
engine_add_input(&en_flow_output, &en_addr_sets,
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_ct_zones, NULL);
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);
engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL);
engine_add_input(&en_flow_output, &en_ovs_bridge, NULL);
diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h
index 5d9466880..40c358cef 100644
--- a/controller/ovn-controller.h
+++ b/controller/ovn-controller.h
@@ -72,6 +72,28 @@ struct local_datapath {
struct local_datapath *get_local_datapath(const struct hmap *,
uint32_t tunnel_key);
+enum local_binding_type {
+ BT_VIF,
+ BT_CHILD,
+ BT_VIRTUAL
+};
+
+struct local_binding {
+ struct ovs_list node; /* In parent if any. */
+ char *name;
+ enum local_binding_type type;
+ const struct ovsrec_interface *iface;
+ const struct sbrec_port_binding *pb;
+
+ struct ovs_list children;
+};
+
+static inline struct local_binding *
+local_binding_find(struct shash *local_bindings, const char *name)
+{
+ return shash_find_data(local_bindings, name);
+}
+
const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table *,
const char *br_name);
diff --git a/controller/physical.c b/controller/physical.c
index 144aeb7bd..03eb677d1 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1780,6 +1780,57 @@ physical_run(struct physical_ctx *p_ctx,
simap_destroy(&new_tunnel_to_ofport);
}
+bool
+physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx,
+ struct ovn_desired_flow_table *flow_table)
+{
+ const struct ovsrec_interface *iface_rec;
+ OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) {
+ if (!strcmp(iface_rec->type, "geneve") ||
+ !strcmp(iface_rec->type, "patch")) {
+ return false;
+ }
+ }
+
+ struct ofpbuf ofpacts;
+ ofpbuf_init(&ofpacts, 0);
+
+ OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) {
+ const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id");
+ if (!iface_id) {
+ continue;
+ }
+
+ const struct local_binding *lb =
+ local_binding_find(p_ctx->local_bindings, iface_id);
+
+ if (!lb || !lb->pb) {
+ continue;
+ }
+
+ int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
+ if (ovsrec_interface_is_deleted(iface_rec)) {
+ ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid);
+ simap_find_and_delete(&localvif_to_ofport, iface_id);
+ } else {
+ if (!ovsrec_interface_is_new(iface_rec)) {
+ ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid);
+ }
+
+ simap_put(&localvif_to_ofport, iface_id, ofport);
+ consider_port_binding(p_ctx->sbrec_port_binding_by_name,
+ p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
+ p_ctx->active_tunnels,
+ p_ctx->local_datapaths,
+ lb->pb, p_ctx->chassis,
+ flow_table, &ofpacts);
+ }
+ }
+
+ ofpbuf_uninit(&ofpacts);
+ return true;
+}
+
bool
get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t *ofport)
{
diff --git a/controller/physical.h b/controller/physical.h
index dadf84ea1..9ca34436a 100644
--- a/controller/physical.h
+++ b/controller/physical.h
@@ -49,11 +49,13 @@ struct physical_ctx {
const struct ovsrec_bridge *br_int;
const struct sbrec_chassis_table *chassis_table;
const struct sbrec_chassis *chassis;
+ const struct ovsrec_interface_table *iface_table;
const struct sset *active_tunnels;
struct hmap *local_datapaths;
struct sset *local_lports;
const struct simap *ct_zones;
enum mf_field_id mff_ovn_geneve;
+ struct shash *local_bindings;
};
void physical_register_ovs_idl(struct ovsdb_idl *);
@@ -63,7 +65,8 @@ void physical_handle_port_binding_changes(struct physical_ctx *,
struct ovn_desired_flow_table *);
void physical_handle_mc_group_changes(struct physical_ctx *,
struct ovn_desired_flow_table *);
-
+bool physical_handle_ovs_iface_changes(struct physical_ctx *,
+ struct ovn_desired_flow_table *);
bool get_tunnel_ofport(const char *chassis_name, char *encap_ip,
ofp_port_t *ofport);
#endif /* controller/physical.h */
--
2.26.2
More information about the dev
mailing list