From patchwork Thu Mar 30 07:52:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1763082 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=2605:bc80:3010::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=LJ47bxN3; dkim-atps=neutral Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::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 4PnFyt5Kd0z1yYb for ; Thu, 30 Mar 2023 18:53:10 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 627EB615DC; Thu, 30 Mar 2023 07:53:08 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 627EB615DC 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=LJ47bxN3 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 7TO4blmikbSI; Thu, 30 Mar 2023 07:53:07 +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 53904615D0; Thu, 30 Mar 2023 07:53:06 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 53904615D0 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1CD3DC0037; Thu, 30 Mar 2023 07:53:06 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id D84E8C002F for ; Thu, 30 Mar 2023 07:53:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id AA979842AC for ; Thu, 30 Mar 2023 07:53:04 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org AA979842AC Authentication-Results: smtp1.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=LJ47bxN3 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0h0awi8s03au for ; Thu, 30 Mar 2023 07:53:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 733BE841D0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 733BE841D0 for ; Thu, 30 Mar 2023 07:53:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680162782; 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; bh=zr24vDPOzYXLc24j+6clED/V9CKR3w8aNpHYpVBGmgc=; b=LJ47bxN3z76r/ESTadoEdmhMVgzijdi9O5YBRZKRMDBhXSoFEG3lit1q00gik2FmMOLzrA XCygulCerN8lCg8EOblhy+yleVtP8DsMpeS2UlVPNxdZouR9t+eBvmcLpP1RpYXBm9tiC6 2B+TNzceUY4hFqxMuwsJbCGFq1upLaw= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-513-oArjbrhVMvylxmtvDgap3A-1; Thu, 30 Mar 2023 03:53:00 -0400 X-MC-Unique: oArjbrhVMvylxmtvDgap3A-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 486AE889047 for ; Thu, 30 Mar 2023 07:53:00 +0000 (UTC) Received: from amusil.. (unknown [10.34.131.44]) by smtp.corp.redhat.com (Postfix) with ESMTP id C6A52492B00; Thu, 30 Mar 2023 07:52:59 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Thu, 30 Mar 2023 09:52:59 +0200 Message-Id: <20230330075259.371472-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] controller: Clear tunnels from old integration bridge 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" After integration bridge change the tunnels would stay on the old bridge preventing new tunnels creation and disrupting traffic. Detect the bridge change and clear the tunnels from the old integration bridge. Reported-at: https://bugzilla.redhat.com/2173635 Signed-off-by: Ales Musil --- controller/encaps.c | 36 +++++++++++++++- controller/encaps.h | 4 +- controller/ovn-controller.c | 26 +++++++++++- tests/ovn.at | 83 +++++++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 4 deletions(-) diff --git a/controller/encaps.c b/controller/encaps.c index 2662eaf98..c66743d3b 100644 --- a/controller/encaps.c +++ b/controller/encaps.c @@ -386,6 +386,20 @@ chassis_tzones_overlap(const struct sset *transport_zones, return false; } +static void +clear_old_tunnels(const struct ovsrec_bridge *old_br_int) +{ + for (size_t i = 0; i < old_br_int->n_ports; i++) { + const struct ovsrec_port *port = old_br_int->ports[i]; + const char *id = smap_get(&port->external_ids, "ovn-chassis-id"); + if (id) { + VLOG_DBG("Clearing old tunnel port \"%s\" (%s) from bridge " + "\"%s\".", port->name, id, old_br_int->name); + ovsrec_bridge_update_ports_delvalue(old_br_int, port); + } + } +} + void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge *br_int, @@ -393,12 +407,32 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_chassis *this_chassis, const struct sbrec_sb_global *sbg, const struct ovsrec_open_vswitch_table *ovs_table, - const struct sset *transport_zones) + const struct sset *transport_zones, + const struct ovsrec_bridge_table *bridge_table, + const char *br_int_name) { if (!ovs_idl_txn || !br_int) { return; } + if (!br_int_name) { + /* The controller has just started, we need to look through all + * bridges for old tunnel ports. */ + const struct ovsrec_bridge *br; + OVSREC_BRIDGE_TABLE_FOR_EACH (br, bridge_table) { + if (!strcmp(br->name, br_int->name)) { + continue; + } + clear_old_tunnels(br); + } + } else if (strcmp(br_int_name, br_int->name)) { + /* The integration bridge was changed, clear tunnel ports from + * the old one. */ + const struct ovsrec_bridge *old_br_int = + get_bridge(bridge_table, br_int_name); + clear_old_tunnels(old_br_int); + } + const struct sbrec_chassis *chassis_rec; struct tunnel_ctx tc = { diff --git a/controller/encaps.h b/controller/encaps.h index 867c6f28c..cf38dac1a 100644 --- a/controller/encaps.h +++ b/controller/encaps.h @@ -35,7 +35,9 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct sbrec_chassis *, const struct sbrec_sb_global *, const struct ovsrec_open_vswitch_table *, - const struct sset *transport_zones); + const struct sset *transport_zones, + const struct ovsrec_bridge_table *bridge_table, + const char *br_int_name); bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge *br_int); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 76be2426e..242d93823 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -536,6 +536,21 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn, *br_int_ = br_int; } +static void +consider_br_int_change(const struct ovsrec_bridge *br_int, char **current_name) +{ + ovs_assert(current_name); + + if (!*current_name) { + *current_name = xstrdup(br_int->name); + } + + if (strcmp(*current_name, br_int->name)) { + free(*current_name); + *current_name = xstrdup(br_int->name); + } +} + static void update_ssl_config(const struct ovsrec_ssl_table *ssl_table) { @@ -4918,6 +4933,8 @@ main(int argc, char *argv[]) char *ovn_version = ovn_get_internal_version(); VLOG_INFO("OVN internal version is : [%s]", ovn_version); + char *current_br_int_name = NULL; + /* Main loop. */ exiting = false; restart = false; @@ -5070,7 +5087,9 @@ main(int argc, char *argv[]) chassis, sbrec_sb_global_first(ovnsb_idl_loop.idl), ovs_table, - &transport_zones); + &transport_zones, + bridge_table, + current_br_int_name); stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME, time_msec()); @@ -5257,7 +5276,10 @@ main(int argc, char *argv[]) stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME, time_msec()); } - + /* The name needs to be reflected at the end of the block. + * This allows us to detect br-int changes and act + * accordingly. */ + consider_br_int_change(br_int, ¤t_br_int_name); } if (!engine_has_run()) { diff --git a/tests/ovn.at b/tests/ovn.at index a892691ca..8ca2e2a0c 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -35179,3 +35179,86 @@ AT_CHECK([test "$(grep -c "Flushing CT for 5-tuple" hv1/ovn-controller.log)" = " OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([Re-create encap tunnels during integration bridge migration]) +ovn_start +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +sim_add hv2 +as hv2 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.2 + +check ovn-nbctl --wait=hv sync + +check_tunnel_port() { + local hv=$1 + local br=$2 + local id=$3 + + as $hv + OVS_WAIT_UNTIL([ + test "$(ovs-vsctl --format=table --no-headings find port external_ids:ovn-chassis-id="$id" | wc -l)" = "1" + ]) + local tunnel_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="$id") + AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" | grep -q "$tunnel_id"]) +} + +# Check that both chassis have tunnel +check_tunnel_port hv1 br-int hv2@192.168.0.2 +check_tunnel_port hv2 br-int hv1@192.168.0.1 + +# Stop ovn-controller on hv1 +check as hv1 ovn-appctl -t ovn-controller exit --restart + +# The tunnel should remain intact +check_tunnel_port hv1 br-int hv2@192.168.0.2 + +# Change the bridge to br-int1 on hv1 +as hv1 +check ovs-vsctl add-br br-int1 +check ovs-vsctl set open . external_ids:ovn-bridge="br-int1" +start_daemon ovn-controller --verbose="encaps:dbg" +check ovn-nbctl --wait=hv sync + +# Check that the tunnel was created on br-int1 instead +check_tunnel_port hv1 br-int1 hv2@192.168.0.2 +check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2) from bridge \"br-int\"" hv1/ovn-controller.log + +# Change the bridge to br-int1 on hv2 +as hv2 +check ovn-appctl vlog/set encaps:dbg +check ovs-vsctl add-br br-int1 +check ovs-vsctl set open . external_ids:ovn-bridge="br-int1" +check ovn-nbctl --wait=hv sync + + +# Check that the tunnel was created on br-int1 instead +check_tunnel_port hv2 br-int1 hv1@192.168.0.1 +check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1) from bridge \"br-int\"" hv2/ovn-controller.log + +# Stop ovn-controller on hv1 +check as hv1 ovn-appctl -t ovn-controller exit --restart + +# The tunnel should remain intact +check_tunnel_port hv1 br-int1 hv2@192.168.0.2 +prev_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2") + +# Start the controller again +start_daemon ovn-controller --verbose="encaps:dbg" +check ovn-nbctl --wait=hv sync +check_tunnel_port hv1 br-int1 hv2@192.168.0.2 +current_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2") + +# The tunnel should be the same after restart +check test "$current_id" = "$prev_id" + +OVN_CLEANUP([hv1],[hv2]) +AT_CLEANUP +])