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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.