From: Numan Siddique <num...@ovn.org>
When changes to port bindings corresponding to router ports are
handled by northd engine node, incorrect warning logs (like below)
are logged.
----
northd|WARN|A port-binding for lrp0 is created but the LSP is not found.
----
Fix these warnings.
Since we are preserving the "lr_ports" data in the northd engine
across engine runs, it is important to store the port binding
address in 'op->sb' after the transaction is committed. And this
patch does that.
Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding in
"northd" node.")
CC: Han Zhou <hz...@ovn.org>
Signed-off-by: Numan Siddique <num...@ovn.org>
---
controller/binding.c | 75 --------------------------------
controller/binding.h | 20 +--------
lib/ovn-util.c | 101 +++++++++++++++++++++++++++++++++++++++++++
lib/ovn-util.h | 21 +++++++++
northd/en-northd.c | 2 +-
northd/northd.c | 19 ++++++--
northd/northd.h | 3 +-
7 files changed, 141 insertions(+), 100 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index 3ac0c35df3..a521f2828e 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct
binding_lport *);
static bool binding_lport_update_port_sec(
struct binding_lport *, const struct sbrec_port_binding *);
-static char *get_lport_type_str(enum en_lport_type lport_type);
static bool ovs_iface_matches_lport_iface_id_ver(
const struct ovsrec_interface *,
const struct sbrec_port_binding *);
@@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct local_binding_data
*lbinding_data,
free(nodes);
}
-static bool
-is_lport_vif(const struct sbrec_port_binding *pb)
-{
- return !pb->type[0];
-}
-
-enum en_lport_type
-get_lport_type(const struct sbrec_port_binding *pb)
-{
- if (is_lport_vif(pb)) {
- if (pb->parent_port && pb->parent_port[0]) {
- return LP_CONTAINER;
- }
- return LP_VIF;
- } else if (!strcmp(pb->type, "patch")) {
- return LP_PATCH;
- } else if (!strcmp(pb->type, "chassisredirect")) {
- return LP_CHASSISREDIRECT;
- } else if (!strcmp(pb->type, "l3gateway")) {
- return LP_L3GATEWAY;
- } else if (!strcmp(pb->type, "localnet")) {
- return LP_LOCALNET;
- } else if (!strcmp(pb->type, "localport")) {
- return LP_LOCALPORT;
- } else if (!strcmp(pb->type, "l2gateway")) {
- return LP_L2GATEWAY;
- } else if (!strcmp(pb->type, "virtual")) {
- return LP_VIRTUAL;
- } else if (!strcmp(pb->type, "external")) {
- return LP_EXTERNAL;
- } else if (!strcmp(pb->type, "remote")) {
- return LP_REMOTE;
- } else if (!strcmp(pb->type, "vtep")) {
- return LP_VTEP;
- }
-
- return LP_UNKNOWN;
-}
-
-static char *
-get_lport_type_str(enum en_lport_type lport_type)
-{
- switch (lport_type) {
- case LP_VIF:
- return "VIF";
- case LP_CONTAINER:
- return "CONTAINER";
- case LP_VIRTUAL:
- return "VIRTUAL";
- case LP_PATCH:
- return "PATCH";
- case LP_CHASSISREDIRECT:
- return "CHASSISREDIRECT";
- case LP_L3GATEWAY:
- return "L3GATEWAY";
- case LP_LOCALNET:
- return "PATCH";
- case LP_LOCALPORT:
- return "LOCALPORT";
- case LP_L2GATEWAY:
- return "L2GATEWAY";
- case LP_EXTERNAL:
- return "EXTERNAL";
- case LP_REMOTE:
- return "REMOTE";
- case LP_VTEP:
- return "VTEP";
- case LP_UNKNOWN:
- return "UNKNOWN";
- }
-
- OVS_NOT_REACHED();
-}
-
void
set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb,
const struct sbrec_chassis *chassis_rec,
diff --git a/controller/binding.h b/controller/binding.h
index 235e5860d0..24bc840791 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -22,6 +22,7 @@
#include "openvswitch/hmap.h"
#include "openvswitch/uuid.h"
#include "openvswitch/list.h"
+# include "lib/ovn-util.h"
#include "sset.h"
#include "lport.h"
@@ -217,25 +218,6 @@ void port_binding_set_down(const struct sbrec_chassis
*chassis_rec,
const char *iface_id,
const struct uuid *pb_uuid);
-/* Corresponds to each Port_Binding.type. */
-enum en_lport_type {
- LP_UNKNOWN,
- LP_VIF,
- LP_CONTAINER,
- LP_PATCH,
- LP_L3GATEWAY,
- LP_LOCALNET,
- LP_LOCALPORT,
- LP_L2GATEWAY,
- LP_VTEP,
- LP_CHASSISREDIRECT,
- LP_VIRTUAL,
- LP_EXTERNAL,
- LP_REMOTE
-};
-
-enum en_lport_type get_lport_type(const struct sbrec_port_binding *);
-
/* This structure represents a logical port (or port binding)
* which is associated with 'struct local_binding'.
*
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 080ad4c0ce..3a237b7fe3 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -1168,3 +1168,104 @@ encode_fqdn_string(const char *fqdn, size_t *len)
return encoded;
}
+
+static bool
+is_lport_vif(const struct sbrec_port_binding *pb)
+{
+ return !pb->type[0];
+}
+
+enum en_lport_type
+get_lport_type(const struct sbrec_port_binding *pb)
+{
+ if (is_lport_vif(pb)) {
+ if (pb->parent_port && pb->parent_port[0]) {
+ return LP_CONTAINER;
+ }
+ return LP_VIF;
+ } else if (!strcmp(pb->type, "patch")) {
+ return LP_PATCH;
+ } else if (!strcmp(pb->type, "chassisredirect")) {
+ return LP_CHASSISREDIRECT;
+ } else if (!strcmp(pb->type, "l3gateway")) {
+ return LP_L3GATEWAY;
+ } else if (!strcmp(pb->type, "localnet")) {
+ return LP_LOCALNET;
+ } else if (!strcmp(pb->type, "localport")) {
+ return LP_LOCALPORT;
+ } else if (!strcmp(pb->type, "l2gateway")) {
+ return LP_L2GATEWAY;
+ } else if (!strcmp(pb->type, "virtual")) {
+ return LP_VIRTUAL;
+ } else if (!strcmp(pb->type, "external")) {
+ return LP_EXTERNAL;
+ } else if (!strcmp(pb->type, "remote")) {
+ return LP_REMOTE;
+ } else if (!strcmp(pb->type, "vtep")) {
+ return LP_VTEP;
+ }
+
+ return LP_UNKNOWN;
+}
+
+char *
+get_lport_type_str(enum en_lport_type lport_type)
+{
+ switch (lport_type) {
+ case LP_VIF:
+ return "VIF";
+ case LP_CONTAINER:
+ return "CONTAINER";
+ case LP_VIRTUAL:
+ return "VIRTUAL";
+ case LP_PATCH:
+ return "PATCH";
+ case LP_CHASSISREDIRECT:
+ return "CHASSISREDIRECT";
+ case LP_L3GATEWAY:
+ return "L3GATEWAY";
+ case LP_LOCALNET:
+ return "LOCALNET";
+ case LP_LOCALPORT:
+ return "LOCALPORT";
+ case LP_L2GATEWAY:
+ return "L2GATEWAY";
+ case LP_EXTERNAL:
+ return "EXTERNAL";
+ case LP_REMOTE:
+ return "REMOTE";
+ case LP_VTEP:
+ return "VTEP";
+ case LP_UNKNOWN:
+ return "UNKNOWN";
+ }
+
+ OVS_NOT_REACHED();
+}
+
+bool
+is_pb_router_type(const struct sbrec_port_binding *pb)
+{
+ enum en_lport_type lport_type = get_lport_type(pb);
+
+ switch (lport_type) {
+ case LP_PATCH:
+ case LP_CHASSISREDIRECT:
+ case LP_L3GATEWAY:
+ case LP_L2GATEWAY:
+ return true;
+
+ case LP_VIF:
+ case LP_CONTAINER:
+ case LP_VIRTUAL:
+ case LP_LOCALNET:
+ case LP_LOCALPORT:
+ case LP_REMOTE:
+ case LP_VTEP:
+ case LP_EXTERNAL:
+ case LP_UNKNOWN:
+ return false;
+ }
+
+ return false;
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 5ebdd8adb0..b056a6c0ed 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -409,4 +409,25 @@ void flow_collector_ids_clear(struct flow_collector_ids *);
* The returned pointer has to be freed by caller. */
char *encode_fqdn_string(const char *fqdn, size_t *len);
+/* Corresponds to each Port_Binding.type. */
+enum en_lport_type {
+ LP_UNKNOWN,
+ LP_VIF,
+ LP_CONTAINER,
+ LP_PATCH,
+ LP_L3GATEWAY,
+ LP_LOCALNET,
+ LP_LOCALPORT,
+ LP_L2GATEWAY,
+ LP_VTEP,
+ LP_CHASSISREDIRECT,
+ LP_VIRTUAL,
+ LP_EXTERNAL,
+ LP_REMOTE
+};
+
+enum en_lport_type get_lport_type(const struct sbrec_port_binding *);
+char *get_lport_type_str(enum en_lport_type lport_type);
+bool is_pb_router_type(const struct sbrec_port_binding *);
+
#endif /* OVN_UTIL_H */
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 044fa70190..b931f812ab 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -198,7 +198,7 @@ northd_sb_port_binding_handler(struct engine_node *node,
northd_get_input_data(node, &input_data);
if (!northd_handle_sb_port_binding_changes(
- input_data.sbrec_port_binding_table, &nd->ls_ports)) {
+ input_data.sbrec_port_binding_table, &nd->ls_ports, &nd->lr_ports)) {
return false;
}
diff --git a/northd/northd.c b/northd/northd.c
index 9a12a94ae2..d355cf7c68 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5262,22 +5262,32 @@ fail:
bool
northd_handle_sb_port_binding_changes(
const struct sbrec_port_binding_table *sbrec_port_binding_table,
- struct hmap *ls_ports)
+ struct hmap *ls_ports, struct hmap *lr_ports)
{
const struct sbrec_port_binding *pb;
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, sbrec_port_binding_table) {
struct ovn_port *op = ovn_port_find(ls_ports, pb->logical_port);
+ bool is_router_port = false;
if (op && !op->lsp_can_be_inc_processed) {
return false;
}
+
+ if (!op) {
+ is_router_port = is_pb_router_type(pb);
+ if (is_router_port) {
+ op = ovn_port_find(lr_ports, pb->logical_port);
+ }
+ }
+
if (sbrec_port_binding_is_new(pb)) {
/* Most likely the PB was created by northd and this is the
* notification of that trasaction. So we just update the sb
* pointer in northd data. Fallback to recompute otherwise. */
if (!op) {
VLOG_WARN_RL(&rl, "A port-binding for %s is created but the "
- "LSP is not found.", pb->logical_port);
+ "%s is not found.", pb->logical_port,
+ is_router_port ? "LRP" : "LSP");
return false;
}
op->sb = pb;
@@ -5288,7 +5298,7 @@ northd_handle_sb_port_binding_changes(
* sb idl pointers and other unexpected behavior. */
if (op) {
VLOG_WARN_RL(&rl, "A port-binding for %s is deleted but the "
- "LSP still exists.", pb->logical_port);
+ "LSP/LRP still exists.", pb->logical_port);
return false;
}
} else {
@@ -5298,7 +5308,8 @@ northd_handle_sb_port_binding_changes(
* Fallback to recompute for anything unexpected. */
if (!op) {
VLOG_WARN_RL(&rl, "A port-binding for %s is updated but the "
- "LSP is not found.", pb->logical_port);
+ "%s is not found.", pb->logical_port,
+ is_router_port ? "LRP" : "LSP");
return false;
}
if (op->sb != pb) {
diff --git a/northd/northd.h b/northd/northd.h
index f3e63b1e1a..5759a4ee0e 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -342,7 +342,8 @@ bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn
*ovnsb_txn,
struct tracked_ls_changes *,
struct lflow_input *, struct hmap *lflows);
bool northd_handle_sb_port_binding_changes(
- const struct sbrec_port_binding_table *, struct hmap *ls_ports);
+ const struct sbrec_port_binding_table *, struct hmap *ls_ports,
+ struct hmap *lr_ports);
void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
const struct nbrec_bfd_table *,
--
2.40.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
