From patchwork Thu Aug 25 14:47:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Simonart X-Patchwork-Id: 1670296 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::138; helo=smtp1.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=Yiqj8aBk; dkim-atps=neutral Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (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 4MD5Qv5HbRz1yhC for ; Fri, 26 Aug 2022 00:47:19 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C237283E36; Thu, 25 Aug 2022 14:47:17 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org C237283E36 Authentication-Results: smtp1.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=Yiqj8aBk 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 nsmBx0eaC6bQ; Thu, 25 Aug 2022 14:47:16 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 9C93982FA9; Thu, 25 Aug 2022 14:47:15 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 9C93982FA9 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 83D48C0032; Thu, 25 Aug 2022 14:47:15 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4DC92C002D for ; Thu, 25 Aug 2022 14:47:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 284CB60C2C for ; Thu, 25 Aug 2022 14:47:14 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 284CB60C2C Authentication-Results: smtp3.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Yiqj8aBk 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 A4Iite5nniUi for ; Thu, 25 Aug 2022 14:47:13 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 4260E60C12 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4260E60C12 for ; Thu, 25 Aug 2022 14:47:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661438831; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=v33J9EORbxCDsRdOt9PgGOy8KT8Uv0fXVV5wNFWxuKQ=; b=Yiqj8aBkIQZWiZjUCcFJTXsZZoqOcIb5AZzQaSUtKBmIb1J0ZZBUPklc+Pip7RRyi3TS12 r5y3qUtZPprWa+MF8Gey6x5clW3GaVOeESuQ2MooUDpjQpTQdOkeKgQ+4mON5Uhq4hHgFW mx6bx2xR7k0gHu2Ho0Zx5deiYtpQ2W4= 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-5-JRIQXSZuOnumLPTb9RVuiQ-1; Thu, 25 Aug 2022 10:47:10 -0400 X-MC-Unique: JRIQXSZuOnumLPTb9RVuiQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 59F4E8032F6 for ; Thu, 25 Aug 2022 14:47: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 422762026D4C; Thu, 25 Aug 2022 14:47:10 +0000 (UTC) From: Xavier Simonart To: xsimonar@redhat.com, dev@openvswitch.org Date: Thu, 25 Aug 2022 10:47:10 -0400 Message-Id: <20220825144710.4149344-1-xsimonar@redhat.com> In-Reply-To: <20220823104415.2197632-1-xsimonar@redhat.com> References: <20220823104415.2197632-1-xsimonar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2] controller: fix potential segmentation violation when removing ports 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" If a logical switch port is added and connected to a logical router port (through options: router-port) before the router port is created, then this might cause further issues such as segmentation violation when the switch and router ports are deleted. Signed-off-by: Xavier Simonart --- v2: handled Han's comments (avoid wasting CPU cycles searching for peer_ld) --- controller/local_data.c | 36 ++++++++++++++---------------------- tests/ovn.at | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/controller/local_data.c b/controller/local_data.c index 7f874fc19..669e686ab 100644 --- a/controller/local_data.c +++ b/controller/local_data.c @@ -34,7 +34,7 @@ VLOG_DEFINE_THIS_MODULE(ldata); -static void add_local_datapath__( +static struct local_datapath *add_local_datapath__( struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, @@ -194,17 +194,7 @@ add_local_datapath_peer_port( return; } - bool present = false; - for (size_t i = 0; i < ld->n_peer_ports; i++) { - if (ld->peer_ports[i].local == pb) { - present = true; - break; - } - } - - if (!present) { - local_datapath_peer_port_add(ld, pb, peer); - } + local_datapath_peer_port_add(ld, pb, peer); struct local_datapath *peer_ld = get_local_datapath(local_datapaths, @@ -218,12 +208,6 @@ add_local_datapath_peer_port( return; } - for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { - if (peer_ld->peer_ports[i].local == peer) { - return; - } - } - local_datapath_peer_port_add(peer_ld, peer, pb); } @@ -541,7 +525,7 @@ chassis_tunnel_find(const struct hmap *chassis_tunnels, const char *chassis_id, } /* static functions. */ -static void +static struct local_datapath * add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, @@ -553,7 +537,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, uint32_t dp_key = dp->tunnel_key; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); if (ld) { - return; + return ld; } ld = local_datapath_alloc(dp); @@ -568,7 +552,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, if (depth >= 100) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "datapaths nested too deep"); - return; + return ld; } struct sbrec_port_binding *target = @@ -589,12 +573,14 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, if (peer && peer->datapath) { if (need_add_patch_peer_to_local( sbrec_port_binding_by_name, pb, chassis)) { - add_local_datapath__(sbrec_datapath_binding_by_key, + struct local_datapath *peer_ld = + add_local_datapath__(sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, depth + 1, peer->datapath, chassis, local_datapaths, tracked_datapaths); + local_datapath_peer_port_add(peer_ld, peer, pb); } local_datapath_peer_port_add(ld, pb, peer); } @@ -602,6 +588,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, } } sbrec_port_binding_index_destroy_row(target); + return ld; } static struct tracked_datapath * @@ -622,6 +609,11 @@ local_datapath_peer_port_add(struct local_datapath *ld, const struct sbrec_port_binding *local, const struct sbrec_port_binding *remote) { + for (size_t i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == local) { + return; + } + } ld->n_peer_ports++; if (ld->n_peer_ports > ld->n_allocated_peer_ports) { size_t old_n_ports = ld->n_allocated_peer_ports; diff --git a/tests/ovn.at b/tests/ovn.at index bba2c9c1d..ae0918d55 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -32580,3 +32580,33 @@ check ovn-nbctl --wait=hv sync OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([router port add then remove - lsp first]) +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 +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +check ovn-nbctl ls-add sw0 +check ovn-nbctl lr-add ro0 +check ovn-nbctl lsp-add sw0 sw0-p1 +check ovn-nbctl lsp-add sw0 lsp +check ovn-nbctl lsp-set-type lsp router +check ovn-nbctl lsp-set-options lsp router-port=lrp +check ovn-nbctl lsp-set-addresses lsp 00:00:00:00:00:1 +check ovn-nbctl --wait=hv lrp-add ro0 lrp 00:00:00:00:00:1 aef0:0:0:0:0:0:0:1/64 +check ovn-nbctl --wait=hv lsp-del lsp +check ovn-nbctl lrp-del lrp +check ovn-nbctl --wait=hv sync +OVN_CLEANUP([hv1]) +AT_CLEANUP +])