From: Thomas Haller <thaller@redhat.com>
To: NetFilter <netfilter-devel@vger.kernel.org>
Cc: Thomas Haller <thaller@redhat.com>
Subject: [PATCH nft 4/5] datatype: extend set_datatype_alloc() to change size
Date: Wed, 27 Sep 2023 21:57:27 +0200 [thread overview]
Message-ID: <20230927200143.3798124-5-thaller@redhat.com> (raw)
In-Reply-To: <20230927200143.3798124-1-thaller@redhat.com>
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 <thaller@redhat.com>
---
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);
}
--
2.41.0
next prev parent reply other threads:[~2023-09-27 20:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-27 19:57 [PATCH nft 0/5] more various cleanups related to struct datatype Thomas Haller
2023-09-27 19:57 ` [PATCH nft 1/5] datatype: make "flags" field of datatype struct simple booleans Thomas Haller
2024-08-19 22:41 ` Pablo Neira Ayuso
2023-09-27 19:57 ` [PATCH nft 2/5] datatype: don't clone static name/desc strings for datatype Thomas Haller
2023-09-27 19:57 ` [PATCH nft 3/5] datatype: don't clone datatype in set_datatype_alloc() if byteorder already matches Thomas Haller
2023-09-27 19:57 ` Thomas Haller [this message]
2023-09-27 19:57 ` [PATCH nft 5/5] datatype: use xmalloc() for allocating datatype in datatype_clone() Thomas Haller
2023-09-28 19:11 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230927200143.3798124-5-thaller@redhat.com \
--to=thaller@redhat.com \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).