From patchwork Wed Sep 27 19:57:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Haller X-Patchwork-Id: 1840415 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=boihEwf2; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RwnbS6g2Xz1ypJ for ; Thu, 28 Sep 2023 06:03:00 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229819AbjI0UDA (ORCPT ); Wed, 27 Sep 2023 16:03:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60144 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229817AbjI0UC7 (ORCPT ); Wed, 27 Sep 2023 16:02:59 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8DC78126 for ; Wed, 27 Sep 2023 13:02:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695844928; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+TCLL8ggyPYHaTKeMaDPRoCRE64OltaaMKUScs1Seg=; b=boihEwf20jMKKnD1hlYLlfOZFyfAjJBL9/o1oSJeviuTi+I+DmKfRlMk4T0QqJc9hF3Dl3 gP3/fIQImGIUODtiWx6POQ4cMQ0UmCYVyYdvUsIdCjSuJUVjf9myuMI1Vpq+V+wdUTQbOx b8jNLLrc/ZP+N1JQwUp5mAye2YM9Z3U= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-64-zbq94umbN2WjCjzv4oisLg-1; Wed, 27 Sep 2023 16:01:56 -0400 X-MC-Unique: zbq94umbN2WjCjzv4oisLg-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2F1C9101AA42 for ; Wed, 27 Sep 2023 20:01:56 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.194.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8312940C6EA8; Wed, 27 Sep 2023 20:01:55 +0000 (UTC) From: Thomas Haller To: NetFilter Cc: Thomas Haller Subject: [PATCH nft 1/5] datatype: make "flags" field of datatype struct simple booleans Date: Wed, 27 Sep 2023 21:57:24 +0200 Message-ID: <20230927200143.3798124-2-thaller@redhat.com> In-Reply-To: <20230927200143.3798124-1-thaller@redhat.com> References: <20230927200143.3798124-1-thaller@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Flags are not always bad. For example, as a function argument they allow easier extension in the future. But with datatype's "flags" argument and enum datatype_flags there are no advantages of this approach. - replace DTYPE_F_PREFIX with a "bool f_prefix:1" field. - replace DTYPE_F_ALLOC with a "bool f_alloc:1" field. - the new boolean fields are made bitfields, although for the moment that does not reduce the size of the struct. If we add more flags, that will be different. - also reorder fields in "struct datatype" so that fields of similar alignment (and type) are beside each other. Specifically moving "refcnt" field beside other integer fields saves one pointer size on x86-64. Signed-off-by: Thomas Haller --- include/datatype.h | 32 +++++++++++++++----------------- src/datatype.c | 21 ++++++++++----------- src/meta.c | 2 +- src/netlink_delinearize.c | 2 +- src/rt.c | 2 +- src/segtree.c | 5 ++--- 6 files changed, 30 insertions(+), 34 deletions(-) diff --git a/include/datatype.h b/include/datatype.h index 09a7894567e4..b53a739e1e6c 100644 --- a/include/datatype.h +++ b/include/datatype.h @@ -120,26 +120,18 @@ enum byteorder { struct expr; -/** - * enum datatype_flags - * - * @DTYPE_F_ALLOC: datatype is dynamically allocated - * @DTYPE_F_PREFIX: preferred representation for ranges is a prefix - */ -enum datatype_flags { - DTYPE_F_ALLOC = (1 << 0), - DTYPE_F_PREFIX = (1 << 1), -}; - struct parse_ctx; /** * struct datatype * * @type: numeric identifier - * @byteorder: byteorder of type (non-basetypes only) - * @flags: flags * @size: type size (fixed sized non-basetypes only) * @subtypes: number of subtypes (concat type) + * @refcnt: reference counter for dynamically allocated instances. + * @byteorder: byteorder of type (non-basetypes only) + * @f_prefix: preferred representation for ranges is a prefix + * @f_alloc: whether the instance is dynamically allocated. If not, datatype_get() and + * datatype_free() are NOPs. * @name: type name * @desc: type description * @basetype: basetype for subtypes, determines type compatibility @@ -147,14 +139,21 @@ struct parse_ctx; * @print: function to print a constant of this type * @parse: function to parse a symbol and return an expression * @sym_tbl: symbol table for this type - * @refcnt: reference counter (only for DTYPE_F_ALLOC) */ struct datatype { uint32_t type; - enum byteorder byteorder; - unsigned int flags; unsigned int size; unsigned int subtypes; + + /* Refcount for dynamically allocated instances. For static instances + * this is zero (get() and free() are NOPs). + */ + unsigned int refcnt; + + enum byteorder byteorder; + bool f_prefix:1; + bool f_alloc:1; + const char *name; const char *desc; const struct datatype *basetype; @@ -169,7 +168,6 @@ struct datatype { struct error_record *(*err)(const struct expr *sym); void (*describe)(struct output_ctx *octx); const struct symbol_table *sym_tbl; - unsigned int refcnt; }; extern const struct datatype *datatype_lookup(enum datatypes type); diff --git a/src/datatype.c b/src/datatype.c index 6fe46e899c4b..464eb49171c6 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -642,7 +642,7 @@ const struct datatype ipaddr_type = { .basetype = &integer_type, .print = ipaddr_type_print, .parse = ipaddr_type_parse, - .flags = DTYPE_F_PREFIX, + .f_prefix = true, }; static void ip6addr_type_print(const struct expr *expr, struct output_ctx *octx) @@ -709,7 +709,7 @@ const struct datatype ip6addr_type = { .basetype = &integer_type, .print = ip6addr_type_print, .parse = ip6addr_type_parse, - .flags = DTYPE_F_PREFIX, + .f_prefix = true, }; static void inet_protocol_type_print(const struct expr *expr, @@ -945,7 +945,7 @@ const struct datatype mark_type = { .print = mark_type_print, .json = mark_type_json, .parse = mark_type_parse, - .flags = DTYPE_F_PREFIX, + .f_prefix = true, }; static const struct symbol_table icmp_code_tbl = { @@ -1204,7 +1204,7 @@ static struct datatype *datatype_alloc(void) struct datatype *dtype; dtype = xzalloc(sizeof(*dtype)); - dtype->flags = DTYPE_F_ALLOC; + dtype->f_alloc = true; dtype->refcnt = 1; return dtype; @@ -1216,10 +1216,10 @@ const struct datatype *datatype_get(const struct datatype *ptr) if (!dtype) return NULL; - if (!(dtype->flags & DTYPE_F_ALLOC)) - return dtype; - dtype->refcnt++; + if (dtype->f_alloc) + dtype->refcnt++; + return dtype; } @@ -1246,7 +1246,7 @@ struct datatype *datatype_clone(const struct datatype *orig_dtype) *dtype = *orig_dtype; dtype->name = xstrdup(orig_dtype->name); dtype->desc = xstrdup(orig_dtype->desc); - dtype->flags = DTYPE_F_ALLOC | orig_dtype->flags; + dtype->f_alloc = true; dtype->refcnt = 1; return dtype; @@ -1258,10 +1258,9 @@ void datatype_free(const struct datatype *ptr) if (!dtype) return; - if (!(dtype->flags & DTYPE_F_ALLOC)) - return; - assert(dtype->refcnt != 0); + if (!dtype->f_alloc) + return; if (--dtype->refcnt > 0) return; diff --git a/src/meta.c b/src/meta.c index b578d5e24c06..536954a7f9eb 100644 --- a/src/meta.c +++ b/src/meta.c @@ -367,7 +367,7 @@ const struct datatype devgroup_type = { .print = devgroup_type_print, .json = devgroup_type_json, .parse = devgroup_type_parse, - .flags = DTYPE_F_PREFIX, + .f_prefix = true, }; const struct datatype ifname_type = { diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index e21451044451..9dc1ffa533d4 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -2567,7 +2567,7 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, default: break; } - } else if (binop->left->dtype->flags & DTYPE_F_PREFIX && + } else if (binop->left->dtype->f_prefix && binop->op == OP_AND && expr->right->etype == EXPR_VALUE && expr_mask_is_prefix(binop->right)) { expr->left = expr_get(binop->left); diff --git a/src/rt.c b/src/rt.c index f5c80559ffee..63939c23604c 100644 --- a/src/rt.c +++ b/src/rt.c @@ -54,7 +54,7 @@ const struct datatype realm_type = { .basetype = &integer_type, .print = realm_type_print, .parse = realm_type_parse, - .flags = DTYPE_F_PREFIX, + .f_prefix = true, }; const struct rt_template rt_templates[] = { diff --git a/src/segtree.c b/src/segtree.c index 28172b30c5b3..768d27b8188c 100644 --- a/src/segtree.c +++ b/src/segtree.c @@ -401,8 +401,7 @@ void concat_range_aggregate(struct expr *set) goto next; } - if (prefix_len < 0 || - !(r1->dtype->flags & DTYPE_F_PREFIX)) { + if (prefix_len < 0 || !r1->dtype->f_prefix) { tmp = range_expr_alloc(&r1->location, r1, r2); @@ -517,7 +516,7 @@ add_interval(struct expr *set, struct expr *low, struct expr *i) expr = expr_get(low); } else if (range_is_prefix(range) && !mpz_cmp_ui(p, 0)) { - if (i->dtype->flags & DTYPE_F_PREFIX) + if (i->dtype->f_prefix) expr = interval_to_prefix(low, i, range); else if (expr_basetype(i)->type == TYPE_STRING) expr = interval_to_string(low, i, range); From patchwork Wed Sep 27 19:57:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Haller X-Patchwork-Id: 1840412 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=QtsyJP8y; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RwnbD1vNbz1yqf for ; Thu, 28 Sep 2023 06:02:48 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229811AbjI0UCr (ORCPT ); Wed, 27 Sep 2023 16:02:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229754AbjI0UCq (ORCPT ); Wed, 27 Sep 2023 16:02:46 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0E52410E for ; Wed, 27 Sep 2023 13:01:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695844919; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=d4uoDgQZ9loIRgkRJbUYDfJ+YeZKQ9gYnUT7Y6BARMs=; b=QtsyJP8yC7DJrQF7UmKD80efdwHQYMRJDPNlCdt9EQlJtvMFuNZUBETpVbVkq/Ix8ZILsC AsQ5P07zbGarAgOUXTtmgDQKcAxn77JSY8umeGesU9mfnwxEvOFf05NAaBAaM+c3h/vTao qWMe/mAhnnWKHweMPeY24drohZvViW0= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-311-AXaTIj3hMi6PZUjG0XOkjQ-1; Wed, 27 Sep 2023 16:01:57 -0400 X-MC-Unique: AXaTIj3hMi6PZUjG0XOkjQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E9C041C00BBC for ; Wed, 27 Sep 2023 20:01:56 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.194.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 690DF40C6EA8; Wed, 27 Sep 2023 20:01:56 +0000 (UTC) From: Thomas Haller To: NetFilter Cc: Thomas Haller Subject: [PATCH nft 2/5] datatype: don't clone static name/desc strings for datatype Date: Wed, 27 Sep 2023 21:57:25 +0200 Message-ID: <20230927200143.3798124-3-thaller@redhat.com> In-Reply-To: <20230927200143.3798124-1-thaller@redhat.com> References: <20230927200143.3798124-1-thaller@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Spam-Status: No, score=1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SBL_CSS,SPF_HELO_NONE, SPF_NONE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Avoid cloning static strings for "struct datatype". With concat_type_alloc(), the name/desc are generated dynamically and need to be allocated. However, datatype_clone() only reuses the original name/desc strings. If those strings are static already, we don't need to clone them. Note that there are no other places that also want to change or set the name/desc. If there were, they would need to handle the new fact that the strings may or may not be dynamically allocated. Signed-off-by: Thomas Haller --- include/datatype.h | 2 ++ src/datatype.c | 15 ++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/include/datatype.h b/include/datatype.h index b53a739e1e6c..465ade290652 100644 --- a/include/datatype.h +++ b/include/datatype.h @@ -132,6 +132,7 @@ struct parse_ctx; * @f_prefix: preferred representation for ranges is a prefix * @f_alloc: whether the instance is dynamically allocated. If not, datatype_get() and * datatype_free() are NOPs. + * @f_allocated_strings: whether @name and @desc are heap allocated or static. * @name: type name * @desc: type description * @basetype: basetype for subtypes, determines type compatibility @@ -153,6 +154,7 @@ struct datatype { enum byteorder byteorder; bool f_prefix:1; bool f_alloc:1; + bool f_allocated_strings:1; const char *name; const char *desc; diff --git a/src/datatype.c b/src/datatype.c index 464eb49171c6..1c557a06c751 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -1244,8 +1244,10 @@ struct datatype *datatype_clone(const struct datatype *orig_dtype) dtype = xzalloc(sizeof(*dtype)); *dtype = *orig_dtype; - dtype->name = xstrdup(orig_dtype->name); - dtype->desc = xstrdup(orig_dtype->desc); + if (orig_dtype->f_allocated_strings) { + dtype->name = xstrdup(orig_dtype->name); + dtype->desc = xstrdup(orig_dtype->desc); + } dtype->f_alloc = true; dtype->refcnt = 1; @@ -1265,8 +1267,10 @@ void datatype_free(const struct datatype *ptr) if (--dtype->refcnt > 0) return; - xfree(dtype->name); - xfree(dtype->desc); + if (dtype->f_allocated_strings) { + xfree(dtype->name); + xfree(dtype->desc); + } xfree(dtype); } @@ -1299,7 +1303,8 @@ const struct datatype *concat_type_alloc(uint32_t type) dtype = datatype_alloc(); dtype->type = type; dtype->size = size; - dtype->subtypes = subtypes; + dtype->subtypes = subtypes; + dtype->f_allocated_strings = true; dtype->name = xstrdup(name); dtype->desc = xstrdup(desc); dtype->parse = concat_type_parse; From patchwork Wed Sep 27 19:57:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Haller X-Patchwork-Id: 1840413 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=JF2z4gvn; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RwnbF639Zz1ypJ for ; Thu, 28 Sep 2023 06:02:49 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229796AbjI0UCs (ORCPT ); Wed, 27 Sep 2023 16:02:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39832 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229754AbjI0UCs (ORCPT ); Wed, 27 Sep 2023 16:02:48 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4E76611D for ; Wed, 27 Sep 2023 13:02:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695844919; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pAQyOebgcJoENGEftU+qtV6q4yx2qCBobs/aJO3PDNo=; b=JF2z4gvnExsI3eQ3aEDl8yG9UYoo+wIUNlhmHwhlN23t5lAWZPl5TCPyNZvLO90LwSni8f fEw/7b8O7nioDyAQGR0780IgHg302lomVYLrw0JTBZLYnSgF3ywIXTgxkkTd87EdIl0Jgx G4hd6fxBkcWN4H6nGE81Sw6QHZD0NNc= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-502-TRZ8XNY-MyCaWelBKMylWA-1; Wed, 27 Sep 2023 16:01:58 -0400 X-MC-Unique: TRZ8XNY-MyCaWelBKMylWA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B206E1019C88 for ; Wed, 27 Sep 2023 20:01:57 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.194.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 30F3A40C6EA8; Wed, 27 Sep 2023 20:01:57 +0000 (UTC) From: Thomas Haller To: NetFilter Cc: Thomas Haller Subject: [PATCH nft 3/5] datatype: don't clone datatype in set_datatype_alloc() if byteorder already matches Date: Wed, 27 Sep 2023 21:57:26 +0200 Message-ID: <20230927200143.3798124-4-thaller@redhat.com> In-Reply-To: <20230927200143.3798124-1-thaller@redhat.com> References: <20230927200143.3798124-1-thaller@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org "struct datatype" instances are treated immutable and not changed after creation. set_datatype_alloc() returns a const pointer, indicating that the caller must not modify the instance anymore. In particular it must not, because for non-integer types we just return a reference to the (already immutable) orig_dtype. If the byteorder that we are about to set is already as-requested, there is no need to clone the instance either. Just return a reference to orig_dtype() too. Also drop the same optimization from key_fix_dtype_byteorder(). Signed-off-by: Thomas Haller --- src/datatype.c | 6 +++++- src/evaluate.c | 19 +++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/datatype.c b/src/datatype.c index 1c557a06c751..6a35c6a76028 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -1321,9 +1321,13 @@ const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype, if (orig_dtype != &integer_type) return datatype_get(orig_dtype); + if (orig_dtype->byteorder == byteorder) { + /* The (immutable) type instance is already as requested. */ + return datatype_get(orig_dtype); + } + dtype = datatype_clone(orig_dtype); dtype->byteorder = byteorder; - return dtype; } diff --git a/src/evaluate.c b/src/evaluate.c index c699a9bc7b86..e84895bf1610 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -74,17 +74,8 @@ static int __fmtstring(3, 4) set_error(struct eval_ctx *ctx, return -1; } -static void key_fix_dtype_byteorder(struct expr *key) -{ - const struct datatype *dtype = key->dtype; - - if (dtype->byteorder == key->byteorder) - return; - - __datatype_set(key, set_datatype_alloc(dtype, key->byteorder)); -} - static int set_evaluate(struct eval_ctx *ctx, struct set *set); + static struct expr *implicit_set_declaration(struct eval_ctx *ctx, const char *name, struct expr *key, @@ -95,8 +86,12 @@ static struct expr *implicit_set_declaration(struct eval_ctx *ctx, struct set *set; struct handle h; - if (set_is_datamap(expr->set_flags)) - key_fix_dtype_byteorder(key); + if (set_is_datamap(expr->set_flags)) { + const struct datatype *dtype; + + dtype = set_datatype_alloc(key->dtype, key->byteorder); + __datatype_set(key, dtype); + } set = set_alloc(&expr->location); set->flags = NFT_SET_ANONYMOUS | expr->set_flags; From patchwork Wed Sep 27 19:57:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Haller X-Patchwork-Id: 1840414 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Xex6PFW0; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RwnbH3Y80z1ypJ for ; Thu, 28 Sep 2023 06:02:51 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229816AbjI0UCu (ORCPT ); Wed, 27 Sep 2023 16:02:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60148 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229754AbjI0UCu (ORCPT ); Wed, 27 Sep 2023 16:02:50 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7FF1B122 for ; Wed, 27 Sep 2023 13:02:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695844927; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=PGujRVaYXUH8BO+fgPtArbje9h1B90yQx8nGGxueSdQ=; b=Xex6PFW0v5kl2i9OaEYEGqlXCl0I5tJEbf3TbVRsweAgK7RRtprIXzMvVyaW/JWPyuTfDq HX952++mJ0gCw8Kdoq62mDFeZiNuYaNwd/vU6JBrFtMeVWivNRL4swzVLC8NSW0pHcL3QV 5jLHYVluGnznkXH6fTp2CX2CCwSCq5U= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-414-aLyWzfMuPaqTs2ZDxVesFg-1; Wed, 27 Sep 2023 16:01:58 -0400 X-MC-Unique: aLyWzfMuPaqTs2ZDxVesFg-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7A6EA29AA2F0 for ; Wed, 27 Sep 2023 20:01:58 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.194.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id ED8D740C6EA8; Wed, 27 Sep 2023 20:01:57 +0000 (UTC) From: Thomas Haller To: NetFilter Cc: Thomas Haller Subject: [PATCH nft 4/5] datatype: extend set_datatype_alloc() to change size Date: Wed, 27 Sep 2023 21:57:27 +0200 Message-ID: <20230927200143.3798124-5-thaller@redhat.com> In-Reply-To: <20230927200143.3798124-1-thaller@redhat.com> References: <20230927200143.3798124-1-thaller@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Spam-Status: No, score=1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RCVD_IN_SBL_CSS,SPF_HELO_NONE, SPF_NONE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org All remaining users of datatype_clone() only use this function to set the byteorder and the size. Extend set_datatype_alloc() to not only change the byteorder but also the size. With that, it can be used instead of clone. The benefit is that set_datatype_alloc() takes care to re-use the same instance, if the values end up being the same. Another benefit is to expose and make clear the oddity related to the "for_any_integer" argument. The argument exists to preserve the previous behavior. However, it's not clear why some places would only want to change the values for &integer_type and some check for orig_dtype->type == TYPE_INTEGER. This is possibly a bug, especially because it means once you clone an &integer_type, you cannot change it's byteorder/size again. So &integer_type is very special here, not only based on the TYPE_INTEGER. Signed-off-by: Thomas Haller --- include/datatype.h | 5 ++++- src/datatype.c | 21 ++++++++++++++++----- src/evaluate.c | 26 ++++++++++++-------------- src/netlink.c | 4 ++-- src/payload.c | 7 +++---- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/include/datatype.h b/include/datatype.h index 465ade290652..f01e15b6ff3e 100644 --- a/include/datatype.h +++ b/include/datatype.h @@ -301,7 +301,10 @@ concat_subtype_lookup(uint32_t type, unsigned int n) } extern const struct datatype * -set_datatype_alloc(const struct datatype *orig_dtype, enum byteorder byteorder); +set_datatype_alloc(const struct datatype *orig_dtype, + bool for_any_integer, + enum byteorder byteorder, + unsigned int size); extern void time_print(uint64_t msec, struct output_ctx *octx); extern struct error_record *time_parse(const struct location *loc, diff --git a/src/datatype.c b/src/datatype.c index 6a35c6a76028..f9570603467a 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -1313,21 +1313,32 @@ const struct datatype *concat_type_alloc(uint32_t type) } const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype, - enum byteorder byteorder) + bool for_any_integer, + enum byteorder byteorder, + unsigned int size) { struct datatype *dtype; - /* Restrict dynamic datatype allocation to generic integer datatype. */ - if (orig_dtype != &integer_type) - return datatype_get(orig_dtype); + if (for_any_integer) { + if (orig_dtype->type != TYPE_INTEGER) { + /* Restrict changing byteorder/size to any integer datatype. */ + return datatype_get(orig_dtype); + } + } else { + if (orig_dtype != &integer_type) { + /* Restrict changing byteorder/size to the generic integer datatype. */ + return datatype_get(orig_dtype); + } + } - if (orig_dtype->byteorder == byteorder) { + if (orig_dtype->byteorder == byteorder && orig_dtype->size == size) { /* The (immutable) type instance is already as requested. */ return datatype_get(orig_dtype); } dtype = datatype_clone(orig_dtype); dtype->byteorder = byteorder; + dtype->size = size; return dtype; } diff --git a/src/evaluate.c b/src/evaluate.c index e84895bf1610..a118aa6a7209 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -89,7 +89,8 @@ static struct expr *implicit_set_declaration(struct eval_ctx *ctx, if (set_is_datamap(expr->set_flags)) { const struct datatype *dtype; - dtype = set_datatype_alloc(key->dtype, key->byteorder); + dtype = set_datatype_alloc(key->dtype, false, key->byteorder, + key->dtype->size); __datatype_set(key, dtype); } @@ -1508,13 +1509,12 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr) return -1; flags &= i->flags; - if (!key && i->dtype->type == TYPE_INTEGER) { - struct datatype *clone; + if (!key) { + const struct datatype *dtype; - clone = datatype_clone(i->dtype); - clone->size = i->len; - clone->byteorder = i->byteorder; - __datatype_set(i, clone); + dtype = set_datatype_alloc(i->dtype, true, + i->byteorder, i->len); + __datatype_set(i, dtype); } if (dtype == NULL && i->dtype->size == 0) @@ -1933,7 +1933,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr) } else { const struct datatype *dtype; - dtype = set_datatype_alloc(ectx.dtype, ectx.byteorder); + dtype = set_datatype_alloc(ectx.dtype, false, ectx.byteorder, ectx.dtype->size); data = constant_expr_alloc(&netlink_location, dtype, dtype->byteorder, ectx.len, NULL); datatype_free(dtype); @@ -4553,13 +4553,11 @@ static int set_expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr) return expr_error(ctx->msgs, i, "specify either ip or ip6 for address matching"); - if (i->etype == EXPR_PAYLOAD && - i->dtype->type == TYPE_INTEGER) { - struct datatype *dtype; + if (i->etype == EXPR_PAYLOAD) { + const struct datatype *dtype; - dtype = datatype_clone(i->dtype); - dtype->size = i->len; - dtype->byteorder = i->byteorder; + dtype = set_datatype_alloc(i->dtype, true, + i->byteorder, i->len); __datatype_set(i, dtype); } diff --git a/src/netlink.c b/src/netlink.c index 120a8ba9ceb1..9fe870885e2e 100644 --- a/src/netlink.c +++ b/src/netlink.c @@ -1034,7 +1034,7 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx, if (datatype) { uint32_t dlen; - dtype2 = set_datatype_alloc(datatype, databyteorder); + dtype2 = set_datatype_alloc(datatype, false, databyteorder, datatype->size); klen = nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN) * BITS_PER_BYTE; dlen = data_interval ? klen / 2 : klen; @@ -1058,7 +1058,7 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx, set->data->flags |= EXPR_F_INTERVAL; } - dtype = set_datatype_alloc(keytype, keybyteorder); + dtype = set_datatype_alloc(keytype, false, keybyteorder, keytype->size); klen = nftnl_set_get_u32(nls, NFTNL_SET_KEY_LEN) * BITS_PER_BYTE; if (set_udata_key_valid(typeof_expr_key, klen)) { diff --git a/src/payload.c b/src/payload.c index 89bb38eb0099..55e075dab033 100644 --- a/src/payload.c +++ b/src/payload.c @@ -243,15 +243,14 @@ static struct expr *payload_expr_parse_udata(const struct nftnl_udata *attr) expr->len = len; if (is_raw) { - struct datatype *dtype; + const struct datatype *dtype; expr->payload.base = base; expr->payload.offset = offset; expr->payload.is_raw = true; expr->len = len; - dtype = datatype_clone(&xinteger_type); - dtype->size = len; - dtype->byteorder = BYTEORDER_BIG_ENDIAN; + dtype = set_datatype_alloc(&xinteger_type, true, + BYTEORDER_BIG_ENDIAN, len); __datatype_set(expr, dtype); } From patchwork Wed Sep 27 19:57:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Haller X-Patchwork-Id: 1840410 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=iCFzVkE3; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RwnbC2lh5z1ypJ for ; Thu, 28 Sep 2023 06:02:47 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229664AbjI0UCo (ORCPT ); Wed, 27 Sep 2023 16:02:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39858 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229721AbjI0UCn (ORCPT ); Wed, 27 Sep 2023 16:02:43 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E6B46114 for ; Wed, 27 Sep 2023 13:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695844922; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Q73ylzHrw8q8wYBZ7ABaz6/wz5xUZOgQ2psHtD+omfg=; b=iCFzVkE3nF47oaamEglslJWV5o/RzIhckeTk1zqRYPkBCTn9MhLWMWLWkDMpmTReL0Vcw6 AxftX+Lflawh1JHYrn45uij8iS4vrfY3R3arjG3rgKJ5lxFyqOxfu/Hrfq1RVaIKQc+T0m we61o6FbblIVb6BHMsq+y+/2thdUctk= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-78-UqAHs7CEPvmKeno-brolzg-1; Wed, 27 Sep 2023 16:02:00 -0400 X-MC-Unique: UqAHs7CEPvmKeno-brolzg-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4292E8002B2 for ; Wed, 27 Sep 2023 20:01:59 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.194.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B557940C6EA8; Wed, 27 Sep 2023 20:01:58 +0000 (UTC) From: Thomas Haller To: NetFilter Cc: Thomas Haller Subject: [PATCH nft 5/5] datatype: use xmalloc() for allocating datatype in datatype_clone() Date: Wed, 27 Sep 2023 21:57:28 +0200 Message-ID: <20230927200143.3798124-6-thaller@redhat.com> In-Reply-To: <20230927200143.3798124-1-thaller@redhat.com> References: <20230927200143.3798124-1-thaller@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org The returned memory will be initialized. No need to zero it first. Use xmalloc() instead of xzalloc(). Signed-off-by: Thomas Haller --- src/datatype.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/datatype.c b/src/datatype.c index f9570603467a..eae7f4c71fbe 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -1242,7 +1242,7 @@ struct datatype *datatype_clone(const struct datatype *orig_dtype) { struct datatype *dtype; - dtype = xzalloc(sizeof(*dtype)); + dtype = xmalloc(sizeof(*dtype)); *dtype = *orig_dtype; if (orig_dtype->f_allocated_strings) { dtype->name = xstrdup(orig_dtype->name);