From patchwork Thu Nov 30 16:53:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Simonart X-Patchwork-Id: 1870301 X-Patchwork-Delegate: dceara@redhat.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org 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=Cr6GBD9F; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Sh2Mn625Yz23mq for ; Fri, 1 Dec 2023 03:53:57 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 8F31B41E7D; Thu, 30 Nov 2023 16:53:55 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8F31B41E7D Authentication-Results: smtp4.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=Cr6GBD9F X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id x54GKlLCFlJo; Thu, 30 Nov 2023 16:53:54 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 8194D41ADF; Thu, 30 Nov 2023 16:53:53 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8194D41ADF Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 58FF7C0077; Thu, 30 Nov 2023 16:53:53 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 70994C0037 for ; Thu, 30 Nov 2023 16:53:51 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 3F3CC41ADF for ; Thu, 30 Nov 2023 16:53:51 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 3F3CC41ADF X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Hc-vH88fDy6a for ; Thu, 30 Nov 2023 16:53:50 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 5F42A41A3A for ; Thu, 30 Nov 2023 16:53:50 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 5F42A41A3A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701363229; 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=rzLFR4dGbJj770/PrSXjQfSK1KJtMGRKMXBPo+fr6zM=; b=Cr6GBD9FebxL6nnY+I6xum/EIAXySe+K3zMGoQ/8BJnrGsLhyRs+eleiZCPpUi3fDnTjZp 6fA38jWk5ysp5uW9zGwzBiNBqMpKVP2kzCNnltZtdBRjUGtU6ltiBjR7Mtk6TKfBLlafp5 0oTjNSfYttvZ1RCiTfCTgVGzNjKU9tw= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-578-a0-8KeP6PE6qDW9dXxz3Kg-1; Thu, 30 Nov 2023 11:53:45 -0500 X-MC-Unique: a0-8KeP6PE6qDW9dXxz3Kg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 74667811E7B for ; Thu, 30 Nov 2023 16:53:45 +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 54E8D1C060AE; Thu, 30 Nov 2023 16:53:45 +0000 (UTC) From: Xavier Simonart To: xsimonar@redhat.com, dev@openvswitch.org Date: Thu, 30 Nov 2023 17:53:44 +0100 Message-Id: <20231130165344.3240652-1-xsimonar@redhat.com> In-Reply-To: <20231129154521.313487-1-xsimonar@redhat.com> References: <20231129154521.313487-1-xsimonar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.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] controller: fix group_table and meter_table allocation 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" The group_table and meter_table are initialized in ovn-controller, with n_ids = 0. Then they are re-initialized in ofctrl, with proper number of n_ids, in state S_CLEAR_FLOWS. However, nothing prevented to start adding flows (by adding logical ports) using groups before ofctrl reaches this state. This was causing some wrong flows (missing group in the flow). With this patch, as soon as the feature rconn is available, i.e. before adding flows, those table are properly re-initialized. This issue is usually not visible in main and recent branches ci, since "Wait for new ovn-controllers to connect to Southbound." as this was slowing down the moment when the test started to add ports. This was causing the following test to fail: "ECMP static routes -- ovn-northd -- parallelization=yes -- ovn_monitor_all=yes". Fixes: 1d6d953bf883 ("controller: Don't artificially limit group and meter IDs to 16bit.") Signed-off-by: Xavier Simonart --- v2: - Updated based on Dumitru's feedback i.e. call ovn_extend_table_clear within ovn_extend_table_reinit and do not call ovn_extend_table_reinit anymore from ofctrl - Modified existing test case (ofctrl wait before clearing flows) to catch this error --- controller/ofctrl.c | 2 -- controller/ovn-controller.c | 8 ++++++++ lib/extend-table.c | 1 + tests/ovn-controller.at | 18 +++++++++++++++++- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index c6b1272ba..7aac0128b 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -675,13 +675,11 @@ run_S_CLEAR_FLOWS(void) /* Clear existing groups, to match the state of the switch. */ if (groups) { ovn_extend_table_clear(groups, true); - ovn_extend_table_reinit(groups, ovs_feature_max_select_groups_get()); } /* Clear existing meters, to match the state of the switch. */ if (meters) { ovn_extend_table_clear(meters, true); - ovn_extend_table_reinit(meters, ovs_feature_max_meters_get()); ofctrl_meter_bands_clear(); } diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 44605eb4e..98d95b1f3 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -5654,6 +5654,14 @@ main(int argc, char *argv[]) br_int ? br_int->name : NULL)) { VLOG_INFO("OVS feature set changed, force recompute."); engine_set_force_recompute(true); + if (ovs_feature_set_discovered()) { + lflow_output_data = + engine_get_internal_data(&en_lflow_output); + ovn_extend_table_reinit(&lflow_output_data->group_table, + ovs_feature_max_select_groups_get()); + ovn_extend_table_reinit(&lflow_output_data->meter_table, + ovs_feature_max_meters_get()); + } } if (br_int && ovs_feature_set_discovered()) { diff --git a/lib/extend-table.c b/lib/extend-table.c index 7f2358778..87828772c 100644 --- a/lib/extend-table.c +++ b/lib/extend-table.c @@ -47,6 +47,7 @@ ovn_extend_table_init(struct ovn_extend_table *table, const char *table_name, void ovn_extend_table_reinit(struct ovn_extend_table *table, uint32_t n_ids) { + ovn_extend_table_clear(table, true); if (n_ids != table->n_ids) { id_pool_destroy(table->table_ids); table->table_ids = id_pool_create(1, n_ids); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 5acb380db..1f826b21c 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2202,6 +2202,10 @@ OVS_APP_EXIT_AND_WAIT([ovn-controller]) # The old OVS flows should remain (this is regardless of the configuration) AT_CHECK([ovs-ofctl dump-flows br-int | grep 10.1.2.3], [0], [ignore]) +# We should have 2 flows with groups +AT_CHECK([ovs-ofctl dump-flows br-int | grep group | wc -l], [0], [2 +]) + # Make a change to the ls1-lp1's IP check ovn-nbctl --wait=sb lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01 10.1.2.4" @@ -2214,10 +2218,18 @@ sleep 2 lflow_run_1=$(ovn-appctl -t ovn-controller coverage/read-counter lflow_run) AT_CHECK([ovs-ofctl dump-flows br-int | grep 10.1.2.3], [0], [ignore]) +# We should have 2 flows with groups +AT_CHECK([ovs-ofctl dump-flows br-int | grep group | wc -l], [0], [2 +]) + sleep 5 # Check after the wait OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 10.1.2.4]) + +# We should have 2 flows with groups +AT_CHECK([ovs-ofctl dump-flows br-int | grep group | wc -l], [0], [2 +]) lflow_run_2=$(ovn-appctl -t ovn-controller coverage/read-counter lflow_run) # Verify that the flow compute completed during the wait (after the wait it @@ -2230,7 +2242,7 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif -vunixctl OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 10.1.2.4]) -check ovn-nbctl --wait=hv lb-add lb3 2.2.2.2 10.1.2.5 \ +check ovn-nbctl --wait=hv lb-add lb3 3.3.3.3 10.1.2.5 \ -- ls-lb-add ls1 lb3 # There should be 3 group IDs allocated (this is to ensure the group ID @@ -2238,6 +2250,10 @@ check ovn-nbctl --wait=hv lb-add lb3 2.2.2.2 10.1.2.5 \ AT_CHECK([ovn-appctl -t ovn-controller group-table-list | awk '{print $2}' | sort | uniq | wc -l], [0], [3 ]) +# We should have 3 flows with groups +AT_CHECK([ovs-ofctl dump-flows br-int | grep group | wc -l], [0], [3 +]) + OVN_CLEANUP([hv1]) AT_CLEANUP