From patchwork Wed Jun 26 05:56:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1952374 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=fzE9aVuo; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4W89ts1qGlz20X6 for ; Wed, 26 Jun 2024 15:56:37 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C8BB6813B8; Wed, 26 Jun 2024 05:56:34 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id Z1PRiL2qmpPK; Wed, 26 Jun 2024 05:56:30 +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 smtp1.osuosl.org A9D7782105 Authentication-Results: smtp1.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=fzE9aVuo Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id A9D7782105; Wed, 26 Jun 2024 05:56:30 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4372EC1DD7; Wed, 26 Jun 2024 05:56:30 +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 61283C1DD6 for ; Wed, 26 Jun 2024 05:56:28 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 35D44407D0 for ; Wed, 26 Jun 2024 05:56:28 +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 RYlpFObAAm3s for ; Wed, 26 Jun 2024 05:56:25 +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 0E0AE4076B 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 0E0AE4076B 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=fzE9aVuo 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 0E0AE4076B for ; Wed, 26 Jun 2024 05:56:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1719381383; 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=QX1EWzyaL4hGVJQeI83fnMH/DVd6QsU/K6AytWIWt8k=; b=fzE9aVuo1UiIuGFA74KseIeDnUDN+5ldCBeLcdKuN7wbJbwkalmsCT24Yz0S/BKijwe53T vmPGlBqNBy0/2IIuVsj2/Y3gVu7UALOW/cAHAvHTTdaREXIU1JXsaINBYxUW8mL74CTDyL tiuhg0cHpKTNxtgJLuLpMs9eFmoyENA= 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-168-VClqdGl6OeKP7sZhj-hyRA-1; Wed, 26 Jun 2024 01:56:22 -0400 X-MC-Unique: VClqdGl6OeKP7sZhj-hyRA-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (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 582641956096 for ; Wed, 26 Jun 2024 05:56:21 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 867F01956087; Wed, 26 Jun 2024 05:56:19 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Wed, 26 Jun 2024 07:56:13 +0200 Message-ID: <20240626055616.516625-2-amusil@redhat.com> In-Reply-To: <20240626055616.516625-1-amusil@redhat.com> References: <20240626055616.516625-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v4 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 --- v4: Rebase on top of latest main. Added ack from Lorenzo. 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 ed95818a0..ffbd41490 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, @@ -2519,8 +2191,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; } @@ -2537,8 +2209,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 @@ -2555,11 +2227,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); @@ -2590,7 +2262,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. @@ -2612,7 +2284,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) { @@ -2664,25 +2336,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; } } @@ -4709,7 +4372,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; @@ -5643,7 +5306,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, @@ -5871,7 +5534,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); @@ -5931,7 +5594,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()); } @@ -6074,7 +5738,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(), @@ -6185,14 +5849,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 8adec6179..2ecec7baf 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 Wed Jun 26 05:56:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1952375 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=eWBetyPQ; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::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 4W89ts43Mbz20Z9 for ; Wed, 26 Jun 2024 15:56:37 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 1DCCC417D4; Wed, 26 Jun 2024 05:56:35 +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 xuIHLMsFB7ld; Wed, 26 Jun 2024 05:56:34 +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 smtp2.osuosl.org 77C31417D6 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=eWBetyPQ Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 77C31417D6; Wed, 26 Jun 2024 05:56:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1CB7DC1DD9; Wed, 26 Jun 2024 05:56:33 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 38172C1DD6 for ; Wed, 26 Jun 2024 05:56:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 6025D4076B for ; Wed, 26 Jun 2024 05:56:28 +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 fJ9XgzMhz087 for ; Wed, 26 Jun 2024 05:56:27 +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 4AA87407D4 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 4AA87407D4 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=eWBetyPQ 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 4AA87407D4 for ; Wed, 26 Jun 2024 05:56:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1719381386; 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=pp+y6sMUrDAvW8p5BUTAhQRAiQvf+GiJ8HLSZNrrNNI=; b=eWBetyPQerKpVX8k85DdFFnZe0G6/uOUpoBuVm1sMXyTkiRnEQf4Bi4mdDa8BpHsqxhHCv Tr+ZoMvjeimEDJs6a5+TPcN4SS5WVberUgbWLa4N80uMGoSH7MQkne56/Dd7UDwevQPQlV 86vR2XEulYbMgCOaot+I/LBMVI14pfI= 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-688-_JgIMdkKOCm2OtgzCMjHoA-1; Wed, 26 Jun 2024 01:56:23 -0400 X-MC-Unique: _JgIMdkKOCm2OtgzCMjHoA-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (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 CB96D19560AB for ; Wed, 26 Jun 2024 05:56:22 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 94B231956087; Wed, 26 Jun 2024 05:56:21 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Wed, 26 Jun 2024 07:56:14 +0200 Message-ID: <20240626055616.516625-3-amusil@redhat.com> In-Reply-To: <20240626055616.516625-1-amusil@redhat.com> References: <20240626055616.516625-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v4 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 --- v4: Rebase on top of latest main.     Added ack from Lorenzo and fixed one nit. v3: Rebase on top of latest main. --- controller/ct-zone.c | 156 +++++++++++++++++++++++++----------- controller/ct-zone.h | 8 +- controller/ovn-controller.c | 50 ++---------- 3 files changed, 119 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 ffbd41490..ee020eda0 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2262,34 +2262,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; } } @@ -2334,21 +2307,12 @@ 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_NEW || + t_lport->tracked_type == TRACKED_RESOURCE_UPDATED; + updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, + t_lport->pb->logical_port, + port_updated, &scan_start); } } From patchwork Wed Jun 26 05:56:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1952376 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=akzBCPvE; 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 4W89tz3Yp4z20X6 for ; Wed, 26 Jun 2024 15:56:43 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 9F6AD60B22; Wed, 26 Jun 2024 05:56:41 +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 i9LjnI78XWD2; Wed, 26 Jun 2024 05:56:36 +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 4460B60AE6 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=akzBCPvE Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4460B60AE6; Wed, 26 Jun 2024 05:56:35 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9DFCCC1DD6; Wed, 26 Jun 2024 05:56:34 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id A5BE2C1DD6 for ; Wed, 26 Jun 2024 05:56:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 956B560AD2 for ; Wed, 26 Jun 2024 05:56:29 +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 DM7dQn7Q60_r for ; Wed, 26 Jun 2024 05:56:28 +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 smtp3.osuosl.org 2226A60ACD 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 2226A60ACD Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 2226A60ACD for ; Wed, 26 Jun 2024 05:56:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1719381386; 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=pQoWoXAG4zpYFOXWymEnWi/gtsmVRv6dDdxOLCE7fac=; b=akzBCPvEZCqr3aCAat40WmMFHfzVklnGvnOXssRSdFwEqV23NGc0/hcDPjT+clE4SnJqqW 4xu9BDFbhzaijTFnoR53dGkqO0LMhaswYSZqvFZfXGQbjvLkrRXyZgEap79xKSN0lgPdQq PEqALIVx/PQdi5u8T4dlZ4xRgoIDR/s= 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-70-2CHa636gONu5vfnuBAfJHg-1; Wed, 26 Jun 2024 01:56:25 -0400 X-MC-Unique: 2CHa636gONu5vfnuBAfJHg-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (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 4BB2519560AF for ; Wed, 26 Jun 2024 05:56:24 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 36A541956087; Wed, 26 Jun 2024 05:56:22 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Wed, 26 Jun 2024 07:56:15 +0200 Message-ID: <20240626055616.516625-4-amusil@redhat.com> In-Reply-To: <20240626055616.516625-1-amusil@redhat.com> References: <20240626055616.516625-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v4 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 --- v4: Rebase on top of latest main. Address comments from Lorenzo. v3: Rebase on top of latest main. v2: Fix NULL ptr deref. --- controller/ct-zone.c | 169 +++++++++++++++++++++--------------- 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, 126 insertions(+), 92 deletions(-) diff --git a/controller/ct-zone.c b/controller/ct-zone.c index e4f66a52a..6ed1e4108 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,53 @@ 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->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 +381,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 +444,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 ee020eda0..a47ed5d9a 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2192,7 +2192,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; } @@ -2209,7 +2209,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); } @@ -4336,7 +4336,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; @@ -6056,12 +6056,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 Wed Jun 26 05:56:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 1952377 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=D/0/oz76; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::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 4W89tz3j5Mz20Z9 for ; Wed, 26 Jun 2024 15:56:43 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 82183417DB; Wed, 26 Jun 2024 05:56:41 +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 QJB0mkdkVpbX; Wed, 26 Jun 2024 05:56:38 +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 71C1B41803 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=D/0/oz76 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 71C1B41803; Wed, 26 Jun 2024 05:56:37 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id F3655C1DD7; Wed, 26 Jun 2024 05:56:36 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id B0546C1DD7 for ; Wed, 26 Jun 2024 05:56:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 7F8E582120 for ; Wed, 26 Jun 2024 05:56:34 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id L31BCSSAkEtg for ; Wed, 26 Jun 2024 05:56:31 +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 smtp1.osuosl.org 43CE98210B Authentication-Results: smtp1.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 43CE98210B Authentication-Results: smtp1.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=D/0/oz76 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 43CE98210B for ; Wed, 26 Jun 2024 05:56:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1719381390; 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=hFDUqgqPGF4/B9/cdmgMHmtBA8/jQC5rqOGukEgFLSo=; b=D/0/oz76GcUvdPE5Q4Ym0lKXxDk+QphQYsO2z/mC3nFNrtq3ZiMzD/QlS/PPp84jY1uF4W 5WoZcbUWAETSXkyeQW7Alvj/INOr0M/xzmpbo6v6ua9Lza+KM8BT8eIIoYZxy7wAYeW/XN KSN5NR2U4yrSMFc1nq+l1UIqKWsMess= 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-493-UZPui91xPTOZd97_zf8JPw-1; Wed, 26 Jun 2024 01:56:26 -0400 X-MC-Unique: UZPui91xPTOZd97_zf8JPw-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (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 C026219560B0 for ; Wed, 26 Jun 2024 05:56:25 +0000 (UTC) Received: from amusil.brq.redhat.com (unknown [10.43.17.32]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id AA75E1956087; Wed, 26 Jun 2024 05:56:24 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Wed, 26 Jun 2024 07:56:16 +0200 Message-ID: <20240626055616.516625-5-amusil@redhat.com> In-Reply-To: <20240626055616.516625-1-amusil@redhat.com> References: <20240626055616.516625-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v4 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 --- v4: Rebase on top of latest main. Change naming of the ct_zone_limit_sync to avoid potential confusion as suggested by Lorenzo. v3: Rebase on top of latest main. --- NEWS | 3 + controller/ct-zone.c | 175 ++++++++++++++++++++++++++++++++---- 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, 349 insertions(+), 22 deletions(-) diff --git a/NEWS b/NEWS index 01db77d57..f50d68a82 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,9 @@ Post v24.03.0 - NATs can now be given an arbitrary match condition and priority. This allows for conditional NATs to be configured. See the ovn-nb(5) man page for more information. + - 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 6ed1e4108..97f0de6c3 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -34,6 +34,18 @@ 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_update_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_update(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 +208,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 +227,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 +242,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 +275,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_update_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 +298,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 +321,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_update(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 +346,25 @@ ct_zone_find_zone(const struct shash *ct_zones, const char *name) return ct_zone ? ct_zone->zone : 0; } +void +ct_zones_limits_sync(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_update_per_dp(ctx, ld->datapath, name, pb_by_dp); + } +} static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name, @@ -363,7 +417,10 @@ ct_zone_add(struct ct_zone_ctx *ctx, const char *name, uint16_t zone, shash_add(&ctx->current, name, ct_zone); } - ct_zone->zone = zone; + *ct_zone = (struct ct_zone) { + .zone = zone, + .limit = -1, + }; if (set_pending) { ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED, @@ -446,6 +503,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. */ @@ -461,3 +519,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_update_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_update(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_update(ctx, dnat, dp_limit); + ct_zone_limit_update(ctx, snat, dp_limit); + + free(dnat); + free(snat); +} + +static void +ct_zone_limit_update(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..5d7f66224 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_limits_sync(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 a47ed5d9a..1b4c89cda 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); @@ -807,6 +808,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) 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_column(ovs_idl, &ovsrec_interface_col_link_state); + 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); @@ -2219,6 +2222,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)); @@ -2232,6 +2239,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_limits_sync(&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); @@ -2249,6 +2258,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, @@ -2262,7 +2275,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; } } @@ -2311,8 +2325,8 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) t_lport->tracked_type == TRACKED_RESOURCE_NEW || t_lport->tracked_type == TRACKED_RESOURCE_UPDATED; 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); } } @@ -5154,6 +5168,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); @@ -5558,7 +5575,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 7e474a7b8..7d3ad63a1 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 b0f8d3687..53e697308 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 +])