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.

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>
---

v2 -> v3
------
  * Checked if type of the 'pb' and return false if its a router port
    before doing a lookup in the ls_ports map.

v1 -> v2
-------
  * v1 was not returning false in northd_handle_sb_port_binding_changes()
    for router port - port bindings because of which IPv6 PD system test
    was failing.  Fixed it in v2.


 controller/binding.c |  75 --------------------------------
 controller/binding.h |  20 +--------
 lib/ovn-util.c       | 101 +++++++++++++++++++++++++++++++++++++++++++
 lib/ovn-util.h       |  21 +++++++++
 northd/northd.c      |   7 +++
 5 files changed, 130 insertions(+), 94 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index 3ac0c35df..a521f2828 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 235e5860d..24bc84079 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 080ad4c0c..3a237b7fe 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 5ebdd8adb..b056a6c0e 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/northd.c b/northd/northd.c
index 9a12a94ae..0a749931e 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5267,10 +5267,17 @@ northd_handle_sb_port_binding_changes(
     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) {
+        /* Return false if the 'pb' belongs to a router port.  We don't handle
+         * I-P for router ports yet. */
+        if (is_pb_router_type(pb)) {
+            return false;
+        }
+
         struct ovn_port *op = ovn_port_find(ls_ports, pb->logical_port);
         if (op && !op->lsp_can_be_inc_processed) {
             return false;
         }
+
         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
-- 
2.40.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to