From: Numan Siddique <num...@ovn.org>
The implementation of util functions "is_cr_port()" and "is_l3dgw_port()"
are confusing and not very intuitive. This patch adds some
documentation. It also renames the struct ovn_port member 'l3dgw_port'
to 'primary_port'. If struct ovn_port->primary_port is set, then it
means 'ovn_port' instance is a chassis resident port and it is derived
from the primary port. An upcoming patch creates a chassisresident port
even for a logical switch port of type "patch" and hence renamed to
avoid confusion.
Signed-off-by: Numan Siddique <num...@ovn.org>
---
northd/northd.c | 48 +++++++++++++++++++++++++++++++++++-------------
northd/northd.h | 10 ++++------
2 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 2d0946218a..6393d688f6 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1077,16 +1077,36 @@ struct ovn_port_routable_addresses {
static bool lsp_can_be_inc_processed(const struct nbrec_logical_switch_port *);
+/* This function returns true if 'op' is a gateway router port.
+ * False otherwise.
+ * For 'op' to be a gateway router port.
+ * 1. op->nbrp->gateway_chassis or op->nbrp->ha_chassis_group should
+ * be configured.
+ * 2. op->cr_port should not be NULL. If op->nbrp->gateway_chassis or
+ * op->nbrp->ha_chassis_group is set by the user, northd WILL create
+ * a chassis resident port in the SB port binding.
+ * See join_logical_ports().
+ */
static bool
is_l3dgw_port(const struct ovn_port *op)
{
- return op->cr_port;
+ return op->cr_port && op->nbrp &&
+ (op->nbrp->n_gateway_chassis || op->nbrp->ha_chassis_group);
}
+/* This function returns true if 'op' is a chassis resident
+ * derived port. False otherwise.
+ * There are 2 ways to check if 'op' is chassis resident port.
+ * 1. op->sb->type is "chassisresident"
+ * 2. op->primary_port is not NULL. If op->primary_port is set,
+ * it means 'op' is derived from the ovn_port op->primary_port.
+ *
+ * This function uses the (2) method as it doesn't involve strcmp().
+ */
static bool
is_cr_port(const struct ovn_port *op)
{
- return op->l3dgw_port;
+ return op->primary_port;
}
static void
@@ -1171,7 +1191,7 @@ ovn_port_create(struct hmap *ports, const char *key,
op->key = xstrdup(key);
op->sb = sb;
ovn_port_set_nb(op, nbsp, nbrp);
- op->l3dgw_port = op->cr_port = NULL;
+ op->primary_port = op->cr_port = NULL;
hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
op->lflow_ref = lflow_ref_create();
@@ -1228,7 +1248,7 @@ ovn_port_destroy(struct hmap *ports, struct ovn_port
*port)
/* Don't remove port->list. The node should be removed from such lists
* before calling this function. */
hmap_remove(ports, &port->key_node);
- if (port->od && !port->l3dgw_port) {
+ if (port->od && !port->primary_port) {
hmap_remove(&port->od->ports, &port->dp_node);
}
ovn_port_destroy_orphan(port);
@@ -1398,7 +1418,7 @@ lsp_force_fdb_lookup(const struct ovn_port *op)
static struct ovn_port *
ovn_port_get_peer(const struct hmap *lr_ports, struct ovn_port *op)
{
- if (!op->nbsp || !lsp_is_router(op->nbsp) || op->l3dgw_port) {
+ if (!op->nbsp || !lsp_is_router(op->nbsp) || is_cr_port(op)) {
return NULL;
}
@@ -2278,7 +2298,7 @@ join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table,
NULL, nbrp, NULL);
ovs_list_push_back(nb_only, &crp->list);
}
- crp->l3dgw_port = op;
+ crp->primary_port = op;
op->cr_port = crp;
crp->od = od;
free(redirect_name);
@@ -2299,7 +2319,7 @@ join_logical_ports(const struct sbrec_port_binding_table
*sbrec_pb_table,
* to their peers. */
struct ovn_port *op;
HMAP_FOR_EACH (op, key_node, ports) {
- if (op->nbsp && lsp_is_router(op->nbsp) && !op->l3dgw_port) {
+ if (op->nbsp && lsp_is_router(op->nbsp) && !op->primary_port) {
struct ovn_port *peer = ovn_port_get_peer(ports, op);
if (!peer || !peer->nbrp) {
continue;
@@ -2353,7 +2373,7 @@ join_logical_ports(const struct sbrec_port_binding_table
*sbrec_pb_table,
op->enable_router_port_acl = smap_get_bool(
&op->nbsp->options, "enable_router_port_acl", false);
}
- } else if (op->nbrp && op->nbrp->peer && !op->l3dgw_port) {
+ } else if (op->nbrp && op->nbrp->peer && !is_cr_port(op)) {
struct ovn_port *peer = ovn_port_find(ports, op->nbrp->peer);
if (peer) {
if (peer->nbrp) {
@@ -3889,7 +3909,7 @@ sync_pb_for_lrp(struct ovn_port *op,
bool always_redirect =
!lr_stateful_rec->lrnat_rec->has_distributed_nat &&
- !l3dgw_port_has_associated_vtep_lports(op->l3dgw_port);
+ !l3dgw_port_has_associated_vtep_lports(op->primary_port);
const char *redirect_type = smap_get(&op->nbrp->options,
"redirect-type");
@@ -4384,7 +4404,7 @@ ls_port_reinit(struct ovn_port *op, struct ovsdb_idl_txn
*ovnsb_txn,
ovn_port_cleanup(op);
op->sb = sb;
ovn_port_set_nb(op, nbsp, NULL);
- op->l3dgw_port = op->cr_port = NULL;
+ op->primary_port = op->cr_port = NULL;
return ls_port_init(op, ovnsb_txn, od, sb,
sbrec_mirror_table, sbrec_chassis_table,
sbrec_chassis_by_name, sbrec_chassis_by_hostname);
@@ -13692,7 +13712,7 @@ build_dhcpv6_reply_flows_for_lrouter_port(
struct lflow_ref *lflow_ref)
{
ovs_assert(op->nbrp);
- if (op->l3dgw_port) {
+ if (is_cr_port(op)) {
return;
}
for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
@@ -17809,9 +17829,11 @@ build_ha_chassis_group_ref_chassis(struct hmapx
*lr_groups,
* table indicating which chassis the distributed port is bond to. */
static void
handle_cr_port_binding_changes(const struct sbrec_port_binding *sb,
- struct ovn_port *orp)
+ struct ovn_port *orp)
{
- const struct nbrec_logical_router_port *nbrec_lrp = orp->l3dgw_port->nbrp;
+ ovs_assert(orp->primary_port);
+ const struct nbrec_logical_router_port *nbrec_lrp
+ = orp->primary_port->nbrp;
if (sb->chassis) {
nbrec_logical_router_port_update_status_setkey(nbrec_lrp,
diff --git a/northd/northd.h b/northd/northd.h
index 940926945e..146139bebc 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -586,13 +586,11 @@ struct ovn_port {
/* Logical port multicast data. */
struct mcast_port_info mcast_info;
- /* At most one of l3dgw_port and cr_port can be not NULL. */
+ /* At most one of primary_port and cr_port can be not NULL. */
- /* This is set to a distributed gateway port if and only if this ovn_port
- * is "derived" from it. Otherwise this is set to NULL. The derived
- * ovn_port represents the instance of distributed gateway port on the
- * gateway chassis.*/
- struct ovn_port *l3dgw_port;
+ /* If this ovn_port is a derived port, then 'primary_port' points to the
+ * port from which this ovn_port is derived. */
+ struct ovn_port *primary_port;
/* This is set to the "derived" chassis-redirect port of this port if and
* only if this port is a distributed gateway port. Otherwise this is set
