[ovs-dev] [PATCH ovn 2/2] if-status: Add OVS interface status management module.
Dumitru Ceara
dceara at redhat.com
Fri Apr 23 14:17:55 UTC 2021
The initial implementation of the notification mechanism that indicates
if OVS flows corresponding to a logical switch port have been installed
is not resilient enough. It didn't cover the case when transactions to
the local OVS DB or to the Southbound fail.
In order to deal with that, factor out the code that tracks the state
of the interfaces to a new module, 'if-status', and implement it with
a state machine that will retry setting Port_Binding.UP and
OVS.Interface.external_ids:ovn-installed in the Southbound and local
OVS databases.
Having a separate module makes sense because tracking the interface
state doesn't really depend on the rest of the binding module, except
for interacting with the Port_Binding and Interface IDL records. For
this we add some additional APIs to binding.c.
Reported-at: https://bugzilla.redhat.com/1952846
Fixes: 4d3cb42b076b ("binding: Set Logical_Switch_Port.up when all OVS flows are installed.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
controller/automake.mk | 2
controller/binding.c | 302 +++++++++++----------------------
controller/binding.h | 14 +-
controller/if-status.c | 395 +++++++++++++++++++++++++++++++++++++++++++
controller/if-status.h | 37 ++++
controller/ovn-controller.c | 24 ++-
6 files changed, 562 insertions(+), 212 deletions(-)
create mode 100644 controller/if-status.c
create mode 100644 controller/if-status.h
diff --git a/controller/automake.mk b/controller/automake.mk
index e664f1980..2f6c50890 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -10,6 +10,8 @@ controller_ovn_controller_SOURCES = \
controller/encaps.h \
controller/ha-chassis.c \
controller/ha-chassis.h \
+ controller/if-status.c \
+ controller/if-status.h \
controller/ip-mcast.c \
controller/ip-mcast.h \
controller/lflow.c \
diff --git a/controller/binding.c b/controller/binding.c
index 451f00e34..3baf4cf17 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -16,13 +16,14 @@
#include <config.h>
#include "binding.h"
#include "ha-chassis.h"
+#include "if-status.h"
#include "lflow.h"
#include "lport.h"
-#include "ofctrl-seqno.h"
#include "patch.h"
#include "lib/bitmap.h"
#include "openvswitch/poll-loop.h"
+#include "lib/hmapx.h"
#include "lib/sset.h"
#include "lib/util.h"
#include "lib/netdev.h"
@@ -41,32 +42,6 @@ VLOG_DEFINE_THIS_MODULE(binding);
*/
#define OVN_INSTALLED_EXT_ID "ovn-installed"
-/* Set of OVS interface IDs that have been released in the most recent
- * processing iterations. This gets updated in release_lport() and is
- * periodically emptied in binding_seqno_run().
- */
-static struct sset binding_iface_released_set =
- SSET_INITIALIZER(&binding_iface_released_set);
-
-/* Set of OVS interface IDs that have been bound in the most recent
- * processing iterations. This gets updated in release_lport() and is
- * periodically emptied in binding_seqno_run().
- */
-static struct sset binding_iface_bound_set =
- SSET_INITIALIZER(&binding_iface_bound_set);
-
-static void
-binding_iface_released_add(const char *iface_id)
-{
- sset_add(&binding_iface_released_set, iface_id);
-}
-
-static void
-binding_iface_bound_add(const char *iface_id)
-{
- sset_add(&binding_iface_bound_set, iface_id);
-}
-
#define OVN_QOS_TYPE "linux-htb"
struct qos_queue {
@@ -672,7 +647,8 @@ static void local_binding_destroy(struct local_binding *,
struct shash *binding_lports);
static void local_binding_delete(struct local_binding *,
struct shash *local_bindings,
- struct shash *binding_lports);
+ struct shash *binding_lports,
+ struct if_status_mgr *if_mgr);
static struct binding_lport *local_binding_add_lport(
struct shash *binding_lports,
struct local_binding *,
@@ -739,6 +715,80 @@ local_binding_get_primary_pb(struct shash *local_bindings, const char *pb_name)
return b_lport ? b_lport->pb : NULL;
}
+bool
+local_binding_is_up(struct shash *local_bindings, const char *pb_name)
+{
+ struct local_binding *lbinding =
+ local_binding_find(local_bindings, pb_name);
+ struct binding_lport *b_lport = local_binding_get_primary_lport(lbinding);
+ if (lbinding && b_lport && lbinding->iface) {
+ if (b_lport->pb->n_up && !b_lport->pb->up[0]) {
+ return false;
+ }
+ return smap_get_bool(&lbinding->iface->external_ids,
+ OVN_INSTALLED_EXT_ID, false);
+ }
+ return false;
+}
+
+bool
+local_binding_is_down(struct shash *local_bindings, const char *pb_name)
+{
+ struct local_binding *lbinding =
+ local_binding_find(local_bindings, pb_name);
+
+ return !lbinding
+ || !lbinding->iface
+ || !smap_get_bool(&lbinding->iface->external_ids,
+ OVN_INSTALLED_EXT_ID, false);
+}
+
+void
+local_binding_set_up(struct shash *local_bindings, const char *pb_name,
+ bool sb_readonly, bool ovs_readonly)
+{
+ struct local_binding *lbinding =
+ local_binding_find(local_bindings, pb_name);
+ struct binding_lport *b_lport = local_binding_get_primary_lport(lbinding);
+ if (lbinding && b_lport && lbinding->iface) {
+ if (!ovs_readonly && !smap_get_bool(&lbinding->iface->external_ids,
+ OVN_INSTALLED_EXT_ID, false)) {
+ ovsrec_interface_update_external_ids_setkey(lbinding->iface,
+ OVN_INSTALLED_EXT_ID,
+ "true");
+ }
+ if (!sb_readonly && b_lport->pb->n_up) {
+ bool up = true;
+
+ sbrec_port_binding_set_up(b_lport->pb, &up, 1);
+ LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
+ sbrec_port_binding_set_up(b_lport->pb, &up, 1);
+ }
+ }
+ }
+}
+
+void
+local_binding_set_down(struct shash *local_bindings, const char *pb_name,
+ bool sb_readonly, bool ovs_readonly)
+{
+ struct local_binding *lbinding =
+ local_binding_find(local_bindings, pb_name);
+ struct binding_lport *b_lport = local_binding_get_primary_lport(lbinding);
+
+ if (!ovs_readonly && lbinding && lbinding->iface
+ && smap_get_bool(&lbinding->iface->external_ids,
+ OVN_INSTALLED_EXT_ID, false)) {
+ ovsrec_interface_update_external_ids_delkey(lbinding->iface,
+ OVN_INSTALLED_EXT_ID);
+ }
+
+ if (!sb_readonly && b_lport && b_lport->pb->n_up) {
+ bool up = false;
+ sbrec_port_binding_set_up(b_lport->pb, &up, 1);
+ }
+}
+
void
binding_dump_local_bindings(struct local_binding_data *lbinding_data,
struct ds *out_data)
@@ -959,7 +1009,7 @@ static void
claimed_lport_set_up(const struct sbrec_port_binding *pb,
const struct sbrec_port_binding *parent_pb,
const struct sbrec_chassis *chassis_rec,
- bool notify_up)
+ bool notify_up, struct if_status_mgr *if_mgr)
{
if (!notify_up) {
bool up = true;
@@ -970,7 +1020,7 @@ claimed_lport_set_up(const struct sbrec_port_binding *pb,
}
if (pb->chassis != chassis_rec || (pb->n_up && !pb->up[0])) {
- binding_iface_bound_add(pb->logical_port);
+ if_status_mgr_claim_iface(if_mgr, pb->logical_port);
}
}
@@ -983,10 +1033,11 @@ claim_lport(const struct sbrec_port_binding *pb,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface *iface_rec,
bool sb_readonly, bool notify_up,
- struct hmap *tracked_datapaths)
+ struct hmap *tracked_datapaths,
+ struct if_status_mgr *if_mgr)
{
if (!sb_readonly) {
- claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up);
+ claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up, if_mgr);
}
if (pb->chassis != chassis_rec) {
@@ -1034,7 +1085,7 @@ claim_lport(const struct sbrec_port_binding *pb,
*/
static bool
release_lport(const struct sbrec_port_binding *pb, bool sb_readonly,
- struct hmap *tracked_datapaths)
+ struct hmap *tracked_datapaths, struct if_status_mgr *if_mgr)
{
if (pb->encap) {
if (sb_readonly) {
@@ -1057,12 +1108,8 @@ release_lport(const struct sbrec_port_binding *pb, bool sb_readonly,
sbrec_port_binding_set_virtual_parent(pb, NULL);
}
- if (pb->n_up) {
- bool up = false;
- sbrec_port_binding_set_up(pb, &up, 1);
- }
update_lport_tracking(pb, tracked_datapaths);
- binding_iface_released_add(pb->logical_port);
+ if_status_mgr_release_iface(if_mgr, pb->logical_port);
VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port);
return true;
}
@@ -1113,7 +1160,8 @@ release_binding_lport(const struct sbrec_chassis *chassis_rec,
if (is_binding_lport_this_chassis(b_lport, chassis_rec)) {
remove_local_lport_ids(b_lport->pb, b_ctx_out);
if (!release_lport(b_lport->pb, sb_readonly,
- b_ctx_out->tracked_dp_bindings)) {
+ b_ctx_out->tracked_dp_bindings,
+ b_ctx_out->if_mgr)) {
return false;
}
}
@@ -1140,7 +1188,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
if (!claim_lport(pb, parent_pb, b_ctx_in->chassis_rec,
b_lport->lbinding->iface,
!b_ctx_in->ovnsb_idl_txn,
- !parent_pb, b_ctx_out->tracked_dp_bindings)){
+ !parent_pb, b_ctx_out->tracked_dp_bindings,
+ b_ctx_out->if_mgr)){
return false;
}
@@ -1171,7 +1220,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
/* Release the lport if there is no lbinding. */
if (!lbinding_set || !can_bind) {
return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,
- b_ctx_out->tracked_dp_bindings);
+ b_ctx_out->tracked_dp_bindings,
+ b_ctx_out->if_mgr);
}
}
@@ -1269,7 +1319,8 @@ consider_container_lport(const struct sbrec_port_binding *pb,
if (is_binding_lport_this_chassis(container_b_lport,
b_ctx_in->chassis_rec)) {
return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,
- b_ctx_out->tracked_dp_bindings);
+ b_ctx_out->tracked_dp_bindings,
+ b_ctx_out->if_mgr);
}
return true;
@@ -1367,10 +1418,12 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb,
update_local_lport_ids(pb, b_ctx_out);
return claim_lport(pb, NULL, b_ctx_in->chassis_rec, NULL,
!b_ctx_in->ovnsb_idl_txn, false,
- b_ctx_out->tracked_dp_bindings);
+ b_ctx_out->tracked_dp_bindings,
+ b_ctx_out->if_mgr);
} else if (pb->chassis == b_ctx_in->chassis_rec) {
return release_lport(pb, !b_ctx_in->ovnsb_idl_txn,
- b_ctx_out->tracked_dp_bindings);
+ b_ctx_out->tracked_dp_bindings,
+ b_ctx_out->if_mgr);
}
return true;
@@ -1978,7 +2031,8 @@ consider_iface_release(const struct ovsrec_interface *iface_rec,
/* Check if the lbinding has children of type PB_CONTAINER.
* If so, don't delete the local_binding. */
if (lbinding && !is_lbinding_container_parent(lbinding)) {
- local_binding_delete(lbinding, local_bindings, binding_lports);
+ local_binding_delete(lbinding, local_bindings, binding_lports,
+ b_ctx_out->if_mgr);
}
remove_local_lports(iface_id, b_ctx_out);
@@ -2533,160 +2587,6 @@ delete_done:
return handled;
}
-/* Registered ofctrl seqno type for port_binding flow installation. */
-static size_t binding_seq_type_pb_cfg;
-
-/* Binding specific seqno to be acked by ofctrl when flows for new interfaces
- * have been installed.
- */
-static uint32_t binding_iface_seqno = 0;
-
-/* Map indexed by iface-id containing the sequence numbers that when acked
- * indicate that the OVS flows for the iface-id have been installed.
- */
-static struct simap binding_iface_seqno_map =
- SIMAP_INITIALIZER(&binding_iface_seqno_map);
-
-void
-binding_init(void)
-{
- binding_seq_type_pb_cfg = ofctrl_seqno_add_type();
-}
-
-/* Processes new release/bind operations OVN ports. For newly bound ports
- * it creates ofctrl seqno update requests that will be acked when
- * corresponding OVS flows have been installed.
- *
- * NOTE: Should be called only when valid SB and OVS transactions are
- * available.
- */
-void
-binding_seqno_run(struct local_binding_data *lbinding_data)
-{
- const char *iface_id;
- const char *iface_id_next;
- struct shash *local_bindings = &lbinding_data->bindings;
- SSET_FOR_EACH_SAFE (iface_id, iface_id_next, &binding_iface_released_set) {
- struct shash_node *lb_node = shash_find(local_bindings, iface_id);
-
- /* If the local binding still exists (i.e., the OVS interface is
- * still configured locally) then remove the external id and remove
- * it from the in-flight seqno map.
- */
- if (lb_node) {
- struct local_binding *lb = lb_node->data;
-
- if (lb->iface && smap_get(&lb->iface->external_ids,
- OVN_INSTALLED_EXT_ID)) {
- ovsrec_interface_update_external_ids_delkey(
- lb->iface, OVN_INSTALLED_EXT_ID);
- }
- }
- simap_find_and_delete(&binding_iface_seqno_map, iface_id);
- sset_delete(&binding_iface_released_set,
- SSET_NODE_FROM_NAME(iface_id));
- }
-
- bool new_ifaces = false;
- uint32_t new_seqno = binding_iface_seqno + 1;
-
- SSET_FOR_EACH_SAFE (iface_id, iface_id_next, &binding_iface_bound_set) {
- struct shash_node *lb_node = shash_find(local_bindings, iface_id);
-
- struct local_binding *lb = lb_node ? lb_node->data : NULL;
-
- /* Make sure the binding is still complete, i.e., both SB port_binding
- * and OVS interface still exist.
- *
- * If so, then this is a newly bound interface, make sure we reset the
- * Port_Binding 'up' field and the OVS Interface 'external-id'.
- */
- struct binding_lport *b_lport = local_binding_get_primary_lport(lb);
- if (lb && b_lport && lb->iface
- && !simap_contains(&binding_iface_seqno_map, lb->name)) {
- new_ifaces = true;
-
- if (smap_get(&lb->iface->external_ids, OVN_INSTALLED_EXT_ID)) {
- ovsrec_interface_update_external_ids_delkey(
- lb->iface, OVN_INSTALLED_EXT_ID);
- }
- if (b_lport->pb->n_up) {
- bool up = false;
- sbrec_port_binding_set_up(b_lport->pb, &up, 1);
- }
- simap_put(&binding_iface_seqno_map, lb->name, new_seqno);
- }
- sset_delete(&binding_iface_bound_set, SSET_NODE_FROM_NAME(iface_id));
- }
-
- /* Request a seqno update when the flows for new interfaces have been
- * installed in OVS.
- */
- if (new_ifaces) {
- binding_iface_seqno = new_seqno;
- ofctrl_seqno_update_create(binding_seq_type_pb_cfg, new_seqno);
- }
-}
-
-/* Processes ofctrl seqno ACKs for new bindings. Sets the
- * 'OVN_INSTALLED_EXT_ID' external-id in the OVS interface and the
- * Port_Binding.up field for all ports for which OVS flows have been
- * installed.
- *
- * NOTE: Should be called only when valid SB and OVS transactions are
- * available.
- */
-void
-binding_seqno_install(struct local_binding_data *lbinding_data)
-{
- struct ofctrl_acked_seqnos *acked_seqnos =
- ofctrl_acked_seqnos_get(binding_seq_type_pb_cfg);
- struct simap_node *node;
- struct simap_node *node_next;
- struct shash *local_bindings = &lbinding_data->bindings;
-
- SIMAP_FOR_EACH_SAFE (node, node_next, &binding_iface_seqno_map) {
- struct shash_node *lb_node = shash_find(local_bindings, node->name);
-
- if (!lb_node) {
- goto del_seqno;
- }
-
- struct local_binding *lb = lb_node->data;
- struct binding_lport *b_lport = local_binding_get_primary_lport(lb);
- if (!b_lport || !lb->iface) {
- goto del_seqno;
- }
-
- if (!ofctrl_acked_seqnos_contains(acked_seqnos, node->data)) {
- continue;
- }
-
- ovsrec_interface_update_external_ids_setkey(lb->iface,
- OVN_INSTALLED_EXT_ID,
- "true");
- if (b_lport->pb->n_up) {
- bool up = true;
-
- sbrec_port_binding_set_up(b_lport->pb, &up, 1);
- LIST_FOR_EACH (b_lport, list_node, &lb->binding_lports) {
- sbrec_port_binding_set_up(b_lport->pb, &up, 1);
- }
- }
-
-del_seqno:
- simap_delete(&binding_iface_seqno_map, node);
- }
-
- ofctrl_acked_seqnos_destroy(acked_seqnos);
-}
-
-void
-binding_seqno_flush(void)
-{
- simap_clear(&binding_iface_seqno_map);
-}
-
/* Static functions for local_lbindind and binding_lport. */
static struct local_binding *
local_binding_create(const char *name, const struct ovsrec_interface *iface)
@@ -2728,9 +2628,11 @@ local_binding_destroy(struct local_binding *lbinding,
static void
local_binding_delete(struct local_binding *lbinding,
struct shash *local_bindings,
- struct shash *binding_lports)
+ struct shash *binding_lports,
+ struct if_status_mgr *if_mgr)
{
shash_find_and_delete(local_bindings, lbinding->name);
+ if_status_mgr_delete_iface(if_mgr, lbinding->name);
local_binding_destroy(lbinding, binding_lports);
}
diff --git a/controller/binding.h b/controller/binding.h
index 4fc9ef207..4f733426b 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -37,6 +37,7 @@ struct sbrec_port_binding_table;
struct sset;
struct sbrec_port_binding;
struct ds;
+struct if_status_mgr;
struct binding_ctx_in {
struct ovsdb_idl_txn *ovnsb_idl_txn;
@@ -85,6 +86,8 @@ struct binding_ctx_out {
* binding_handle_port_binding_changes) fills in for
* the changed datapaths and port bindings. */
struct hmap *tracked_dp_bindings;
+
+ struct if_status_mgr *if_mgr;
};
struct local_binding_data {
@@ -97,6 +100,12 @@ void local_binding_data_destroy(struct local_binding_data *);
const struct sbrec_port_binding *local_binding_get_primary_pb(
struct shash *local_bindings, const char *pb_name);
+bool local_binding_is_up(struct shash *local_bindings, const char *pb_name);
+bool local_binding_is_down(struct shash *local_bindings, const char *pb_name);
+void local_binding_set_up(struct shash *local_bindings, const char *pb_name,
+ bool sb_readonly, bool ovs_readonly);
+void local_binding_set_down(struct shash *local_bindings, const char *pb_name,
+ bool sb_readonly, bool ovs_readonly);
/* Represents a tracked binding logical port. */
struct tracked_binding_lport {
@@ -123,9 +132,6 @@ bool binding_handle_port_binding_changes(struct binding_ctx_in *,
struct binding_ctx_out *);
void binding_tracked_dp_destroy(struct hmap *tracked_datapaths);
-void binding_init(void);
-void binding_seqno_run(struct local_binding_data *lbinding_data);
-void binding_seqno_install(struct local_binding_data *lbinding_data);
-void binding_seqno_flush(void);
void binding_dump_local_bindings(struct local_binding_data *, struct ds *);
+
#endif /* controller/binding.h */
diff --git a/controller/if-status.c b/controller/if-status.c
new file mode 100644
index 000000000..c6b1a7fa6
--- /dev/null
+++ b/controller/if-status.c
@@ -0,0 +1,395 @@
+/* Copyright (c) 2021, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#include "binding.h"
+#include "if-status.h"
+#include "ofctrl-seqno.h"
+
+#include "lib/hmapx.h"
+#include "lib/util.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(if_status);
+
+/* This module implements an interface manager that maintains the state of
+ * the interfaces wrt. their flows being completely installed in OVS and
+ * their corresponding bindings being marked up/down.
+ *
+ * A state machine is maintained for each interface.
+ *
+ * Transitions are triggered between states by three types of events:
+ * A. Events received from the binding module:
+ * - interface is claimed: if_status_mgr_claim_iface()
+ * - interface is released: if_status_mgr_release_iface()
+ * - interface is deleted: if_status_mgr_delete_iface()
+ *
+ * B. At every iteration, based on SB/OVS updates, handled in
+ * if_status_mgr_update():
+ * - an interface binding has been marked "up" both in the Southbound and OVS
+ * databases.
+ * - an interface binding has been marked "down" both in the Southbound and OVS
+ * databases.
+ * - new interface has been claimed.
+ *
+ * C. At every iteration, based on ofctrl_seqno updates, handled in
+ * if_status_mgr_run():
+ * - the flows for a previously claimed interface have been installed in OVS.
+ */
+
+enum if_state {
+ OIF_CLAIMED, /* Newly claimed interface. */
+ OIF_INSTALL_FLOWS, /* Already claimed interface for which flows are still
+ * being installed.
+ */
+ OIF_MARK_UP, /* Interface with flows successfully installed in OVS
+ * but not yet marked "up" in the binding module (in
+ * SB and OVS databases).
+ */
+ OIF_MARK_DOWN, /* Released interface but not yet marked "down" in the
+ * binding module (in SB and/or OVS databases).
+ */
+ OIF_INSTALLED, /* Interface flows programmed in OVS and binding marked
+ * "up" in the binding module.
+ */
+ OIF_MAX,
+};
+
+static const char *if_state_names[] = {
+ [OIF_CLAIMED] = "CLAIMED",
+ [OIF_INSTALL_FLOWS] = "INSTALL_FLOWS",
+ [OIF_MARK_UP] = "MARK_UP",
+ [OIF_MARK_DOWN] = "MARK_DOWN",
+ [OIF_INSTALLED] = "INSTALLED",
+};
+
+struct ovs_iface {
+ char *id; /* Extracted from OVS external_ids.iface_id. */
+ enum if_state state; /* State of the interface in the state machine. */
+ uint32_t install_seqno; /* Seqno at which this interface is expected to
+ * be fully programmed in OVS. Only used in state
+ * OIF_INSTALL_FLOWS.
+ */
+};
+
+/* State machine manager for all local OVS interfaces. */
+struct if_status_mgr {
+ /* All local interfaces, mapping from 'iface-id' to 'struct ovs_iface'. */
+ struct shash ifaces;
+
+ /* All local interfaces, stored per state. */
+ struct hmapx ifaces_per_state[OIF_MAX];
+
+ /* Registered ofctrl seqno type for port_binding flow installation. */
+ size_t iface_seq_type_pb_cfg;
+
+ /* Interface specific seqno to be acked by ofctrl when flows for new
+ * interfaces have been installed.
+ */
+ uint32_t iface_seqno;
+};
+
+static struct ovs_iface *ovs_iface_create(struct if_status_mgr *,
+ const char *iface_id,
+ enum if_state );
+static void ovs_iface_destroy(struct if_status_mgr *, struct ovs_iface *);
+static void ovs_iface_set_state(struct if_status_mgr *, struct ovs_iface *,
+ enum if_state);
+
+static void if_status_mgr_update_bindings(
+ struct if_status_mgr *mgr, struct local_binding_data *binding_data,
+ bool sb_readonly, bool ovs_readonly);
+
+struct if_status_mgr *
+if_status_mgr_create(void)
+{
+ struct if_status_mgr *mgr = xzalloc(sizeof *mgr);
+
+ mgr->iface_seq_type_pb_cfg = ofctrl_seqno_add_type();
+ for (size_t i = 0; i < ARRAY_SIZE(mgr->ifaces_per_state); i++) {
+ hmapx_init(&mgr->ifaces_per_state[i]);
+ }
+ shash_init(&mgr->ifaces);
+ return mgr;
+}
+
+void
+if_status_mgr_clear(struct if_status_mgr *mgr)
+{
+ struct shash_node *node_next;
+ struct shash_node *node;
+
+ SHASH_FOR_EACH_SAFE (node, node_next, &mgr->ifaces) {
+ ovs_iface_destroy(mgr, node->data);
+ }
+ ovs_assert(shash_is_empty(&mgr->ifaces));
+
+ for (size_t i = 0; i < ARRAY_SIZE(mgr->ifaces_per_state); i++) {
+ ovs_assert(hmapx_is_empty(&mgr->ifaces_per_state[i]));
+ }
+}
+
+void
+if_status_mgr_destroy(struct if_status_mgr *mgr)
+{
+ if_status_mgr_clear(mgr);
+ shash_destroy(&mgr->ifaces);
+ for (size_t i = 0; i < ARRAY_SIZE(mgr->ifaces_per_state); i++) {
+ hmapx_destroy(&mgr->ifaces_per_state[i]);
+ }
+ free(mgr);
+}
+
+void
+if_status_mgr_claim_iface(struct if_status_mgr *mgr, const char *iface_id)
+{
+ struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
+
+ if (!iface) {
+ iface = ovs_iface_create(mgr, iface_id, OIF_CLAIMED);
+ }
+
+ switch (iface->state) {
+ case OIF_CLAIMED:
+ case OIF_INSTALL_FLOWS:
+ case OIF_MARK_UP:
+ /* Nothing to do here. */
+ break;
+ case OIF_INSTALLED:
+ case OIF_MARK_DOWN:
+ ovs_iface_set_state(mgr, iface, OIF_CLAIMED);
+ break;
+ case OIF_MAX:
+ OVS_NOT_REACHED();
+ break;
+ }
+}
+
+void
+if_status_mgr_release_iface(struct if_status_mgr *mgr, const char *iface_id)
+{
+ struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
+
+ if (!iface) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "Trying to release unknown interface %s", iface_id);
+ return;
+ }
+
+ switch (iface->state) {
+ case OIF_CLAIMED:
+ case OIF_INSTALL_FLOWS:
+ /* Not yet fully installed interfaces can be safely deleted. */
+ ovs_iface_destroy(mgr, iface);
+ break;
+ case OIF_MARK_UP:
+ case OIF_INSTALLED:
+ /* Properly mark interfaces "down" if their flows were already
+ * programmed in OVS.
+ */
+ ovs_iface_set_state(mgr, iface, OIF_MARK_DOWN);
+ break;
+ case OIF_MARK_DOWN:
+ /* Nothing to do here. */
+ break;
+ case OIF_MAX:
+ OVS_NOT_REACHED();
+ break;
+ }
+}
+
+void
+if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id)
+{
+ struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
+
+ if (!iface) {
+ return;
+ }
+ ovs_iface_destroy(mgr, iface);
+}
+
+void
+if_status_mgr_update(struct if_status_mgr *mgr,
+ struct local_binding_data *binding_data)
+{
+ if (!binding_data) {
+ return;
+ }
+
+ struct shash *bindings = &binding_data->bindings;
+ struct hmapx_node *node_next;
+ struct hmapx_node *node;
+
+ /* Move all interfaces that have been confirmed "up" by the binding module,
+ * from OIF_MARK_UP to OIF_INSTALLED.
+ */
+ HMAPX_FOR_EACH_SAFE (node, node_next,
+ &mgr->ifaces_per_state[OIF_MARK_UP]) {
+ struct ovs_iface *iface = node->data;
+
+ if (local_binding_is_up(bindings, iface->id)) {
+ ovs_iface_set_state(mgr, iface, OIF_INSTALLED);
+ }
+ }
+
+ /* Cleanup all interfaces that have been confirmed "down" by the binding
+ * module.
+ */
+ HMAPX_FOR_EACH_SAFE (node, node_next,
+ &mgr->ifaces_per_state[OIF_MARK_DOWN]) {
+ struct ovs_iface *iface = node->data;
+
+ if (local_binding_is_down(bindings, iface->id)) {
+ ovs_iface_destroy(mgr, iface);
+ }
+ }
+
+ /* Register for a notification about flows being installed in OVS for all
+ * newly claimed interfaces.
+ *
+ * Move them from OIF_CLAIMED to OIF_INSTALL_FLOWS.
+ */
+ bool new_ifaces = false;
+ HMAPX_FOR_EACH_SAFE (node, node_next,
+ &mgr->ifaces_per_state[OIF_CLAIMED]) {
+ struct ovs_iface *iface = node->data;
+
+ ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS);
+ iface->install_seqno = mgr->iface_seqno + 1;
+ new_ifaces = true;
+ }
+
+ /* Request a seqno update when the flows for new interfaces have been
+ * installed in OVS.
+ */
+ if (new_ifaces) {
+ mgr->iface_seqno++;
+ ofctrl_seqno_update_create(mgr->iface_seq_type_pb_cfg,
+ mgr->iface_seqno);
+ VLOG_DBG("Seqno requested: %"PRIu32, mgr->iface_seqno);
+ }
+}
+
+void
+if_status_mgr_run(struct if_status_mgr *mgr,
+ struct local_binding_data *binding_data,
+ bool sb_readonly, bool ovs_readonly)
+{
+ struct ofctrl_acked_seqnos *acked_seqnos =
+ ofctrl_acked_seqnos_get(mgr->iface_seq_type_pb_cfg);
+ struct hmapx_node *node_next;
+ struct hmapx_node *node;
+
+ /* Move interfaces from state OIF_INSTALL_FLOWS to OIF_MARK_UP if a
+ * notification has been received aabout their flows being installed
+ * in OVS.
+ */
+ HMAPX_FOR_EACH_SAFE (node, node_next,
+ &mgr->ifaces_per_state[OIF_INSTALL_FLOWS]) {
+ struct ovs_iface *iface = node->data;
+
+ if (!ofctrl_acked_seqnos_contains(acked_seqnos,
+ iface->install_seqno)) {
+ continue;
+ }
+ ovs_iface_set_state(mgr, iface, OIF_MARK_UP);
+ }
+ ofctrl_acked_seqnos_destroy(acked_seqnos);
+
+ /* Update binding states. */
+ if_status_mgr_update_bindings(mgr, binding_data, sb_readonly,
+ ovs_readonly);
+}
+
+static struct ovs_iface *
+ovs_iface_create(struct if_status_mgr *mgr, const char *iface_id,
+ enum if_state state)
+{
+ struct ovs_iface *iface = xzalloc(sizeof *iface);
+
+ VLOG_DBG("Interface %s create.", iface->id);
+ iface->id = xstrdup(iface_id);
+ shash_add(&mgr->ifaces, iface_id, iface);
+ ovs_iface_set_state(mgr, iface, state);
+ return iface;
+}
+
+static void
+ovs_iface_destroy(struct if_status_mgr *mgr, struct ovs_iface *iface)
+{
+ VLOG_DBG("Interface %s destroy: state %s", iface->id,
+ if_state_names[iface->state]);
+ hmapx_find_and_delete(&mgr->ifaces_per_state[iface->state], iface);
+ shash_find_and_delete(&mgr->ifaces, iface->id);
+ free(iface->id);
+ free(iface);
+}
+
+static void
+ovs_iface_set_state(struct if_status_mgr *mgr, struct ovs_iface *iface,
+ enum if_state state)
+{
+ VLOG_DBG("Interface %s set state: old %s, new %s", iface->id,
+ if_state_names[iface->state],
+ if_state_names[state]);
+
+ hmapx_find_and_delete(&mgr->ifaces_per_state[iface->state], iface);
+ iface->state = state;
+ hmapx_add(&mgr->ifaces_per_state[iface->state], iface);
+ iface->install_seqno = 0;
+}
+
+static void
+if_status_mgr_update_bindings(struct if_status_mgr *mgr,
+ struct local_binding_data *binding_data,
+ bool sb_readonly, bool ovs_readonly)
+{
+ if (!binding_data) {
+ return;
+ }
+
+ struct shash *bindings = &binding_data->bindings;
+ struct hmapx_node *node;
+
+ /* Notify the binding module to set "down" all bindings that are still
+ * in the process of being installed in OVS, i.e., are not yet instsalled.
+ */
+ HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_INSTALL_FLOWS]) {
+ struct ovs_iface *iface = node->data;
+
+ local_binding_set_down(bindings, iface->id, sb_readonly, ovs_readonly);
+ }
+
+ /* Notifiy the binding module to set "up" all bindings that have had
+ * their flows installed but are not yet marked "up" in the binding
+ * module.
+ */
+ HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_UP]) {
+ struct ovs_iface *iface = node->data;
+
+ local_binding_set_up(bindings, iface->id, sb_readonly, ovs_readonly);
+ }
+
+ /* Notify the binding module to set "down" all bindings that have been
+ * released but are not yet marked as "down" in the binding module.
+ */
+ HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_DOWN]) {
+ struct ovs_iface *iface = node->data;
+
+ local_binding_set_down(bindings, iface->id, sb_readonly, ovs_readonly);
+ }
+}
diff --git a/controller/if-status.h b/controller/if-status.h
new file mode 100644
index 000000000..51fe7c684
--- /dev/null
+++ b/controller/if-status.h
@@ -0,0 +1,37 @@
+/* Copyright (c) 2021, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef IF_STATUS_H
+#define IF_STATUS_H 1
+
+#include "openvswitch/shash.h"
+
+#include "binding.h"
+
+struct if_status_mgr;
+
+struct if_status_mgr *if_status_mgr_create(void);
+void if_status_mgr_clear(struct if_status_mgr *);
+void if_status_mgr_destroy(struct if_status_mgr *);
+
+void if_status_mgr_claim_iface(struct if_status_mgr *, const char *iface_id);
+void if_status_mgr_release_iface(struct if_status_mgr *, const char *iface_id);
+void if_status_mgr_delete_iface(struct if_status_mgr *, const char *iface_id);
+
+void if_status_mgr_update(struct if_status_mgr *, struct local_binding_data *);
+void if_status_mgr_run(struct if_status_mgr *mgr, struct local_binding_data *,
+ bool sb_readonly, bool ovs_readonly);
+
+# endif /* controller/if-status.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6f7c9ea61..e599ad1df 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -33,6 +33,7 @@
#include "openvswitch/dynamic-string.h"
#include "encaps.h"
#include "fatal-signal.h"
+#include "if-status.h"
#include "ip-mcast.h"
#include "openvswitch/hmap.h"
#include "lflow.h"
@@ -103,6 +104,7 @@ OVS_NO_RETURN static void usage(void);
struct controller_engine_ctx {
struct lflow_cache *lflow_cache;
+ struct if_status_mgr *if_mgr;
};
/* Pending packet to be injected into connected OVS. */
@@ -996,6 +998,7 @@ en_ofctrl_is_connected_cleanup(void *data OVS_UNUSED)
static void
en_ofctrl_is_connected_run(struct engine_node *node, void *data)
{
+ struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
struct ed_type_ofctrl_is_connected *of_data = data;
if (of_data->connected != ofctrl_is_connected()) {
of_data->connected = !of_data->connected;
@@ -1003,7 +1006,7 @@ en_ofctrl_is_connected_run(struct engine_node *node, void *data)
/* Flush ofctrl seqno requests when the ofctrl connection goes down. */
if (!of_data->connected) {
ofctrl_seqno_flush();
- binding_seqno_flush();
+ if_status_mgr_clear(ctrl_ctx->if_mgr);
}
engine_set_node_state(node, EN_UPDATED);
return;
@@ -1375,6 +1378,8 @@ init_binding_ctx(struct engine_node *node,
engine_get_input("SB_port_binding", node),
"datapath");
+ struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+
b_ctx_in->ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn;
b_ctx_in->ovs_idl_txn = engine_get_context()->ovs_idl_txn;
b_ctx_in->sbrec_datapath_binding_by_key = sbrec_datapath_binding_by_key;
@@ -1401,6 +1406,7 @@ init_binding_ctx(struct engine_node *node,
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;
+ b_ctx_out->if_mgr = ctrl_ctx->if_mgr;
}
static void
@@ -2440,7 +2446,6 @@ main(int argc, char *argv[])
/* Register ofctrl seqno types. */
ofctrl_seq_type_nb_cfg = ofctrl_seqno_add_type();
- binding_init();
patch_init();
pinctrl_init();
lflow_init();
@@ -2744,7 +2749,9 @@ main(int argc, char *argv[])
struct controller_engine_ctx ctrl_engine_ctx = {
.lflow_cache = lflow_cache_create(),
+ .if_mgr = if_status_mgr_create(),
};
+ struct if_status_mgr *if_mgr = ctrl_engine_ctx.if_mgr;
char *ovn_version = ovn_get_internal_version();
VLOG_INFO("OVN internal version is : [%s]", ovn_version);
@@ -2954,9 +2961,10 @@ main(int argc, char *argv[])
ovnsb_idl_loop.idl),
ovnsb_cond_seqno,
ovnsb_expected_cond_seqno));
- if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) {
- binding_seqno_run(&runtime_data->lbinding_data);
- }
+
+ struct local_binding_data *binding_data =
+ runtime_data ? &runtime_data->lbinding_data : NULL;
+ if_status_mgr_update(if_mgr, binding_data);
flow_output_data = engine_get_data(&en_flow_output);
if (flow_output_data && ct_zones_data) {
@@ -2967,9 +2975,8 @@ main(int argc, char *argv[])
engine_node_changed(&en_flow_output));
}
ofctrl_seqno_run(ofctrl_get_cur_cfg());
- if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) {
- binding_seqno_install(&runtime_data->lbinding_data);
- }
+ if_status_mgr_run(if_mgr, binding_data, !ovnsb_idl_txn,
+ !ovs_idl_txn);
}
}
@@ -3135,6 +3142,7 @@ loop_done:
ofctrl_destroy();
pinctrl_destroy();
patch_destroy();
+ if_status_mgr_destroy(if_mgr);
ovsdb_idl_loop_destroy(&ovs_idl_loop);
ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
More information about the dev
mailing list