netdev - Re: [PATCH iproute2-next, v4 2/2] iproute2: add 'ip monitor maddr' support

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] [day] [month] [year] [list]
Message-ID: <dfbea06a-16ee-4d53-8d45-7a7c7d0a32ef@6wind.com>
Date: Fri, 6 Dec 2024 16:49:56 +0100
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Yuyang Huang <yuyanghuang@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 David Ahern <dsahern@...nel.org>, roopa@...ulusnetworks.com,
 jiri@...nulli.us, stephen@...workplumber.org, jimictw@...gle.com,
 prohr@...gle.com, liuhangbin@...il.com, andrew@...n.ch,
 netdev@...r.kernel.org, Maciej Żenczykowski
 <maze@...gle.com>, Lorenzo Colitti <lorenzo@...gle.com>
Subject: Re: [PATCH iproute2-next, v4 2/2] iproute2: add 'ip monitor maddr'
 support

Le 06/12/2024 à 13:45, Yuyang Huang a écrit :
> Enhanced the 'ip monitor' command to track changes in IPv4 and IPv6
> multicast addresses. This update allows the command to listen for
> events related to multicast address additions and deletions by
> registering to the newly introduced RTNLGRP_IPV4_MCADDR and
> RTNLGRP_IPV6_MCADDR netlink groups.
> 
> This patch depends on the kernel patch that adds RTNLGRP_IPV4_MCADDR
> and RTNLGRP_IPV6_MCADDR being merged first.
> 
> Here is an example usage:
> 
> root@...-x86-64:/# ip monitor maddr
> 8: nettest123    inet6 mcast ff01::1 scope global
> 8: nettest123    inet6 mcast ff02::1 scope global
> 8: nettest123    inet mcast 224.0.0.1 scope global
> 8: nettest123    inet6 mcast ff02::1:ff00:7b01 scope global
> Deleted 8: nettest123    inet mcast 224.0.0.1 scope global
> Deleted 8: nettest123    inet6 mcast ff02::1:ff00:7b01 scope global
> Deleted 8: nettest123    inet6 mcast ff02::1 scope global
> 
> Cc: Maciej Żenczykowski <maze@...gle.com>
> Cc: Lorenzo Colitti <lorenzo@...gle.com>
> Signed-off-by: Yuyang Huang <yuyanghuang@...gle.com>
> ---
> 
> Changelog since v3:
> - Update man/man8/ip-monitor.8 page.
> - Use 'ip monitor maddr' for naming consistency with 'ip maddr' command.
> 
> Changelog since v1:
> - Move the UAPI constants to a separate patch.
> - Update the commit message.
> - Fix the indentation format.
> 
>  ip/ipaddress.c        | 17 +++++++++++++++--
>  ip/ipmonitor.c        | 25 ++++++++++++++++++++++++-
>  man/man8/ip-monitor.8 |  2 +-
>  3 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index d90ba94d..373f613f 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -1504,7 +1504,10 @@ int print_addrinfo(struct nlmsghdr *n, void *arg)
>  
>  	SPRINT_BUF(b1);
>  
> -	if (n->nlmsg_type != RTM_NEWADDR && n->nlmsg_type != RTM_DELADDR)
> +	if (n->nlmsg_type != RTM_NEWADDR
> +	    && n->nlmsg_type != RTM_DELADDR
> +	    && n->nlmsg_type != RTM_NEWMULTICAST
> +	    && n->nlmsg_type != RTM_DELMULTICAST)
We usually put the boolean operator at the end of the line.

>  		return 0;
>  	len -= NLMSG_LENGTH(sizeof(*ifa));
>  	if (len < 0) {
> @@ -1564,7 +1567,7 @@ int print_addrinfo(struct nlmsghdr *n, void *arg)
>  
>  	print_headers(fp, "[ADDR]");
>  
> -	if (n->nlmsg_type == RTM_DELADDR)
> +	if (n->nlmsg_type == RTM_DELADDR || n->nlmsg_type == RTM_DELMULTICAST)
>  		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
>  
>  	if (!brief) {
> @@ -1639,6 +1642,16 @@ int print_addrinfo(struct nlmsghdr *n, void *arg)
>  						   rta_tb[IFA_ANYCAST]));
>  	}
>  
> +	if (rta_tb[IFA_MULTICAST]) {
> +		print_string(PRINT_FP, NULL, "%s ", "mcast");
> +		print_color_string(PRINT_ANY,
> +				   ifa_family_color(ifa->ifa_family),
> +				   "multicast",
> +				   "%s ",
> +				   format_host_rta(ifa->ifa_family,
> +						   rta_tb[IFA_MULTICAST]));
> +	}
> +
>  	print_string(PRINT_ANY,
>  		     "scope",
>  		     "scope %s ",
> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
> index de67f2c9..beefba4a 100644
> --- a/ip/ipmonitor.c
> +++ b/ip/ipmonitor.c
> @@ -30,7 +30,7 @@ static void usage(void)
>  	fprintf(stderr,
>  		"Usage: ip monitor [ all | OBJECTS ] [ FILE ] [ label ] [ all-nsid ]\n"
>  		"                  [ dev DEVICE ]\n"
> -		"OBJECTS :=  address | link | mroute | neigh | netconf |\n"
> +		"OBJECTS :=  address | link | mroute | maddr | neigh | netconf |\n"
I was probably not clear. Please use 'maddress', like the existing one.

>  		"            nexthop | nsid | prefix | route | rule | stats\n"
>  		"FILE := file FILENAME\n");
>  	exit(-1);
> @@ -152,6 +152,11 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
>  		ipstats_print(n, arg);
>  		return 0;
>  
> +	case RTM_DELMULTICAST:
> +	case RTM_NEWMULTICAST:
> +		print_addrinfo(n, arg);
> +		return 0;
> +
>  	case NLMSG_ERROR:
>  	case NLMSG_NOOP:
>  	case NLMSG_DONE:
> @@ -178,6 +183,7 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
>  #define IPMON_LRULE		BIT(8)
>  #define IPMON_LNSID		BIT(9)
>  #define IPMON_LNEXTHOP		BIT(10)
> +#define IPMON_LMADDR		BIT(11)
>  
>  #define IPMON_L_ALL		(~0)
>  
> @@ -220,6 +226,8 @@ int do_ipmonitor(int argc, char **argv)
>  			lmask |= IPMON_LNEXTHOP;
>  		} else if (strcmp(*argv, "stats") == 0) {
>  			lmask |= IPMON_LSTATS;
> +		} else if (strcmp(*argv, "maddr") == 0) {
> +			lmask |= IPMON_LMADDR;
And here, youc an use 'matches(*argv, "maddress")' so that it will be consistent
with
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/ip.c?h=v6.12.0#n92

Note that it should be put after the '} else if (matches(*argv, "address") == 0)
{' block also.


Regards,
Nicolas