From patchwork Mon Aug 14 09:07:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1820954 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RPT7y2hDlz1yfZ for ; Mon, 14 Aug 2023 19:07:56 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id CE49161028; Mon, 14 Aug 2023 09:07:53 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org CE49161028 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3Vn76J16rk0F; Mon, 14 Aug 2023 09:07:52 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 7FAEB60B48; Mon, 14 Aug 2023 09:07:51 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 7FAEB60B48 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5ACF3C0072; Mon, 14 Aug 2023 09:07:51 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 40E9FC0032 for ; Mon, 14 Aug 2023 09:07:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 1B84240A5D for ; Mon, 14 Aug 2023 09:07:50 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 1B84240A5D X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2_bHte-Xv0Gs for ; Mon, 14 Aug 2023 09:07:48 +0000 (UTC) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp2.osuosl.org (Postfix) with ESMTPS id AE03940A61 for ; Mon, 14 Aug 2023 09:07:47 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org AE03940A61 Received: by mail.gandi.net (Postfix) with ESMTPSA id 7CF45FF802; Mon, 14 Aug 2023 09:07:43 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 14 Aug 2023 14:37:38 +0530 Message-Id: <20230814090738.58016-1-numans@ovn.org> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 X-GND-Sasl: numans@ovn.org Subject: [ovs-dev] [PATCH ovn] northd: Fix incorrect warning logs when handling port binding changes. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique 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 Signed-off-by: Numan Siddique --- 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 *,