From patchwork Wed Nov 29 07:23:33 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1869586 X-Patchwork-Delegate: i.maximets@samsung.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=cjFx5Twv; dkim-atps=neutral Authentication-Results: legolas.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=patchwork.ozlabs.org) 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 ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Sg9mf0FF1z1ySY for ; Wed, 29 Nov 2023 18:24:01 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id B1DC260FA1; Wed, 29 Nov 2023 07:23:59 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org B1DC260FA1 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=cjFx5Twv 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 GIdUV236kt_t; Wed, 29 Nov 2023 07:23:56 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 67EBB60BA9; Wed, 29 Nov 2023 07:23:52 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 67EBB60BA9 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 82215C0DD3; Wed, 29 Nov 2023 07:23:50 +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 04651C0DD9 for ; Wed, 29 Nov 2023 07:23:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id BFA4B60FA4 for ; Wed, 29 Nov 2023 07:23:48 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org BFA4B60FA4 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 EceSmvq4-JX2 for ; Wed, 29 Nov 2023 07:23:47 +0000 (UTC) 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 D7E5560E8B for ; Wed, 29 Nov 2023 07:23:46 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org D7E5560E8B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701242625; 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=Ea4GDaB+cm5jUREpM23Ej/w2YtTlc4/yrD0WfF0QTmA=; b=cjFx5TwvxpXYwc6Fe1CK0ywAdu4dA2Ja4kOt6x6F6z98aeOzruBdlcX8cFqu10qOWbYo0O TK4IpGkZeF7oroaJo8IzMc0AMFozHrYvvOVF31vsuwhcNgrvCvvtDCwdmJjJTk2FAg7TQm gnHMXy9RSOj3VkVv8KgsDhdDecT0+KA= 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-221-r3CWc1paM6u-16tIgblumg-1; Wed, 29 Nov 2023 02:23:41 -0500 X-MC-Unique: r3CWc1paM6u-16tIgblumg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (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 21B43101A529; Wed, 29 Nov 2023 07:23:41 +0000 (UTC) Received: from amusil.. (unknown [10.34.130.152]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4E315C1596F; Wed, 29 Nov 2023 07:23:40 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Wed, 29 Nov 2023 08:23:33 +0100 Message-ID: <20231129072334.91442-6-amusil@redhat.com> In-Reply-To: <20231129072334.91442-1-amusil@redhat.com> References: <20231129072334.91442-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: i.maximets@ovn.org Subject: [ovs-dev] [PATCH v7 5/6] ct-dpif: Enforce CT zone limit protection. 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" Make sure that if any zone limit was set via DB all zones are forced to be set there also. This is done by tracking which datapath has zone limit protection and it is reflected in the dpctl command. If the datapath is protected the dpctl command will return permission error. Signed-off-by: Ales Musil --- v7: Rebase on top of current master. Remove leftover comments from testing. v6: Rebase on top of current master. Address comments from Ilya: - Drop the log message about protection. - Make the dpctl error message more user-friendly. - Do not ignore error messages in the system-test. v5: Rebase on top of current master. Address comments from Ilya: - Add more user friendly error message to the dpctl. - Fix style related problems. v4: Rebase on top of current master. Make the protection datapath wide. --- lib/ct-dpif.c | 25 +++++++++++++++++++++ lib/ct-dpif.h | 2 ++ lib/dpctl.c | 14 ++++++++++++ ofproto/ofproto-dpif.c | 13 +++++++++++ ofproto/ofproto-provider.h | 5 +++++ ofproto/ofproto.c | 11 +++++++++ ofproto/ofproto.h | 2 ++ tests/system-traffic.at | 46 ++++++++++++++++++++++++++++++++++++++ vswitchd/bridge.c | 7 ++++++ 9 files changed, 125 insertions(+) diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c index 2ee045164..5115c886b 100644 --- a/lib/ct-dpif.c +++ b/lib/ct-dpif.c @@ -23,6 +23,7 @@ #include "openvswitch/ofp-ct.h" #include "openvswitch/ofp-parse.h" #include "openvswitch/vlog.h" +#include "sset.h" VLOG_DEFINE_THIS_MODULE(ct_dpif); @@ -32,6 +33,10 @@ struct flags { const char *name; }; +/* Protection for CT zone limit per datapath. */ +static struct sset ct_limit_protection = + SSET_INITIALIZER(&ct_limit_protection); + static void ct_dpif_format_counters(struct ds *, const struct ct_dpif_counters *); static void ct_dpif_format_timestamp(struct ds *, @@ -1064,3 +1069,23 @@ ct_dpif_get_features(struct dpif *dpif, enum ct_features *features) ? dpif->dpif_class->ct_get_features(dpif, features) : EOPNOTSUPP); } + +void +ct_dpif_set_zone_limit_protection(struct dpif *dpif, bool protected) +{ + if (sset_contains(&ct_limit_protection, dpif->full_name) == protected) { + return; + } + + if (protected) { + sset_add(&ct_limit_protection, dpif->full_name); + } else { + sset_find_and_delete(&ct_limit_protection, dpif->full_name); + } +} + +bool +ct_dpif_is_zone_limit_protected(struct dpif *dpif) +{ + return sset_contains(&ct_limit_protection, dpif->full_name); +} diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h index c8a7c155e..c3786d5ae 100644 --- a/lib/ct-dpif.h +++ b/lib/ct-dpif.h @@ -350,5 +350,7 @@ int ct_dpif_get_timeout_policy_name(struct dpif *dpif, uint32_t tp_id, uint16_t dl_type, uint8_t nw_proto, char **tp_name, bool *is_generic); int ct_dpif_get_features(struct dpif *dpif, enum ct_features *features); +void ct_dpif_set_zone_limit_protection(struct dpif *, bool protected); +bool ct_dpif_is_zone_limit_protected(struct dpif *); #endif /* CT_DPIF_H */ diff --git a/lib/dpctl.c b/lib/dpctl.c index a8c654747..2a1aac5e5 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -2234,6 +2234,13 @@ dpctl_ct_set_limits(int argc, const char *argv[], ct_dpif_push_zone_limit(&zone_limits, zone, limit, 0); } + if (ct_dpif_is_zone_limit_protected(dpif)) { + ds_put_cstr(&ds, "the zone limits are set via database, " + "use 'ovs-vsctl set-zone-limit <...>' instead."); + error = EPERM; + goto error; + } + error = ct_dpif_set_limits(dpif, &zone_limits); if (!error) { ct_dpif_free_zone_limits(&zone_limits); @@ -2310,6 +2317,13 @@ dpctl_ct_del_limits(int argc, const char *argv[], } } + if (ct_dpif_is_zone_limit_protected(dpif)) { + ds_put_cstr(&ds, "the zone limits are set via database, " + "use 'ovs-vsctl del-zone-limit <...>' instead."); + error = EPERM; + goto error; + } + error = ct_dpif_del_limits(dpif, &zone_limits); if (!error) { goto out; diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index bfae28d96..6e62ed1f9 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -5673,6 +5673,18 @@ ct_zone_limits_commit(struct dpif_backer *backer) } } +static void +ct_zone_limit_protection_update(const char *datapath_type, bool protected) +{ + struct dpif_backer *backer = shash_find_data(&all_dpif_backers, + datapath_type); + if (!backer) { + return; + } + + ct_dpif_set_zone_limit_protection(backer->dpif, protected); +} + static void get_datapath_cap(const char *datapath_type, struct smap *cap) { @@ -6964,4 +6976,5 @@ const struct ofproto_class ofproto_dpif_class = { ct_set_zone_timeout_policy, ct_del_zone_timeout_policy, ct_zone_limit_update, + ct_zone_limit_protection_update, }; diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index face0b574..83c509fcf 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -1929,6 +1929,11 @@ struct ofproto_class { * within proper range (0 - UINT32_MAX). */ void (*ct_zone_limit_update)(const char *dp_type, int32_t zone, int64_t *limit); + + /* Sets the CT zone limit protection to "protected" for the specified + * datapath type. */ + void (*ct_zone_limit_protection_update)(const char *dp_type, + bool protected); }; extern const struct ofproto_class ofproto_dpif_class; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 649add089..122a06f30 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1038,6 +1038,17 @@ ofproto_ct_zone_limit_update(const char *datapath_type, int32_t zone_id, } } +void +ofproto_ct_zone_limit_protection_update(const char *datapath_type, + bool protected) +{ + datapath_type = ofproto_normalize_type(datapath_type); + const struct ofproto_class *class = ofproto_class_find__(datapath_type); + + if (class && class->ct_zone_limit_protection_update) { + class->ct_zone_limit_protection_update(datapath_type, protected); + } +} /* Spanning Tree Protocol (STP) configuration. */ diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 7ce6a65e1..1c07df275 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -386,6 +386,8 @@ void ofproto_ct_del_zone_timeout_policy(const char *datapath_type, uint16_t zone); void ofproto_ct_zone_limit_update(const char *datapath_type, int32_t zone_id, int64_t *limit); +void ofproto_ct_zone_limit_protection_update(const char *datapath_type, + bool protected); void ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap); diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 9dc7e48cd..ac705a0a8 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -5275,6 +5275,52 @@ OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], [dnl default limit=0 zone=0,limit=3,count=0]) +dnl Try to overwrite the zone limit via dpctl command. +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=3,limit=5 zone=0,limit=5], [2], [ignore], [dnl +ovs-vswitchd: the zone limits are set via database, use 'ovs-vsctl set-zone-limit <...>' instead. (Operation not permitted) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=0 +zone=0,limit=3,count=0 +]) + +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=0], [2], [ignore], [dnl +ovs-vswitchd: the zone limits are set via database, use 'ovs-vsctl del-zone-limit <...>' instead. (Operation not permitted) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=0 +zone=0,limit=3,count=0 +]) + +AT_CHECK([ovs-vsctl del-zone-limit $DP_TYPE zone=0]) +AT_CHECK([ovs-vsctl set-zone-limit $DP_TYPE default limit=10]) +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=10 +]) + +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=1,limit=5], [2], [ignore], [dnl +ovs-vswitchd: the zone limits are set via database, use 'ovs-vsctl set-zone-limit <...>' instead. (Operation not permitted) +ovs-appctl: ovs-vswitchd: server returned an error +]) + +dnl Delete all zones from DB, that should remove the protection. +AT_CHECK([ovs-vsctl del-zone-limit $DP_TYPE default]) + +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=1,limit=5]) +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=15 +zone=1,limit=5,count=0 +]) + +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=1]) +AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl +default limit=15 +]) + OVS_TRAFFIC_VSWITCHD_STOP(["dnl /could not create datapath/d /(Cannot allocate memory) on packet/d"]) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 5be38b890..95a65fcdc 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -740,6 +740,7 @@ datapath_destroy(struct datapath *dp) NULL); } + ofproto_ct_zone_limit_protection_update(dp->type, false); hmap_remove(&all_datapaths, &dp->node); hmap_destroy(&dp->ct_zones); free(dp->type); @@ -752,6 +753,7 @@ static void ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) { struct ct_zone *ct_zone; + bool protected = false; /* Add new 'ct_zone's or update existing 'ct_zone's based on the database * state. */ @@ -785,6 +787,8 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) } ct_zone->last_used = idl_seqno; + + protected = protected || !!zone_cfg->limit; } /* Purge 'ct_zone's no longer found in the database. */ @@ -805,6 +809,9 @@ ct_zones_reconfigure(struct datapath *dp, struct ovsrec_datapath *dp_cfg) dp->ct_zone_default_limit = default_limit; } + protected = protected || !!dp_cfg->ct_zone_default_limit; + + ofproto_ct_zone_limit_protection_update(dp->type, protected); } static void