From patchwork Wed Jan 4 08:32:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Simonart X-Patchwork-Id: 1721263 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=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ivbn4AmF; dkim-atps=neutral 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 4Nn2sP46Bsz23fc for ; Wed, 4 Jan 2023 19:32:25 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 18A0B61022; Wed, 4 Jan 2023 08:32:23 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 18A0B61022 Authentication-Results: smtp3.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ivbn4AmF 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 hutfAe3eKNs6; Wed, 4 Jan 2023 08:32:20 +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 A307161012; Wed, 4 Jan 2023 08:32:18 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org A307161012 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8CB3CC007F; Wed, 4 Jan 2023 08:32:16 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id DE7F9C002D for ; Wed, 4 Jan 2023 08:32:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id ACC9F40A95 for ; Wed, 4 Jan 2023 08:32:14 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org ACC9F40A95 Authentication-Results: smtp2.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ivbn4AmF 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 zo6NuP6HCRFS for ; Wed, 4 Jan 2023 08:32:13 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 80AAE400EC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp2.osuosl.org (Postfix) with ESMTPS id 80AAE400EC for ; Wed, 4 Jan 2023 08:32:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1672821132; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3GpDdXzSW4CzUSDQHQ+6VCcyY/sup9FrqBHZmNxP1BE=; b=Ivbn4AmFgXHgw5TPBHD0oX1VzxA3VUKKSvdXRHGIWsMJimudEr8qixbH5KZtmdfRVwU3t5 0UGDbi3m7LCLg3o5cr+PTpVNBFTKaoIe5znJ3KOOxWpx1nPToaQBOtvEk480Cu71iGhiQi UeGPqiBEguDjTSaLaSz9rpcEnxyQCzM= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-641-tZLUL-uPOp-JZb0ShYi4qA-1; Wed, 04 Jan 2023 03:32:11 -0500 X-MC-Unique: tZLUL-uPOp-JZb0ShYi4qA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B3B332805582 for ; Wed, 4 Jan 2023 08:32:10 +0000 (UTC) Received: from wsfd-netdev90.ntdv.lab.eng.bos.redhat.com (wsfd-netdev90.ntdv.lab.eng.bos.redhat.com [10.19.188.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 97C10140EBF5; Wed, 4 Jan 2023 08:32:10 +0000 (UTC) From: Xavier Simonart To: xsimonar@redhat.com, dev@openvswitch.org Date: Wed, 4 Jan 2023 03:32:09 -0500 Message-Id: <20230104083209.3880669-2-xsimonar@redhat.com> In-Reply-To: <20230104083209.3880669-1-xsimonar@redhat.com> References: <20220627085202.1187277-1-xsimonar@redhat.com> <20230104083209.3880669-1-xsimonar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: dceara@redhat.com Subject: [ovs-dev] [PATCH ovn v2 2/2] ovn-controller: fixed port not always set down when unbinding interface 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" When interface was unbound, the port was not always set down and the port_binding->chassis not always removed. Fixes: a7c7d4519e50 ("controller: avoid recomputes triggered by SBDB Port_Binding updates.") Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2150905 Signed-off-by: Xavier Simonart --- controller/binding.c | 2 +- controller/if-status.c | 44 +++++++++++++++++++++++++++++++++++++ controller/if-status.h | 4 ++++ controller/ovn-controller.c | 12 ++++++++++ tests/ovn.at | 12 ++++------ 5 files changed, 65 insertions(+), 9 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index d85a17730..eb8d580c8 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -894,7 +894,6 @@ local_binding_set_down(struct shash *local_bindings, const char *pb_name, if (!sb_readonly && b_lport && b_lport->pb->n_up && b_lport->pb->up[0] && (!b_lport->pb->chassis || b_lport->pb->chassis == chassis_rec)) { - VLOG_INFO("Setting lport %s down in Southbound", pb_name); binding_lport_set_down(b_lport, sb_readonly); LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { binding_lport_set_down(b_lport, sb_readonly); @@ -3384,6 +3383,7 @@ binding_lport_set_down(struct binding_lport *b_lport, bool sb_readonly) if (sb_readonly || !b_lport || !b_lport->pb->n_up || !b_lport->pb->up[0]) { return; } + VLOG_INFO("Setting lport %s down in Southbound", b_lport->name); bool up = false; sbrec_port_binding_set_up(b_lport->pb, &up, 1); diff --git a/controller/if-status.c b/controller/if-status.c index c008aa79e..26535c9e8 100644 --- a/controller/if-status.c +++ b/controller/if-status.c @@ -16,6 +16,7 @@ #include #include "binding.h" +#include "lport.h" #include "if-status.h" #include "ofctrl-seqno.h" #include "simap.h" @@ -75,6 +76,9 @@ enum if_state { OIF_INSTALLED, /* Interface flows programmed in OVS and binding * marked "up" in the binding module. */ + OIF_UPDATE_PORT, /* Logical ports need to be set down, and pb->chassis + * removed. + */ OIF_MAX, }; @@ -85,6 +89,7 @@ static const char *if_state_names[] = { [OIF_MARK_UP] = "MARK_UP", [OIF_MARK_DOWN] = "MARK_DOWN", [OIF_INSTALLED] = "INSTALLED", + [OIF_UPDATE_PORT] = "UPDATE_PORT", }; /* @@ -264,6 +269,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, break; case OIF_INSTALLED: case OIF_MARK_DOWN: + case OIF_UPDATE_PORT: ovs_iface_set_state(mgr, iface, OIF_CLAIMED); break; case OIF_MAX: @@ -304,6 +310,7 @@ if_status_mgr_release_iface(struct if_status_mgr *mgr, const char *iface_id) ovs_iface_set_state(mgr, iface, OIF_MARK_DOWN); break; case OIF_MARK_DOWN: + case OIF_UPDATE_PORT: /* Nothing to do here. */ break; case OIF_MAX: @@ -336,6 +343,7 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id) ovs_iface_set_state(mgr, iface, OIF_MARK_DOWN); break; case OIF_MARK_DOWN: + case OIF_UPDATE_PORT: /* Nothing to do here. */ break; case OIF_MAX: @@ -344,6 +352,36 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id) } } +void +if_status_handle_lports(struct if_status_mgr *mgr, + const struct sbrec_chassis *chassis_rec, + struct ovsdb_idl_index *sbrec_port_binding_by_name, + bool sb_readonly) +{ + if (sb_readonly) { + return; + } + + struct hmapx_node *node; + + HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_UPDATE_PORT]) { + struct ovs_iface *iface = node->data; + const struct sbrec_port_binding *pb = lport_lookup_by_name( + sbrec_port_binding_by_name, iface->id); + + if ((pb == NULL) || sbrec_port_binding_is_deleted(pb)) { + VLOG_DBG("Removing lport %s from list of ports to set down", + iface->id); + } else { + bool up = false; + sbrec_port_binding_set_up(pb, &up, 1); + VLOG_INFO("Setting lport %s down in Southbound", iface->id); + set_pb_chassis_in_sbrec(pb, chassis_rec, false); + } + ovs_iface_destroy(mgr, node->data); + } +} + bool if_status_handle_claims(struct if_status_mgr *mgr, struct local_binding_data *binding_data, @@ -424,6 +462,12 @@ if_status_mgr_update(struct if_status_mgr *mgr, HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_MARK_DOWN]) { struct ovs_iface *iface = node->data; + if (!local_binding_find(bindings, iface->id) && sb_readonly) { + VLOG_DBG("%s not found in local_bindings and sb read only => " + "port down delayed", iface->id); + ovs_iface_set_state(mgr, iface, OIF_UPDATE_PORT); + continue; + } if (!sb_readonly) { local_binding_set_pb(bindings, iface->id, chassis_rec, NULL, false); diff --git a/controller/if-status.h b/controller/if-status.h index 5bd187a25..593597af7 100644 --- a/controller/if-status.h +++ b/controller/if-status.h @@ -48,5 +48,9 @@ bool if_status_handle_claims(struct if_status_mgr *mgr, const struct sbrec_chassis *chassis_rec, struct hmap *tracked_datapath, bool sb_readonly); +void if_status_handle_lports(struct if_status_mgr *mgr, + const struct sbrec_chassis *chassis_rec, + struct ovsdb_idl_index *sbrec_port_binding_by_name, + bool sb_readonly); # endif /* controller/if-status.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 814a88117..4094eb56e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1603,6 +1603,11 @@ runtime_data_sb_ro_handler(struct engine_node *node, void *data) engine_get_input("SB_chassis", node), "name"); + struct ovsdb_idl_index *sbrec_port_binding_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_port_binding", node), + "name"); + if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); } @@ -1620,6 +1625,13 @@ runtime_data_sb_ro_handler(struct engine_node *node, void *data) engine_set_node_state(node, EN_UPDATED); rt_data->tracked = true; } + + if (sbrec_port_binding_by_name) { + if_status_handle_lports(ctrl_ctx->if_mgr, + chassis, + sbrec_port_binding_by_name, + sb_readonly); + } } return true; } diff --git a/tests/ovn.at b/tests/ovn.at index a0378a728..a849a483d 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -34403,10 +34403,8 @@ sleep_controller wake_up_sb wake_up_controller check_ovn_uninstalled -# Port_down not always set on iface-id removal -# check_ports_down -# Port_Binding(chassis) not always removed on iface-id removal -# check_ports_unbound +check_ports_down +check_ports_unbound add_iface_ids check ovn-nbctl --wait=hv sync @@ -34443,10 +34441,8 @@ sleep_controller wake_up_sb wake_up_controller check_ovn_uninstalled -# Port_down not always set on Interface removal -# check_ports_down -# Port_Binding(chassis) not always removed on Interface removal -# check_ports_unbound +check_ports_down +check_ports_unbound add_ovs_interfaces check ovn-nbctl --wait=hv sync