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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox