Re: [PATCH RFC iproute2-next v3 2/2] seg6: add support for the End.MAP behavior under seg6mobile encap

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 17 Sep 2026 06:39:01 +0900
Yuya Kusakabe <yuya.kusakabe@gmail.com> wrote:

> Wire up a new "seg6mobile" lightweight tunnel encap type that mirrors
> the kernel's LWTUNNEL_ENCAP_SEG6_MOBILE namespace, and add parse and
> print support for the first RFC 9433 behavior it carries: End.MAP.
> 
> End.MAP swaps the IPv6 destination address with the configured mapped SID
> and forwards the packet via the IPv6 FIB without consuming the SRH:
> 
>     ip -6 route add 2001:db8:f::/64 \
>         encap seg6mobile action End.MAP mapped_sid 2001:db8:2::e \
>         dev <dev>
> 
> Help text and the ip-route(8) man page get the corresponding ENCAPTYPE
> and SEG6MOBILE entries.
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
> ---

The AI review for this is:

On Thu, 17 Sep 2026 06:39:01 +0900, Yuya Kusakabe wrote:
> Subject: [PATCH RFC iproute2-next v3 2/2] seg6: add support for the
>  End.MAP behavior under seg6mobile encap

Reviewed the full v3 series (1/2 uapi + 2/2 implementation). Applied
cleanly on fce739fa, builds with no new warnings, and I exercised the
parse and print paths directly.

Overall this is in good shape. The code follows the seg6local patterns
closely: strcmp() rather than matches(), duparg2() for duplicates,
print_XXX() helpers throughout, paired open/close_json_object(), errors
to stderr. I have no functional objections; the comments below are a
documentation inconsistency and two suggestions.

Test results, for the record:

  # ip -6 route add ... encap seg6mobile action End.MAP \
        mapped_sid 2001:db8:2::e count dev lo

produces a well-formed message -- SEG6_MOBILE_ACTION=1,
SEG6_MOBILE_MAPPED_SID=2001:db8:2::e, nested SEG6_MOBILE_COUNTERS with
three zeroed u64s, RTA_ENCAP_TYPE=11. Argument consumption is correct
in every ordering I tried, including "count" as the final token and
with no trailing "dev".

Feeding a synthetic reply through lwt_print_encap() gives:

  text: encap seg6mobile action End.MAP mapped_sid 2001:db8:2::e \
        packets 1234 bytes 567890 errors 7

  json: {"encap":{"encap_type":"seg6mobile","action":"End.MAP",
         "mapped_sid":"2001:db8:2::e",
         "stats64":{"packets":1234,"bytes":567890,"errors":7}}}

Both correct, and the JSON parses.


1. usage() and the man page disagree on whether mapped_sid is optional

> +		"SEG6MOBILE := action MOBILE_ACTION mapped_sid ADDR [ count ]\n"
> +		"MOBILE_ACTION := { End.MAP }\n"

The usage string makes "mapped_sid ADDR" mandatory, but the man page
synopsis makes it optional:

> +.IR ENCAP_SEG6MOBILE " := "
> +.B seg6mobile
> +.BR action
> +.IR SEG6_MOBILE_ACTION " [ "
> +.IR SEG6_MOBILE_PARAM " ] [ "
> +.BR count " ] "

and the parser enforces neither -- "action End.MAP" with no mapped_sid
is accepted and sent to the kernel.

Please make the three agree. Since End.MAP is meaningless without a
mapped SID, I would keep the usage string as-is and follow seg6local's
convention in the man page, which spells the parameter as part of the
action description rather than as an optional token.

2. Consider rejecting End.MAP without mapped_sid in userspace

> +	if (!action) {
> +		fprintf(stderr, "Missing action type\n");
> +		exit(-1);
> +	}

This catches a missing action but not a missing mapped_sid, so the
kernel has to reject it. seg6local has the same gap, so this is not a
regression and I will not insist -- but End.MAP has exactly one
required parameter, which makes the check cheap and the diagnostic much
better than whatever errno comes back:

	if (action == SEG6_MOBILE_ACTION_END_MAP && !mapped_sid_ok) {
		fprintf(stderr, "Missing mapped_sid for End.MAP\n");
		exit(-1);
	}

3. Series depends on an unmerged kernel RFC

Patch 1/2 adds LWTUNNEL_ENCAP_SEG6_MOBILE and seg6_mobile.h ahead of
the kernel side, which is still an RFC. That is fine for an RFC posting
and the cover letter is upfront about it, but for the non-RFC repost:
iproute2-next takes uapi header syncs from accepted kernel commits, so
this cannot be applied until the kernel patches land in net-next.
Please cite the upstream commit id in 1/2's changelog when you repost.

One note on 1/2 while I am here: the value of LWTUNNEL_ENCAP_SEG6_MOBILE
is fixed by where it lands in the kernel's enum. If the kernel series
gains another encap type before it is merged, the header needs a
re-sync rather than a hand-edit.

Nothing else. The uapi definitions line up with what the code assumes --
SEG6_MOBILE_ACTION_UNSPEC is 0, so the "if (!action)" validity test
works; the counters carry a CNT_PAD for 64-bit alignment as seg6local
does.

Thanks,
Stephen