[PATCH iproute2-next] lwt: ioam6: allow options in any order

[PATCH iproute2-next] lwt: ioam6: allow options in any order

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

The ioam6 encap options must be given in a fixed order (freq, mode,
tunsrc, tundst, trace, prealloc, type, ns, size). If a user uses a
different order, the command fails with a misleading error even
though every option is present:

  # ip -6 route change 2001:db8:2:1::/64 encap ioam6 mode encap \
      trace prealloc tundst 2001:db8:2:1::2 type 0x800000 ns 0 size 12 \
      via 2001:db8:1::3 dev dummy0
  Error: argument "tundst" is required

Parse all options in a single while loop instead, then run the
cross-option and missing-argument checks after the loop, reporting
the missing arguments in the order the options are documented in
ip-route(8).

Use while (argc > 0), same with parse_encap_ip(), so that a
valueless option in the last position (trace, prealloc) is not
skipped, and reject duplicated options the same way type/ns/size
already did.

Rename ns_found and has_src to x_ok to align with other variable names.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
This change fixes one main issue plus several related mirror updates.
I grouped them into a single patch, but the diff is a bit unwieldy for
review. Let me know if you’d prefer I split this into smaller patches.

Run kernel selftest ioam6.sh and all tests passed.
---
 ip/iproute_lwtunnel.c | 142 +++++++++++++++++++++++---------------------------
 1 file changed, 66 insertions(+), 76 deletions(-)

diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 9a1e747c6951..798ef8143ce1 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -1170,7 +1170,8 @@ static int parse_ioam6_freq(char *buf, __u32 *freq_k, __u32 *freq_n)
 static int parse_encap_ioam6(struct rtattr *rta, size_t len, int *argcp,
 			     char ***argvp)
 {
-	int ns_found = 0, argc = *argcp;
+	int tunsrc_ok = 0, tundst_ok = 0, trace_ok = 0, prealloc_ok = 0;
+	int freq_ok = 0, mode_ok = 0, ns_ok = 0, argc = *argcp;
 	__u16 trace_ns, trace_size = 0;
 	struct ioam6_trace_hdr *trace;
 	inet_prefix saddr, daddr;
@@ -1178,94 +1179,65 @@ static int parse_encap_ioam6(struct rtattr *rta, size_t len, int *argcp,
 	__u32 trace_type = 0;
 	__u32 freq_k, freq_n;
 	char buf[16] = {0};
-	bool has_src;
 	__u8 mode;
 
-	if (strcmp(*argv, "freq") != 0) {
-		freq_k = IOAM6_IPTUNNEL_FREQ_MIN;
-		freq_n = IOAM6_IPTUNNEL_FREQ_MIN;
-	} else {
-		NEXT_ARG();
-
-		if (strlen(*argv) > sizeof(buf) - 1)
-			invarg("Invalid frequency (too long)", *argv);
-
-		strncpy(buf, *argv, sizeof(buf));
-
-		if (parse_ioam6_freq(buf, &freq_k, &freq_n))
-			invarg("Invalid frequency (malformed)", *argv);
-
-		if (freq_k < IOAM6_IPTUNNEL_FREQ_MIN ||
-		    freq_k > IOAM6_IPTUNNEL_FREQ_MAX)
-			invarg("Out of bound \"k\" frequency", *argv);
+	mode = IOAM6_IPTUNNEL_MODE_INLINE;
+	freq_k = IOAM6_IPTUNNEL_FREQ_MIN;
+	freq_n = IOAM6_IPTUNNEL_FREQ_MIN;
 
-		if (freq_n < IOAM6_IPTUNNEL_FREQ_MIN ||
-		    freq_n > IOAM6_IPTUNNEL_FREQ_MAX)
-			invarg("Out of bound \"n\" frequency", *argv);
-
-		if (freq_k > freq_n)
-			invarg("Frequency with k > n is forbidden", *argv);
-
-		NEXT_ARG();
-	}
-
-	if (strcmp(*argv, "mode") != 0) {
-		mode = IOAM6_IPTUNNEL_MODE_INLINE;
-	} else {
-		NEXT_ARG();
-
-		mode = read_ioam6mode_type(*argv);
-		if (!mode)
-			invarg("Invalid mode", *argv);
-
-		NEXT_ARG();
-	}
-
-	if (strcmp(*argv, "tunsrc") != 0) {
-		has_src = false;
-	} else {
-		has_src = true;
+	while (argc > 0) {
+		if (strcmp(*argv, "freq") == 0) {
+			NEXT_ARG();
 
-		if (mode == IOAM6_IPTUNNEL_MODE_INLINE)
-			invarg("Inline mode does not need tunsrc", *argv);
+			if (freq_ok++)
+				duparg2("freq", *argv);
 
-		NEXT_ARG();
+			if (strlen(*argv) > sizeof(buf) - 1)
+				invarg("Invalid frequency (too long)", *argv);
 
-		get_addr(&saddr, *argv, AF_INET6);
-		if (saddr.family != AF_INET6 || saddr.bytelen != 16)
-			invarg("Invalid IPv6 address for tunsrc", *argv);
+			strncpy(buf, *argv, sizeof(buf));
 
-		NEXT_ARG();
-	}
+			if (parse_ioam6_freq(buf, &freq_k, &freq_n))
+				invarg("Invalid frequency (malformed)", *argv);
 
-	if (strcmp(*argv, "tundst") != 0) {
-		if (mode != IOAM6_IPTUNNEL_MODE_INLINE)
-			missarg("tundst");
-	} else {
-		if (mode == IOAM6_IPTUNNEL_MODE_INLINE)
-			invarg("Inline mode does not need tundst", *argv);
+			if (freq_k < IOAM6_IPTUNNEL_FREQ_MIN ||
+			    freq_k > IOAM6_IPTUNNEL_FREQ_MAX)
+				invarg("Out of bound \"k\" frequency", *argv);
 
-		NEXT_ARG();
+			if (freq_n < IOAM6_IPTUNNEL_FREQ_MIN ||
+			    freq_n > IOAM6_IPTUNNEL_FREQ_MAX)
+				invarg("Out of bound \"n\" frequency", *argv);
 
-		get_addr(&daddr, *argv, AF_INET6);
-		if (daddr.family != AF_INET6 || daddr.bytelen != 16)
-			invarg("Invalid IPv6 address for tundst", *argv);
+			if (freq_k > freq_n)
+				invarg("Frequency with k > n is forbidden", *argv);
+		} else if (strcmp(*argv, "mode") == 0) {
+			NEXT_ARG();
 
-		NEXT_ARG();
-	}
+			if (mode_ok++)
+				duparg2("mode", *argv);
 
-	if (strcmp(*argv, "trace") != 0)
-		missarg("trace");
+			mode = read_ioam6mode_type(*argv);
+			if (!mode)
+				invarg("Invalid mode", *argv);
+		} else if (strcmp(*argv, "tunsrc") == 0) {
+			NEXT_ARG();
 
-	NEXT_ARG();
+			if (tunsrc_ok++)
+				duparg2("tunsrc", *argv);
 
-	if (strcmp(*argv, "prealloc") != 0)
-		missarg("prealloc");
+			get_addr(&saddr, *argv, AF_INET6);
+			if (saddr.family != AF_INET6 || saddr.bytelen != 16)
+				invarg("Invalid IPv6 address for tunsrc", *argv);
+		} else if (strcmp(*argv, "tundst") == 0) {
+			NEXT_ARG();
 
-	while (NEXT_ARG_OK()) {
-		NEXT_ARG_FWD();
+			if (tundst_ok++)
+				duparg2("tundst", *argv);
 
-		if (strcmp(*argv, "type") == 0) {
+			get_addr(&daddr, *argv, AF_INET6);
+			if (daddr.family != AF_INET6 || daddr.bytelen != 16)
+				invarg("Invalid IPv6 address for tundst", *argv);
+		} else if (strcmp(*argv, "type") == 0) {
 			NEXT_ARG();
 
 			if (trace_type)
@@ -1276,7 +1248,7 @@ static int parse_encap_ioam6(struct rtattr *rta, size_t len, int *argcp,
 		} else if (strcmp(*argv, "ns") == 0) {
 			NEXT_ARG();
 
-			if (ns_found++)
+			if (ns_ok++)
 				duparg2("ns", *argv);
 
 			if (get_u16(&trace_ns, *argv, 0))
@@ -1296,14 +1268,32 @@ static int parse_encap_ioam6(struct rtattr *rta, size_t len, int *argcp,
 
 			if (trace_size > IOAM6_TRACE_DATA_SIZE_MAX)
 				invarg("Trace size is too big", *argv);
+		} else if (strcmp(*argv, "trace") == 0) {
+			if (trace_ok++)
+				duparg("trace", *argv);
+		} else if (strcmp(*argv, "prealloc") == 0) {
+			if (prealloc_ok++)
+				duparg("prealloc", *argv);
 		} else {
 			break;
 		}
+
+		NEXT_ARG_FWD();
 	}
 
+	if (mode == IOAM6_IPTUNNEL_MODE_INLINE && tunsrc_ok)
+		invarg("Inline mode does not need tunsrc", "tunsrc");
+	if (mode == IOAM6_IPTUNNEL_MODE_INLINE && tundst_ok)
+		invarg("Inline mode does not need tundst", "tundst");
+	if (mode != IOAM6_IPTUNNEL_MODE_INLINE && !tundst_ok)
+		missarg("tundst");
+	if (!trace_ok)
+		missarg("trace");
+	if (!prealloc_ok)
+		missarg("prealloc");
 	if (!trace_type)
 		missarg("type");
-	if (!ns_found)
+	if (!ns_ok)
 		missarg("ns");
 	if (!trace_size)
 		missarg("size");
@@ -1319,7 +1309,7 @@ static int parse_encap_ioam6(struct rtattr *rta, size_t len, int *argcp,
 	if (rta_addattr32(rta, len, IOAM6_IPTUNNEL_FREQ_K, freq_k) ||
 	    rta_addattr32(rta, len, IOAM6_IPTUNNEL_FREQ_N, freq_n) ||
 	    rta_addattr8(rta, len, IOAM6_IPTUNNEL_MODE, mode) ||
-	    (mode != IOAM6_IPTUNNEL_MODE_INLINE && has_src &&
+	    (mode != IOAM6_IPTUNNEL_MODE_INLINE && tunsrc_ok &&
 	     rta_addattr_l(rta, len, IOAM6_IPTUNNEL_SRC, &saddr.data, saddr.bytelen)) ||
 	    (mode != IOAM6_IPTUNNEL_MODE_INLINE &&
 	     rta_addattr_l(rta, len, IOAM6_IPTUNNEL_DST, &daddr.data, daddr.bytelen)) ||

---
base-commit: 204a721033556114c81c47d00375f2d52e98e463
change-id: 20261021-ioam_fix_order-ff9d472023dd

Best regards,
--  
Hangbin Liu <liuhangbin@kylinos.cn>

Re: [PATCH iproute2-next] lwt: ioam6: allow options in any order

[Justin Iurman]

On 10/22/26 04:53, Hangbin Liu wrote:
> From: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> The ioam6 encap options must be given in a fixed order (freq, mode,
> tunsrc, tundst, trace, prealloc, type, ns, size). If a user uses a
> different order, the command fails with a misleading error even
> though every option is present:
> 
>    # ip -6 route change 2001:db8:2:1::/64 encap ioam6 mode encap \
>        trace prealloc tundst 2001:db8:2:1::2 type 0x800000 ns 0 size 12 \
>        via 2001:db8:1::3 dev dummy0
>    Error: argument "tundst" is required
> 
> Parse all options in a single while loop instead, then run the
> cross-option and missing-argument checks after the loop, reporting
> the missing arguments in the order the options are documented in
> ip-route(8).

The current order is not arbitrary by intent:

1. freq
2. mode
3. tunsrc
4. tundst
5. trace
6. prealloc
7. type|ns|size

While I agree some could be swappable and the above order is too strict, 
most are dependent on context (i.e., a previous keyword). For example, 
(type|ns|size) are only valid in a 'prealloc' context, which one is only 
valid in a 'trace' context. They form a group and their current order 
should remain inside this group.

OTOH, as mentioned, some could be swapped. For example, both tunsrc and 
tundst could come in whatever order. However, they are only valid in a 
specific 'mode' context, so they should appear after it. With 'mode', 
they form another group. 'freq' forms another group by itself.

So, I guess a new accepted (relaxed) order should accept groups in 
whatever order, but the order inside a same group should remain. That 
said, I'd be open to this proposal if other iproute2 commands also 
accept parameters in any order, regardless of their scope or dependencies.

Justin

Re: [PATCH iproute2-next] lwt: ioam6: allow options in any order

[Hangbin Liu]

Hi Justin,
On Sat, Oct 24, 2026 at 05:26:28PM +0200, Justin Iurman wrote:
> On 10/22/26 04:53, Hangbin Liu wrote:
> > From: Hangbin Liu <liuhangbin@kylinos.cn>
> > 
> > The ioam6 encap options must be given in a fixed order (freq, mode,
> > tunsrc, tundst, trace, prealloc, type, ns, size). If a user uses a
> > different order, the command fails with a misleading error even
> > though every option is present:
> > 
> >    # ip -6 route change 2001:db8:2:1::/64 encap ioam6 mode encap \
> >        trace prealloc tundst 2001:db8:2:1::2 type 0x800000 ns 0 size 12 \
> >        via 2001:db8:1::3 dev dummy0
> >    Error: argument "tundst" is required
> > 
> > Parse all options in a single while loop instead, then run the
> > cross-option and missing-argument checks after the loop, reporting
> > the missing arguments in the order the options are documented in
> > ip-route(8).
> 
> The current order is not arbitrary by intent:
> 
> 1. freq
> 2. mode
> 3. tunsrc
> 4. tundst
> 5. trace
> 6. prealloc
> 7. type|ns|size
> 
> While I agree some could be swappable and the above order is too strict,
> most are dependent on context (i.e., a previous keyword). For example,
> (type|ns|size) are only valid in a 'prealloc' context, which one is only
> valid in a 'trace' context. They form a group and their current order should
> remain inside this group.
> 
> OTOH, as mentioned, some could be swapped. For example, both tunsrc and
> tundst could come in whatever order. However, they are only valid in a
> specific 'mode' context, so they should appear after it. With 'mode', they
> form another group. 'freq' forms another group by itself.
> 
> So, I guess a new accepted (relaxed) order should accept groups in whatever
> order, but the order inside a same group should remain. That said, I'd be
> open to this proposal if other iproute2 commands also accept parameters in
> any order, regardless of their scope or dependencies.

Thanks for the reply. I agree your explanation about the ordering makes sense.
Personally, though, I still think the ordering constraint is too strict. And
the new patch also validates for mode mismatches and missing parameters.

I’ve checked all encap types: only mpls and ila require the address to be the
first parameter, seg6 has an error info about the tunsrc order. All others allow
arbitrary order.

I’m fine with keeping the existing behavior, as I can read the source code and
know the order is fixed. But new users will likely hit this same issue again.

Thanks
Hangbin