netdev - Re: [PATCH iproute2-next 1/1] ip: add NLM_F

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fba2ec25-d15c-8c1a-32f8-d20d81c1f1cb@kernel.org>
Date:   Thu, 22 Sep 2022 16:17:08 -0700
From:   David Ahern <dsahern@...nel.org>
To:     Hangbin Liu <liuhangbin@...il.com>, netdev@...r.kernel.org
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        Guillaume Nault <gnault@...hat.com>
Subject: Re: [PATCH iproute2-next 1/1] ip: add NLM_F_ECHO support

On 9/15/22 9:34 PM, Hangbin Liu wrote:
> When user space configures the kernel with netlink messages, it can set the
> NLM_F_ECHO flag to request the kernel to send the applied configuration back
> to the caller. This allows user space to retrieve configuration information
> that are filled by the kernel (either because these parameters can only be
> set by the kernel or because user space let the kernel choose a default
> value).
> 
> NLM_F_ACK is also supplied incase the kernel doesn't support NLM_F_ECHO
> and we will wait for the reply forever. Just like the update in
> iplink.c, which I plan to post a patch to kernel later.
> 
> A new parameter -echo is added when user want to get feedback from kernel.
> e.g.
> 
>   # ip -echo addr add 192.168.0.1/24 dev eth1
>   3: eth1    inet 192.168.0.1/24 scope global eth1
>          valid_lft forever preferred_lft forever
>   # ip -j -p -echo addr del 192.168.0.1/24 dev eth1
>   [ {
>           "deleted": true,
>           "index": 3,
>           "dev": "eth1",
>           "family": "inet",
>           "local": "192.168.0.1",
>           "prefixlen": 24,
>           "scope": "global",
>           "label": "eth1",
>           "valid_life_time": 4294967295,
>           "preferred_life_time": 4294967295
>       } ]
> 
> Suggested-by: Guillaume Nault <gnault@...hat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> ---
>  include/utils.h |  1 +
>  ip/ip.c         |  3 +++
>  ip/ipaddress.c  | 23 +++++++++++++++++++++--
>  ip/iplink.c     | 20 +++++++++++++++++++-
>  ip/ipnexthop.c  | 21 ++++++++++++++++++++-
>  ip/iproute.c    | 21 ++++++++++++++++++++-
>  ip/iprule.c     | 21 ++++++++++++++++++++-
>  man/man8/ip.8   |  4 ++++
>  8 files changed, 108 insertions(+), 6 deletions(-)
> 

> @@ -2416,6 +2416,11 @@ 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;
> +	struct nlmsghdr *answer;
> +	int ret;
> +
> +	if (echo_request)
> +		req.n.nlmsg_flags |= NLM_F_ECHO|NLM_F_ACK;

fixed the spacing on the flags (all locations) and applied to iproute2-next.


>  
>  	while (argc > 0) {
>  		if (strcmp(*argv, "peer") == 0 ||
> @@ -2597,9 +2602,23 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
>  		return -1;
>  	}
>  
> -	if (rtnl_talk(&rth, &req.n, NULL) < 0)
> +	if (echo_request)
> +		ret = rtnl_talk(&rth, &req.n, &answer);
> +	else
> +		ret = rtnl_talk(&rth, &req.n, NULL);
> +
> +	if (ret < 0)
>  		return -2;
>  
> +	if (echo_request) {
> +		new_json_obj(json);
> +		open_json_object(NULL);
> +		print_addrinfo(answer, stdout);
> +		close_json_object();
> +		delete_json_obj();
> +		free(answer);
> +	}

That list is redundant. I think it can be turned into a util function
that takes an the print function as an input argument.