Re: [PATCHv3 iproute2-next] rtnetlink: add new function rtnl_echo_talk()

[Ido Schimmel <idosch@idosch.org>]

On Thu, Sep 29, 2022 at 04:10:16PM +0800, Hangbin Liu wrote:
> diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
> index 59f8f2fb..c87e847f 100644
> --- a/ip/ipnexthop.c
> +++ b/ip/ipnexthop.c
> @@ -919,12 +919,7 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
>  		.n.nlmsg_type = cmd,
>  		.nhm.nh_family = preferred_family,
>  	};
> -	struct nlmsghdr *answer;
>  	__u32 nh_flags = 0;
> -	int ret;
> -
> -	if (echo_request)
> -		req.n.nlmsg_flags |= NLM_F_ECHO | NLM_F_ACK;
>  
>  	while (argc > 0) {
>  		if (!strcmp(*argv, "id")) {
> @@ -1005,23 +1000,9 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
>  	req.nhm.nh_flags = nh_flags;
>  
>  	if (echo_request)
> -		ret = rtnl_talk(&rth, &req.n, &answer);
> -	else
> -		ret = rtnl_talk(&rth, &req.n, NULL);
> -
> -	if (ret < 0)
> -		return -2;
> +		return rtnl_echo_talk(&rth, &req.n, json, print_nexthop_nocache);
>  
> -	if (echo_request) {
> -		new_json_obj(json);
> -		open_json_object(NULL);
> -		print_nexthop_nocache(answer, (void *)stdout);
> -		close_json_object();
> -		delete_json_obj();
> -		free(answer);
> -	}
> -
> -	return 0;
> +	return rtnl_talk(&rth, &req.n, NULL);
>  }

Hangbin,

This change breaks the nexthop selftest:
tools/testing/selftests/net/fib_nexthops.sh

Which is specifically checking for "2" as the error code. Example:

# attempt to create nh without a device or gw - fails
run_cmd "$IP nexthop add id 1"
log_test $? 2 "Nexthop with no device or gateway"

I think it's better to restore the original error code than "fixing" all
the tests / applications that rely on it.

The return code of other subcommands was also changed by this patch, but
so far all the failures I have seen are related to "nexthop" subcommand.

Thanks