From patchwork Wed May 14 08:12:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 2085463 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=DUQLKVKR; 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 4Zy5gJ2R4wz1yPv for ; Wed, 14 May 2025 18:12:44 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 6285A414D3; Wed, 14 May 2025 08:12:57 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id G6z40umGH7he; Wed, 14 May 2025 08:12:56 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org EFE6A414A1 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=DUQLKVKR Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id EFE6A414A1; Wed, 14 May 2025 08:12:55 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D99ACC007B; Wed, 14 May 2025 08:12:55 +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 79135C08A9 for ; Wed, 14 May 2025 08:12:54 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 7720361060 for ; Wed, 14 May 2025 08:12:54 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id tLdQVyL5rDVY for ; Wed, 14 May 2025 08:12:53 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=amusil@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org 9E86161062 Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 9E86161062 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=DUQLKVKR 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 9E86161062 for ; Wed, 14 May 2025 08:12:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1747210372; 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=Ovc46vwyvEs9DMyj5Nf2VySGVNiWtgpNZR0bLpaUvj0=; b=DUQLKVKR1fcLc7Rc67newhXRfOLfTKb96+GhWb8lzdijRhZXpvNHUCC4kxST+/sJIbCkIw vfQnj2TEJ0hrBBJ9JZbannLEXApo+SkEDLqnpR62BxeOcafHc3F+/ZlHtUehbATwaVefrP rTZHwRSoO4LMdf42Ucc3lEXCUBIS/CY= Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-513-OXusdl9aPuCD89tcmQy8XQ-1; Wed, 14 May 2025 04:12:50 -0400 X-MC-Unique: OXusdl9aPuCD89tcmQy8XQ-1 X-Mimecast-MFC-AGG-ID: OXusdl9aPuCD89tcmQy8XQ_1747210369 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (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 mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id B1F0C195DE36 for ; Wed, 14 May 2025 08:12:49 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.21]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id C6B7B30002E5; Wed, 14 May 2025 08:12:48 +0000 (UTC) To: dev@openvswitch.org Date: Wed, 14 May 2025 10:12:44 +0200 Message-ID: <20250514081246.985906-2-amusil@redhat.com> In-Reply-To: <20250514081246.985906-1-amusil@redhat.com> References: <20250514081246.985906-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: _-PNMtfL2-LERDazIU3JU43KhWIw5oCxkanboZp2PzY_1747210369 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 1/3] northd: Avoid recompute of lflow from ACLs without meters. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Ales Musil via dev From: Ales Musil Reply-To: Ales Musil Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" The lflow has dependency on sync_meters node so any change in sync_meters will cause full lflow recompute. Add handler for ACLs into sync_meters node that will check if the ACL actually requires any meters. In case it does it will trigger recompute. However, in most cases the ACLs are without metering which should avoid expensive recomopute of lflow node. Signed-off-by: Ales Musil --- v2: Rebase on top of latest main. --- northd/en-meters.c | 24 ++++++++++++ northd/en-meters.h | 2 + northd/inc-proc-northd.c | 2 +- tests/ovn-northd.at | 80 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 105 insertions(+), 3 deletions(-) diff --git a/northd/en-meters.c b/northd/en-meters.c index a0352c34b..288134108 100644 --- a/northd/en-meters.c +++ b/northd/en-meters.c @@ -79,6 +79,30 @@ en_sync_meters_run(struct engine_node *node, void *data_) return EN_UPDATED; } +enum engine_input_handler_result +sync_meters_nb_acl_handler(struct engine_node *node, void *data OVS_UNUSED) +{ + const struct nbrec_acl_table *acl_table = + EN_OVSDB_GET(engine_get_input("NB_acl", node)); + + const struct nbrec_acl *nb_acl; + NBREC_ACL_TABLE_FOR_EACH_TRACKED (nb_acl, acl_table) { + /* New or deleted ACL with meter needs to be recomputed. */ + if ((nbrec_acl_is_new(nb_acl) || nbrec_acl_is_deleted(nb_acl)) && + (nb_acl->log || nb_acl->meter)) { + return EN_UNHANDLED; + } + + /* Addition or removal of meter requires recompute. */ + if (nbrec_acl_is_updated(nb_acl, NBREC_ACL_COL_LOG) || + nbrec_acl_is_updated(nb_acl, NBREC_ACL_COL_METER)) { + return EN_UNHANDLED; + } + } + + return EN_HANDLED_UNCHANGED; +} + const struct nbrec_meter* fair_meter_lookup_by_name(const struct shash *meter_groups, const char *meter_name) diff --git a/northd/en-meters.h b/northd/en-meters.h index e0ef07fad..e1deb5daf 100644 --- a/northd/en-meters.h +++ b/northd/en-meters.h @@ -29,6 +29,8 @@ struct sync_meters_data { void *en_sync_meters_init(struct engine_node *, struct engine_arg *); void en_sync_meters_cleanup(void *data); enum engine_node_state en_sync_meters_run(struct engine_node *, void *data); +enum engine_input_handler_result +sync_meters_nb_acl_handler(struct engine_node *, void *data); const struct nbrec_meter *fair_meter_lookup_by_name( const struct shash *meter_groups, diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index b1e4994a4..5905462ec 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -319,7 +319,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_group_ecmp_route, &en_learned_route_sync, group_ecmp_route_learned_route_change_handler); - engine_add_input(&en_sync_meters, &en_nb_acl, NULL); + engine_add_input(&en_sync_meters, &en_nb_acl, sync_meters_nb_acl_handler); engine_add_input(&en_sync_meters, &en_nb_meter, NULL); engine_add_input(&en_sync_meters, &en_sb_meter, NULL); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 69b75fe9d..39f27772b 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -11352,14 +11352,14 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb meter-add m drop 1 pktps check ovn-nbctl --wait=sb acl-add ls from-lport 1 1 allow dnl Only triggers recompute of the sync_meters and lflow nodes. -check_recompute_counter 0 2 2 +check_recompute_counter 0 2 1 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb meter-del m check ovn-nbctl --wait=sb acl-del ls dnl Only triggers recompute of the sync_meters and lflow nodes. -check_recompute_counter 0 2 2 +check_recompute_counter 0 2 1 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) AT_CLEANUP @@ -17290,3 +17290,79 @@ AT_CHECK([cat trace | grep output], [0], [dnl AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([ACL incremental processing]) +ovn_start + +check ovn-nbctl ls-add ls +check ovn-nbctl meter-add meter1 drop 10 kbps +check ovn-nbctl meter-add meter2 drop 20 kbps + +# Wait for sb to be connected before clearing stats. +check ovn-nbctl --wait=sb sync +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +AS_BOX([ACL with log]) +check ovn-nbctl --wait=sb --log acl-add ls from-lport 100 tcp drop +acl_id=$(fetch_column nb:Acl _uuid action=drop) +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb set acl $acl_id match=udp +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb set acl $acl_id log=false +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check ovn-nbctl --wait=sb set acl $acl_id log=true +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb acl-del ls +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +AS_BOX([ACL with meter]) +check ovn-nbctl --wait=sb --meter=meter1 acl-add ls from-lport 100 tcp drop +acl_id=$(fetch_column nb:Acl _uuid action=drop) +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb set acl $acl_id match=udp +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb set acl $acl_id meter=meter2 +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb acl-del ls +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +AT_CLEANUP +]) From patchwork Wed May 14 08:12:45 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 2085464 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=NiHQy75I; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (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 4Zy5gJ67Y3z1yZ2 for ; Wed, 14 May 2025 18:12:44 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 75FD541FF8; Wed, 14 May 2025 08:12:58 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 1N7G_eSJCNRE; Wed, 14 May 2025 08:12:57 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 28ACB41FDA Authentication-Results: smtp2.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=NiHQy75I Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 28ACB41FDA; Wed, 14 May 2025 08:12:57 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 03A18C08A9; Wed, 14 May 2025 08:12:57 +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 543F7C0009 for ; Wed, 14 May 2025 08:12:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 46BF840BB1 for ; Wed, 14 May 2025 08:12:55 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id Ax7yFgfVmmrW for ; Wed, 14 May 2025 08:12:54 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.133.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=amusil@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp2.osuosl.org 46C04409B0 Authentication-Results: smtp2.osuosl.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 46C04409B0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp2.osuosl.org (Postfix) with ESMTPS id 46C04409B0 for ; Wed, 14 May 2025 08:12:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1747210373; 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=skkYtXFxfW6NTRiPwZHaMOdIIUZHVdg/tZkPufSls40=; b=NiHQy75Ig1ZWgLCzJEzzszg0p5r6QbwEEbiICtyFrh2awjUkytjeNtKFU2nleY5U+AOUFI i5uT8HsYOP6mHbYu9YPWTZQXVqh6TiRV/LRyxE6PMksbXGLvdW+nID/v/uaKkKbmubuNUr QHYbs9qJDx+6wORg2slRHz4aeKStGO0= Received: from mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-464-EJwrBTXWPSqPV34Pdnv-kA-1; Wed, 14 May 2025 04:12:51 -0400 X-MC-Unique: EJwrBTXWPSqPV34Pdnv-kA-1 X-Mimecast-MFC-AGG-ID: EJwrBTXWPSqPV34Pdnv-kA_1747210371 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (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 mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 181AB1800374 for ; Wed, 14 May 2025 08:12:51 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.21]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 2E62130002E3; Wed, 14 May 2025 08:12:49 +0000 (UTC) To: dev@openvswitch.org Date: Wed, 14 May 2025 10:12:45 +0200 Message-ID: <20250514081246.985906-3-amusil@redhat.com> In-Reply-To: <20250514081246.985906-1-amusil@redhat.com> References: <20250514081246.985906-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 9GCsuZTzRZ1_RAmkTca59bJCR5msMRcCTb20p_V-HRE_1747210371 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 2/3] northd: Handle port group ACL changes incrementally. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Ales Musil via dev From: Ales Musil Reply-To: Ales Musil Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Handle ACL changes in port group incrementally. The logical switch ACL changes are already handled, add similar method for port groups which should prevent lflow recomputes. There is still lflow dependency on ACL table that results in full lflow recompute. That will be solved in future commit. Signed-off-by: Ales Musil --- v2: Rebase on top of latest main. --- northd/en-northd.c | 24 ++++++++++++ northd/en-northd.h | 2 + northd/inc-proc-northd.c | 2 + northd/northd.c | 56 +++++++++++++++++++++++++++ northd/northd.h | 3 ++ tests/ovn-northd.at | 81 +++++++++++++++++++++++++++++++++++----- 6 files changed, 159 insertions(+), 9 deletions(-) diff --git a/northd/en-northd.c b/northd/en-northd.c index 3359d8d0e..0f1425ec7 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -78,6 +78,8 @@ northd_get_input_data(struct engine_node *node, EN_OVSDB_GET(engine_get_input("NB_mirror", node)); input_data->nbrec_mirror_rule_table = EN_OVSDB_GET(engine_get_input("NB_mirror_rule", node)); + input_data->nbrec_port_gorup_table = + EN_OVSDB_GET(engine_get_input("NB_port_group", node)); input_data->sbrec_datapath_binding_table = EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); @@ -238,6 +240,28 @@ northd_global_config_handler(struct engine_node *node, void *data OVS_UNUSED) return EN_HANDLED_UNCHANGED; } +enum engine_input_handler_result +northd_nb_port_group_handler(struct engine_node *node, void *data) +{ + struct northd_data *nd = data; + + struct northd_input input_data; + northd_get_input_data(node, &input_data); + + /* This handler cares only about ACLs, the port group itself has separate + * node. */ + if (!northd_handle_pgs_acl_changes(&input_data, nd)) { + return EN_UNHANDLED; + } + + if (northd_has_tracked_data(&nd->trk_data)) { + return EN_HANDLED_UPDATED; + } + + return EN_HANDLED_UNCHANGED; +} + + enum engine_input_handler_result route_policies_northd_change_handler(struct engine_node *node, void *data OVS_UNUSED) diff --git a/northd/en-northd.h b/northd/en-northd.h index b19b73270..4f744a6c5 100644 --- a/northd/en-northd.h +++ b/northd/en-northd.h @@ -26,6 +26,8 @@ northd_sb_port_binding_handler(struct engine_node *, void *data); enum engine_input_handler_result northd_lb_data_handler(struct engine_node *, void *data); enum engine_input_handler_result +northd_nb_port_group_handler(struct engine_node *node, void *data); +enum engine_input_handler_result northd_sb_fdb_change_handler(struct engine_node *node, void *data); void *en_routes_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED); diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 5905462ec..be2f36e33 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -246,6 +246,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_nb_logical_router, northd_nb_logical_router_handler); engine_add_input(&en_northd, &en_lb_data, northd_lb_data_handler); + engine_add_input(&en_northd, &en_nb_port_group, + northd_nb_port_group_handler); engine_add_input(&en_lr_nat, &en_northd, lr_nat_northd_handler); diff --git a/northd/northd.c b/northd/northd.c index 7b05147b4..2e8e7e63f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5192,6 +5192,62 @@ fail: return false; } +static bool +is_pg_acls_changed(const struct nbrec_port_group *npg) { + + return (nbrec_port_group_is_updated(npg, NBREC_PORT_GROUP_COL_ACLS) + || is_acls_seqno_changed(npg->acls, npg->n_acls)); +} + +bool +northd_handle_pgs_acl_changes(const struct northd_input *ni, + struct northd_data *nd) +{ + const struct nbrec_port_group *nb_pg; + struct northd_tracked_data *trk_data = &nd->trk_data; + + NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, + ni->nbrec_port_gorup_table) { + if (!is_pg_acls_changed(nb_pg)) { + continue; + } + + for (size_t i = 0; i < nb_pg->n_ports; i++) { + const char *port_name = nb_pg->ports[i]->name; + const struct ovn_datapath *od = + northd_get_datapath_for_port(&nd->ls_ports, port_name); + + if (!od) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_ERR_RL(&rl, "lport %s in port group %s not found.", + port_name, nb_pg->name); + goto fail; + } + + if (!od->nbs) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "lport %s in port group %s has no lswitch.", + nb_pg->ports[i]->name, + nb_pg->name); + goto fail; + } + + hmapx_add(&trk_data->ls_with_changed_acls, + CONST_CAST(struct ovn_datapath *, od)); + } + } + + if (!hmapx_is_empty(&trk_data->ls_with_changed_acls)) { + trk_data->type |= NORTHD_TRACKED_LS_ACLS; + } + + return true; + +fail: + destroy_northd_data_tracked_changes(nd); + return false; +} + /* Returns true if the logical router has changes which can be * incrementally handled. * Presently supports i-p for the below changes: diff --git a/northd/northd.h b/northd/northd.h index 5a698458f..55155f7be 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -38,6 +38,7 @@ struct northd_input { *nbrec_chassis_template_var_table; const struct nbrec_mirror_table *nbrec_mirror_table; const struct nbrec_mirror_rule_table *nbrec_mirror_rule_table; + const struct nbrec_port_group_table *nbrec_port_gorup_table; /* Southbound table references */ const struct sbrec_datapath_binding_table *sbrec_datapath_binding_table; @@ -829,6 +830,8 @@ bool northd_handle_ls_changes(struct ovsdb_idl_txn *, struct northd_data *); bool northd_handle_lr_changes(const struct northd_input *, struct northd_data *); +bool northd_handle_pgs_acl_changes(const struct northd_input *ni, + struct northd_data *nd); void destroy_northd_data_tracked_changes(struct northd_data *); void northd_destroy(struct northd_data *data); void northd_init(struct northd_data *data); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 39f27772b..6d0f8f86f 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -10095,7 +10095,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl The port_group node recomputes every time a NB port group is added/deleted. @@ -10132,7 +10132,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl The port_group node recomputes also every time a port from a new switch @@ -10168,7 +10168,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl The port_group node recomputes also every time a port from a new switch @@ -10205,7 +10205,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl We did not change the set of switches a pg is applied to, there should be @@ -10243,7 +10243,7 @@ dnl though, therefore "compute: 1". AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl We did not change the set of switches a pg is applied to, there should be @@ -10279,7 +10279,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl We changed the set of switches a pg is applied to, there should be @@ -10316,7 +10316,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl We changed the set of switches a pg is applied to, there should be @@ -10353,7 +10353,7 @@ dnl The northd node should not recompute. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl We changed the set of switches a pg is applied to, there should be a @@ -10392,7 +10392,7 @@ dnl The northd node should not recompute,. AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd], [0], [dnl Node: northd - recompute: 0 -- compute: 0 +- compute: 1 - cancel: 0 ]) dnl We changed the set of switches a pg is applied to, there should be a @@ -17296,6 +17296,9 @@ AT_SETUP([ACL incremental processing]) ovn_start check ovn-nbctl ls-add ls +check ovn-nbctl lsp-add ls lsp0 +check ovn-nbctl pg-add pg lsp0 + check ovn-nbctl meter-add meter1 drop 10 kbps check ovn-nbctl meter-add meter2 drop 20 kbps @@ -17364,5 +17367,65 @@ check_engine_stats sync_meters recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +AS_BOX([ACLs attached to LS]) +check ovn-nbctl --wait=sb acl-add ls from-lport 100 tcp drop +acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop) +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb acl-add ls from-lport 101 ip4 drop +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb set acl $acl_id match=udp +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb acl-del ls +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +AS_BOX([ACLs attached to PG]) +check ovn-nbctl --wait=sb acl-add pg from-lport 100 tcp drop +acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop) +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb acl-add pg from-lport 101 ip4 drop +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb set acl $acl_id match=udp +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + +check ovn-nbctl --wait=sb acl-del pg +check_engine_stats northd norecompute compute +check_engine_stats lflow recompute nocompute +check_engine_stats sync_meters norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + AT_CLEANUP ]) From patchwork Wed May 14 08:12:46 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 2085467 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=crzfXvb7; dkim-atps=neutral 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=patchwork.ozlabs.org) 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 (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Zy5gN4c0gz1yPv for ; Wed, 14 May 2025 18:12:48 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 7D1F3835C8; Wed, 14 May 2025 08:13:02 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id XTyE6IH6UKw6; Wed, 14 May 2025 08:12:59 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 50AC683388 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=crzfXvb7 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 50AC683388; Wed, 14 May 2025 08:12:59 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id F19F5C007B; Wed, 14 May 2025 08:12:58 +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 B6CD9C0A47 for ; Wed, 14 May 2025 08:12:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 9E1326107E for ; Wed, 14 May 2025 08:12:57 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id Oo3sLolUP-TB for ; Wed, 14 May 2025 08:12:56 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=amusil@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org 19A3161062 Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 19A3161062 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=crzfXvb7 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 19A3161062 for ; Wed, 14 May 2025 08:12:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1747210375; 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=sdfhiiph4ZofBLufDOGogZG3VbK7wHiqwG7tUZ9lacM=; b=crzfXvb7GbS7o1+J/B4ORlqB2dslyTePMt2abSgrlCaHK8pD7pt7+BrihG5cnC/oGrFlAW xdq49Bawto3tdStQppROxNGQrg+g0ZcKbFnwfz7Iov3RmVVw/QeDT4r3sutH5BfNbdDblD 3vFj1sQtNquBQuMntbwCHEMenCPoTfc= Received: from mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-683-z0n-RtZoPkW-F-NDzn1h_A-1; Wed, 14 May 2025 04:12:53 -0400 X-MC-Unique: z0n-RtZoPkW-F-NDzn1h_A-1 X-Mimecast-MFC-AGG-ID: z0n-RtZoPkW-F-NDzn1h_A_1747210372 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (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 mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id AACBA1800368 for ; Wed, 14 May 2025 08:12:52 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.21]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 8951430001A1; Wed, 14 May 2025 08:12:51 +0000 (UTC) To: dev@openvswitch.org Date: Wed, 14 May 2025 10:12:46 +0200 Message-ID: <20250514081246.985906-4-amusil@redhat.com> In-Reply-To: <20250514081246.985906-1-amusil@redhat.com> References: <20250514081246.985906-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: DJQCPbVoFicQGG1Ii-jgDRG1IxXGLChmrKzbTTd8BzE_1747210372 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2 3/3] northd: Process ACL changes incrementally. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Ales Musil via dev From: Ales Musil Reply-To: Ales Musil Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Any change to ACL would result in full lflow recompute. Instead of that reprocess only the relevant logical switch. In order to get track the relation between ACL and LS from the ACL side we need to store all related ACL uuids in the ls_stateful_record. Reported-at: https://issues.redhat.com/browse/FDP-755 Signed-off-by: Ales Musil --- v2: Rebase on top of latest main. Optimize the related ACLs and flags computation further. optimization --- northd/en-ls-stateful.c | 178 +++++++++++++++------------------------ northd/en-ls-stateful.h | 6 ++ northd/inc-proc-northd.c | 2 +- tests/ovn-northd.at | 24 +++--- 4 files changed, 85 insertions(+), 125 deletions(-) diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c index b713b5bce..88c850916 100644 --- a/northd/en-ls-stateful.c +++ b/northd/en-ls-stateful.c @@ -65,20 +65,13 @@ static void ls_stateful_record_destroy(struct ls_stateful_record *); static void ls_stateful_record_init( struct ls_stateful_record *, const struct ovn_datapath *, - const struct ls_port_group *, - const struct ls_port_group_table *); -static void ls_stateful_record_reinit( - struct ls_stateful_record *, - const struct ovn_datapath *, - const struct ls_port_group *, const struct ls_port_group_table *); static bool ls_has_lb_vip(const struct ovn_datapath *); -static void ls_stateful_record_set_acl_flags( +static void ls_stateful_record_set_acls( struct ls_stateful_record *, const struct ovn_datapath *, - const struct ls_port_group *, const struct ls_port_group_table *); -static bool ls_stateful_record_set_acl_flags_(struct ls_stateful_record *, - struct nbrec_acl **, - size_t n_acls); + const struct ls_port_group_table *); +static void ls_stateful_record_set_acls_(struct ls_stateful_record *, + struct nbrec_acl **, size_t n_acls); static struct ls_stateful_input ls_stateful_get_input_data( struct engine_node *); @@ -148,30 +141,31 @@ ls_stateful_northd_handler(struct engine_node *node, void *data_) struct ed_type_ls_stateful *data = data_; struct hmapx_node *hmapx_node; - struct hmapx changed_stateful_od = HMAPX_INITIALIZER(&changed_stateful_od); HMAPX_FOR_EACH (hmapx_node, &nd_changes->ls_with_changed_lbs) { - hmapx_add(&changed_stateful_od, hmapx_node->data); - } + const struct ovn_datapath *od = hmapx_node->data; - HMAPX_FOR_EACH (hmapx_node, &nd_changes->ls_with_changed_acls) { - hmapx_add(&changed_stateful_od, hmapx_node->data); + struct ls_stateful_record *ls_stateful_rec = + ls_stateful_table_find_(&data->table, od->nbs); + ovs_assert(ls_stateful_rec); + ls_stateful_rec->has_lb_vip = ls_has_lb_vip(od); + + /* Add the ls_stateful_rec to the tracking data. */ + hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); } - HMAPX_FOR_EACH (hmapx_node, &changed_stateful_od) { + HMAPX_FOR_EACH (hmapx_node, &nd_changes->ls_with_changed_acls) { const struct ovn_datapath *od = hmapx_node->data; - struct ls_stateful_record *ls_stateful_rec = ls_stateful_table_find_( - &data->table, od->nbs); + struct ls_stateful_record *ls_stateful_rec = + ls_stateful_table_find_(&data->table, od->nbs); ovs_assert(ls_stateful_rec); - ls_stateful_record_reinit(ls_stateful_rec, od, NULL, - input_data.ls_port_groups); + ls_stateful_record_set_acls(ls_stateful_rec, od, + input_data.ls_port_groups); /* Add the ls_stateful_rec to the tracking data. */ hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); } - hmapx_destroy(&changed_stateful_od); - if (ls_stateful_has_tracked_data(&data->trk_data)) { return EN_HANDLED_UPDATED; } @@ -189,49 +183,47 @@ ls_stateful_port_group_handler(struct engine_node *node, void *data_) return EN_UNHANDLED; } - /* port_group engine node doesn't provide the tracking data yet. - * Loop through all the ls port groups and update the ls_stateful_rec. - * This is still better than returning false. */ - struct ls_stateful_input input_data = ls_stateful_get_input_data(node); struct ed_type_ls_stateful *data = data_; - const struct ls_port_group *ls_pg; + if (ls_stateful_has_tracked_data(&data->trk_data)) { + return EN_HANDLED_UPDATED; + } + return EN_HANDLED_UNCHANGED; +} - LS_PORT_GROUP_TABLE_FOR_EACH (ls_pg, input_data.ls_port_groups) { - struct ls_stateful_record *ls_stateful_rec = - ls_stateful_table_find_(&data->table, ls_pg->nbs); - ovs_assert(ls_stateful_rec); - const struct ovn_datapath *od = - ovn_datapaths_find_by_index(input_data.ls_datapaths, - ls_stateful_rec->ls_index); - bool had_stateful_acl = ls_stateful_rec->has_stateful_acl; - struct acl_tier old_max = ls_stateful_rec->max_acl_tier; - bool had_acls = ls_stateful_rec->has_acls; - bool modified = false; - - ls_stateful_record_reinit(ls_stateful_rec, od, ls_pg, - input_data.ls_port_groups); - - struct acl_tier new_max = ls_stateful_rec->max_acl_tier; - - /* Using memcmp for struct acl_tier is fine since there is no padding - * in the struct. However, if the structure is changed, the memcmp - * may need to be updated to compare individual struct fields. - */ - if ((had_stateful_acl != ls_stateful_rec->has_stateful_acl) - || (had_acls != ls_stateful_rec->has_acls) - || memcmp(&old_max, &new_max, sizeof(old_max))) { - modified = true; +enum engine_input_handler_result +ls_stateful_acl_handler(struct engine_node *node, void *data_) +{ + struct ed_type_ls_stateful *data = data_; + const struct nbrec_acl_table *nbrec_acl_table = + EN_OVSDB_GET(engine_get_input("NB_acl", node)); + + const struct nbrec_acl *acl; + NBREC_ACL_TABLE_FOR_EACH_TRACKED (acl, nbrec_acl_table) { + /* The creation and deletion is handled in relation to LS/PG rather + * than the ACL itself. */ + if (nbrec_acl_is_new(acl) || nbrec_acl_is_deleted(acl)) { + continue; } - if (modified) { - /* Add the ls_stateful_rec to the tracking data. */ - hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); + /* Meter changes are handled separately in the en_sync_meters node. */ + if (nbrec_acl_is_updated(acl, NBREC_ACL_COL_LOG) || + nbrec_acl_is_updated(acl, NBREC_ACL_COL_METER)) { + continue; + } + + struct ls_stateful_record *ls_stateful_rec; + LS_STATEFUL_TABLE_FOR_EACH (ls_stateful_rec, &data->table) { + if (uuidset_contains(&ls_stateful_rec->related_acls, + &acl->header_.uuid)) { + hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); + } } } if (ls_stateful_has_tracked_data(&data->trk_data)) { return EN_HANDLED_UPDATED; } + return EN_HANDLED_UNCHANGED; } @@ -295,7 +287,8 @@ ls_stateful_record_create(struct ls_stateful_table *table, xzalloc(sizeof *ls_stateful_rec); ls_stateful_rec->ls_index = od->index; ls_stateful_rec->nbs_uuid = od->nbs->header_.uuid; - ls_stateful_record_init(ls_stateful_rec, od, NULL, ls_pgs); + uuidset_init(&ls_stateful_rec->related_acls); + ls_stateful_record_init(ls_stateful_rec, od, ls_pgs); ls_stateful_rec->lflow_ref = lflow_ref_create(); hmap_insert(&table->entries, &ls_stateful_rec->key_node, @@ -307,6 +300,7 @@ ls_stateful_record_create(struct ls_stateful_table *table, static void ls_stateful_record_destroy(struct ls_stateful_record *ls_stateful_rec) { + uuidset_destroy(&ls_stateful_rec->related_acls); lflow_ref_destroy(ls_stateful_rec->lflow_ref); free(ls_stateful_rec); } @@ -314,20 +308,10 @@ ls_stateful_record_destroy(struct ls_stateful_record *ls_stateful_rec) static void ls_stateful_record_init(struct ls_stateful_record *ls_stateful_rec, const struct ovn_datapath *od, - const struct ls_port_group *ls_pg, const struct ls_port_group_table *ls_pgs) { ls_stateful_rec->has_lb_vip = ls_has_lb_vip(od); - ls_stateful_record_set_acl_flags(ls_stateful_rec, od, ls_pg, ls_pgs); -} - -static void -ls_stateful_record_reinit(struct ls_stateful_record *ls_stateful_rec, - const struct ovn_datapath *od, - const struct ls_port_group *ls_pg, - const struct ls_port_group_table *ls_pgs) -{ - ls_stateful_record_init(ls_stateful_rec, od, ls_pg, ls_pgs); + ls_stateful_record_set_acls(ls_stateful_rec, od, ls_pgs); } static bool @@ -365,36 +349,28 @@ ls_has_lb_vip(const struct ovn_datapath *od) } static void -ls_stateful_record_set_acl_flags(struct ls_stateful_record *ls_stateful_rec, - const struct ovn_datapath *od, - const struct ls_port_group *ls_pg, - const struct ls_port_group_table *ls_pgs) +ls_stateful_record_set_acls(struct ls_stateful_record *ls_stateful_rec, + const struct ovn_datapath *od, + const struct ls_port_group_table *ls_pgs) { ls_stateful_rec->has_stateful_acl = false; memset(&ls_stateful_rec->max_acl_tier, 0, sizeof ls_stateful_rec->max_acl_tier); ls_stateful_rec->has_acls = false; + uuidset_clear(&ls_stateful_rec->related_acls); - if (ls_stateful_record_set_acl_flags_(ls_stateful_rec, od->nbs->acls, - od->nbs->n_acls)) { - return; - } - - if (!ls_pg) { - ls_pg = ls_port_group_table_find(ls_pgs, od->nbs); - } + ls_stateful_record_set_acls_(ls_stateful_rec, od->nbs->acls, + od->nbs->n_acls); + struct ls_port_group *ls_pg = ls_port_group_table_find(ls_pgs, od->nbs); if (!ls_pg) { return; } const struct ls_port_group_record *ls_pg_rec; HMAP_FOR_EACH (ls_pg_rec, key_node, &ls_pg->nb_pgs) { - if (ls_stateful_record_set_acl_flags_(ls_stateful_rec, - ls_pg_rec->nb_pg->acls, - ls_pg_rec->nb_pg->n_acls)) { - return; - } + ls_stateful_record_set_acls_(ls_stateful_rec, ls_pg_rec->nb_pg->acls, + ls_pg_rec->nb_pg->n_acls); } } @@ -421,46 +397,24 @@ update_ls_max_acl_tier(struct ls_stateful_record *ls_stateful_rec, *tier = MAX(*tier, acl->tier); } -static bool -ls_acl_tiers_are_maxed_out(struct acl_tier *acl_tier, - uint64_t max_allowed_acl_tier) -{ - return acl_tier->ingress_post_lb == max_allowed_acl_tier && - acl_tier->ingress_pre_lb == max_allowed_acl_tier && - acl_tier->egress == max_allowed_acl_tier; -} - -static bool -ls_stateful_record_set_acl_flags_(struct ls_stateful_record *ls_stateful_rec, - struct nbrec_acl **acls, - size_t n_acls) +static void +ls_stateful_record_set_acls_(struct ls_stateful_record *ls_stateful_rec, + struct nbrec_acl **acls, size_t n_acls) { - /* A true return indicates that there are no possible ACL flags - * left to set on ls_stateful record. A false return indicates that - * further ACLs should be explored in case more flags need to be - * set on ls_stateful record. - */ if (!n_acls) { - return false; + return; } ls_stateful_rec->has_acls = true; for (size_t i = 0; i < n_acls; i++) { const struct nbrec_acl *acl = acls[i]; update_ls_max_acl_tier(ls_stateful_rec, acl); + uuidset_insert(&ls_stateful_rec->related_acls, &acl->header_.uuid); if (!ls_stateful_rec->has_stateful_acl && !strcmp(acl->action, "allow-related")) { ls_stateful_rec->has_stateful_acl = true; } - if (ls_stateful_rec->has_stateful_acl && - ls_acl_tiers_are_maxed_out( - &ls_stateful_rec->max_acl_tier, - nbrec_acl_col_tier.type.key.integer.max)) { - return true; - } } - - return false; } static struct ls_stateful_input diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h index 1d98b3695..217adf696 100644 --- a/northd/en-ls-stateful.h +++ b/northd/en-ls-stateful.h @@ -20,6 +20,7 @@ /* OVS includes. */ #include "lib/hmapx.h" +#include "lib/uuidset.h" #include "openvswitch/hmap.h" #include "sset.h" @@ -54,6 +55,9 @@ struct ls_stateful_record { bool has_acls; struct acl_tier max_acl_tier; + /* Set of ACLs that are related to this LS. */ + struct uuidset related_acls; + /* 'lflow_ref' is used to reference logical flows generated for * this ls_stateful record. * @@ -109,6 +113,8 @@ enum engine_input_handler_result ls_stateful_northd_handler(struct engine_node *, void *data); enum engine_input_handler_result ls_stateful_port_group_handler(struct engine_node *, void *data); +enum engine_input_handler_result +ls_stateful_acl_handler(struct engine_node *node, void *data); const struct ls_stateful_record *ls_stateful_table_find( const struct ls_stateful_table *, const struct nbrec_logical_switch *); diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index be2f36e33..d43bfc16c 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -259,6 +259,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_ls_stateful, &en_northd, ls_stateful_northd_handler); engine_add_input(&en_ls_stateful, &en_port_group, ls_stateful_port_group_handler); + engine_add_input(&en_ls_stateful, &en_nb_acl, ls_stateful_acl_handler); engine_add_input(&en_mac_binding_aging, &en_sb_mac_binding, NULL); engine_add_input(&en_mac_binding_aging, &en_northd, NULL); @@ -330,7 +331,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_multicast_igmp, &en_sb_multicast_group, NULL); engine_add_input(&en_multicast_igmp, &en_sb_igmp_group, NULL); - engine_add_input(&en_lflow, &en_nb_acl, NULL); engine_add_input(&en_lflow, &en_sync_meters, NULL); engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 6d0f8f86f..a984cfce6 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -11352,14 +11352,14 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb meter-add m drop 1 pktps check ovn-nbctl --wait=sb acl-add ls from-lport 1 1 allow dnl Only triggers recompute of the sync_meters and lflow nodes. -check_recompute_counter 0 2 1 +check_recompute_counter 0 1 1 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb meter-del m check ovn-nbctl --wait=sb acl-del ls dnl Only triggers recompute of the sync_meters and lflow nodes. -check_recompute_counter 0 2 1 +check_recompute_counter 0 1 1 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1) AT_CLEANUP @@ -17317,7 +17317,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb set acl $acl_id match=udp check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats @@ -17348,7 +17348,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb set acl $acl_id match=udp check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats @@ -17371,28 +17371,28 @@ AS_BOX([ACLs attached to LS]) check ovn-nbctl --wait=sb acl-add ls from-lport 100 tcp drop acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop) check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb acl-add ls from-lport 101 ip4 drop check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb set acl $acl_id match=udp check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb acl-del ls check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats @@ -17401,28 +17401,28 @@ AS_BOX([ACLs attached to PG]) check ovn-nbctl --wait=sb acl-add pg from-lport 100 tcp drop acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop) check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb acl-add pg from-lport 101 ip4 drop check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb set acl $acl_id match=udp check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats check ovn-nbctl --wait=sb acl-del pg check_engine_stats northd norecompute compute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute check_engine_stats sync_meters norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats