From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Sutter <phil@nwl.cc>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [nft PATCH v2 02/11] mergesort: Fix sorting of string values
Date: Wed, 28 Jan 2026 00:28:16 +0100 [thread overview]
Message-ID: <aXlKECq5p9SUYuJO@chamomile> (raw)
In-Reply-To: <20251114002542.22667-3-phil@nwl.cc>
On Fri, Nov 14, 2025 at 01:25:33AM +0100, Phil Sutter wrote:
> Sorting order was obviously wrong, e.g. "ppp0" ordered before "eth1".
> Moreover, this happened on Little Endian only so sorting order actually
> depended on host's byteorder. By reimporting string values as Big
> Endian, both issues are fixed: On one hand, GMP-internal byteorder no
> longer depends on host's byteorder, on the other comparing strings
> really starts with the first character, not the last.
>
> Fixes: 14ee0a979b622 ("src: sort set elements in netlink_get_setelems()")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> src/mergesort.c | 7 +++
> tests/py/any/meta.t.json.output | 54 -------------------
> tests/py/any/queue.t.json.output | 4 +-
> tests/py/inet/osf.t.json.output | 54 +++++++++++++++++++
> .../testcases/maps/dumps/0012map_0.json-nft | 20 +++----
> .../shell/testcases/maps/dumps/0012map_0.nft | 8 +--
> .../maps/dumps/named_ct_objects.json-nft | 4 +-
> .../testcases/maps/dumps/named_ct_objects.nft | 4 +-
> .../sets/dumps/sets_with_ifnames.json-nft | 4 +-
> .../sets/dumps/sets_with_ifnames.nft | 2 +-
> 10 files changed, 84 insertions(+), 77 deletions(-)
>
> diff --git a/src/mergesort.c b/src/mergesort.c
> index a9cba614612ed..97e36917280f3 100644
> --- a/src/mergesort.c
> +++ b/src/mergesort.c
> @@ -37,6 +37,13 @@ static mpz_srcptr expr_msort_value(const struct expr *expr, mpz_t value)
> case EXPR_RANGE:
> return expr_msort_value(expr->left, value);
> case EXPR_VALUE:
> + if (expr_basetype(expr)->type == TYPE_STRING) {
> + char buf[expr->len];
> +
> + mpz_export_data(buf, expr->value, BYTEORDER_HOST_ENDIAN, expr->len);
> + mpz_import_data(value, buf, BYTEORDER_BIG_ENDIAN, expr->len);
> + return value;
> + }
This is also used for automerge, not only get_setelems().
Are you sure this is correct?
> return expr->value;
> case EXPR_RANGE_VALUE:
> return expr->range.low;
> diff --git a/tests/py/any/meta.t.json.output b/tests/py/any/meta.t.json.output
> index 8f4d597a5034e..4454bb960385d 100644
> --- a/tests/py/any/meta.t.json.output
> +++ b/tests/py/any/meta.t.json.output
> @@ -233,60 +233,6 @@
> }
> ]
>
> -# meta iifname {"dummy0", "lo"}
> -[
> - {
> - "match": {
> - "left": {
> - "meta": { "key": "iifname" }
> - },
> - "op": "==",
> - "right": {
> - "set": [
> - "lo",
> - "dummy0"
> - ]
> - }
> - }
> - }
> -]
> -
> -# meta iifname != {"dummy0", "lo"}
> -[
> - {
> - "match": {
> - "left": {
> - "meta": { "key": "iifname" }
> - },
> - "op": "!=",
> - "right": {
> - "set": [
> - "lo",
> - "dummy0"
> - ]
> - }
> - }
> - }
> -]
> -
> -# meta oifname { "dummy0", "lo"}
> -[
> - {
> - "match": {
> - "left": {
> - "meta": { "key": "oifname" }
> - },
> - "op": "==",
> - "right": {
> - "set": [
> - "lo",
> - "dummy0"
> - ]
> - }
> - }
> - }
> -]
> -
> # meta skuid {"bin", "root", "daemon"} accept
> [
> {
> diff --git a/tests/py/any/queue.t.json.output b/tests/py/any/queue.t.json.output
> index ea3722383f113..90670cc938866 100644
> --- a/tests/py/any/queue.t.json.output
> +++ b/tests/py/any/queue.t.json.output
> @@ -104,11 +104,11 @@
> 0
> ],
> [
> - "ppp0",
> + "eth1",
> 2
> ],
> [
> - "eth1",
> + "ppp0",
> 2
> ]
> ]
> diff --git a/tests/py/inet/osf.t.json.output b/tests/py/inet/osf.t.json.output
> index 922e395f202c7..77ca7e30e0f77 100644
> --- a/tests/py/inet/osf.t.json.output
> +++ b/tests/py/inet/osf.t.json.output
> @@ -18,6 +18,26 @@
> }
> ]
>
> +# osf version { "Windows:XP", "MacOs:Sierra" }
> +[
> + {
> + "match": {
> + "left": {
> + "osf": {
> + "key": "version"
> + }
> + },
> + "op": "==",
> + "right": {
> + "set": [
> + "MacOs:Sierra",
> + "Windows:XP"
> + ]
> + }
> + }
> + }
> +]
> +
> # ct mark set osf name map { "Windows" : 0x00000001, "MacOs" : 0x00000002 }
> [
> {
> @@ -51,3 +71,37 @@
> }
> }
> ]
> +
> +# ct mark set osf version map { "Windows:XP" : 0x00000003, "MacOs:Sierra" : 0x00000004 }
> +[
> + {
> + "mangle": {
> + "key": {
> + "ct": {
> + "key": "mark"
> + }
> + },
> + "value": {
> + "map": {
> + "data": {
> + "set": [
> + [
> + "MacOs:Sierra",
> + 4
> + ],
> + [
> + "Windows:XP",
> + 3
> + ]
> + ]
> + },
> + "key": {
> + "osf": {
> + "key": "version"
> + }
> + }
> + }
> + }
> + }
> + }
> +]
> diff --git a/tests/shell/testcases/maps/dumps/0012map_0.json-nft b/tests/shell/testcases/maps/dumps/0012map_0.json-nft
> index 2892e11d71f54..6c885703ffd6b 100644
> --- a/tests/shell/testcases/maps/dumps/0012map_0.json-nft
> +++ b/tests/shell/testcases/maps/dumps/0012map_0.json-nft
> @@ -32,21 +32,21 @@
> "map": "verdict",
> "elem": [
> [
> - "lo",
> + "eth0",
> {
> - "accept": null
> + "drop": null
> }
> ],
> [
> - "eth0",
> + "eth1",
> {
> "drop": null
> }
> ],
> [
> - "eth1",
> + "lo",
> {
> - "drop": null
> + "accept": null
> }
> ]
> ]
> @@ -69,21 +69,21 @@
> "data": {
> "set": [
> [
> - "lo",
> + "eth0",
> {
> - "accept": null
> + "drop": null
> }
> ],
> [
> - "eth0",
> + "eth1",
> {
> "drop": null
> }
> ],
> [
> - "eth1",
> + "lo",
> {
> - "drop": null
> + "accept": null
> }
> ]
> ]
> diff --git a/tests/shell/testcases/maps/dumps/0012map_0.nft b/tests/shell/testcases/maps/dumps/0012map_0.nft
> index e734fc1c70b93..0df329a550518 100644
> --- a/tests/shell/testcases/maps/dumps/0012map_0.nft
> +++ b/tests/shell/testcases/maps/dumps/0012map_0.nft
> @@ -1,12 +1,12 @@
> table ip x {
> map z {
> type ifname : verdict
> - elements = { "lo" : accept,
> - "eth0" : drop,
> - "eth1" : drop }
> + elements = { "eth0" : drop,
> + "eth1" : drop,
> + "lo" : accept }
> }
>
> chain y {
> - iifname vmap { "lo" : accept, "eth0" : drop, "eth1" : drop }
> + iifname vmap { "eth0" : drop, "eth1" : drop, "lo" : accept }
> }
> }
> diff --git a/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft b/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft
> index c0f270e372b24..34c8798dee8fb 100644
> --- a/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft
> +++ b/tests/shell/testcases/maps/dumps/named_ct_objects.json-nft
> @@ -195,8 +195,8 @@
> },
> "handle": 0,
> "elem": [
> - "sip",
> - "ftp"
> + "ftp",
> + "sip"
> ]
> }
> },
> diff --git a/tests/shell/testcases/maps/dumps/named_ct_objects.nft b/tests/shell/testcases/maps/dumps/named_ct_objects.nft
> index 59f18932b28ad..dab683bf5cdbd 100644
> --- a/tests/shell/testcases/maps/dumps/named_ct_objects.nft
> +++ b/tests/shell/testcases/maps/dumps/named_ct_objects.nft
> @@ -50,8 +50,8 @@ table inet t {
>
> set helpname {
> typeof ct helper
> - elements = { "sip",
> - "ftp" }
> + elements = { "ftp",
> + "sip" }
> }
>
> chain y {
> diff --git a/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft b/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft
> index ac4284293c32a..7b4849e0530d3 100644
> --- a/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft
> +++ b/tests/shell/testcases/sets/dumps/sets_with_ifnames.json-nft
> @@ -260,8 +260,8 @@
> },
> "right": {
> "set": [
> - "eth0",
> - "abcdef0"
> + "abcdef0",
> + "eth0"
> ]
> }
> }
> diff --git a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
> index 77a8baf58cef2..8abca03a080ec 100644
> --- a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
> +++ b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
> @@ -39,7 +39,7 @@ table inet testifsets {
> chain v4icmp {
> iifname @simple counter packets 0 bytes 0
> iifname @simple_wild counter packets 0 bytes 0
> - iifname { "eth0", "abcdef0" } counter packets 0 bytes 0
> + iifname { "abcdef0", "eth0" } counter packets 0 bytes 0
> iifname { "abcdef*", "eth0" } counter packets 0 bytes 0
> iifname vmap @map_wild
> }
> --
> 2.51.0
>
next prev parent reply other threads:[~2026-01-27 23:28 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-14 0:25 [nft PATCH v2 00/11] Fix netlink debug output on Big Endian Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 01/11] segtree: Fix range aggregation " Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 02/11] mergesort: Fix sorting of string values Phil Sutter
2026-01-27 23:28 ` Pablo Neira Ayuso [this message]
2026-01-28 12:11 ` Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 03/11] mergesort: Align concatenation sort order with Big Endian Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 04/11] intervals: Convert byte order implicitly Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 05/11] expression: Set range expression 'len' field Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 06/11] netlink: Introduce struct nft_data_linearize::byteorder Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 07/11] netlink: Introduce struct nft_data_linearize::sizes Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 08/11] netlink: Make use of nftnl_{expr,set_elem}_set_imm() Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 09/11] tests: py: tools: Add regen_payloads.sh Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 10/11] tests: py: Update payload records Phil Sutter
2025-11-14 0:25 ` [nft PATCH v2 11/11] utils: Introduce expr_print_debug() Phil Sutter
2026-01-27 22:04 ` [nft PATCH v2 00/11] Fix netlink debug output on Big Endian Phil Sutter
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=aXlKECq5p9SUYuJO@chamomile \
--to=pablo@netfilter.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=phil@nwl.cc \
/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.