From patchwork Thu Feb 18 18:31:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441803 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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: 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=L1dqAaoB; 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DhNcl5070z9sCD for ; Fri, 19 Feb 2021 05:33:11 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id C686A60684 for ; Thu, 18 Feb 2021 18:33:08 +0000 (UTC) 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 E4VfT-L2Qk0c for ; Thu, 18 Feb 2021 18:33:05 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id 661B960699; Thu, 18 Feb 2021 18:33:05 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id C14A860662; Thu, 18 Feb 2021 18:32:54 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 82A41C000F; Thu, 18 Feb 2021 18:32:54 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7DB57C000D for ; Thu, 18 Feb 2021 18:32:53 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 5533160661 for ; Thu, 18 Feb 2021 18:32:53 +0000 (UTC) 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 ec7xoW3YgI5d for ; Thu, 18 Feb 2021 18:32:52 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id 6639C60663; Thu, 18 Feb 2021 18:32:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id A6BAB6064C for ; Thu, 18 Feb 2021 18:32:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1613673169; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type; bh=HbDXbTtdnpH+ZBx5ixfR+9lHirwJEcGhlFnfZ+/4hEg=; b=L1dqAaoBbi5vUav7vxrxXqGAgh/6BNVSjVVXthhaV02X8KXDf45Sdzfh+A2BbRzXYrpCfd nkLsSpWZIN1jjopzaYwYvaYcw2k6NVIq5e2v7XERJVPyoxSslrZe4Bm3CbQxy/E4YaqGmX 7yFuyFxYqSykVlcDKzyWikm1Nsv3n9c= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-32-E8u_6Lt1OpuHuYFdxJec-A-1; Thu, 18 Feb 2021 13:31:37 -0500 X-MC-Unique: E8u_6Lt1OpuHuYFdxJec-A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8F2FD801965; Thu, 18 Feb 2021 18:31:36 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-27.ams2.redhat.com [10.36.114.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id AB5546A03D; Thu, 18 Feb 2021 18:31:35 +0000 (UTC) From: Dumitru Ceara To: ovs-dev@openvswitch.org Date: Thu, 18 Feb 2021 19:31:32 +0100 Message-Id: <1613673092-14093-1-git-send-email-dceara@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: dceara@redhat.com Subject: [ovs-dev] [RFC PATCH ovn] binding: Do not try to incrementally process former VIF updates. 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" If a Port_Binding corresponding to a VIF changes in the Southbound to represent a port that cannot be bound locally (e.g., localport, localnet, patch) then ovn-controller should not try to process this update incrementally. If it would try, it would need to first remove cleanup all previous lbindings from memory which is not trivial. Failing to do so might cause ovn-controller to access already freed memory later on. However, because this is an unlikely scenario, to avoid over complicating the incremental processing engine, we now rely to a regular recompute when such Port_Binding updates are detected. This implies though that when the runtime_data I-P node is updated we also need to recompute physical flows to make sure that flows for specific types of ports, e.g., localports, are properly installed. To reproduce the issue, this patch also modifies the "ovn -- 2 HVs, 1 lport/HV, localport ports" test. More tests are added for all types of port bindings that shouldn't be claimed locally. Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data.") Signed-off-by: Dumitru Ceara --- Note: sending this patch as RFC for now because I'm not completely sure if the physical_run() call in the I-P engine can be avoided. --- controller/binding.c | 32 ++++++++++++++++++++++++ controller/ovn-controller.c | 2 ++ tests/ovn.at | 59 ++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index efaa109..9ce5c01 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1027,6 +1027,17 @@ is_lbinding_container_parent(struct local_binding *lbinding) } static bool +is_non_local_port_bound(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type) +{ + if (lport_type == LP_PATCH || lport_type == LP_LOCALNET || + lport_type == LP_LOCALPORT) { + return !!pb->chassis; + } + return false; +} + +static bool release_local_binding_children(const struct sbrec_chassis *chassis_rec, struct local_binding *lbinding, bool sb_readonly, @@ -1486,9 +1497,17 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) b_ctx_in->port_binding_table) { enum en_lport_type lport_type = get_lport_type(pb); + if (is_non_local_port_bound(pb, lport_type)) { + release_lport(pb, !b_ctx_in->ovnsb_idl_txn, NULL); + } + switch (lport_type) { case LP_PATCH: + update_local_lport_ids(pb, b_ctx_out); + break; case LP_LOCALPORT: + update_local_lport_ids(pb, b_ctx_out); + break; case LP_VTEP: update_local_lport_ids(pb, b_ctx_out); break; @@ -1818,6 +1837,12 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec, } if (lbinding->pb) { + enum en_lport_type lport_type = get_lport_type(lbinding->pb); + + if (lport_type != LP_VIF && lport_type != LP_VIRTUAL) { + return true; + } + if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out, lbinding, qos_map)) { return false; @@ -2419,6 +2444,13 @@ delete_done: break; } + /* Former VIFs that changed to non-local ports should be released, + * but we cannot handle this incrementally. + */ + if (is_non_local_port_bound(pb, lport_type)) { + handled = false; + } + if (!handled) { break; } diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 4343650..15c78db 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2306,6 +2306,8 @@ flow_output_runtime_data_handler(struct engine_node *node, struct physical_ctx p_ctx; init_physical_ctx(node, rt_data, &p_ctx); + physical_run(&p_ctx, &fo->flow_table); + struct tracked_binding_datapath *tdp; HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { if (tdp->is_new) { diff --git a/tests/ovn.at b/tests/ovn.at index 344e6bf..b3a8b03 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -11319,11 +11319,6 @@ ovn_start ovn-nbctl ls-add ls1 -# Add localport to the switch -ovn-nbctl lsp-add ls1 lp01 -ovn-nbctl lsp-set-addresses lp01 f0:00:00:00:00:01 -ovn-nbctl lsp-set-type lp01 localport - net_add n1 for i in 1 2; do @@ -11350,10 +11345,17 @@ for i in 1 2; do OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp${i}1` = xup]) done +# Add localport to the switch +ovn-nbctl lsp-add ls1 lp01 +ovn-nbctl lsp-set-addresses lp01 f0:00:00:00:00:01 +ovn-nbctl lsp-set-type lp01 localport + wait_for_ports_up -ovn-nbctl --wait=sb sync +ovn-nbctl --wait=hv sync ovn-sbctl dump-flows +check_column "" Port_Binding chassis logical_port=lp01 + OVN_POPULATE_ARP # Given the name of a logical port, prints the name of the hypervisor @@ -22606,6 +22608,51 @@ wait_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch OVN_CLEANUP([hv1]) AT_CLEANUP +# Test that OVS.external_ids:iface-id doesn't affect non-VIF port bindings. +AT_SETUP([ovn -- Non-VIF ports incremental processing]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.10 + +check ovn-nbctl ls-add ls1 -- lsp-add ls1 lsp1 + +as hv1 +check ovs-vsctl \ + -- add-port br-int vif1 \ + -- set Interface vif1 external_ids:iface-id=lsp1 + +# ovn-controller should bind the interface. +wait_for_ports_up +hv_uuid=$(fetch_column Chassis _uuid name=hv1) +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 + +# Change the port type to router, ovn-controller should release it. +check ovn-nbctl --wait=hv lsp-set-type lsp1 router +check_column "" Port_Binding chassis logical_port=lsp1 + +# Clear port type, ovn-controller should rebind it. +check ovn-nbctl --wait=hv lsp-set-type lsp1 '' +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 + +# Change the port type to localnet, ovn-controller should release it. +check ovn-nbctl --wait=hv lsp-set-type lsp1 localnet +check_column "" Port_Binding chassis logical_port=lsp1 + +# Clear port type, ovn-controller should rebind it. +check ovn-nbctl --wait=hv lsp-set-type lsp1 '' +check_column "$hv_uuid" Port_Binding chassis logical_port=lsp1 + +# Change the port type to localport, ovn-controller should release it. +check ovn-nbctl --wait=hv lsp-set-type lsp1 localport +check_column "" Port_Binding chassis logical_port=lsp1 + +OVN_CLEANUP([hv1]) +AT_CLEANUP + # Test dropping traffic destined to router owned IPs. AT_SETUP([ovn -- gateway router drop traffic for own IPs]) ovn_start