From patchwork Mon Jun 17 06:58:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1948474 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=FWQtrpY2; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.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 4W2ghm4JcYz20XJ for ; Mon, 17 Jun 2024 16:58:48 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id E5E5E40892; Mon, 17 Jun 2024 06:58:44 +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 wYoluHqurGuN; Mon, 17 Jun 2024 06:58:40 +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 2A323405EB 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=FWQtrpY2 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 2A323405EB; Mon, 17 Jun 2024 06:58:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id F204DC0013; Mon, 17 Jun 2024 06:58:39 +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 CDB85C0011 for ; Mon, 17 Jun 2024 06:58:37 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 9146D405C4 for ; Mon, 17 Jun 2024 06:58:37 +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 5hd2gOLNlUxM for ; Mon, 17 Jun 2024 06:58:34 +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 smtp2.osuosl.org 2C497401C2 Authentication-Results: smtp2.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 2C497401C2 Authentication-Results: smtp2.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=FWQtrpY2 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp2.osuosl.org (Postfix) with ESMTPS id 2C497401C2 for ; Mon, 17 Jun 2024 06:58:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718607513; 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=V6y8AylJIml/HACdMVzh9+yFeyPNvPg8r7D9HUZYqBc=; b=FWQtrpY2r2VKxqQ1dkrlfxCsmxtU8mIKhil8Ml7pHudA/E/JDo48fdhcMbGXGSgQyJ0rqG OzvdsiNNysVIsvwG88+gxX9lJgqmcoUYt4zWIf123n2Wgdo3OP1VkwbZuCwJh1pljTwHF/ ormyalOPsTfNWZfY/yv3HWyU8reBDGY= 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-247-0Nb0XaHtMWWqQiFxSkrTIw-1; Mon, 17 Jun 2024 02:58:31 -0400 X-MC-Unique: 0Nb0XaHtMWWqQiFxSkrTIw-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (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 6157E195608B for ; Mon, 17 Jun 2024 06:58:30 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id EC2ED19560AE; Mon, 17 Jun 2024 06:58:28 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Mon, 17 Jun 2024 08:58:23 +0200 Message-ID: <20240617065826.76656-2-amusil@redhat.com> In-Reply-To: <20240617065826.76656-1-amusil@redhat.com> References: <20240617065826.76656-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v3 1/4] controller: Move CT zone handling into separate module. 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: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Move the CT zone handling specific bits into its own module. This allows for easier changes done within the module and separates the logic that is unrelated from ovn-controller. Signed-off-by: Ales Musil Acked-by: Lorenzo Bianconi --- v3: Rebase on top of latest main. --- controller/automake.mk | 4 +- controller/ct-zone.c | 378 ++++++++++++++++++++++++++++++++++ controller/ct-zone.h | 74 +++++++ controller/ofctrl.c | 3 +- controller/ovn-controller.c | 393 +++--------------------------------- controller/ovn-controller.h | 21 +- controller/pinctrl.c | 2 +- tests/ovn.at | 4 +- 8 files changed, 486 insertions(+), 393 deletions(-) create mode 100644 controller/ct-zone.c create mode 100644 controller/ct-zone.h diff --git a/controller/automake.mk b/controller/automake.mk index 1b1b3aeb1..ed93cfb3c 100644 --- a/controller/automake.mk +++ b/controller/automake.mk @@ -47,7 +47,9 @@ controller_ovn_controller_SOURCES = \ controller/mac-cache.h \ controller/mac-cache.c \ controller/statctrl.h \ - controller/statctrl.c + controller/statctrl.c \ + controller/ct-zone.h \ + controller/ct-zone.c controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la man_MANS += controller/ovn-controller.8 diff --git a/controller/ct-zone.c b/controller/ct-zone.c new file mode 100644 index 000000000..3e37fedb6 --- /dev/null +++ b/controller/ct-zone.c @@ -0,0 +1,378 @@ +/* Copyright (c) 2024, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "ct-zone.h" +#include "local_data.h" +#include "openvswitch/vlog.h" + +VLOG_DEFINE_THIS_MODULE(ct_zone); + +static void +ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, + struct ct_zone_ctx *ctx, const char *name, int zone); +static void ct_zone_add_pending(struct shash *pending_ct_zones, + enum ct_zone_pending_state state, + int zone, bool add, const char *name); + +void +ct_zones_restore(struct ct_zone_ctx *ctx, + const struct ovsrec_open_vswitch_table *ovs_table, + const struct sbrec_datapath_binding_table *dp_table, + const struct ovsrec_bridge *br_int) +{ + memset(ctx->bitmap, 0, sizeof ctx->bitmap); + bitmap_set1(ctx->bitmap, 0); /* Zone 0 is reserved. */ + + struct shash_node *pending_node; + SHASH_FOR_EACH (pending_node, &ctx->pending) { + struct ct_zone_pending_entry *ctpe = pending_node->data; + + if (ctpe->add) { + ct_zone_restore(dp_table, ctx, pending_node->name, ctpe->zone); + } + } + + const struct ovsrec_open_vswitch *cfg; + cfg = ovsrec_open_vswitch_table_first(ovs_table); + if (!cfg) { + return; + } + + if (!br_int) { + /* If the integration bridge hasn't been defined, assume that + * any existing ct-zone definitions aren't valid. */ + return; + } + + struct smap_node *node; + SMAP_FOR_EACH (node, &br_int->external_ids) { + if (strncmp(node->key, "ct-zone-", 8)) { + continue; + } + + const char *user = node->key + 8; + if (!user[0]) { + continue; + } + + if (shash_find(&ctx->pending, user)) { + continue; + } + + unsigned int zone; + if (!str_to_uint(node->value, 10, &zone)) { + continue; + } + + ct_zone_restore(dp_table, ctx, user, zone); + } +} + +bool +ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, + int *scan_start) +{ + /* We assume that there are 64K zones and that we own them all. */ + int zone = bitmap_scan(ctx->bitmap, 0, *scan_start, MAX_CT_ZONES + 1); + if (zone == MAX_CT_ZONES + 1) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "exhausted all ct zones"); + return false; + } + + *scan_start = zone + 1; + + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + zone, true, zone_name); + + bitmap_set1(ctx->bitmap, zone); + simap_put(&ctx->current, zone_name, zone); + return true; +} + +bool +ct_zone_remove(struct ct_zone_ctx *ctx, const char *name) +{ + struct simap_node *ct_zone = simap_find(&ctx->current, name); + if (!ct_zone) { + return false; + } + + VLOG_DBG("removing ct zone %"PRId32" for '%s'", ct_zone->data, + ct_zone->name); + + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + ct_zone->data, false, ct_zone->name); + bitmap_set0(ctx->bitmap, ct_zone->data); + simap_delete(&ctx->current, ct_zone); + + return true; +} + +void +ct_zones_update(const struct sset *local_lports, + const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) +{ + struct simap_node *ct_zone; + int scan_start = 1; + const char *user; + struct sset all_users = SSET_INITIALIZER(&all_users); + struct simap req_snat_zones = SIMAP_INITIALIZER(&req_snat_zones); + unsigned long *unreq_snat_zones_map = bitmap_allocate(MAX_CT_ZONES); + struct simap unreq_snat_zones = SIMAP_INITIALIZER(&unreq_snat_zones); + + const char *local_lport; + SSET_FOR_EACH (local_lport, local_lports) { + sset_add(&all_users, local_lport); + } + + /* Local patched datapath (gateway routers) need zones assigned. */ + const struct local_datapath *ld; + HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { + /* XXX Add method to limit zone assignment to logical router + * datapaths with NAT */ + const char *name = smap_get(&ld->datapath->external_ids, "name"); + if (!name) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' " + "skipping zone assignment.", + UUID_ARGS(&ld->datapath->header_.uuid)); + continue; + } + + char *dnat = alloc_nat_zone_key(name, "dnat"); + char *snat = alloc_nat_zone_key(name, "snat"); + sset_add(&all_users, dnat); + sset_add(&all_users, snat); + + int req_snat_zone = ct_zone_get_snat(ld->datapath); + if (req_snat_zone >= 0) { + simap_put(&req_snat_zones, snat, req_snat_zone); + } + free(dnat); + free(snat); + } + + /* Delete zones that do not exist in above sset. */ + SIMAP_FOR_EACH_SAFE (ct_zone, &ctx->current) { + if (!sset_contains(&all_users, ct_zone->name)) { + ct_zone_remove(ctx, ct_zone->name); + } else if (!simap_find(&req_snat_zones, ct_zone->name)) { + bitmap_set1(unreq_snat_zones_map, ct_zone->data); + simap_put(&unreq_snat_zones, ct_zone->name, ct_zone->data); + } + } + + /* Prioritize requested CT zones */ + struct simap_node *snat_req_node; + SIMAP_FOR_EACH (snat_req_node, &req_snat_zones) { + /* Determine if someone already had this zone auto-assigned. + * If so, then they need to give up their assignment since + * that zone is being explicitly requested now. + */ + if (bitmap_is_set(unreq_snat_zones_map, snat_req_node->data)) { + struct simap_node *unreq_node; + SIMAP_FOR_EACH_SAFE (unreq_node, &unreq_snat_zones) { + if (unreq_node->data == snat_req_node->data) { + simap_find_and_delete(&ctx->current, unreq_node->name); + simap_delete(&unreq_snat_zones, unreq_node); + } + } + + /* Set this bit to 0 so that if multiple datapaths have requested + * this zone, we don't needlessly double-detect this condition. + */ + bitmap_set0(unreq_snat_zones_map, snat_req_node->data); + } + + struct simap_node *node = simap_find(&ctx->current, + snat_req_node->name); + if (node) { + if (node->data != snat_req_node->data) { + /* Zone request has changed for this node. delete old entry and + * create new one*/ + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + snat_req_node->data, true, + snat_req_node->name); + bitmap_set0(ctx->bitmap, node->data); + } + bitmap_set1(ctx->bitmap, snat_req_node->data); + node->data = snat_req_node->data; + } else { + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + snat_req_node->data, true, + snat_req_node->name); + bitmap_set1(ctx->bitmap, snat_req_node->data); + simap_put(&ctx->current, snat_req_node->name, snat_req_node->data); + } + } + + /* xxx This is wasteful to assign a zone to each port--even if no + * xxx security policy is applied. */ + + /* Assign a unique zone id for each logical port and two zones + * to a gateway router. */ + SSET_FOR_EACH (user, &all_users) { + if (simap_contains(&ctx->current, user)) { + continue; + } + + ct_zone_assign_unused(ctx, user, &scan_start); + } + + simap_destroy(&req_snat_zones); + simap_destroy(&unreq_snat_zones); + sset_destroy(&all_users); + bitmap_free(unreq_snat_zones_map); +} + +void +ct_zones_commit(const struct ovsrec_bridge *br_int, + struct shash *pending_ct_zones) +{ + struct shash_node *iter; + SHASH_FOR_EACH (iter, pending_ct_zones) { + struct ct_zone_pending_entry *ctzpe = iter->data; + + /* The transaction is open, so any pending entries in the + * CT_ZONE_DB_QUEUED must be sent and any in CT_ZONE_DB_QUEUED + * need to be retried. */ + if (ctzpe->state != CT_ZONE_DB_QUEUED + && ctzpe->state != CT_ZONE_DB_SENT) { + continue; + } + + char *user_str = xasprintf("ct-zone-%s", iter->name); + if (ctzpe->add) { + char *zone_str = xasprintf("%"PRId32, ctzpe->zone); + struct smap_node *node = + smap_get_node(&br_int->external_ids, user_str); + if (!node || strcmp(node->value, zone_str)) { + ovsrec_bridge_update_external_ids_setkey(br_int, + user_str, zone_str); + } + free(zone_str); + } else { + if (smap_get(&br_int->external_ids, user_str)) { + ovsrec_bridge_update_external_ids_delkey(br_int, user_str); + } + } + free(user_str); + + ctzpe->state = CT_ZONE_DB_SENT; + } +} + +int +ct_zone_get_snat(const struct sbrec_datapath_binding *dp) +{ + return smap_get_int(&dp->external_ids, "snat-ct-zone", -1); +} + +void +ct_zones_pending_clear_commited(struct shash *pending) +{ + struct shash_node *iter; + SHASH_FOR_EACH_SAFE (iter, pending) { + struct ct_zone_pending_entry *ctzpe = iter->data; + if (ctzpe->state == CT_ZONE_DB_SENT) { + shash_delete(pending, iter); + free(ctzpe); + } + } +} + +static void +ct_zone_add_pending(struct shash *pending_ct_zones, + enum ct_zone_pending_state state, + int zone, bool add, const char *name) +{ + VLOG_DBG("%s ct zone %"PRId32" for '%s'", + add ? "assigning" : "removing", zone, name); + + struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending); + *pending = (struct ct_zone_pending_entry) { + .state = state, + .zone = zone, + .add = add, + }; + + /* Its important that we add only one entry for the key 'name'. + * Replace 'pending' with 'existing' and free up 'existing'. + * Otherwise, we may end up in a continuous loop of adding + * and deleting the zone entry in the 'external_ids' of + * integration bridge. + */ + struct ct_zone_pending_entry *existing = + shash_replace(pending_ct_zones, name, pending); + free(existing); +} + +/* Replaces a UUID prefix from 'uuid_zone' (if any) with the + * corresponding Datapath_Binding.external_ids.name. Returns it + * as a new string that will be owned by the caller. */ +static char * +ct_zone_name_from_uuid(const struct sbrec_datapath_binding_table *dp_table, + const char *uuid_zone) +{ + struct uuid uuid; + if (!uuid_from_string_prefix(&uuid, uuid_zone)) { + return NULL; + } + + const struct sbrec_datapath_binding *dp = + sbrec_datapath_binding_table_get_for_uuid(dp_table, &uuid); + if (!dp) { + return NULL; + } + + const char *entity_name = smap_get(&dp->external_ids, "name"); + if (!entity_name) { + return NULL; + } + + return xasprintf("%s%s", entity_name, uuid_zone + UUID_LEN); +} + +static void +ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, + struct ct_zone_ctx *ctx, const char *name, int zone) +{ + VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, name); + + char *new_name = ct_zone_name_from_uuid(dp_table, name); + const char *current_name = name; + if (new_name) { + VLOG_DBG("ct zone %"PRId32" replace uuid name '%s' with '%s'", + zone, name, new_name); + + /* Make sure we remove the uuid one in the next OvS DB commit without + * flush. */ + ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED, + zone, false, name); + /* Store the zone in OvS DB with name instead of uuid without flush. + * */ + ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED, + zone, true, new_name); + current_name = new_name; + } + + simap_put(&ctx->current, current_name, zone); + bitmap_set1(ctx->bitmap, zone); + + free(new_name); +} diff --git a/controller/ct-zone.h b/controller/ct-zone.h new file mode 100644 index 000000000..6b14de935 --- /dev/null +++ b/controller/ct-zone.h @@ -0,0 +1,74 @@ +/* Copyright (c) 2024, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef OVN_CT_ZONE_H +#define OVN_CT_ZONE_H + +#include + +#include "bitmap.h" +#include "openvswitch/hmap.h" +#include "openvswitch/shash.h" +#include "openvswitch/types.h" +#include "ovn-sb-idl.h" +#include "simap.h" +#include "vswitch-idl.h" + +/* Linux supports a maximum of 64K zones, which seems like a fine default. */ +#define MAX_CT_ZONES 65535 +#define BITMAP_SIZE BITMAP_N_LONGS(MAX_CT_ZONES) + +/* Context of CT zone assignment. */ +struct ct_zone_ctx { + unsigned long bitmap[BITMAP_SIZE]; /* Bitmap indication of allocated + * zones. */ + struct shash pending; /* Pending entries, + * 'struct ct_zone_pending_entry' + * by name. */ + struct simap current; /* Current CT zones mapping + * (zone id by name). */ +}; + +/* States to move through when a new conntrack zone has been allocated. */ +enum ct_zone_pending_state { + CT_ZONE_OF_QUEUED, /* Waiting to send conntrack flush command. */ + CT_ZONE_OF_SENT, /* Sent and waiting for confirmation on flush. */ + CT_ZONE_DB_QUEUED, /* Waiting for DB transaction to open. */ + CT_ZONE_DB_SENT, /* Sent and waiting for confirmation from DB. */ +}; + +struct ct_zone_pending_entry { + int zone; + bool add; /* Is the entry being added? */ + ovs_be32 of_xid; /* Transaction id for barrier. */ + enum ct_zone_pending_state state; +}; + +void ct_zones_restore(struct ct_zone_ctx *ctx, + const struct ovsrec_open_vswitch_table *ovs_table, + const struct sbrec_datapath_binding_table *dp_table, + const struct ovsrec_bridge *br_int); +bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, + int *scan_start); +bool ct_zone_remove(struct ct_zone_ctx *ctx, const char *name); +void ct_zones_update(const struct sset *local_lports, + const struct hmap *local_datapaths, + struct ct_zone_ctx *ctx); +void ct_zones_commit(const struct ovsrec_bridge *br_int, + struct shash *pending_ct_zones); +int ct_zone_get_snat(const struct sbrec_datapath_binding *dp); +void ct_zones_pending_clear_commited(struct shash *pending); + +#endif /* controller/ct-zone.h */ diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 9d181a782..8a19106c2 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -42,7 +42,6 @@ #include "openvswitch/ofp-util.h" #include "openvswitch/ofpbuf.h" #include "openvswitch/vlog.h" -#include "ovn-controller.h" #include "ovn/actions.h" #include "lib/extend-table.h" #include "lib/lb.h" @@ -53,6 +52,8 @@ #include "timeval.h" #include "util.h" #include "vswitch-idl.h" +#include "ovn-sb-idl.h" +#include "ct-zone.h" VLOG_DEFINE_THIS_MODULE(ofctrl); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6874f99a3..54a9d7a13 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -86,6 +86,7 @@ #include "mac-cache.h" #include "statctrl.h" #include "lib/dns-resolve.h" +#include "ct-zone.h" VLOG_DEFINE_THIS_MODULE(main); @@ -673,343 +674,14 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, } } -static void -add_pending_ct_zone_entry(struct shash *pending_ct_zones, - enum ct_zone_pending_state state, - int zone, bool add, const char *name) -{ - VLOG_DBG("%s ct zone %"PRId32" for '%s'", - add ? "assigning" : "removing", zone, name); - - struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending); - pending->state = state; /* Skip flushing zone. */ - pending->zone = zone; - pending->add = add; - - /* Its important that we add only one entry for the key 'name'. - * Replace 'pending' with 'existing' and free up 'existing'. - * Otherwise, we may end up in a continuous loop of adding - * and deleting the zone entry in the 'external_ids' of - * integration bridge. - */ - struct ct_zone_pending_entry *existing = - shash_replace(pending_ct_zones, name, pending); - free(existing); -} - -static bool -alloc_id_to_ct_zone(const char *zone_name, struct simap *ct_zones, - unsigned long *ct_zone_bitmap, int *scan_start, - struct shash *pending_ct_zones) -{ - /* We assume that there are 64K zones and that we own them all. */ - int zone = bitmap_scan(ct_zone_bitmap, 0, *scan_start, MAX_CT_ZONES + 1); - if (zone == MAX_CT_ZONES + 1) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "exhausted all ct zones"); - return false; - } - - *scan_start = zone + 1; - - add_pending_ct_zone_entry(pending_ct_zones, CT_ZONE_OF_QUEUED, - zone, true, zone_name); - - bitmap_set1(ct_zone_bitmap, zone); - simap_put(ct_zones, zone_name, zone); - return true; -} - -static int -get_snat_ct_zone(const struct sbrec_datapath_binding *dp) -{ - return smap_get_int(&dp->external_ids, "snat-ct-zone", -1); -} - -static void -update_ct_zones(const struct sset *local_lports, - const struct hmap *local_datapaths, - struct simap *ct_zones, unsigned long *ct_zone_bitmap, - struct shash *pending_ct_zones) -{ - struct simap_node *ct_zone; - int scan_start = 1; - const char *user; - struct sset all_users = SSET_INITIALIZER(&all_users); - struct simap req_snat_zones = SIMAP_INITIALIZER(&req_snat_zones); - unsigned long *unreq_snat_zones_map = bitmap_allocate(MAX_CT_ZONES); - struct simap unreq_snat_zones = SIMAP_INITIALIZER(&unreq_snat_zones); - - const char *local_lport; - SSET_FOR_EACH (local_lport, local_lports) { - sset_add(&all_users, local_lport); - } - - /* Local patched datapath (gateway routers) need zones assigned. */ - const struct local_datapath *ld; - HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { - /* XXX Add method to limit zone assignment to logical router - * datapaths with NAT */ - const char *name = smap_get(&ld->datapath->external_ids, "name"); - if (!name) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' " - "skipping zone assignment.", - UUID_ARGS(&ld->datapath->header_.uuid)); - continue; - } - - char *dnat = alloc_nat_zone_key(name, "dnat"); - char *snat = alloc_nat_zone_key(name, "snat"); - sset_add(&all_users, dnat); - sset_add(&all_users, snat); - - int req_snat_zone = get_snat_ct_zone(ld->datapath); - if (req_snat_zone >= 0) { - simap_put(&req_snat_zones, snat, req_snat_zone); - } - free(dnat); - free(snat); - } - - /* Delete zones that do not exist in above sset. */ - SIMAP_FOR_EACH_SAFE (ct_zone, ct_zones) { - if (!sset_contains(&all_users, ct_zone->name)) { - VLOG_DBG("removing ct zone %"PRId32" for '%s'", - ct_zone->data, ct_zone->name); - - add_pending_ct_zone_entry(pending_ct_zones, CT_ZONE_OF_QUEUED, - ct_zone->data, false, ct_zone->name); - - bitmap_set0(ct_zone_bitmap, ct_zone->data); - simap_delete(ct_zones, ct_zone); - } else if (!simap_find(&req_snat_zones, ct_zone->name)) { - bitmap_set1(unreq_snat_zones_map, ct_zone->data); - simap_put(&unreq_snat_zones, ct_zone->name, ct_zone->data); - } - } - - /* Prioritize requested CT zones */ - struct simap_node *snat_req_node; - SIMAP_FOR_EACH (snat_req_node, &req_snat_zones) { - /* Determine if someone already had this zone auto-assigned. - * If so, then they need to give up their assignment since - * that zone is being explicitly requested now. - */ - if (bitmap_is_set(unreq_snat_zones_map, snat_req_node->data)) { - struct simap_node *unreq_node; - SIMAP_FOR_EACH_SAFE (unreq_node, &unreq_snat_zones) { - if (unreq_node->data == snat_req_node->data) { - simap_find_and_delete(ct_zones, unreq_node->name); - simap_delete(&unreq_snat_zones, unreq_node); - } - } - - /* Set this bit to 0 so that if multiple datapaths have requested - * this zone, we don't needlessly double-detect this condition. - */ - bitmap_set0(unreq_snat_zones_map, snat_req_node->data); - } - - struct simap_node *node = simap_find(ct_zones, snat_req_node->name); - if (node) { - if (node->data != snat_req_node->data) { - /* Zone request has changed for this node. delete old entry and - * create new one*/ - add_pending_ct_zone_entry(pending_ct_zones, CT_ZONE_OF_QUEUED, - snat_req_node->data, true, - snat_req_node->name); - bitmap_set0(ct_zone_bitmap, node->data); - } - bitmap_set1(ct_zone_bitmap, snat_req_node->data); - node->data = snat_req_node->data; - } else { - add_pending_ct_zone_entry(pending_ct_zones, CT_ZONE_OF_QUEUED, - snat_req_node->data, true, snat_req_node->name); - bitmap_set1(ct_zone_bitmap, snat_req_node->data); - simap_put(ct_zones, snat_req_node->name, snat_req_node->data); - } - } - - /* xxx This is wasteful to assign a zone to each port--even if no - * xxx security policy is applied. */ - - /* Assign a unique zone id for each logical port and two zones - * to a gateway router. */ - SSET_FOR_EACH(user, &all_users) { - if (simap_contains(ct_zones, user)) { - continue; - } - - alloc_id_to_ct_zone(user, ct_zones, ct_zone_bitmap, &scan_start, - pending_ct_zones); - } - - simap_destroy(&req_snat_zones); - simap_destroy(&unreq_snat_zones); - sset_destroy(&all_users); - bitmap_free(unreq_snat_zones_map); -} - -static void -commit_ct_zones(const struct ovsrec_bridge *br_int, - struct shash *pending_ct_zones) -{ - struct shash_node *iter; - SHASH_FOR_EACH(iter, pending_ct_zones) { - struct ct_zone_pending_entry *ctzpe = iter->data; - - /* The transaction is open, so any pending entries in the - * CT_ZONE_DB_QUEUED must be sent and any in CT_ZONE_DB_QUEUED - * need to be retried. */ - if (ctzpe->state != CT_ZONE_DB_QUEUED - && ctzpe->state != CT_ZONE_DB_SENT) { - continue; - } - - char *user_str = xasprintf("ct-zone-%s", iter->name); - if (ctzpe->add) { - char *zone_str = xasprintf("%"PRId32, ctzpe->zone); - struct smap_node *node = - smap_get_node(&br_int->external_ids, user_str); - if (!node || strcmp(node->value, zone_str)) { - ovsrec_bridge_update_external_ids_setkey(br_int, - user_str, zone_str); - } - free(zone_str); - } else { - if (smap_get(&br_int->external_ids, user_str)) { - ovsrec_bridge_update_external_ids_delkey(br_int, user_str); - } - } - free(user_str); - - ctzpe->state = CT_ZONE_DB_SENT; - } -} - /* Connection tracking zones. */ struct ed_type_ct_zones { - unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; - struct shash pending; - struct simap current; + struct ct_zone_ctx ctx; /* Tracked data. */ bool recomputed; }; -/* Replaces a UUID prefix from 'uuid_zone' (if any) with the - * corresponding Datapath_Binding.external_ids.name. Returns it - * as a new string that will be owned by the caller. */ -static char * -ct_zone_name_from_uuid(const struct sbrec_datapath_binding_table *dp_table, - const char *uuid_zone) -{ - struct uuid uuid; - if (!uuid_from_string_prefix(&uuid, uuid_zone)) { - return NULL; - } - - const struct sbrec_datapath_binding *dp = - sbrec_datapath_binding_table_get_for_uuid(dp_table, &uuid); - if (!dp) { - return NULL; - } - - const char *entity_name = smap_get(&dp->external_ids, "name"); - if (!entity_name) { - return NULL; - } - - return xasprintf("%s%s", entity_name, uuid_zone + UUID_LEN); -} - -static void -ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, - struct ed_type_ct_zones *ct_zones_data, const char *name, - int zone) -{ - VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, name); - - char *new_name = ct_zone_name_from_uuid(dp_table, name); - const char *current_name = name; - if (new_name) { - VLOG_DBG("ct zone %"PRId32" replace uuid name '%s' with '%s'", - zone, name, new_name); - - /* Make sure we remove the uuid one in the next OvS DB commit without - * flush. */ - add_pending_ct_zone_entry(&ct_zones_data->pending, CT_ZONE_DB_QUEUED, - zone, false, name); - /* Store the zone in OvS DB with name instead of uuid without flush. - * */ - add_pending_ct_zone_entry(&ct_zones_data->pending, CT_ZONE_DB_QUEUED, - zone, true, new_name); - current_name = new_name; - } - - simap_put(&ct_zones_data->current, current_name, zone); - bitmap_set1(ct_zones_data->bitmap, zone); - - free(new_name); -} - -static void -restore_ct_zones(const struct ovsrec_bridge_table *bridge_table, - const struct ovsrec_open_vswitch_table *ovs_table, - const struct sbrec_datapath_binding_table *dp_table, - struct ed_type_ct_zones *ct_zones_data) -{ - memset(ct_zones_data->bitmap, 0, sizeof ct_zones_data->bitmap); - bitmap_set1(ct_zones_data->bitmap, 0); /* Zone 0 is reserved. */ - - struct shash_node *pending_node; - SHASH_FOR_EACH (pending_node, &ct_zones_data->pending) { - struct ct_zone_pending_entry *ctpe = pending_node->data; - - if (ctpe->add) { - ct_zone_restore(dp_table, ct_zones_data, - pending_node->name, ctpe->zone); - } - } - - const struct ovsrec_open_vswitch *cfg; - cfg = ovsrec_open_vswitch_table_first(ovs_table); - if (!cfg) { - return; - } - - const struct ovsrec_bridge *br_int; - br_int = get_bridge(bridge_table, br_int_name(ovs_table)); - if (!br_int) { - /* If the integration bridge hasn't been defined, assume that - * any existing ct-zone definitions aren't valid. */ - return; - } - - struct smap_node *node; - SMAP_FOR_EACH(node, &br_int->external_ids) { - if (strncmp(node->key, "ct-zone-", 8)) { - continue; - } - - const char *user = node->key + 8; - if (!user[0]) { - continue; - } - - if (shash_find(&ct_zones_data->pending, user)) { - continue; - } - - unsigned int zone; - if (!str_to_uint(node->value, 10, &zone)) { - continue; - } - - ct_zone_restore(dp_table, ct_zones_data, user, zone); - } -} static uint64_t get_nb_cfg(const struct sbrec_sb_global_table *sb_global_table, @@ -2518,8 +2190,8 @@ en_ct_zones_init(struct engine_node *node OVS_UNUSED, { struct ed_type_ct_zones *data = xzalloc(sizeof *data); - shash_init(&data->pending); - simap_init(&data->current); + shash_init(&data->ctx.pending); + simap_init(&data->ctx.current); return data; } @@ -2536,8 +2208,8 @@ en_ct_zones_cleanup(void *data) { struct ed_type_ct_zones *ct_zones_data = data; - simap_destroy(&ct_zones_data->current); - shash_destroy_free_data(&ct_zones_data->pending); + simap_destroy(&ct_zones_data->ctx.current); + shash_destroy_free_data(&ct_zones_data->ctx.pending); } static void @@ -2554,11 +2226,11 @@ en_ct_zones_run(struct engine_node *node, void *data) const struct sbrec_datapath_binding_table *dp_table = EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); - restore_ct_zones(bridge_table, ovs_table, dp_table, ct_zones_data); - update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths, - &ct_zones_data->current, ct_zones_data->bitmap, - &ct_zones_data->pending); + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); + ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); + ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths, + &ct_zones_data->ctx); ct_zones_data->recomputed = true; engine_set_node_state(node, EN_UPDATED); @@ -2589,7 +2261,7 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data) return false; } - int req_snat_zone = get_snat_ct_zone(dp); + int req_snat_zone = ct_zone_get_snat(dp); if (req_snat_zone == -1) { /* datapath snat ct zone is not set. This condition will also hit * when CMS clears the snat-ct-zone for the logical router. @@ -2611,7 +2283,7 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data) } char *snat_dp_zone_key = alloc_nat_zone_key(name, "snat"); - struct simap_node *simap_node = simap_find(&ct_zones_data->current, + struct simap_node *simap_node = simap_find(&ct_zones_data->ctx.current, snat_dp_zone_key); free(snat_dp_zone_key); if (!simap_node || simap_node->data != req_snat_zone) { @@ -2663,25 +2335,16 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) if (t_lport->tracked_type == TRACKED_RESOURCE_NEW || t_lport->tracked_type == TRACKED_RESOURCE_UPDATED) { - if (!simap_contains(&ct_zones_data->current, + if (!simap_contains(&ct_zones_data->ctx.current, t_lport->pb->logical_port)) { - alloc_id_to_ct_zone(t_lport->pb->logical_port, - &ct_zones_data->current, - ct_zones_data->bitmap, &scan_start, - &ct_zones_data->pending); + ct_zone_assign_unused(&ct_zones_data->ctx, + t_lport->pb->logical_port, + &scan_start); updated = true; } } else if (t_lport->tracked_type == TRACKED_RESOURCE_REMOVED) { - struct simap_node *ct_zone = - simap_find(&ct_zones_data->current, - t_lport->pb->logical_port); - if (ct_zone) { - add_pending_ct_zone_entry( - &ct_zones_data->pending, CT_ZONE_OF_QUEUED, - ct_zone->data, false, ct_zone->name); - - bitmap_set0(ct_zones_data->bitmap, ct_zone->data); - simap_delete(&ct_zones_data->current, ct_zone); + if (ct_zone_remove(&ct_zones_data->ctx, + t_lport->pb->logical_port)) { updated = true; } } @@ -4708,7 +4371,7 @@ static void init_physical_ctx(struct engine_node *node, struct ed_type_ct_zones *ct_zones_data = engine_get_input_data("ct_zones", node); - struct simap *ct_zones = &ct_zones_data->current; + struct simap *ct_zones = &ct_zones_data->ctx.current; parse_encap_ips(ovs_table, &p_ctx->n_encap_ips, &p_ctx->encap_ips); p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; @@ -5642,7 +5305,7 @@ main(int argc, char *argv[]) unixctl_command_register("ct-zone-list", "", 0, 0, ct_zone_list, - &ct_zones_data->current); + &ct_zones_data->ctx.current); struct pending_pkt pending_pkt = { .conn = NULL }; unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt, @@ -5870,7 +5533,7 @@ main(int argc, char *argv[]) ct_zones_data = engine_get_data(&en_ct_zones); if (ofctrl_run(br_int_remote.target, br_int_remote.probe_interval, ovs_table, - ct_zones_data ? &ct_zones_data->pending + ct_zones_data ? &ct_zones_data->ctx.pending : NULL)) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); @@ -5930,7 +5593,8 @@ main(int argc, char *argv[]) if (ct_zones_data) { stopwatch_start(CT_ZONE_COMMIT_STOPWATCH_NAME, time_msec()); - commit_ct_zones(br_int, &ct_zones_data->pending); + ct_zones_commit(br_int, + &ct_zones_data->ctx.pending); stopwatch_stop(CT_ZONE_COMMIT_STOPWATCH_NAME, time_msec()); } @@ -6073,7 +5737,7 @@ main(int argc, char *argv[]) time_msec()); ofctrl_put(&lflow_output_data->flow_table, &pflow_output_data->flow_table, - &ct_zones_data->pending, + &ct_zones_data->ctx.pending, &lb_data->removed_tuples, sbrec_meter_by_name, ofctrl_seqno_get_req_cfg(), @@ -6184,14 +5848,7 @@ main(int argc, char *argv[]) * (or it did not change anything in the database). */ ct_zones_data = engine_get_data(&en_ct_zones); if (ct_zones_data) { - struct shash_node *iter; - SHASH_FOR_EACH_SAFE (iter, &ct_zones_data->pending) { - struct ct_zone_pending_entry *ctzpe = iter->data; - if (ctzpe->state == CT_ZONE_DB_SENT) { - shash_delete(&ct_zones_data->pending, iter); - free(ctzpe); - } - } + ct_zones_pending_clear_commited(&ct_zones_data->ctx.pending); } vif_plug_finish_deleted( diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h index 3a0e95377..fafd704df 100644 --- a/controller/ovn-controller.h +++ b/controller/ovn-controller.h @@ -17,29 +17,10 @@ #ifndef OVN_CONTROLLER_H #define OVN_CONTROLLER_H 1 -#include "simap.h" -#include "lib/ovn-sb-idl.h" +#include struct ovsrec_bridge_table; -/* Linux supports a maximum of 64K zones, which seems like a fine default. */ -#define MAX_CT_ZONES 65535 - -/* States to move through when a new conntrack zone has been allocated. */ -enum ct_zone_pending_state { - CT_ZONE_OF_QUEUED, /* Waiting to send conntrack flush command. */ - CT_ZONE_OF_SENT, /* Sent and waiting for confirmation on flush. */ - CT_ZONE_DB_QUEUED, /* Waiting for DB transaction to open. */ - CT_ZONE_DB_SENT, /* Sent and waiting for confirmation from DB. */ -}; - -struct ct_zone_pending_entry { - int zone; - bool add; /* Is the entry being added? */ - ovs_be32 of_xid; /* Transaction id for barrier. */ - enum ct_zone_pending_state state; -}; - const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table *, const char *br_name); diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 6d7a6f329..ce5516eca 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -45,7 +45,6 @@ #include "lib/crc32c.h" #include "lib/dhcp.h" -#include "ovn-controller.h" #include "ovn/actions.h" #include "ovn/lex.h" #include "lib/acl-log.h" @@ -62,6 +61,7 @@ #include "vswitch-idl.h" #include "lflow.h" #include "ip-mcast.h" +#include "ovn-sb-idl.h" VLOG_DEFINE_THIS_MODULE(pinctrl); diff --git a/tests/ovn.at b/tests/ovn.at index 44ba26465..f485fc533 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -37004,7 +37004,7 @@ check ovn-nbctl lsp-set-type sw0-lr0 router check ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 -check ovn-appctl -t ovn-controller vlog/set dbg:main +check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone wait_for_ports_up ovn-nbctl --wait=hv sync @@ -37112,7 +37112,7 @@ OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" != "running" replace_with_uuid lr0 replace_with_uuid sw0 -start_daemon ovn-controller --verbose="main:dbg" +start_daemon ovn-controller --verbose="ct_zone:dbg" OVS_WAIT_UNTIL([test "$(grep -c 'ct zone .* replace uuid name' hv1/ovn-controller.log)" = "8"]) From patchwork Mon Jun 17 06:58:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1948473 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=eaWDY2pG; 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 4W2ghl3Q6Jz20Wg for ; Mon, 17 Jun 2024 16:58:47 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 2EFFD405E0; Mon, 17 Jun 2024 06:58:45 +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 uDiohRes0XAF; Mon, 17 Jun 2024 06:58:43 +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 AAEA0405EF 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=eaWDY2pG Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id AAEA0405EF; Mon, 17 Jun 2024 06:58:41 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1DECDC0016; Mon, 17 Jun 2024 06:58:41 +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 ACE3AC0011 for ; Mon, 17 Jun 2024 06:58:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 99C09405C8 for ; Mon, 17 Jun 2024 06:58:38 +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 qrQ6xmL3YBHs for ; Mon, 17 Jun 2024 06:58:37 +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 8EFCC405C2 Authentication-Results: smtp2.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 8EFCC405C2 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 8EFCC405C2 for ; Mon, 17 Jun 2024 06:58:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718607515; 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=UvvVe5rom5OJqA/xjJ2c6cJXdqSPJr6kGQID1h8gcRg=; b=eaWDY2pGbz8LNoUo0pXAVFmrNRHU3frqok0Gt34iiZ9HU4aPkLbhHDzuJY+vDDEYbHPwkD QmWbrKvK7O/TasWwasURgXaEj3Qqt5TMxtWJ5QInzy1bzBAc92ezOdgqYdRlSec8a4LBNe 3gJpzPi7tJtPoPHsn9gcoE2aD0W+QHc= 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-26-l2cLxcgtPIKsxLXyxffTgQ-1; Mon, 17 Jun 2024 02:58:32 -0400 X-MC-Unique: l2cLxcgtPIKsxLXyxffTgQ-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (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 E3C481956094 for ; Mon, 17 Jun 2024 06:58:31 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id CA2C019560AE; Mon, 17 Jun 2024 06:58:30 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Mon, 17 Jun 2024 08:58:24 +0200 Message-ID: <20240617065826.76656-3-amusil@redhat.com> In-Reply-To: <20240617065826.76656-1-amusil@redhat.com> References: <20240617065826.76656-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v3 2/4] controller: Further encapsulate the CT zone handling. 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: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Move more code into the new ct-zone module and encapsulate functionality that is strictly related to CT zone handling. Signed-off-by: Ales Musil Acked-by: Lorenzo Bianconi --- v3: Rebase on top of latest main. --- controller/ct-zone.c | 156 +++++++++++++++++++++++++----------- controller/ct-zone.h | 8 +- controller/ovn-controller.c | 49 ++--------- 3 files changed, 118 insertions(+), 95 deletions(-) diff --git a/controller/ct-zone.c b/controller/ct-zone.c index 3e37fedb6..e4f66a52a 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -27,6 +27,11 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, static void ct_zone_add_pending(struct shash *pending_ct_zones, enum ct_zone_pending_state state, int zone, bool add, const char *name); +static int ct_zone_get_snat(const struct sbrec_datapath_binding *dp); +static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, + const char *zone_name, int *scan_start); +static bool ct_zone_remove(struct ct_zone_ctx *ctx, + struct simap_node *ct_zone); void ct_zones_restore(struct ct_zone_ctx *ctx, @@ -82,47 +87,6 @@ ct_zones_restore(struct ct_zone_ctx *ctx, } } -bool -ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, - int *scan_start) -{ - /* We assume that there are 64K zones and that we own them all. */ - int zone = bitmap_scan(ctx->bitmap, 0, *scan_start, MAX_CT_ZONES + 1); - if (zone == MAX_CT_ZONES + 1) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "exhausted all ct zones"); - return false; - } - - *scan_start = zone + 1; - - ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, - zone, true, zone_name); - - bitmap_set1(ctx->bitmap, zone); - simap_put(&ctx->current, zone_name, zone); - return true; -} - -bool -ct_zone_remove(struct ct_zone_ctx *ctx, const char *name) -{ - struct simap_node *ct_zone = simap_find(&ctx->current, name); - if (!ct_zone) { - return false; - } - - VLOG_DBG("removing ct zone %"PRId32" for '%s'", ct_zone->data, - ct_zone->name); - - ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, - ct_zone->data, false, ct_zone->name); - bitmap_set0(ctx->bitmap, ct_zone->data); - simap_delete(&ctx->current, ct_zone); - - return true; -} - void ct_zones_update(const struct sset *local_lports, const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) @@ -170,7 +134,7 @@ ct_zones_update(const struct sset *local_lports, /* Delete zones that do not exist in above sset. */ SIMAP_FOR_EACH_SAFE (ct_zone, &ctx->current) { if (!sset_contains(&all_users, ct_zone->name)) { - ct_zone_remove(ctx, ct_zone->name); + ct_zone_remove(ctx, ct_zone); } else if (!simap_find(&req_snat_zones, ct_zone->name)) { bitmap_set1(unreq_snat_zones_map, ct_zone->data); simap_put(&unreq_snat_zones, ct_zone->name, ct_zone->data); @@ -277,12 +241,6 @@ ct_zones_commit(const struct ovsrec_bridge *br_int, } } -int -ct_zone_get_snat(const struct sbrec_datapath_binding *dp) -{ - return smap_get_int(&dp->external_ids, "snat-ct-zone", -1); -} - void ct_zones_pending_clear_commited(struct shash *pending) { @@ -296,6 +254,108 @@ ct_zones_pending_clear_commited(struct shash *pending) } } +/* Returns "true" when there is no need for full recompute. */ +bool +ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, + const struct sbrec_datapath_binding *dp) +{ + int req_snat_zone = ct_zone_get_snat(dp); + if (req_snat_zone == -1) { + /* datapath snat ct zone is not set. This condition will also hit + * when CMS clears the snat-ct-zone for the logical router. + * In this case there is no harm in using the previosly specified + * snat ct zone for this datapath. Also it is hard to know + * if this option was cleared or if this option is never set. */ + return true; + } + + const char *name = smap_get(&dp->external_ids, "name"); + if (!name) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' skipping" + "zone check.", UUID_ARGS(&dp->header_.uuid)); + return true; + } + + /* Check if the requested snat zone has changed for the datapath + * or not. If so, then fall back to full recompute of + * ct_zone engine. */ + char *snat_dp_zone_key = alloc_nat_zone_key(name, "snat"); + struct simap_node *simap_node = + simap_find(&ctx->current, snat_dp_zone_key); + free(snat_dp_zone_key); + if (!simap_node || simap_node->data != req_snat_zone) { + /* There is no entry yet or the requested snat zone has changed. + * Trigger full recompute of ct_zones engine. */ + return false; + } + + return true; +} + +/* Returns "true" if there was an update to the context. */ +bool +ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name, + bool updated, int *scan_start) +{ + struct simap_node *ct_zone = simap_find(&ctx->current, name); + if (updated && !ct_zone) { + ct_zone_assign_unused(ctx, name, scan_start); + return true; + } else if (!updated && ct_zone_remove(ctx, ct_zone)) { + return true; + } + + return false; +} + + +static bool +ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, + int *scan_start) +{ + /* We assume that there are 64K zones and that we own them all. */ + int zone = bitmap_scan(ctx->bitmap, 0, *scan_start, MAX_CT_ZONES + 1); + if (zone == MAX_CT_ZONES + 1) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "exhausted all ct zones"); + return false; + } + + *scan_start = zone + 1; + + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + zone, true, zone_name); + + bitmap_set1(ctx->bitmap, zone); + simap_put(&ctx->current, zone_name, zone); + return true; +} + +static bool +ct_zone_remove(struct ct_zone_ctx *ctx, struct simap_node *ct_zone) +{ + if (!ct_zone) { + return false; + } + + VLOG_DBG("removing ct zone %"PRId32" for '%s'", ct_zone->data, + ct_zone->name); + + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + ct_zone->data, false, ct_zone->name); + bitmap_set0(ctx->bitmap, ct_zone->data); + simap_delete(&ctx->current, ct_zone); + + return true; +} + +static int +ct_zone_get_snat(const struct sbrec_datapath_binding *dp) +{ + return smap_get_int(&dp->external_ids, "snat-ct-zone", -1); +} + static void ct_zone_add_pending(struct shash *pending_ct_zones, enum ct_zone_pending_state state, diff --git a/controller/ct-zone.h b/controller/ct-zone.h index 6b14de935..889bdf2fc 100644 --- a/controller/ct-zone.h +++ b/controller/ct-zone.h @@ -60,15 +60,15 @@ void ct_zones_restore(struct ct_zone_ctx *ctx, const struct ovsrec_open_vswitch_table *ovs_table, const struct sbrec_datapath_binding_table *dp_table, const struct ovsrec_bridge *br_int); -bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, - int *scan_start); -bool ct_zone_remove(struct ct_zone_ctx *ctx, const char *name); void ct_zones_update(const struct sset *local_lports, const struct hmap *local_datapaths, struct ct_zone_ctx *ctx); void ct_zones_commit(const struct ovsrec_bridge *br_int, struct shash *pending_ct_zones); -int ct_zone_get_snat(const struct sbrec_datapath_binding *dp); void ct_zones_pending_clear_commited(struct shash *pending); +bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, + const struct sbrec_datapath_binding *dp); +bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name, + bool updated, int *scan_start); #endif /* controller/ct-zone.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 54a9d7a13..2152195c3 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2261,34 +2261,7 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data) return false; } - int req_snat_zone = ct_zone_get_snat(dp); - if (req_snat_zone == -1) { - /* datapath snat ct zone is not set. This condition will also hit - * when CMS clears the snat-ct-zone for the logical router. - * In this case there is no harm in using the previosly specified - * snat ct zone for this datapath. Also it is hard to know - * if this option was cleared or if this option is never set. */ - continue; - } - - /* Check if the requested snat zone has changed for the datapath - * or not. If so, then fall back to full recompute of - * ct_zone engine. */ - const char *name = smap_get(&dp->external_ids, "name"); - if (!name) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' skipping" - "zone check.", UUID_ARGS(&dp->header_.uuid)); - continue; - } - - char *snat_dp_zone_key = alloc_nat_zone_key(name, "snat"); - struct simap_node *simap_node = simap_find(&ct_zones_data->ctx.current, - snat_dp_zone_key); - free(snat_dp_zone_key); - if (!simap_node || simap_node->data != req_snat_zone) { - /* There is no entry yet or the requested snat zone has changed. - * Trigger full recompute of ct_zones engine. */ + if (!ct_zone_handle_dp_update(&ct_zones_data->ctx, dp)) { return false; } } @@ -2333,21 +2306,11 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) continue; } - if (t_lport->tracked_type == TRACKED_RESOURCE_NEW || - t_lport->tracked_type == TRACKED_RESOURCE_UPDATED) { - if (!simap_contains(&ct_zones_data->ctx.current, - t_lport->pb->logical_port)) { - ct_zone_assign_unused(&ct_zones_data->ctx, - t_lport->pb->logical_port, - &scan_start); - updated = true; - } - } else if (t_lport->tracked_type == TRACKED_RESOURCE_REMOVED) { - if (ct_zone_remove(&ct_zones_data->ctx, - t_lport->pb->logical_port)) { - updated = true; - } - } + bool port_updated = + t_lport->tracked_type != TRACKED_RESOURCE_REMOVED; + updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, + t_lport->pb->logical_port, + port_updated, &scan_start); } } From patchwork Mon Jun 17 06:58:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1948476 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=EWUiBJuO; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::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 4W2ghm607pz20Z6 for ; Mon, 17 Jun 2024 16:58:48 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id E9D8760871; Mon, 17 Jun 2024 06:58:46 +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 12aAVwt219S8; Mon, 17 Jun 2024 06:58:44 +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 smtp3.osuosl.org 59BE7605A3 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=EWUiBJuO Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 59BE7605A3; Mon, 17 Jun 2024 06:58:44 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 233BEC0016; Mon, 17 Jun 2024 06:58:44 +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 0007BC0011 for ; Mon, 17 Jun 2024 06:58:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id F1252607D5 for ; Mon, 17 Jun 2024 06:58:38 +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 2Y6m5oohJPNN for ; Mon, 17 Jun 2024 06:58:37 +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 7DF5A605A2 Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 7DF5A605A2 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 7DF5A605A2 for ; Mon, 17 Jun 2024 06:58:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718607516; 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=uLmwK9Bm+xDKjAkNDp1ZDLuXeqby4McGggS00k9AGEA=; b=EWUiBJuOO9svdKsg32xZ/EvosBP1P98lexreNy2LydyDL+kKcMq6U0Z/l22PEsLBO2bcYt pd5GgY5D36raiNmRWk5jovVmcV/CFYhf+cAxQOTt21nhUVankrsag4VldaocpmLpAlzxHu IUYU3WkGFweC7W5NZNnH/R1jIN8o1ms= Received: from mx-prod-mc-02.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-561-_3GW2bXAMxu-hTuiEeujqA-1; Mon, 17 Jun 2024 02:58:34 -0400 X-MC-Unique: _3GW2bXAMxu-hTuiEeujqA-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (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-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id AEAF3195608C for ; Mon, 17 Jun 2024 06:58:33 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 355A219560AE; Mon, 17 Jun 2024 06:58:31 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Mon, 17 Jun 2024 08:58:25 +0200 Message-ID: <20240617065826.76656-4-amusil@redhat.com> In-Reply-To: <20240617065826.76656-1-amusil@redhat.com> References: <20240617065826.76656-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v3 3/4] controller: Prepare structure around CT zone limiting. 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: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" In order to be able to store CT limits for specified zone, store the zone inside separate struct instead of simap. This allows to add the addition of limit without changing the whole infrastructure again. This is a preparation step for the CT zone limits. Signed-off-by: Ales Musil --- v3: Rebase on top of latest main. v2: Fix NULL ptr deref. --- controller/ct-zone.c | 171 +++++++++++++++++++++--------------- controller/ct-zone.h | 13 ++- controller/ofctrl.c | 2 +- controller/ovn-controller.c | 15 ++-- controller/physical.c | 17 ++-- controller/physical.h | 2 +- 6 files changed, 128 insertions(+), 92 deletions(-) diff --git a/controller/ct-zone.c b/controller/ct-zone.c index e4f66a52a..95faec2f1 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -26,12 +26,14 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, struct ct_zone_ctx *ctx, const char *name, int zone); static void ct_zone_add_pending(struct shash *pending_ct_zones, enum ct_zone_pending_state state, - int zone, bool add, const char *name); + struct ct_zone *zone, bool add, + const char *name); static int ct_zone_get_snat(const struct sbrec_datapath_binding *dp); static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, int *scan_start); -static bool ct_zone_remove(struct ct_zone_ctx *ctx, - struct simap_node *ct_zone); +static bool ct_zone_remove(struct ct_zone_ctx *ctx, const char *name); +static void ct_zone_add(struct ct_zone_ctx *ctx, const char *name, + uint16_t zone, bool set_pending); void ct_zones_restore(struct ct_zone_ctx *ctx, @@ -47,7 +49,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx, struct ct_zone_pending_entry *ctpe = pending_node->data; if (ctpe->add) { - ct_zone_restore(dp_table, ctx, pending_node->name, ctpe->zone); + ct_zone_restore(dp_table, ctx, pending_node->name, + ctpe->ct_zone.zone); } } @@ -91,7 +94,6 @@ void ct_zones_update(const struct sset *local_lports, const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) { - struct simap_node *ct_zone; int scan_start = 1; const char *user; struct sset all_users = SSET_INITIALIZER(&all_users); @@ -132,12 +134,14 @@ ct_zones_update(const struct sset *local_lports, } /* Delete zones that do not exist in above sset. */ - SIMAP_FOR_EACH_SAFE (ct_zone, &ctx->current) { - if (!sset_contains(&all_users, ct_zone->name)) { - ct_zone_remove(ctx, ct_zone); - } else if (!simap_find(&req_snat_zones, ct_zone->name)) { - bitmap_set1(unreq_snat_zones_map, ct_zone->data); - simap_put(&unreq_snat_zones, ct_zone->name, ct_zone->data); + struct shash_node *node; + SHASH_FOR_EACH_SAFE (node, &ctx->current) { + struct ct_zone *ct_zone = node->data; + if (!sset_contains(&all_users, node->name)) { + ct_zone_remove(ctx, node->name); + } else if (!simap_find(&req_snat_zones, node->name)) { + bitmap_set1(unreq_snat_zones_map, ct_zone->zone); + simap_put(&unreq_snat_zones, node->name, ct_zone->zone); } } @@ -152,7 +156,7 @@ ct_zones_update(const struct sset *local_lports, struct simap_node *unreq_node; SIMAP_FOR_EACH_SAFE (unreq_node, &unreq_snat_zones) { if (unreq_node->data == snat_req_node->data) { - simap_find_and_delete(&ctx->current, unreq_node->name); + ct_zone_remove(ctx, unreq_node->name); simap_delete(&unreq_snat_zones, unreq_node); } } @@ -163,26 +167,12 @@ ct_zones_update(const struct sset *local_lports, bitmap_set0(unreq_snat_zones_map, snat_req_node->data); } - struct simap_node *node = simap_find(&ctx->current, - snat_req_node->name); - if (node) { - if (node->data != snat_req_node->data) { - /* Zone request has changed for this node. delete old entry and - * create new one*/ - ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, - snat_req_node->data, true, - snat_req_node->name); - bitmap_set0(ctx->bitmap, node->data); - } - bitmap_set1(ctx->bitmap, snat_req_node->data); - node->data = snat_req_node->data; - } else { - ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, - snat_req_node->data, true, - snat_req_node->name); - bitmap_set1(ctx->bitmap, snat_req_node->data); - simap_put(&ctx->current, snat_req_node->name, snat_req_node->data); + struct ct_zone *ct_zone = shash_find_data(&ctx->current, + snat_req_node->name); + if (node && ct_zone->zone != snat_req_node->data) { + ct_zone_remove(ctx, snat_req_node->name); } + ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, true); } /* xxx This is wasteful to assign a zone to each port--even if no @@ -191,7 +181,7 @@ ct_zones_update(const struct sset *local_lports, /* Assign a unique zone id for each logical port and two zones * to a gateway router. */ SSET_FOR_EACH (user, &all_users) { - if (simap_contains(&ctx->current, user)) { + if (shash_find(&ctx->current, user)) { continue; } @@ -222,7 +212,7 @@ ct_zones_commit(const struct ovsrec_bridge *br_int, char *user_str = xasprintf("ct-zone-%s", iter->name); if (ctzpe->add) { - char *zone_str = xasprintf("%"PRId32, ctzpe->zone); + char *zone_str = xasprintf("%"PRIu16, ctzpe->ct_zone.zone); struct smap_node *node = smap_get_node(&br_int->external_ids, user_str); if (!node || strcmp(node->value, zone_str)) { @@ -281,12 +271,17 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, * or not. If so, then fall back to full recompute of * ct_zone engine. */ char *snat_dp_zone_key = alloc_nat_zone_key(name, "snat"); - struct simap_node *simap_node = - simap_find(&ctx->current, snat_dp_zone_key); + struct shash_node *node = shash_find(&ctx->current, snat_dp_zone_key); free(snat_dp_zone_key); - if (!simap_node || simap_node->data != req_snat_zone) { - /* There is no entry yet or the requested snat zone has changed. - * Trigger full recompute of ct_zones engine. */ + + if (!node) { + return false; + } + + struct ct_zone *ct_zone = node->data; + if (ct_zone->zone != req_snat_zone) { + /* The requested snat zone has changed. Trigger full recompute of + * ct_zones engine. */ return false; } @@ -298,17 +293,24 @@ bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name, bool updated, int *scan_start) { - struct simap_node *ct_zone = simap_find(&ctx->current, name); - if (updated && !ct_zone) { + struct shash_node *node = shash_find(&ctx->current, name); + if (updated && !node) { ct_zone_assign_unused(ctx, name, scan_start); return true; - } else if (!updated && ct_zone_remove(ctx, ct_zone)) { + } else if (!updated && node && ct_zone_remove(ctx, node->name)) { return true; } return false; } +uint16_t +ct_zone_find_zone(const struct shash *ct_zones, const char *name) +{ + struct ct_zone *ct_zone = shash_find_data(ct_zones, name); + return ct_zone ? ct_zone->zone : 0; +} + static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, @@ -323,33 +325,55 @@ ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, } *scan_start = zone + 1; + ct_zone_add(ctx, zone_name, zone, true); - ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, - zone, true, zone_name); - - bitmap_set1(ctx->bitmap, zone); - simap_put(&ctx->current, zone_name, zone); return true; } static bool -ct_zone_remove(struct ct_zone_ctx *ctx, struct simap_node *ct_zone) +ct_zone_remove(struct ct_zone_ctx *ctx, const char *name) { - if (!ct_zone) { + struct shash_node *node = shash_find(&ctx->current, name); + if (!node) { return false; } - VLOG_DBG("removing ct zone %"PRId32" for '%s'", ct_zone->data, - ct_zone->name); + struct ct_zone *ct_zone = node->data; + + VLOG_DBG("removing ct zone %"PRIu16" for '%s'", ct_zone->zone, name); ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, - ct_zone->data, false, ct_zone->name); - bitmap_set0(ctx->bitmap, ct_zone->data); - simap_delete(&ctx->current, ct_zone); + ct_zone, false, name); + bitmap_set0(ctx->bitmap, ct_zone->zone); + shash_delete(&ctx->current, node); + free(ct_zone); return true; } +static void +ct_zone_add(struct ct_zone_ctx *ctx, const char *name, uint16_t zone, + bool set_pending) +{ + VLOG_DBG("assigning ct zone %"PRIu16" for '%s'", zone, name); + + struct ct_zone *ct_zone = shash_find_data(&ctx->current, name); + if (!ct_zone) { + ct_zone = xmalloc(sizeof *ct_zone); + shash_add(&ctx->current, name, ct_zone); + } + + *ct_zone = (struct ct_zone) { + .zone = zone, + }; + + if (set_pending) { + ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, + ct_zone, true, name); + } + bitmap_set1(ctx->bitmap, zone); +} + static int ct_zone_get_snat(const struct sbrec_datapath_binding *dp) { @@ -359,27 +383,29 @@ ct_zone_get_snat(const struct sbrec_datapath_binding *dp) static void ct_zone_add_pending(struct shash *pending_ct_zones, enum ct_zone_pending_state state, - int zone, bool add, const char *name) + struct ct_zone *zone, bool add, const char *name) { - VLOG_DBG("%s ct zone %"PRId32" for '%s'", - add ? "assigning" : "removing", zone, name); - - struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending); - *pending = (struct ct_zone_pending_entry) { - .state = state, - .zone = zone, - .add = add, - }; - /* Its important that we add only one entry for the key 'name'. * Replace 'pending' with 'existing' and free up 'existing'. * Otherwise, we may end up in a continuous loop of adding * and deleting the zone entry in the 'external_ids' of * integration bridge. */ - struct ct_zone_pending_entry *existing = - shash_replace(pending_ct_zones, name, pending); - free(existing); + struct ct_zone_pending_entry *entry = + shash_find_data(pending_ct_zones, name); + + if (!entry) { + entry = xmalloc(sizeof *entry); + entry->state = CT_ZONE_STATE_NEW; + + shash_add(pending_ct_zones, name, entry); + } + + *entry = (struct ct_zone_pending_entry) { + .ct_zone = *zone, + .state = MIN(entry->state, state), + .add = add, + }; } /* Replaces a UUID prefix from 'uuid_zone' (if any) with the @@ -420,19 +446,20 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, VLOG_DBG("ct zone %"PRId32" replace uuid name '%s' with '%s'", zone, name, new_name); + struct ct_zone ct_zone = { + .zone = zone, + }; /* Make sure we remove the uuid one in the next OvS DB commit without * flush. */ ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED, - zone, false, name); + &ct_zone, false, name); /* Store the zone in OvS DB with name instead of uuid without flush. * */ ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED, - zone, true, new_name); + &ct_zone, true, new_name); current_name = new_name; } - simap_put(&ctx->current, current_name, zone); - bitmap_set1(ctx->bitmap, zone); - + ct_zone_add(ctx, current_name, zone, false); free(new_name); } diff --git a/controller/ct-zone.h b/controller/ct-zone.h index 889bdf2fc..af68de2d6 100644 --- a/controller/ct-zone.h +++ b/controller/ct-zone.h @@ -37,8 +37,12 @@ struct ct_zone_ctx { struct shash pending; /* Pending entries, * 'struct ct_zone_pending_entry' * by name. */ - struct simap current; /* Current CT zones mapping - * (zone id by name). */ + struct shash current; /* Current CT zones mapping + * (struct ct_zone by name). */ +}; + +struct ct_zone { + uint16_t zone; }; /* States to move through when a new conntrack zone has been allocated. */ @@ -47,10 +51,12 @@ enum ct_zone_pending_state { CT_ZONE_OF_SENT, /* Sent and waiting for confirmation on flush. */ CT_ZONE_DB_QUEUED, /* Waiting for DB transaction to open. */ CT_ZONE_DB_SENT, /* Sent and waiting for confirmation from DB. */ + CT_ZONE_STATE_NEW, /* Indication that the pending state was just + * created. Must remain last in the enum. */ }; struct ct_zone_pending_entry { - int zone; + struct ct_zone ct_zone; bool add; /* Is the entry being added? */ ovs_be32 of_xid; /* Transaction id for barrier. */ enum ct_zone_pending_state state; @@ -70,5 +76,6 @@ bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, const struct sbrec_datapath_binding *dp); bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name, bool updated, int *scan_start); +uint16_t ct_zone_find_zone(const struct shash *ct_zones, const char *name); #endif /* controller/ct-zone.h */ diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 8a19106c2..f9387d375 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -2709,7 +2709,7 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, SHASH_FOR_EACH(iter, pending_ct_zones) { struct ct_zone_pending_entry *ctzpe = iter->data; if (ctzpe->state == CT_ZONE_OF_QUEUED) { - add_ct_flush_zone(ctzpe->zone, &msgs); + add_ct_flush_zone(ctzpe->ct_zone.zone, &msgs); ctzpe->state = CT_ZONE_OF_SENT; ctzpe->of_xid = 0; } diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 2152195c3..7e1279237 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2191,7 +2191,7 @@ en_ct_zones_init(struct engine_node *node OVS_UNUSED, struct ed_type_ct_zones *data = xzalloc(sizeof *data); shash_init(&data->ctx.pending); - simap_init(&data->ctx.current); + shash_init(&data->ctx.current); return data; } @@ -2208,7 +2208,7 @@ en_ct_zones_cleanup(void *data) { struct ed_type_ct_zones *ct_zones_data = data; - simap_destroy(&ct_zones_data->ctx.current); + shash_destroy_free_data(&ct_zones_data->ctx.current); shash_destroy_free_data(&ct_zones_data->ctx.pending); } @@ -4334,7 +4334,7 @@ static void init_physical_ctx(struct engine_node *node, struct ed_type_ct_zones *ct_zones_data = engine_get_input_data("ct_zones", node); - struct simap *ct_zones = &ct_zones_data->ctx.current; + struct shash *ct_zones = &ct_zones_data->ctx.current; parse_encap_ips(ovs_table, &p_ctx->n_encap_ips, &p_ctx->encap_ips); p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; @@ -6054,12 +6054,13 @@ static void ct_zone_list(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, void *ct_zones_) { - struct simap *ct_zones = ct_zones_; + struct shash *ct_zones = ct_zones_; struct ds ds = DS_EMPTY_INITIALIZER; - struct simap_node *zone; + struct shash_node *node; - SIMAP_FOR_EACH(zone, ct_zones) { - ds_put_format(&ds, "%s %d\n", zone->name, zone->data); + SHASH_FOR_EACH (node, ct_zones) { + struct ct_zone *ct_zone = node->data; + ds_put_format(&ds, "%s %d\n", node->name, ct_zone->zone); } unixctl_command_reply(conn, ds_cstr(&ds)); diff --git a/controller/physical.c b/controller/physical.c index 22756810f..082e00eda 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -17,6 +17,7 @@ #include "binding.h" #include "coverage.h" #include "byte-order.h" +#include "ct-zone.h" #include "encaps.h" #include "flow.h" #include "ha-chassis.h" @@ -212,10 +213,10 @@ get_vtep_port(const struct hmap *local_datapaths, int64_t tunnel_key) static struct zone_ids get_zone_ids(const struct sbrec_port_binding *binding, - const struct simap *ct_zones) + const struct shash *ct_zones) { struct zone_ids zone_ids = { - .ct = simap_get(ct_zones, binding->logical_port) + .ct = ct_zone_find_zone(ct_zones, binding->logical_port) }; const char *name = smap_get(&binding->datapath->external_ids, "name"); @@ -228,11 +229,11 @@ get_zone_ids(const struct sbrec_port_binding *binding, } char *dnat = alloc_nat_zone_key(name, "dnat"); - zone_ids.dnat = simap_get(ct_zones, dnat); + zone_ids.dnat = ct_zone_find_zone(ct_zones, dnat); free(dnat); char *snat = alloc_nat_zone_key(name, "snat"); - zone_ids.snat = simap_get(ct_zones, snat); + zone_ids.snat = ct_zone_find_zone(ct_zones, snat); free(snat); return zone_ids; @@ -631,7 +632,7 @@ put_chassis_mac_conj_id_flow(const struct sbrec_chassis_table *chassis_table, } static void -put_replace_chassis_mac_flows(const struct simap *ct_zones, +put_replace_chassis_mac_flows(const struct shash *ct_zones, const struct sbrec_port_binding *localnet_port, const struct hmap *local_datapaths, @@ -1477,7 +1478,7 @@ enforce_tunneling_for_multichassis_ports( static void consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, enum mf_field_id mff_ovn_geneve, - const struct simap *ct_zones, + const struct shash *ct_zones, const struct sset *active_tunnels, const struct hmap *local_datapaths, const struct shash *local_bindings, @@ -2110,7 +2111,7 @@ mc_ofctrl_add_flow(const struct sbrec_multicast_group *mc, static void consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, enum mf_field_id mff_ovn_geneve, - const struct simap *ct_zones, + const struct shash *ct_zones, const struct hmap *local_datapaths, struct shash *local_bindings, struct simap *patch_ofports, @@ -2174,7 +2175,7 @@ consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, continue; } - int zone_id = simap_get(ct_zones, port->logical_port); + int zone_id = ct_zone_find_zone(ct_zones, port->logical_port); if (zone_id) { put_load(zone_id, MFF_LOG_CT_ZONE, 0, 16, &ofpacts); } diff --git a/controller/physical.h b/controller/physical.h index 7fe8ee3c1..8ec90a3ab 100644 --- a/controller/physical.h +++ b/controller/physical.h @@ -61,7 +61,7 @@ struct physical_ctx { const struct if_status_mgr *if_mgr; struct hmap *local_datapaths; struct sset *local_lports; - const struct simap *ct_zones; + const struct shash *ct_zones; enum mf_field_id mff_ovn_geneve; struct shash *local_bindings; struct simap *patch_ofports; From patchwork Mon Jun 17 06:58:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1948479 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=KZbFNsYf; 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 4W2ghs4hc5z20Wg for ; Mon, 17 Jun 2024 16:58:53 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 1C93660867; Mon, 17 Jun 2024 06:58:52 +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 HDBPiIDy4L_b; Mon, 17 Jun 2024 06:58:50 +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 smtp3.osuosl.org 478F560870 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=KZbFNsYf Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 478F560870; Mon, 17 Jun 2024 06:58:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0E6F9C0012; Mon, 17 Jun 2024 06:58:50 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 13993C0015 for ; Mon, 17 Jun 2024 06:58:47 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id A6321408A2 for ; Mon, 17 Jun 2024 06:58:46 +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 beaJ9HiFIhkC for ; Mon, 17 Jun 2024 06:58:43 +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 smtp4.osuosl.org 9FE1B40578 Authentication-Results: smtp4.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 9FE1B40578 Authentication-Results: smtp4.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KZbFNsYf 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 9FE1B40578 for ; Mon, 17 Jun 2024 06:58:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718607520; 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=k340tPZoBqP25KERvvXBK21G+U9OCKkYxLhJ1ph9faY=; b=KZbFNsYf3b1rqS0Es3LdD7m59wGmSH9L4DyelAUrSR4SH4XZOM+51FnMsfgR9qCSxqmC4M MahvYoUs8aVoqGy5CJ0iETlixNXY6WgqPWkVjXAjtphyHLKosNszJBQ669MpBvDITIHkb9 jCQdhwUGNTvUb841gNyqrXWZMmADOTQ= Received: from mx-prod-mc-03.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-446-89qgWXfWMuuc6iGK7DxWEQ-1; Mon, 17 Jun 2024 02:58:35 -0400 X-MC-Unique: 89qgWXfWMuuc6iGK7DxWEQ-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (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-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 1723719560AF for ; Mon, 17 Jun 2024 06:58:35 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 1B40F19560AE; Mon, 17 Jun 2024 06:58:33 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Mon, 17 Jun 2024 08:58:26 +0200 Message-ID: <20240617065826.76656-5-amusil@redhat.com> In-Reply-To: <20240617065826.76656-1-amusil@redhat.com> References: <20240617065826.76656-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v3 4/4] controller, northd: Add support for CT zone limits. 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: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Add support for limiting the CT zone usage per Ls, LR or LSP. When the limit is configured on logical switch it will also implicitly set limits for all ports in that logical switch. The port configuration can be overwritten individually and has priority over the whole logical switch configuration. The value 0 means unlimited, when the value is not specified it is derived from OvS default CT limit specified for given OvS datapath. Reported-at: https://bugzilla.redhat.com/2189924 Signed-off-by: Ales Musil --- v3: Rebase on top of latest main. --- NEWS | 3 + controller/ct-zone.c | 170 ++++++++++++++++++++++++++++++++---- controller/ct-zone.h | 12 ++- controller/ovn-controller.c | 25 +++++- lib/ovn-util.c | 17 ++++ lib/ovn-util.h | 3 + northd/northd.c | 8 ++ ovn-nb.xml | 29 ++++++ tests/ovn-controller.at | 99 +++++++++++++++++++++ 9 files changed, 345 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index 3bdc55172..22c1797e6 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,9 @@ Post v24.03.0 has been renamed to "options:ic-route-denylist" in order to comply with inclusive language guidelines. The previous name is still recognized to aid with backwards compatibility. + - Add support for CT zone limit that can be specified per LR + (options:ct-zone-limit), LS (other_config:ct-zone-limit) or LSP + (options:ct-zone-limit). OVN v24.03.0 - 01 Mar 2024 -------------------------- diff --git a/controller/ct-zone.c b/controller/ct-zone.c index 95faec2f1..820ec061e 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -34,6 +34,17 @@ static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, static bool ct_zone_remove(struct ct_zone_ctx *ctx, const char *name); static void ct_zone_add(struct ct_zone_ctx *ctx, const char *name, uint16_t zone, bool set_pending); +static void ct_zone_limits_sync_per_dp(struct ct_zone_ctx *ctx, + const struct sbrec_datapath_binding *dp, + const char *name, + struct ovsdb_idl_index *pb_by_dp); +static void ct_zone_limit_sync(struct ct_zone_ctx *ctx, const char *name, + int64_t limit); +static int64_t ct_zone_get_dp_limit(const struct sbrec_datapath_binding *dp); +static int64_t ct_zone_get_pb_limit(const struct sbrec_port_binding *pb); +static int64_t ct_zone_limit_normalize(int64_t limit); +static struct ovsrec_ct_zone * +ct_zone_find_ovsrec(const struct ovsrec_datapath *dp, uint16_t zone_id); void ct_zones_restore(struct ct_zone_ctx *ctx, @@ -196,11 +207,14 @@ ct_zones_update(const struct sset *local_lports, void ct_zones_commit(const struct ovsrec_bridge *br_int, + const struct ovsrec_datapath *ovs_dp, + struct ovsdb_idl_txn *ovs_idl_txn, struct shash *pending_ct_zones) { struct shash_node *iter; SHASH_FOR_EACH (iter, pending_ct_zones) { struct ct_zone_pending_entry *ctzpe = iter->data; + struct ct_zone *ct_zone = &ctzpe->ct_zone; /* The transaction is open, so any pending entries in the * CT_ZONE_DB_QUEUED must be sent and any in CT_ZONE_DB_QUEUED @@ -212,7 +226,7 @@ ct_zones_commit(const struct ovsrec_bridge *br_int, char *user_str = xasprintf("ct-zone-%s", iter->name); if (ctzpe->add) { - char *zone_str = xasprintf("%"PRIu16, ctzpe->ct_zone.zone); + char *zone_str = xasprintf("%"PRIu16, ct_zone->zone); struct smap_node *node = smap_get_node(&br_int->external_ids, user_str); if (!node || strcmp(node->value, zone_str)) { @@ -227,6 +241,19 @@ ct_zones_commit(const struct ovsrec_bridge *br_int, } free(user_str); + struct ovsrec_ct_zone *ovs_zone = + ct_zone_find_ovsrec(ovs_dp, ct_zone->zone); + if ((!ctzpe->add || ct_zone->limit < 0) && ovs_zone) { + ovsrec_datapath_update_ct_zones_delkey(ovs_dp, ct_zone->zone); + } else if (ctzpe->add && ct_zone->limit >= 0) { + if (!ovs_zone) { + ovs_zone = ovsrec_ct_zone_insert(ovs_idl_txn); + ovsrec_datapath_update_ct_zones_setkey(ovs_dp, ct_zone->zone, + ovs_zone); + } + ovsrec_ct_zone_set_limit(ovs_zone, &ct_zone->limit, 1); + } + ctzpe->state = CT_ZONE_DB_SENT; } } @@ -247,8 +274,19 @@ ct_zones_pending_clear_commited(struct shash *pending) /* Returns "true" when there is no need for full recompute. */ bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, - const struct sbrec_datapath_binding *dp) + const struct sbrec_datapath_binding *dp, + struct ovsdb_idl_index *pb_by_dp) { + const char *name = smap_get(&dp->external_ids, "name"); + if (!name) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' skipping" + "zone check.", UUID_ARGS(&dp->header_.uuid)); + return true; + } + + ct_zone_limits_sync_per_dp(ctx, dp, name, pb_by_dp); + int req_snat_zone = ct_zone_get_snat(dp); if (req_snat_zone == -1) { /* datapath snat ct zone is not set. This condition will also hit @@ -259,14 +297,6 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, return true; } - const char *name = smap_get(&dp->external_ids, "name"); - if (!name) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' skipping" - "zone check.", UUID_ARGS(&dp->header_.uuid)); - return true; - } - /* Check if the requested snat zone has changed for the datapath * or not. If so, then fall back to full recompute of * ct_zone engine. */ @@ -290,14 +320,18 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, /* Returns "true" if there was an update to the context. */ bool -ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name, +ct_zone_handle_port_update(struct ct_zone_ctx *ctx, + const struct sbrec_port_binding *pb, bool updated, int *scan_start) { - struct shash_node *node = shash_find(&ctx->current, name); - if (updated && !node) { - ct_zone_assign_unused(ctx, name, scan_start); + struct shash_node *node = shash_find(&ctx->current, pb->logical_port); + if (updated) { + if (!node) { + ct_zone_assign_unused(ctx, pb->logical_port, scan_start); + } + ct_zone_limit_sync(ctx, pb->logical_port, ct_zone_get_pb_limit(pb)); return true; - } else if (!updated && node && ct_zone_remove(ctx, node->name)) { + } else if (node && ct_zone_remove(ctx, node->name)) { return true; } @@ -311,6 +345,25 @@ ct_zone_find_zone(const struct shash *ct_zones, const char *name) return ct_zone ? ct_zone->zone : 0; } +void +ct_zones_sync_limit(struct ct_zone_ctx *ctx, + const struct hmap *local_datapaths, + struct ovsdb_idl_index *pb_by_dp) +{ + const struct local_datapath *ld; + HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { + const char *name = smap_get(&ld->datapath->external_ids, "name"); + if (!name) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "Missing name for datapath '"UUID_FMT"' " + "skipping zone assignment.", + UUID_ARGS(&ld->datapath->header_.uuid)); + continue; + } + + ct_zone_limits_sync_per_dp(ctx, ld->datapath, name, pb_by_dp); + } +} static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, @@ -365,6 +418,7 @@ ct_zone_add(struct ct_zone_ctx *ctx, const char *name, uint16_t zone, *ct_zone = (struct ct_zone) { .zone = zone, + .limit = -1, }; if (set_pending) { @@ -448,6 +502,7 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, struct ct_zone ct_zone = { .zone = zone, + .limit = -1, }; /* Make sure we remove the uuid one in the next OvS DB commit without * flush. */ @@ -463,3 +518,88 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table, ct_zone_add(ctx, current_name, zone, false); free(new_name); } + +static void +ct_zone_limits_sync_per_dp(struct ct_zone_ctx *ctx, + const struct sbrec_datapath_binding *dp, + const char *name, + struct ovsdb_idl_index *pb_by_dp) +{ + if (smap_get(&dp->external_ids, "logical-switch")) { + struct sbrec_port_binding *target = + sbrec_port_binding_index_init_row(pb_by_dp); + sbrec_port_binding_index_set_datapath(target, dp); + + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, pb_by_dp) { + ct_zone_limit_sync(ctx, pb->logical_port, + ct_zone_get_pb_limit(pb)); + } + + sbrec_port_binding_index_destroy_row(target); + } + + int64_t dp_limit = ct_zone_get_dp_limit(dp); + char *dnat = alloc_nat_zone_key(name, "dnat"); + char *snat = alloc_nat_zone_key(name, "snat"); + + ct_zone_limit_sync(ctx, dnat, dp_limit); + ct_zone_limit_sync(ctx, snat, dp_limit); + + free(dnat); + free(snat); +} + +static void +ct_zone_limit_sync(struct ct_zone_ctx *ctx, const char *name, int64_t limit) +{ + struct ct_zone *ct_zone = shash_find_data(&ctx->current, name); + + if (!ct_zone) { + return; + } + + if (ct_zone->limit != limit) { + ct_zone->limit = limit; + /* Add pending entry only for DB store to avoid flushing the zone. */ + ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED, ct_zone, + true, name); + VLOG_DBG("setting ct zone %"PRIu16" limit to %"PRId64, + ct_zone->zone, ct_zone->limit); + } +} + +static int64_t +ct_zone_get_dp_limit(const struct sbrec_datapath_binding *dp) +{ + int64_t limit = ovn_smap_get_llong(&dp->external_ids, "ct-zone-limit", -1); + return ct_zone_limit_normalize(limit); +} + +static int64_t +ct_zone_get_pb_limit(const struct sbrec_port_binding *pb) +{ + int64_t dp_limit = ovn_smap_get_llong(&pb->datapath->external_ids, + "ct-zone-limit", -1); + int64_t limit = ovn_smap_get_llong(&pb->options, + "ct-zone-limit", dp_limit); + return ct_zone_limit_normalize(limit); +} + +static int64_t +ct_zone_limit_normalize(int64_t limit) +{ + return limit >= 0 && limit <= UINT32_MAX ? limit : -1; +} + +static struct ovsrec_ct_zone * +ct_zone_find_ovsrec(const struct ovsrec_datapath *dp, uint16_t zone_id) +{ + for (int i = 0; i < dp->n_ct_zones; i++) { + if (dp->key_ct_zones[i] == zone_id) { + return dp->value_ct_zones[i]; + } + } + + return NULL; +} diff --git a/controller/ct-zone.h b/controller/ct-zone.h index af68de2d6..7f157bdfa 100644 --- a/controller/ct-zone.h +++ b/controller/ct-zone.h @@ -43,6 +43,7 @@ struct ct_zone_ctx { struct ct_zone { uint16_t zone; + int64_t limit; }; /* States to move through when a new conntrack zone has been allocated. */ @@ -70,12 +71,19 @@ void ct_zones_update(const struct sset *local_lports, const struct hmap *local_datapaths, struct ct_zone_ctx *ctx); void ct_zones_commit(const struct ovsrec_bridge *br_int, + const struct ovsrec_datapath *ovs_dp, + struct ovsdb_idl_txn *ovs_idl_txn, struct shash *pending_ct_zones); void ct_zones_pending_clear_commited(struct shash *pending); bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, - const struct sbrec_datapath_binding *dp); -bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name, + const struct sbrec_datapath_binding *dp, + struct ovsdb_idl_index *pb_by_dp); +bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, + const struct sbrec_port_binding *pb, bool updated, int *scan_start); uint16_t ct_zone_find_zone(const struct shash *ct_zones, const char *name); +void ct_zones_sync_limit(struct ct_zone_ctx *ctx, + const struct hmap *local_datapaths, + struct ovsdb_idl_index *pb_by_dp); #endif /* controller/ct-zone.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 7e1279237..58d717786 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -798,6 +798,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_private_key); ovsdb_idl_add_table(ovs_idl, &ovsrec_table_datapath); ovsdb_idl_add_column(ovs_idl, &ovsrec_datapath_col_capabilities); + ovsdb_idl_add_column(ovs_idl, &ovsrec_datapath_col_ct_zones); ovsdb_idl_add_table(ovs_idl, &ovsrec_table_flow_sample_collector_set); ovsdb_idl_add_table(ovs_idl, &ovsrec_table_qos); ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_other_config); @@ -806,6 +807,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue); ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_other_config); ovsdb_idl_add_column(ovs_idl, &ovsrec_queue_col_external_ids); + ovsdb_idl_add_table(ovs_idl, &ovsrec_table_ct_zone); + ovsdb_idl_add_column(ovs_idl, &ovsrec_ct_zone_col_limit); chassis_register_ovs_idl(ovs_idl); encaps_register_ovs_idl(ovs_idl); @@ -2218,6 +2221,10 @@ en_ct_zones_run(struct engine_node *node, void *data) struct ed_type_ct_zones *ct_zones_data = data; struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); + struct ovsdb_idl_index *sbrec_port_binding_by_datapath = + engine_ovsdb_node_get_index( + engine_get_input("SB_port_binding", node), + "datapath"); const struct ovsrec_open_vswitch_table *ovs_table = EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node)); @@ -2231,6 +2238,8 @@ en_ct_zones_run(struct engine_node *node, void *data) ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); ct_zones_update(&rt_data->local_lports, &rt_data->local_datapaths, &ct_zones_data->ctx); + ct_zones_sync_limit(&ct_zones_data->ctx, &rt_data->local_datapaths, + sbrec_port_binding_by_datapath); ct_zones_data->recomputed = true; engine_set_node_state(node, EN_UPDATED); @@ -2248,6 +2257,10 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data) engine_get_input_data("runtime_data", node); const struct sbrec_datapath_binding_table *dp_table = EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); + struct ovsdb_idl_index *sbrec_port_binding_by_datapath = + engine_ovsdb_node_get_index( + engine_get_input("SB_port_binding", node), + "datapath"); SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { if (!get_local_datapath(&rt_data->local_datapaths, @@ -2261,7 +2274,8 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data) return false; } - if (!ct_zone_handle_dp_update(&ct_zones_data->ctx, dp)) { + if (!ct_zone_handle_dp_update(&ct_zones_data->ctx, dp, + sbrec_port_binding_by_datapath)) { return false; } } @@ -2309,8 +2323,8 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) bool port_updated = t_lport->tracked_type != TRACKED_RESOURCE_REMOVED; updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, - t_lport->pb->logical_port, - port_updated, &scan_start); + t_lport->pb, port_updated, + &scan_start); } } @@ -5152,6 +5166,9 @@ main(int argc, char *argv[]) ct_zones_datapath_binding_handler); engine_add_input(&en_ct_zones, &en_runtime_data, ct_zones_runtime_data_handler); + /* The CT node just needs the index for port bindings by name. The data + * for ports are processed in the runtime handler. */ + engine_add_input(&en_ct_zones, &en_sb_port_binding, engine_noop_handler); engine_add_input(&en_ovs_interface_shadow, &en_ovs_interface, ovs_interface_shadow_ovs_interface_handler); @@ -5556,7 +5573,7 @@ main(int argc, char *argv[]) if (ct_zones_data) { stopwatch_start(CT_ZONE_COMMIT_STOPWATCH_NAME, time_msec()); - ct_zones_commit(br_int, + ct_zones_commit(br_int, br_int_dp, ovs_idl_txn, &ct_zones_data->ctx.pending); stopwatch_stop(CT_ZONE_COMMIT_STOPWATCH_NAME, time_msec()); diff --git a/lib/ovn-util.c b/lib/ovn-util.c index 58e941193..1ad347419 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -816,6 +816,23 @@ str_tolower(const char *orig) return copy; } +/* This is a wrapper function which get the value associated with 'key' in + * 'smap' and converts it to a long long. If 'key' is not in 'smap' or a + * valid unsigned integer can't be parsed from its value, returns 'def'. + */ +long long +ovn_smap_get_llong(const struct smap *smap, const char *key, long long def) +{ + const char *value = smap_get(smap, key); + long long ll_value; + + if (!value || !str_to_llong(value, 10, &ll_value)) { + return def; + } + + return ll_value; +} + /* For a 'key' of the form "IP:port" or just "IP", sets 'port', * 'ip_address' and 'ip' ('struct in6_addr' IPv6 or IPv4 mapped address). * The caller must free() the memory allocated for 'ip_address'. diff --git a/lib/ovn-util.h b/lib/ovn-util.h index f75b821b6..ae971ce5a 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -211,6 +211,9 @@ char *normalize_v46_prefix(const struct in6_addr *prefix, unsigned int plen); */ char *str_tolower(const char *orig); +long long ovn_smap_get_llong(const struct smap *smap, const char *key, + long long def); + /* OVN daemon options. Taken from ovs/lib/daemon.h. */ #define OVN_DAEMON_OPTION_ENUMS \ OVN_OPT_DETACH, \ diff --git a/northd/northd.c b/northd/northd.c index b0d0c4747..5ff8894da 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -737,6 +737,14 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) smap_add(&ids, "name2", name2); } + int64_t ct_zone_limit = ovn_smap_get_llong(od->nbs ? + &od->nbs->other_config : + &od->nbr->options, + "ct-zone-limit", -1); + if (ct_zone_limit > 0) { + smap_add_format(&ids, "ct-zone-limit", "%"PRId64, ct_zone_limit); + } + /* Set interconn-ts. */ if (od->nbs) { const char *ts = smap_get(&od->nbs->other_config, "interconn-ts"); diff --git a/ovn-nb.xml b/ovn-nb.xml index 83f3d5f83..295139296 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -721,6 +721,17 @@ this timeout will be automatically removed. The value defaults to 0, which means disabled. + + + CT zone limit value for given + . This value will be propagated to all + when configured, but can be + overwritten individually per . The + value 0 means unlimited, when the option is not present the limit + is not set and the zone limit is derived from OvS default datapath + limit. + @@ -1122,6 +1133,16 @@ false. + + CT zone limit value for given + . This value has priority over + limit specified on when configured. + The value 0 means unlimited, when the option is not present the limit + is not set and the zone limit is derived from OvS default datapath + limit. + + @@ -2785,6 +2806,14 @@ or

