* [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED
@ 2026-09-23 0:36 Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP " Yuya Kusakabe
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Yuya Kusakabe @ 2026-09-23 0:36 UTC (permalink / raw)
To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Justin Iurman,
Alexander Aring, Steffen Klassert, Herbert Xu,
Nikolay Aleksandrov, Eyal Birger, Nicolas Dichtel, Xin Long,
Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Yuya Kusakabe
"ip route save" stores a route dump and "ip route restore" sends it
back to the kernel unchanged. The kernel dumps RTA_ENCAP, and some of
the attributes nested in it, without NLA_F_NESTED, but rpl, ioam6,
xfrm and the geneve, vxlan and erspan options of the ip and ip6 encaps
require the flag when parsing them. Restoring such a route fails with
"NLA_F_NESTED is missing", and none of them has been restorable in any
release.
Setting the flag in the dumps would change the UAPI: userspace that
does not mask it off the attribute type, such as parse_rtattr() in
iproute2, would no longer find RTA_ENCAP, and dumps that have already
been saved would still fail. The series therefore only makes the
parsers accept what the kernel itself dumps. The dumps, and the
attributes accepted with the flag set, stay as they are.
Patch 1 adds lwtunnel_nla_parse() to the lwtunnel core, so that the
missing flag is documented once next to the build_state callback
instead of being worked around in each lwtunnel. It is
nla_parse_nested() without the flag check: the nested attributes are
still validated strictly, which nla_parse_nested_deprecated(), used by
the older lwtunnels, would not do. Patch 1 uses it in rpl and ioam6,
and patch 2 in xfrm.
Patch 3 does the same for the ip and ip6 encaps, whose geneve, vxlan
and erspan options are nested one level further down: the level shared
by all options is validated through lwtunnel_nla_validate(), the
counterpart of lwtunnel_nla_parse() for nla_validate(). Nothing but
the NLA_F_NESTED check is relaxed.
Patch 4 adds a selftest that saves and restores a route of each kind.
---
Yuya Kusakabe (4):
net: lwtunnel: accept RTA_ENCAP without NLA_F_NESTED
xfrm: lwtunnel: accept RTA_ENCAP without NLA_F_NESTED
net: ip_tunnel: accept tunnel options without NLA_F_NESTED
selftests: net: add lwtunnel route save and restore test
include/net/lwtunnel.h | 49 ++++++++++
net/ipv4/ip_tunnel_core.c | 24 +++--
net/ipv6/ioam6_iptunnel.c | 4 +-
net/ipv6/rpl_iptunnel.c | 4 +-
net/xfrm/xfrm_interface_core.c | 3 +-
tools/testing/selftests/net/Makefile | 1 +
tools/testing/selftests/net/lwt_save_restore.sh | 116 ++++++++++++++++++++++++
7 files changed, 186 insertions(+), 15 deletions(-)
---
base-commit: 17741334d00bf5ebd37f8c1c36bc9c146a351deb
change-id: 20260918-lwt-encap-noflag-ae795af30b52
Best regards,
--
Yuya Kusakabe <yuya.kusakabe@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP without NLA_F_NESTED
2026-09-23 0:36 [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Yuya Kusakabe
@ 2026-09-23 0:36 ` Yuya Kusakabe
2026-09-27 1:24 ` netdev-bot+sashiko
2026-09-23 0:36 ` [PATCH net 2/4] xfrm: " Yuya Kusakabe
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Yuya Kusakabe @ 2026-09-23 0:36 UTC (permalink / raw)
To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Justin Iurman,
Alexander Aring, Steffen Klassert, Herbert Xu,
Nikolay Aleksandrov, Eyal Birger, Nicolas Dichtel, Xin Long,
Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Yuya Kusakabe
lwtunnel_fill_encap() dumps RTA_ENCAP without NLA_F_NESTED, and
"ip route restore" sends the routes saved by "ip route save" back to
the kernel unchanged. rpl and ioam6 break on the same path:
lwtunnel_build_state() hands the restored RTA_ENCAP to their
build_state callbacks, which parse it with nla_parse_nested() and so
require the flag. Restoring an rpl route fails as below, and an ioam6
route fails with the same error:
# ip -6 route add 2001:db8:1::/64 encap rpl segs 2001:db8::2 dev dummy0
# ip -6 route save 2001:db8:1::/64 > route.bin
# ip -6 route del 2001:db8:1::/64
# ip -6 route restore < route.bin
Error: NLA_F_NESTED is missing.
Setting the flag in the dump is not an option: userspace that does not
mask it off the attribute type, such as parse_rtattr() in iproute2,
would no longer find RTA_ENCAP.
Add lwtunnel_nla_parse(), which validates the nested attributes
strictly but does not require the flag on RTA_ENCAP itself, and use it
in rpl and ioam6. Switching them to nla_parse_nested_deprecated()
instead would also make them accept unknown attributes, which they have
rejected since they were added.
Fixes: a7a29f9c361f ("net: ipv6: add rpl sr tunnel")
Fixes: 3edede08ff37 ("ipv6: ioam: Support for IOAM injection with lwtunnels")
Assisted-by: LLM
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
include/net/lwtunnel.h | 27 +++++++++++++++++++++++++++
net/ipv6/ioam6_iptunnel.c | 4 ++--
net/ipv6/rpl_iptunnel.c | 4 ++--
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 26232f603e33..046978d6224c 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -6,6 +6,7 @@
#include <linux/netdevice.h>
#include <linux/skbuff.h>
#include <linux/types.h>
+#include <net/netlink.h>
#include <net/route.h>
#define LWTUNNEL_HASH_BITS 7
@@ -37,6 +38,7 @@ struct lwtunnel_state {
};
struct lwtunnel_encap_ops {
+ /* encap may lack NLA_F_NESTED, parse it with lwtunnel_nla_parse() */
int (*build_state)(struct net *net, struct nlattr *encap,
unsigned int family, const void *cfg,
struct lwtunnel_state **ts,
@@ -53,6 +55,31 @@ struct lwtunnel_encap_ops {
struct module *owner;
};
+/**
+ * lwtunnel_nla_parse - parse the attributes nested in an lwtunnel encap
+ * @tb: destination array with maxtype+1 elements
+ * @maxtype: maximum attribute type to be expected
+ * @nla: encap attribute passed to &lwtunnel_encap_ops.build_state, or an
+ * attribute nested in it
+ * @policy: validation policy
+ * @extack: extended ACK report struct
+ *
+ * The encap attribute, and some of the attributes nested in it, have always
+ * been dumped without NLA_F_NESTED, and userspace such as "ip route restore"
+ * sends a dump back unchanged, so the flag cannot be required on @nla.
+ * The attributes nested in @nla are still validated strictly.
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static inline int lwtunnel_nla_parse(struct nlattr *tb[], int maxtype,
+ const struct nlattr *nla,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack)
+{
+ return nla_parse(tb, maxtype, nla_data(nla), nla_len(nla), policy,
+ extack);
+}
+
#ifdef CONFIG_LWTUNNEL
DECLARE_STATIC_KEY_FALSE(nf_hooks_lwtunnel_enabled);
diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index cfb2c41634a0..946c360ff214 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -113,8 +113,8 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
if (family != AF_INET6)
return -EINVAL;
- err = nla_parse_nested(tb, IOAM6_IPTUNNEL_MAX, nla,
- ioam6_iptunnel_policy, extack);
+ err = lwtunnel_nla_parse(tb, IOAM6_IPTUNNEL_MAX, nla,
+ ioam6_iptunnel_policy, extack);
if (err < 0)
return err;
diff --git a/net/ipv6/rpl_iptunnel.c b/net/ipv6/rpl_iptunnel.c
index 4e10adcd70e8..1861af408bbc 100644
--- a/net/ipv6/rpl_iptunnel.c
+++ b/net/ipv6/rpl_iptunnel.c
@@ -78,8 +78,8 @@ static int rpl_build_state(struct net *net, struct nlattr *nla,
if (family != AF_INET6)
return -EINVAL;
- err = nla_parse_nested(tb, RPL_IPTUNNEL_MAX, nla,
- rpl_iptunnel_policy, extack);
+ err = lwtunnel_nla_parse(tb, RPL_IPTUNNEL_MAX, nla,
+ rpl_iptunnel_policy, extack);
if (err < 0)
return err;
--
2.50.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 2/4] xfrm: lwtunnel: accept RTA_ENCAP without NLA_F_NESTED
2026-09-23 0:36 [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP " Yuya Kusakabe
@ 2026-09-23 0:36 ` Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 3/4] net: ip_tunnel: accept tunnel options " Yuya Kusakabe
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Yuya Kusakabe @ 2026-09-23 0:36 UTC (permalink / raw)
To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Justin Iurman,
Alexander Aring, Steffen Klassert, Herbert Xu,
Nikolay Aleksandrov, Eyal Birger, Nicolas Dichtel, Xin Long,
Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Yuya Kusakabe
xfrmi_build_state() parses RTA_ENCAP with nla_parse_nested(), which
requires NLA_F_NESTED, but lwtunnel_fill_encap() dumps RTA_ENCAP
without it. As a result, "ip route restore" cannot restore the routes
saved by "ip route save":
# ip -6 route add 2001:db8:3::/64 dev ipsec0 encap xfrm if_id 1
# ip -6 route save 2001:db8:3::/64 > route.bin
# ip -6 route del 2001:db8:3::/64
# ip -6 route restore < route.bin
Error: NLA_F_NESTED is missing.
Use lwtunnel_nla_parse().
Fixes: 2c2493b9da91 ("xfrm: lwtunnel: add lwtunnel support for xfrm interfaces in collect_md mode")
Assisted-by: LLM
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
net/xfrm/xfrm_interface_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index 688306bf62c5..2fcfbfac315a 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -80,7 +80,8 @@ static int xfrmi_build_state(struct net *net, struct nlattr *nla,
struct xfrm_md_info *info;
int ret;
- ret = nla_parse_nested(tb, LWT_XFRM_MAX, nla, xfrm_lwt_policy, extack);
+ ret = lwtunnel_nla_parse(tb, LWT_XFRM_MAX, nla, xfrm_lwt_policy,
+ extack);
if (ret < 0)
return ret;
--
2.50.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 3/4] net: ip_tunnel: accept tunnel options without NLA_F_NESTED
2026-09-23 0:36 [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP " Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 2/4] xfrm: " Yuya Kusakabe
@ 2026-09-23 0:36 ` Yuya Kusakabe
2026-09-27 1:25 ` netdev-bot+sashiko
2026-09-23 0:36 ` [PATCH net 4/4] selftests: net: add lwtunnel route save and restore test Yuya Kusakabe
2026-09-23 11:29 ` [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Ido Schimmel
4 siblings, 1 reply; 10+ messages in thread
From: Yuya Kusakabe @ 2026-09-23 0:36 UTC (permalink / raw)
To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Justin Iurman,
Alexander Aring, Steffen Klassert, Herbert Xu,
Nikolay Aleksandrov, Eyal Birger, Nicolas Dichtel, Xin Long,
Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Yuya Kusakabe
ip_tun_fill_encap_opts() and its helpers dump LWTUNNEL_IP_OPTS,
LWTUNNEL_IP6_OPTS and the geneve, vxlan and erspan options nested in
them without NLA_F_NESTED. Commit ed02551f58b9 ("lwtunnel: change to
use nla_parse_nested on new options") made the parsing of all of them
strict, on the grounds that new attributes should be strict from the
start, but left the dump as it was, and the two sides have disagreed
ever since. So "ip route restore" cannot send back the ip and ip6
encap routes with tunnel options saved by "ip route save":
# ip route add 192.0.2.0/24 encap ip id 1 dst 198.51.100.2 \
geneve_opts 0:0:12121212 dev dummy0
# ip route save 192.0.2.0/24 > route.bin
# ip route del 192.0.2.0/24
# ip route restore < route.bin
Error: NLA_F_NESTED is missing.
The flag is required at three levels:
- ip_tun_policy and ip6_tun_policy validate LWTUNNEL_IP_OPTS and
LWTUNNEL_IP6_OPTS strictly through .strict_start_type.
- ip_tun_parse_opts() validates the options nested in them with
nla_validate(), which is strict as well.
- ip_tun_parse_opts_geneve(), ip_tun_parse_opts_vxlan() and
ip_tun_parse_opts_erspan() parse each option with
nla_parse_nested().
Start the strict validation after LWTUNNEL_IP(6)_OPTS, validate the
options with lwtunnel_nla_validate(), which is nla_validate() without
the NLA_F_NESTED check, and parse each option with lwtunnel_nla_parse().
netlink has no validation level that keeps the other strict checks and
drops that one, so lwtunnel_nla_validate() clears the flag when calling
__nla_validate(). Nothing else is relaxed: unknown option types and
trailing bytes after the last option are still rejected, and attributes
added to ip_tun_policy and ip6_tun_policy later are still validated
strictly.
Fixes: ed02551f58b9 ("lwtunnel: change to use nla_parse_nested on new options")
Fixes: 2f1d370b997a ("lwtunnel: add support for multiple geneve opts")
Fixes: 7b6a70f73764 ("lwtunnel: be STRICT to validate the new LWTUNNEL_IP(6)_OPTS")
Assisted-by: LLM
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
include/net/lwtunnel.h | 22 ++++++++++++++++++++++
net/ipv4/ip_tunnel_core.c | 24 ++++++++++++++----------
2 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 046978d6224c..59d7ec7b04f2 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -80,6 +80,28 @@ static inline int lwtunnel_nla_parse(struct nlattr *tb[], int maxtype,
extack);
}
+/**
+ * lwtunnel_nla_validate - validate the attributes nested in an lwtunnel encap
+ * @nla: encap attribute passed to &lwtunnel_encap_ops.build_state, or an
+ * attribute nested in it
+ * @maxtype: maximum attribute type to be expected
+ * @policy: validation policy
+ * @extack: extended ACK report struct
+ *
+ * Like nla_validate(), except that NLA_F_NESTED is not required on the
+ * attributes nested in @nla, for the reason given for lwtunnel_nla_parse().
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static inline int lwtunnel_nla_validate(const struct nlattr *nla, int maxtype,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack)
+{
+ return __nla_validate(nla_data(nla), nla_len(nla), maxtype, policy,
+ NL_VALIDATE_STRICT & ~NL_VALIDATE_NESTED,
+ extack);
+}
+
#ifdef CONFIG_LWTUNNEL
DECLARE_STATIC_KEY_FALSE(nf_hooks_lwtunnel_enabled);
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index bab42b9e277f..c87827ffc347 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -466,7 +466,9 @@ int skb_tunnel_check_pmtu(struct sk_buff *skb, struct dst_entry *encap_dst,
EXPORT_SYMBOL(skb_tunnel_check_pmtu);
static const struct nla_policy ip_tun_policy[LWTUNNEL_IP_MAX + 1] = {
- [LWTUNNEL_IP_UNSPEC] = { .strict_start_type = LWTUNNEL_IP_OPTS },
+ [LWTUNNEL_IP_UNSPEC] = {
+ .strict_start_type = LWTUNNEL_IP_OPTS + 1
+ },
[LWTUNNEL_IP_ID] = { .type = NLA_U64 },
[LWTUNNEL_IP_DST] = { .type = NLA_U32 },
[LWTUNNEL_IP_SRC] = { .type = NLA_U32 },
@@ -509,8 +511,8 @@ static int ip_tun_parse_opts_geneve(struct nlattr *attr,
struct nlattr *tb[LWTUNNEL_IP_OPT_GENEVE_MAX + 1];
int data_len, err;
- err = nla_parse_nested(tb, LWTUNNEL_IP_OPT_GENEVE_MAX, attr,
- geneve_opt_policy, extack);
+ err = lwtunnel_nla_parse(tb, LWTUNNEL_IP_OPT_GENEVE_MAX, attr,
+ geneve_opt_policy, extack);
if (err)
return err;
@@ -546,8 +548,8 @@ static int ip_tun_parse_opts_vxlan(struct nlattr *attr,
struct nlattr *tb[LWTUNNEL_IP_OPT_VXLAN_MAX + 1];
int err;
- err = nla_parse_nested(tb, LWTUNNEL_IP_OPT_VXLAN_MAX, attr,
- vxlan_opt_policy, extack);
+ err = lwtunnel_nla_parse(tb, LWTUNNEL_IP_OPT_VXLAN_MAX, attr,
+ vxlan_opt_policy, extack);
if (err)
return err;
@@ -575,8 +577,8 @@ static int ip_tun_parse_opts_erspan(struct nlattr *attr,
int err;
u8 ver;
- err = nla_parse_nested(tb, LWTUNNEL_IP_OPT_ERSPAN_MAX, attr,
- erspan_opt_policy, extack);
+ err = lwtunnel_nla_parse(tb, LWTUNNEL_IP_OPT_ERSPAN_MAX, attr,
+ erspan_opt_policy, extack);
if (err)
return err;
@@ -626,8 +628,8 @@ static int ip_tun_parse_opts(struct nlattr *attr, struct ip_tunnel_info *info,
if (!attr)
return 0;
- err = nla_validate(nla_data(attr), nla_len(attr), LWTUNNEL_IP_OPTS_MAX,
- ip_opts_policy, extack);
+ err = lwtunnel_nla_validate(attr, LWTUNNEL_IP_OPTS_MAX,
+ ip_opts_policy, extack);
if (err)
return err;
@@ -975,7 +977,9 @@ static const struct lwtunnel_encap_ops ip_tun_lwt_ops = {
};
static const struct nla_policy ip6_tun_policy[LWTUNNEL_IP6_MAX + 1] = {
- [LWTUNNEL_IP6_UNSPEC] = { .strict_start_type = LWTUNNEL_IP6_OPTS },
+ [LWTUNNEL_IP6_UNSPEC] = {
+ .strict_start_type = LWTUNNEL_IP6_OPTS + 1
+ },
[LWTUNNEL_IP6_ID] = { .type = NLA_U64 },
[LWTUNNEL_IP6_DST] = { .len = sizeof(struct in6_addr) },
[LWTUNNEL_IP6_SRC] = { .len = sizeof(struct in6_addr) },
--
2.50.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 4/4] selftests: net: add lwtunnel route save and restore test
2026-09-23 0:36 [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Yuya Kusakabe
` (2 preceding siblings ...)
2026-09-23 0:36 ` [PATCH net 3/4] net: ip_tunnel: accept tunnel options " Yuya Kusakabe
@ 2026-09-23 0:36 ` Yuya Kusakabe
2026-09-27 1:25 ` netdev-bot+sashiko
2026-09-23 11:29 ` [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Ido Schimmel
4 siblings, 1 reply; 10+ messages in thread
From: Yuya Kusakabe @ 2026-09-23 0:36 UTC (permalink / raw)
To: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Justin Iurman,
Alexander Aring, Steffen Klassert, Herbert Xu,
Nikolay Aleksandrov, Eyal Birger, Nicolas Dichtel, Xin Long,
Shuah Khan
Cc: netdev, linux-kernel, linux-kselftest, Yuya Kusakabe
Add an encap route, save it with "ip route save", delete it and send
the dump back with "ip route restore". The restored route must match
the original one. The test covers the rpl, ioam6 and xfrm encaps and
the ip encap with geneve, vxlan and erspan options. A case is skipped
when the kernel or iproute2 does not support its encap.
Assisted-by: LLM
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
tools/testing/selftests/net/Makefile | 1 +
tools/testing/selftests/net/lwt_save_restore.sh | 116 ++++++++++++++++++++++++
2 files changed, 117 insertions(+)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 3ee3378f8b26..496aed9ca57b 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -62,6 +62,7 @@ TEST_PROGS := \
l2tp.sh \
link_netns.py \
lwt_dst_cache_ref_loop.sh \
+ lwt_save_restore.sh \
macvlan_mcast_shared_mac.sh \
msg_zerocopy.sh \
nat6to4.sh \
diff --git a/tools/testing/selftests/net/lwt_save_restore.sh b/tools/testing/selftests/net/lwt_save_restore.sh
new file mode 100755
index 000000000000..9666386c622d
--- /dev/null
+++ b/tools/testing/selftests/net/lwt_save_restore.sh
@@ -0,0 +1,116 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Check that routes with a lightweight tunnel encap survive "ip route save"
+# followed by "ip route restore", which sends the dumped RTA_ENCAP back to
+# the kernel unchanged.
+
+# shellcheck disable=SC1091,SC2034,SC2154,SC2317
+source lib.sh
+
+ALL_TESTS="
+ save_restore_rpl
+ save_restore_ioam6
+ save_restore_xfrm
+ save_restore_geneve_opts
+ save_restore_vxlan_opts
+ save_restore_erspan_opts
+"
+
+setup_prepare()
+{
+ setup_ns NS
+ defer cleanup_all_ns
+
+ ip -n "$NS" link add name dummy0 up type dummy
+}
+
+# save_restore <description> <prefix> <ip route add arguments>
+#
+# The description starts with the encap type.
+save_restore()
+{
+ local desc=$1; shift
+ local prefix=$1
+ local encap=${desc%% *}
+ local before after dump out rc
+ local family=-4
+
+ [[ $prefix == *:* ]] && family=-6
+
+ RET=0
+
+ if ! ip route help 2>&1 | grep "^ENCAPTYPE" | grep -qw "$encap"; then
+ log_test_skip "$desc" "iproute2 lacks the encap"
+ return
+ fi
+
+ out=$(ip -n "$NS" "$family" route add "$@" 2>&1)
+ rc=$?
+ if ((rc)) && [[ $out == *"encapsulation type"* ||
+ $out == *CONFIG_LWTUNNEL* ]]; then
+ log_test_skip "$desc" "kernel lacks the encap"
+ return
+ fi
+ check_err "$rc" "Failed to add route: $out"
+
+ dump=$(mktemp)
+ defer rm -f "$dump"
+
+ before=$(ip -n "$NS" "$family" route show "$prefix")
+ ip -n "$NS" "$family" route save "$prefix" > "$dump"
+ check_err $? "Failed to save route"
+
+ ip -n "$NS" "$family" route del "$prefix"
+ ip -n "$NS" "$family" route restore < "$dump"
+ check_err $? "Failed to restore route"
+
+ after=$(ip -n "$NS" "$family" route show "$prefix")
+ [ "$before" = "$after" ]
+ check_err $? "Restored route differs from the saved one"
+
+ log_test "encap $desc route save and restore"
+}
+
+save_restore_rpl()
+{
+ save_restore rpl 2001:db8:1::/64 \
+ encap rpl segs 2001:db8::2 dev dummy0
+}
+
+save_restore_ioam6()
+{
+ save_restore ioam6 2001:db8:2::/64 \
+ encap ioam6 trace prealloc type 0x800000 ns 1 size 12 dev dummy0
+}
+
+save_restore_xfrm()
+{
+ # iproute2 takes every argument after "encap xfrm" as an xfrm one
+ save_restore xfrm 2001:db8:3::/64 dev dummy0 encap xfrm if_id 1
+}
+
+save_restore_geneve_opts()
+{
+ save_restore "ip geneve_opts" 192.0.2.0/26 \
+ encap ip id 1 dst 198.51.100.2 geneve_opts 0:0:12121212 \
+ dev dummy0
+}
+
+save_restore_vxlan_opts()
+{
+ save_restore "ip vxlan_opts" 192.0.2.64/26 \
+ encap ip id 1 dst 198.51.100.2 vxlan_opts 456 dev dummy0
+}
+
+save_restore_erspan_opts()
+{
+ save_restore "ip erspan_opts" 192.0.2.128/26 \
+ encap ip id 1 dst 198.51.100.2 erspan_opts 1:123:0:0 dev dummy0
+}
+
+trap defer_scopes_cleanup EXIT
+setup_prepare
+tests_run
+
+exit "$EXIT_STATUS"
--
2.50.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED
2026-09-23 0:36 [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Yuya Kusakabe
` (3 preceding siblings ...)
2026-09-23 0:36 ` [PATCH net 4/4] selftests: net: add lwtunnel route save and restore test Yuya Kusakabe
@ 2026-09-23 11:29 ` Ido Schimmel
2026-09-23 11:59 ` Yuya Kusakabe
4 siblings, 1 reply; 10+ messages in thread
From: Ido Schimmel @ 2026-09-23 11:29 UTC (permalink / raw)
To: Yuya Kusakabe
Cc: David Ahern, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Justin Iurman, Alexander Aring,
Steffen Klassert, Herbert Xu, Nikolay Aleksandrov, Eyal Birger,
Nicolas Dichtel, Xin Long, Shuah Khan, netdev, linux-kernel,
linux-kselftest
On Wed, Sep 23, 2026 at 09:36:55AM +0900, Yuya Kusakabe wrote:
> "ip route save" stores a route dump and "ip route restore" sends it
> back to the kernel unchanged. The kernel dumps RTA_ENCAP, and some of
> the attributes nested in it, without NLA_F_NESTED, but rpl, ioam6,
> xfrm and the geneve, vxlan and erspan options of the ip and ip6 encaps
> require the flag when parsing them. Restoring such a route fails with
> "NLA_F_NESTED is missing", and none of them has been restorable in any
> release.
If this never worked, then why target these patches at net and blaming
up to 7 years old commits? I try to follow [1] when deciding between net
and net-next.
Also, did you hit this in practice or was this flagged by AI? If nobody
is using save/restore and hitting this in practice, then maybe it's not
worth to loosen the validation performed by the kernel.
Note that nowadays many deployments program routes using nexthop objects
and ip-nexthop completely lacks save/restore functionality.
[1] https://lkml.org/lkml/2018/6/24/113
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED
2026-09-23 11:29 ` [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Ido Schimmel
@ 2026-09-23 11:59 ` Yuya Kusakabe
0 siblings, 0 replies; 10+ messages in thread
From: Yuya Kusakabe @ 2026-09-23 11:59 UTC (permalink / raw)
To: idosch
Cc: dsahern, davem, edumazet, kuba, pabeni, horms, justin.iurman,
alex.aring, steffen.klassert, herbert, razor, eyal.birger,
nicolas.dichtel, lucien.xin, shuah, netdev, linux-kernel,
linux-kselftest
On Wed, Sep 23, 2026 at 02:29:17PM +0300, Ido Schimmel wrote:
> If this never worked, then why target these patches at net and blaming
> up to 7 years old commits? I try to follow [1] when deciding between net
> and net-next.
You are right, it is not a regression. If there is a v2 it will go to
net-next without the Fixes tags.
> Also, did you hit this in practice or was this flagged by AI? If nobody
> is using save/restore and hitting this in practice, then maybe it's not
> worth to loosen the validation performed by the kernel.
It was flagged by an AI review of a new encap I am working on (SRv6
Mobile User Plane, still RFC). I reproduced it there, and checking the
existing encaps showed rpl, ioam6, xfrm and the ip tunnel options behave
the same. I have no report from their users.
My motivation was that a feature which exists should not silently fail,
and the series only drops the flag check on the container; everything
nested inside is still validated strictly. But if nobody relies on
save/restore for these encaps, I am fine dropping the series, and the
new encap will keep nla_parse_nested() like rpl, ioam6 and xfrm.
Thanks
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP without NLA_F_NESTED
2026-09-23 0:36 ` [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP " Yuya Kusakabe
@ 2026-09-27 1:24 ` netdev-bot+sashiko
0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 1:24 UTC (permalink / raw)
To: yuya.kusakabe
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms,
justin.iurman, alex.aring, steffen.klassert, herbert, razor,
eyal.birger, nicolas.dichtel, lucien.xin, shuah, netdev,
linux-kernel, linux-kselftest
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] Concern: the patch fixes rpl and ioam6 but leaves
xfrmi_build_state() (net/xfrm/xfrm_interface_core.c:83) on strict…
- [Low] Concern: the IP/IP6 tunnel encap options (LWTUNNEL_IP_OPTS /
LWTUNNEL_IP6_OPTS and the GENEVE/VXLAN/ERSPAN entries) are dumped with…
--
net: lwtunnel: accept RTA_ENCAP without NLA_F_NESTED
This adds lwtunnel_nla_parse(). It parses the attributes nested in an
lwtunnel encap strictly, but does not require NLA_F_NESTED on the encap
attribute itself. rpl and ioam6 now use it, so routes dumped by "ip route
save" can be restored with "ip route restore".
> diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
> index 26232f603e33c..046978d6224c2 100644
> --- a/include/net/lwtunnel.h
> +++ b/include/net/lwtunnel.h
[ ... ]
> @@ -37,6 +38,7 @@ struct lwtunnel_state {
> };
>
> struct lwtunnel_encap_ops {
> + /* encap may lack NLA_F_NESTED, parse it with lwtunnel_nla_parse() */
> int (*build_state)(struct net *net, struct nlattr *encap,
> unsigned int family, const void *cfg,
> struct lwtunnel_state **ts,
[Severity: Low]
Do other build_state callbacks still break this contract after this
patch? xfrmi_build_state() in net/xfrm/xfrm_interface_core.c still uses
the strict parser:
net/xfrm/xfrm_interface_core.c:xfrmi_build_state() {
...
ret = nla_parse_nested(tb, LWT_XFRM_MAX, nla, xfrm_lwt_policy, extack);
...
}
lwtunnel_fill_encap() emits RTA_ENCAP with the flag cleared:
net/core/lwtunnel.c:lwtunnel_fill_encap() {
...
nest = nla_nest_start_noflag(skb, encap_attr);
...
}
Would "ip route save" followed by "ip route restore" of an "encap xfrm
if_id N" route still fail with "NLA_F_NESTED is missing" at this point?
A later patch in this series, "xfrm: lwtunnel: accept RTA_ENCAP without
NLA_F_NESTED", switches xfrmi_build_state() to lwtunnel_nla_parse() and
carries its own Fixes: tag. The series as a whole closes this gap, even
though this patch does not.
> @@ -53,6 +55,31 @@ struct lwtunnel_encap_ops {
> struct module *owner;
> };
>
> +/**
> + * lwtunnel_nla_parse - parse the attributes nested in an lwtunnel encap
> + * @tb: destination array with maxtype+1 elements
> + * @maxtype: maximum attribute type to be expected
> + * @nla: encap attribute passed to &lwtunnel_encap_ops.build_state, or an
> + * attribute nested in it
> + * @policy: validation policy
> + * @extack: extended ACK report struct
> + *
> + * The encap attribute, and some of the attributes nested in it, have always
> + * been dumped without NLA_F_NESTED, and userspace such as "ip route restore"
> + * sends a dump back unchanged, so the flag cannot be required on @nla.
> + * The attributes nested in @nla are still validated strictly.
> + *
> + * Return: 0 on success or a negative error code.
> + */
> +static inline int lwtunnel_nla_parse(struct nlattr *tb[], int maxtype,
> + const struct nlattr *nla,
> + const struct nla_policy *policy,
> + struct netlink_ext_ack *extack)
> +{
> + return nla_parse(tb, maxtype, nla_data(nla), nla_len(nla), policy,
> + extack);
> +}
[Severity: Low]
The kernel-doc says some attributes nested in the encap are also dumped
without NLA_F_NESTED. Is the IP/IP6 tunnel options path still rejected on
restore after this patch?
In net/ipv4/ip_tunnel_core.c, ip_tun_fill_encap_opts() and the
geneve/vxlan/erspan fill helpers emit the option nests without the flag:
nest = nla_nest_start_noflag(skb, type);
...
nest = nla_nest_start_noflag(skb, LWTUNNEL_IP_OPTS_GENEVE);
The input side validates them strictly in three places.
ip_tun_policy and ip6_tun_policy:
[LWTUNNEL_IP_UNSPEC] = { .strict_start_type = LWTUNNEL_IP_OPTS },
ip_tun_parse_opts():
err = nla_validate(nla_data(attr), nla_len(attr), LWTUNNEL_IP_OPTS_MAX,
...
ip_tun_parse_opts_geneve(), and the same in the vxlan and erspan parsers:
err = nla_parse_nested(tb, LWTUNNEL_IP_OPT_GENEVE_MAX, attr,
...
lwtunnel_nla_parse() calls nla_parse(), which uses NL_VALIDATE_STRICT. If
a policy had NLA_NESTED children without the flag, it would reject them
too. Could the kernel-doc sentence "The attributes nested in @nla are
still validated strictly" mislead callers that expect unflagged nested
children to be accepted?
This restore failure for the tunnel options has existed since
ed02551f58b9. This patch does not change it.
A later patch in this series, "net: ip_tunnel: accept tunnel options
without NLA_F_NESTED", makes three changes:
- It moves strict_start_type to LWTUNNEL_IP(6)_OPTS + 1.
- It adds lwtunnel_nla_validate() and uses it in ip_tun_parse_opts().
- It switches the geneve/vxlan/erspan parsers to lwtunnel_nla_parse().
None of the policies passed to lwtunnel_nla_parse() in the series have
NLA_NESTED entries. In practice, its strict mode never rejects an
unflagged child.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-lwt-encap-noflag-v1-0-8de7ab6c86e9%40gmail.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 3/4] net: ip_tunnel: accept tunnel options without NLA_F_NESTED
2026-09-23 0:36 ` [PATCH net 3/4] net: ip_tunnel: accept tunnel options " Yuya Kusakabe
@ 2026-09-27 1:25 ` netdev-bot+sashiko
0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 1:25 UTC (permalink / raw)
To: yuya.kusakabe
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms,
justin.iurman, alex.aring, steffen.klassert, herbert, razor,
eyal.birger, nicolas.dichtel, lucien.xin, shuah, netdev,
linux-kernel, linux-kselftest
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] Restoring a saved route that has two or more geneve options now
succeeds, but the kernel keeps only the last option.
- [Low] The kernel-doc for the new shared helper lwtunnel_nla_validate()
(include/net/lwtunnel.h:91-92) says it is "Like nla_validate(), except…
--
net: ip_tunnel: accept tunnel options without NLA_F_NESTED
This patch makes the LWTUNNEL_IP_OPTS and LWTUNNEL_IP6_OPTS parsing
accept attributes that lack NLA_F_NESTED, so that "ip route restore"
can replay routes that the kernel dumped without the flag. It moves
strict_start_type past the OPTS attribute and adds a helper,
lwtunnel_nla_validate(), which validates with the nested check turned
off. It also parses each option with lwtunnel_nla_parse().
> diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
> index 046978d6224c2..59d7ec7b04f2d 100644
> --- a/include/net/lwtunnel.h
> +++ b/include/net/lwtunnel.h
> @@ -80,6 +80,28 @@ static inline int lwtunnel_nla_parse(struct nlattr *tb[], int maxtype,
> extack);
> }
>
> +/**
> + * lwtunnel_nla_validate - validate the attributes nested in an lwtunnel encap
> + * @nla: encap attribute passed to &lwtunnel_encap_ops.build_state, or an
> + * attribute nested in it
> + * @maxtype: maximum attribute type to be expected
> + * @policy: validation policy
> + * @extack: extended ACK report struct
> + *
> + * Like nla_validate(), except that NLA_F_NESTED is not required on the
> + * attributes nested in @nla, for the reason given for lwtunnel_nla_parse().
> + *
> + * Return: 0 on success or a negative error code.
> + */
> +static inline int lwtunnel_nla_validate(const struct nlattr *nla, int maxtype,
> + const struct nla_policy *policy,
> + struct netlink_ext_ack *extack)
> +{
> + return __nla_validate(nla_data(nla), nla_len(nla), maxtype, policy,
> + NL_VALIDATE_STRICT & ~NL_VALIDATE_NESTED,
> + extack);
> +}
[Severity: Low]
This isn't a bug with the current caller, but does the kernel-doc match
what the NL_VALIDATE_STRICT & ~NL_VALIDATE_NESTED mask does?
The mask differs from the doc in two ways.
First, NL_VALIDATE_NESTED also controls the check in the other
direction. With it cleared, validate_nla() no longer rejects a
non-nested attribute that carries NLA_F_NESTED, although nla_validate()
would:
lib/nlattr.c:validate_nla() {
...
if (validate & NL_VALIDATE_NESTED) {
...
if (pt->type != NLA_NESTED && pt->type != NLA_NESTED_ARRAY &&
pt->type != NLA_UNSPEC && (nla->nla_type & NLA_F_NESTED)) {
NL_SET_ERR_MSG_ATTR_POL(extack, nla, pt,
"NLA_F_NESTED not expected");
return -EINVAL;
}
}
...
}
Second, a policy with strict_start_type turns the bit back on for
attribute types at or above that value:
lib/nlattr.c:validate_nla() {
...
if (strict_start_type && type >= strict_start_type)
validate |= NL_VALIDATE_STRICT;
...
}
For those types, NLA_F_NESTED is required again.
The only caller, ip_tun_parse_opts() with ip_opts_policy, is not
affected. That policy has only NLA_NESTED entries and no
strict_start_type. However, the doc invites use on any attribute nested
in a build_state encap attribute.
Could the doc describe these two cases, so that future callers don't
rely on semantics that differ from the implementation?
[ ... ]
> diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
> index bab42b9e277f7..c87827ffc3475 100644
> --- a/net/ipv4/ip_tunnel_core.c
> +++ b/net/ipv4/ip_tunnel_core.c
> @@ -466,7 +466,9 @@ int skb_tunnel_check_pmtu(struct sk_buff *skb, struct dst_entry *encap_dst,
> EXPORT_SYMBOL(skb_tunnel_check_pmtu);
>
> static const struct nla_policy ip_tun_policy[LWTUNNEL_IP_MAX + 1] = {
> - [LWTUNNEL_IP_UNSPEC] = { .strict_start_type = LWTUNNEL_IP_OPTS },
> + [LWTUNNEL_IP_UNSPEC] = {
> + .strict_start_type = LWTUNNEL_IP_OPTS + 1
> + },
> [LWTUNNEL_IP_ID] = { .type = NLA_U64 },
> [LWTUNNEL_IP_DST] = { .type = NLA_U32 },
> [LWTUNNEL_IP_SRC] = { .type = NLA_U32 },
> @@ -509,8 +511,8 @@ static int ip_tun_parse_opts_geneve(struct nlattr *attr,
> struct nlattr *tb[LWTUNNEL_IP_OPT_GENEVE_MAX + 1];
> int data_len, err;
>
> - err = nla_parse_nested(tb, LWTUNNEL_IP_OPT_GENEVE_MAX, attr,
> - geneve_opt_policy, extack);
> + err = lwtunnel_nla_parse(tb, LWTUNNEL_IP_OPT_GENEVE_MAX, attr,
> + geneve_opt_policy, extack);
> if (err)
> return err;
[Severity: High]
Can restoring a saved route with more than one geneve option now succeed
but keep only the last option?
The dump side puts all geneve options into a single
LWTUNNEL_IP_OPTS_GENEVE nest, as repeated CLASS/TYPE/DATA triples:
net/ipv4/ip_tunnel_core.c:ip_tun_fill_encap_opts_geneve() {
...
nest = nla_nest_start_noflag(skb, LWTUNNEL_IP_OPTS_GENEVE);
...
while (tun_info->options_len > offset) {
opt = ip_tunnel_info_opts(tun_info) + offset;
if (nla_put_be16(skb, LWTUNNEL_IP_OPT_GENEVE_CLASS,
opt->opt_class) ||
nla_put_u8(skb, LWTUNNEL_IP_OPT_GENEVE_TYPE, opt->type) ||
nla_put(skb, LWTUNNEL_IP_OPT_GENEVE_DATA, opt->length * 4,
opt->opt_data)) {
...
}
The parse side expects one LWTUNNEL_IP_OPTS_GENEVE attribute per
option. ip_tun_parse_opts() calls ip_tun_parse_opts_geneve() once for
each GENEVE attribute, and each call rebuilds exactly one option.
When nla_parse() in ip_tun_parse_opts_geneve() sees repeated types,
__nla_validate_parse() just overwrites tb[type] without an error, so
only the last triple is kept. The function then returns
sizeof(struct geneve_opt) + data_len for that single option. As a
result, ip_tun_get_optlen() sizes the state for one option and
ip_tun_set_opts() copies only that one.
Before this patch, a restored message like this was rejected with
"NLA_F_NESTED is missing". The rejection came from the strict_start_type
check in ip_tun_policy/ip6_tun_policy, and from nla_validate() and
nla_parse_nested(). With all three relaxed, the multi-option nest is
accepted and truncated with no error:
ip route add ... encap ip id 1 geneve_opts 0:0:12121212,1:2:12121212 ...
ip route save ...
ip route del ...
ip route restore
ip_tun_build_state()
ip_tun_get_optlen()
ip_tun_parse_opts()
lwtunnel_nla_validate() <- no NLA_F_NESTED check any more
ip_tun_parse_opts_geneve()
nla_parse() <- second triple overwrites the first
The same applies to ip6_tun_build_state(), which goes through the same
ip_tun_get_optlen()/ip_tun_set_opts() path.
The commit message says this fixes "ip route restore" for routes with
tunnel options saved by "ip route save", and lists 2f1d370b997a
("lwtunnel: add support for multiple geneve opts") in Fixes. That
commit created the mismatch between one nest on dump and one nest per
option on parse, and the mismatch is still there after this patch.
No later patch in the series appears to address this. The selftest
added in "selftests: net: add lwtunnel route save and restore test"
uses only a single geneve option (geneve_opts 0:0:12121212), so it
would not catch it.
Should ip_tun_parse_opts()/ip_tun_parse_opts_geneve() accept repeated
CLASS/TYPE/DATA triples inside one LWTUNNEL_IP_OPTS_GENEVE nest, which
is the layout the dump produces? If not, should restore keep rejecting
this input rather than silently dropping options?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-lwt-encap-noflag-v1-0-8de7ab6c86e9%40gmail.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 4/4] selftests: net: add lwtunnel route save and restore test
2026-09-23 0:36 ` [PATCH net 4/4] selftests: net: add lwtunnel route save and restore test Yuya Kusakabe
@ 2026-09-27 1:25 ` netdev-bot+sashiko
0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 1:25 UTC (permalink / raw)
To: yuya.kusakabe
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms,
justin.iurman, alex.aring, steffen.klassert, herbert, razor,
eyal.birger, nicolas.dichtel, lucien.xin, shuah, netdev,
linux-kernel, linux-kselftest
Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 4
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The xfrm case (save_restore_xfrm,
tools/testing/selftests/net/lwt_save_restore.sh:87-91) almost always…
- [Low] save_restore() does not check the exit status of 'ip -n "$NS"
"$family" route del "$prefix"' (line 64).
- [Low] The iproute2 capability probe in save_restore() (line 43) only
checks the base encap keyword.
- [Low] The same case gets a different name depending on its outcome.
- [Low] setup_prepare() ignores the return value of setup_ns.
--
selftests: net: add lwtunnel route save and restore test
This adds a selftest that installs a route with an lwtunnel encap, saves
it with "ip route save", deletes it, restores it with "ip route restore"
and checks that the restored route matches the original. It covers the
rpl, ioam6 and xfrm encaps and the ip encap with geneve, vxlan and erspan
options, skipping cases the kernel or iproute2 does not support.
> diff --git a/tools/testing/selftests/net/lwt_save_restore.sh b/tools/testing/selftests/net/lwt_save_restore.sh
> new file mode 100755
> index 0000000000000..9666386c622d5
> --- /dev/null
> +++ b/tools/testing/selftests/net/lwt_save_restore.sh
> @@ -0,0 +1,116 @@
[ ... ]
> +setup_prepare()
> +{
> + setup_ns NS
[Severity: Low]
Should the return value of setup_ns be checked here?
When "ip netns add" fails, for example when the script is not run as
root, setup_ns() in lib.sh does not exit. NS has already been assigned,
and setup_ns() returns ksft_skip:
tools/testing/selftests/net/lib.sh:setup_ns() {
...
if ! ip netns add "${!ns_name}"; then
echo "Failed to create namespace $ns_name"
cleanup_ns "${ns_list[@]}"
return $ksft_skip
fi
...
}
The script then calls tests_run anyway. Each case passes the host-side
"ip route help" probe, and then "ip -n $NS route add" fails with "Cannot
open network namespace ...".
That error matches neither skip pattern in save_restore(). So every case
is reported as FAIL instead of the whole script being skipped.
Most net selftests call setup_ns without checking it. Still, would
"setup_ns NS || exit $ksft_skip" give a more accurate result here?
> + defer cleanup_all_ns
> +
> + ip -n "$NS" link add name dummy0 up type dummy
> +}
[ ... ]
> + RET=0
> +
> + if ! ip route help 2>&1 | grep "^ENCAPTYPE" | grep -qw "$encap"; then
> + log_test_skip "$desc" "iproute2 lacks the encap"
> + return
> + fi
> +
> + out=$(ip -n "$NS" "$family" route add "$@" 2>&1)
> + rc=$?
> + if ((rc)) && [[ $out == *"encapsulation type"* ||
> + $out == *CONFIG_LWTUNNEL* ]]; then
> + log_test_skip "$desc" "kernel lacks the encap"
> + return
> + fi
> + check_err "$rc" "Failed to add route: $out"
[Severity: Low]
In the geneve, vxlan and erspan cases, encap=${desc%% *} is "ip". The
probe only checks that iproute2 supports "encap ip". Does it also need
to check for geneve_opts, vxlan_opts and erspan_opts?
Some iproute2 versions support "encap ip" but not these option keywords.
There, parse_encap_ip() stops at the unknown token and route add fails
with a generic parse error:
either "to" is duplicate, or "geneve_opts" is a garbage
That message contains neither "encapsulation type" nor "CONFIG_LWTUNNEL".
So check_err() records "Failed to add route" and the case reports FAIL,
not the SKIP described in the commit message.
> +
> + dump=$(mktemp)
> + defer rm -f "$dump"
> +
> + before=$(ip -n "$NS" "$family" route show "$prefix")
> + ip -n "$NS" "$family" route save "$prefix" > "$dump"
> + check_err $? "Failed to save route"
> +
> + ip -n "$NS" "$family" route del "$prefix"
[Severity: Low]
Should the exit status of this route del be checked?
iproute2's restore_handler() sends RTM_NEWROUTE with NLM_F_CREATE and
without NLM_F_EXCL, and it treats EEXIST as success. If the del fails to
remove the route, the restore sends an exact duplicate. Then
fib_table_insert() or fib6_add_rt2node() returns -EEXIST.
In that case restore still exits 0, and before and after match because
the original route was never removed. log_test reports PASS, but the
kernel never parsed the restored RTA_ENCAP.
Would it help to add a check_err after the del, and perhaps to check
that "ip route show $prefix" is empty before the restore?
> + ip -n "$NS" "$family" route restore < "$dump"
> + check_err $? "Failed to restore route"
> +
> + after=$(ip -n "$NS" "$family" route show "$prefix")
> + [ "$before" = "$after" ]
> + check_err $? "Restored route differs from the saved one"
> +
> + log_test "encap $desc route save and restore"
[Severity: Low]
This isn't a bug, but the same case gets a different test name depending
on how it ends. The skip paths above call:
log_test_skip "$desc" "iproute2 lacks the encap"
log_test_skip "$desc" "kernel lacks the encap"
Those produce names like "rpl" or "ip geneve_opts". The pass/fail path
uses "encap $desc route save and restore" instead.
Could both paths use the same test name, so that results tracked by name
line up across runs?
> +}
[ ... ]
> +save_restore_xfrm()
> +{
> + # iproute2 takes every argument after "encap xfrm" as an xfrm one
> + save_restore xfrm 2001:db8:3::/64 dev dummy0 encap xfrm if_id 1
> +}
[Severity: Medium]
Will this case ever run under the in-tree selftest config?
tools/testing/selftests/net/config sets CONFIG_XFRM_INTERFACE=m. Only
the xfrm_interface module registers xfrmi_encap_ops, from xfrmi_init().
Module autoload is disabled for this encap type:
net/core/lwtunnel.c:lwtunnel_encap_str() {
...
case LWTUNNEL_ENCAP_XFRM:
/* module autoload not supported for encap type */
return NULL;
...
}
So unless xfrm_interface is already loaded, the route add goes:
rtm_to_fib6_config()
lwtunnel_valid_encap_type()
lwtunnel_encap_str() returns NULL, so no request_module()
fails with -EOPNOTSUPP, "lwt encapsulation type not supported"
That message matches the *"encapsulation type"* pattern in
save_restore(). The case is then logged as "kernel lacks the encap" and
skipped.
The test routes through dummy0 and never runs modprobe. It also never
creates a "type xfrm" link, which would autoload the module through
MODULE_ALIAS_RTNL_LINK("xfrm").
Suppose the xfrm fix earlier in this series ("xfrm: lwtunnel: accept
RTA_ENCAP without NLA_F_NESTED") were reverted. Would this case still
SKIP instead of FAIL, unless an earlier test happened to load
xfrm_interface?
Would it make sense to create an xfrm interface in $NS before adding the
route, for example with "ip -n $NS link add ipsec0 type xfrm external"?
Running "modprobe -q xfrm_interface" would also work.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-lwt-encap-noflag-v1-0-8de7ab6c86e9%40gmail.com
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-27 1:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 0:36 [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 1/4] net: lwtunnel: accept RTA_ENCAP " Yuya Kusakabe
2026-09-27 1:24 ` netdev-bot+sashiko
2026-09-23 0:36 ` [PATCH net 2/4] xfrm: " Yuya Kusakabe
2026-09-23 0:36 ` [PATCH net 3/4] net: ip_tunnel: accept tunnel options " Yuya Kusakabe
2026-09-27 1:25 ` netdev-bot+sashiko
2026-09-23 0:36 ` [PATCH net 4/4] selftests: net: add lwtunnel route save and restore test Yuya Kusakabe
2026-09-27 1:25 ` netdev-bot+sashiko
2026-09-23 11:29 ` [PATCH net 0/4] net: lwtunnel: accept encap attributes without NLA_F_NESTED Ido Schimmel
2026-09-23 11:59 ` Yuya Kusakabe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®