[PATCH iproute2] ip: fix return value for rtnl_talk failures

[PATCH iproute2] ip: fix return value for rtnl_talk failures

[Hangbin Liu]

Since my last commit "rtnetlink: add new function rtnl_echo_talk()" we
return the kernel rtnl exit code directly, which breaks some kernel
selftest checking. As there are still a lot of tests checking -2 as the
error return value, to keep backward compatibility, let's keep using
-2 for all the rtnl return values.

Reported-by: Ido Schimmel <idosch@idosch.org>
Fixes: 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 ip/ipaddress.c | 10 ++++++++--
 ip/iplink.c    |  2 +-
 ip/ipnexthop.c | 10 ++++++++--
 ip/iproute.c   | 10 ++++++++--
 ip/iprule.c    | 10 ++++++++--
 5 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 456545bb..5e833482 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2422,6 +2422,7 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 	__u32 preferred_lft = INFINITY_LIFE_TIME;
 	__u32 valid_lft = INFINITY_LIFE_TIME;
 	unsigned int ifa_flags = 0;
+	int ret;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "peer") == 0 ||
@@ -2604,9 +2605,14 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 	}
 
 	if (echo_request)
-		return rtnl_echo_talk(&rth, &req.n, json, print_addrinfo);
+		ret = rtnl_echo_talk(&rth, &req.n, json, print_addrinfo);
+	else
+		ret = rtnl_talk(&rth, &req.n, NULL);
 
-	return rtnl_talk(&rth, &req.n, NULL);
+	if (ret)
+		return -2;
+
+	return 0;
 }
 
 int do_ipaddr(int argc, char **argv)
diff --git a/ip/iplink.c b/ip/iplink.c
index 92ce6c47..301a535e 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1129,7 +1129,7 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 		ret = rtnl_talk(&rth, &req.n, NULL);
 
 	if (ret)
-		return ret;
+		return -2;
 
 	/* remove device from cache; next use can refresh with new data */
 	ll_drop_by_index(req.i.ifi_index);
diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index c87e847f..9f16b809 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -920,6 +920,7 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
 		.nhm.nh_family = preferred_family,
 	};
 	__u32 nh_flags = 0;
+	int ret;
 
 	while (argc > 0) {
 		if (!strcmp(*argv, "id")) {
@@ -1000,9 +1001,14 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
 	req.nhm.nh_flags = nh_flags;
 
 	if (echo_request)
-		return rtnl_echo_talk(&rth, &req.n, json, print_nexthop_nocache);
+		ret = rtnl_echo_talk(&rth, &req.n, json, print_nexthop_nocache);
+	else
+		ret = rtnl_talk(&rth, &req.n, NULL);
+
+	if (ret)
+		return -2;
 
-	return rtnl_talk(&rth, &req.n, NULL);
+	return 0;
 }
 
 static int ipnh_get_id(__u32 id)
diff --git a/ip/iproute.c b/ip/iproute.c
index b4b9d1b2..f34289e8 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1134,6 +1134,7 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
 	int raw = 0;
 	int type_ok = 0;
 	__u32 nhid = 0;
+	int ret;
 
 	if (cmd != RTM_DELROUTE) {
 		req.r.rtm_protocol = RTPROT_BOOT;
@@ -1588,9 +1589,14 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
 		req.r.rtm_type = RTN_UNICAST;
 
 	if (echo_request)
-		return rtnl_echo_talk(&rth, &req.n, json, print_route);
+		ret = rtnl_echo_talk(&rth, &req.n, json, print_route);
+	else
+		ret = rtnl_talk(&rth, &req.n, NULL);
+
+	if (ret)
+		return -2;
 
-	return rtnl_talk(&rth, &req.n, NULL);
+	return 0;
 }
 
 static int iproute_flush_cache(void)
diff --git a/ip/iprule.c b/ip/iprule.c
index 8f750425..8e5a2287 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -787,6 +787,7 @@ static int iprule_modify(int cmd, int argc, char **argv)
 		.frh.family = preferred_family,
 		.frh.action = FR_ACT_UNSPEC,
 	};
+	int ret;
 
 	if (cmd == RTM_NEWRULE) {
 		if (argc == 0) {
@@ -1017,9 +1018,14 @@ static int iprule_modify(int cmd, int argc, char **argv)
 		req.frh.table = RT_TABLE_MAIN;
 
 	if (echo_request)
-		return rtnl_echo_talk(&rth, &req.n, json, print_rule);
+		ret = rtnl_echo_talk(&rth, &req.n, json, print_rule);
+	else
+		ret = rtnl_talk(&rth, &req.n, NULL);
+
+	if (ret)
+		return -2;
 
-	return rtnl_talk(&rth, &req.n, NULL);
+	return 0;
 }
 
 int do_iprule(int argc, char **argv)
-- 
2.38.1

Re: [PATCH iproute2] ip: fix return value for rtnl_talk failures

[Ido Schimmel]

On Tue, Nov 08, 2022 at 08:43:44PM +0800, Hangbin Liu wrote:
> Since my last commit "rtnetlink: add new function rtnl_echo_talk()" we
> return the kernel rtnl exit code directly, which breaks some kernel
> selftest checking. As there are still a lot of tests checking -2 as the
> error return value, to keep backward compatibility, let's keep using
> -2 for all the rtnl return values.
> 
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Fixes: 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Tested-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH iproute2] ip: fix return value for rtnl_talk failures

[Ido Schimmel]

On Tue, Nov 08, 2022 at 08:43:44PM +0800, Hangbin Liu wrote:
> Since my last commit "rtnetlink: add new function rtnl_echo_talk()" we
> return the kernel rtnl exit code directly, which breaks some kernel
> selftest checking. As there are still a lot of tests checking -2 as the
> error return value, to keep backward compatibility, let's keep using
> -2 for all the rtnl return values.
> 
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Fixes: 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Stephen, I believe this patch was missed. It appears as "New, archived"
in patchwork:
https://patchwork.kernel.org/project/netdevbpf/patch/20221108124344.192326-1-liuhangbin@gmail.com/

Can you please apply it? It fixes regressions in kernel selftests that
use iproute2 with blamed commit. See:

https://lore.kernel.org/netdev/Y2oWDRIIR6gjkM4a@shredder/

Thanks