+ + + CT zone limit value for given + . The value 0 means unlimited, when the + option is not present the limit is not set and the zone limit is + derived from OvS default datapath limit. + diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 2cb86dc98..0a20cbc09 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -3127,3 +3127,102 @@ OVS_WAIT_UNTIL([grep -q 'tcp:127.0.0.1:1235: connected' hv1/ovn-controller.log]) OVN_CLEANUP([hv1]) AT_CLEANUP + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller - CT zone limit]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone + +check ovs-vsctl add-port br-int lsp \ + -- set Interface lsp external-ids:iface-id=lsp + +check ovn-nbctl lr-add lr + +check ovn-nbctl ls-add ls +check ovn-nbctl lsp-add ls ls-lr +check ovn-nbctl lsp-set-type ls-lr router +check ovn-nbctl lsp-set-addresses ls-lr router +check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1 + +check ovn-nbctl lsp-add ls lsp +check ovn-nbctl lsp-set-addresses lsp "00:00:00:00:00:02 10.0.0.2" + +check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1 +check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1 + +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +get_zone_num() { + output=$1 + name=$2 + + printf "$output" | grep $name | cut -d ' ' -f 2 +} + +check_ovs_ct_limit() { + zone=$1 + limit=$2 + + AT_CHECK_UNQUOTED([ovs-appctl dpctl/ct-get-limits zone=$zone | sed "s/count=.*/count=?/;s/default limit=.*/default limit=?/" | sort], [0], [dnl +default limit=? +zone=$zone,limit=$limit,count=? +]) +} + +wait_ovs_ct_limit_count() { + count=$1 + + OVS_WAIT_UNTIL([test $count -eq $(ovs-vsctl --no-headings --format=table list CT_Zone | wc -l)]) +} + +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +lr_dnat=$(get_zone_num "$ct_zones" lr_dnat) +lr_snat=$(get_zone_num "$ct_zones" lr_snat) + +ls_dnat=$(get_zone_num "$ct_zones" ls_dnat) +ls_snat=$(get_zone_num "$ct_zones" ls_snat) + +lsp=$(get_zone_num "$ct_zones" lsp) + +wait_ovs_ct_limit_count 0 + +check ovn-nbctl --wait=hv set Logical_Router lr options:ct-zone-limit=5 +wait_ovs_ct_limit_count 2 +check_ovs_ct_limit $lr_dnat 5 +check_ovs_ct_limit $lr_snat 5 + +check ovn-nbctl --wait=hv remove Logical_Router lr options ct-zone-limit +wait_ovs_ct_limit_count 0 + +check ovn-nbctl --wait=hv set Logical_Switch ls other_config:ct-zone-limit=10 +wait_ovs_ct_limit_count 3 +check_ovs_ct_limit $ls_dnat 10 +check_ovs_ct_limit $ls_snat 10 +check_ovs_ct_limit $lsp 10 + +check ovn-nbctl --wait=hv set Logical_Switch_Port lsp options:ct-zone-limit=5 +wait_ovs_ct_limit_count 3 +check_ovs_ct_limit $ls_dnat 10 +check_ovs_ct_limit $ls_snat 10 +check_ovs_ct_limit $lsp 5 + +check ovn-nbctl --wait=hv remove Logical_Switch_Port lsp options ct-zone-limit +wait_ovs_ct_limit_count 3 +check_ovs_ct_limit $ls_dnat 10 +check_ovs_ct_limit $ls_snat 10 +check_ovs_ct_limit $lsp 10 + +check ovn-nbctl --wait=hv remove Logical_Switch ls other_config ct-zone-limit +wait_ovs_ct_limit_count 0 + +OVN_CLEANUP([hv1]) +AT_CLEANUP +])