Re: [PATCH v1] tools/hv: update route parsing in kvp daemon

[Wei Liu <wei.liu@kernel.org>]

On Mon, Dec 02, 2024 at 11:19:55AM +0100, Olaf Hering wrote:
> After recent changes in the VM network stack, the host fails to
> display the IP addresses of the VM. As a result the "IP Addresses"
> column in the "Networking" tab in the Windows Hyper-V Manager is
> empty. This is caused by a change in the expected output of the
> "ip route show" command. Previously the gateway address was shown
> in the third row. Now the gateway addresses might be split into
> several lines of output. As a result, the string "ra" instead of
> an IP address is sent to the host.
> 
> To me more specific, a VM with the wellknown wicked network

me -> be.

Heh, it took me a while to realize that "wicked" is the name of a
network manager. :-)

> managing tool still shows the expected output in recent openSUSE
> Tumbleweed snapshots:
> 
> ip a show dev uplink;ip -4 route show;ip -6 route show
> 2: uplink: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state ...
>     link/ether 00:15:5d:d0:93:08 brd ff:ff:ff:ff:ff:ff
>     inet 1.2.3.4/22 brd 1.2.3.255 scope global uplink
>        valid_lft forever preferred_lft forever
>     inet6 fe80::215:5dff:fed0:9308/64 scope link proto kernel_ll
>        valid_lft forever preferred_lft forever
> default via 1.2.3.254 dev uplink proto dhcp
> 1.2.3.0/22 dev uplink proto kernel scope link src 1.2.3.4
> fe80::/64 dev uplink proto kernel metric 256 pref medium
> default via fe80::26fc:4e00:3b:74 dev uplink proto ra metric 1024 exp...
> default via fe80::6a22:8e00:fb:14f8 dev uplink proto ra metric 1024 e...
> 
> A similar VM, but with NetworkManager as network managing tool:
> 
> ip a show dev eth0;ip -4 route show;ip -6 route show
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP...
>     link/ether 00:15:5d:d0:93:0b brd ff:ff:ff:ff:ff:ff
>     inet 1.2.3.8/22 brd 1.2.3.255 scope global dynamic noprefixroute ...
>        valid_lft 1022sec preferred_lft 1022sec
>     inet6 fe80::215:5dff:fed0:930b/64 scope link noprefixroute
>        valid_lft forever preferred_lft forever
> default via 1.2.3.254 dev eth0 proto dhcp src 1.2.3.8 metric 100
> 1.2.3.0/22 dev eth0 proto kernel scope link src 1.2.3.8 metric 100
> fe80::/64 dev eth0 proto kernel metric 1024 pref medium
> default proto ra metric 20100 pref medium
>         nexthop via fe80::6a22:8e00:fb:14f8 dev eth0 weight 1
>         nexthop via fe80::26fc:4e00:3b:74 dev eth0 weight 1
> 
> Adjust the route parsing to use a single line for each line of
> output. Also use a single shell invocation to retrieve both IPv4
> and IPv6 information. The actual IP addresses are expected after
> the "via" keyword.
> 

Shradha, can you help review and test this patch? You changed the code in this
file recently.

Keep in mind that we want this tool to be useable for different network
managers.

Thanks,
Wei.

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/hv/hv_kvp_daemon.c | 108 ++++++++++++++++++++++++++++++---------
>  1 file changed, 84 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index ae57bf69ad4a..63b44b191320 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -24,6 +24,7 @@
>  
>  #include <sys/poll.h>
>  #include <sys/utsname.h>
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> @@ -677,6 +678,88 @@ static void kvp_process_ipconfig_file(char *cmd,
>  	pclose(file);
>  }
>  
> +static bool kvp_verify_ip_address(const void *address_string)
> +{
> +	char verify_buf[sizeof(struct in6_addr)];
> +
> +	if (inet_pton(AF_INET, address_string, verify_buf) == 1)
> +		return true;
> +	if (inet_pton(AF_INET6, address_string, verify_buf) == 1)
> +		return true;
> +	return false;
> +}
> +
> +static void kvp_extract_routes(const char *line, void **output, size_t *remaining)
> +{
> +	static const char needle[] = "via ";
> +	const char *match, *haystack = line;
> +
> +	while ((match = strstr(haystack, needle))) {
> +		const char *address, *next_char;
> +
> +		/* Address starts after needle. */
> +		address = match + strlen(needle);
> +
> +		/* The char following address is a space or end of line. */
> +		next_char = strpbrk(address, " \t\\");
> +		if (!next_char)
> +			next_char = address + strlen(address) + 1;
> +
> +		/* Enough room for address and semicolon. */
> +		if (*remaining >= (next_char - address) + 1) {
> +			memcpy(*output, address, next_char - address);
> +			/* Terminate string for verification. */
> +			memcpy(*output + (next_char - address), "", 1);
> +			if (kvp_verify_ip_address(*output)) {
> +				/* Advance output buffer. */
> +				*output += next_char - address;
> +				*remaining -= next_char - address;
> +
> +				/* Each address needs a trailing semicolon. */
> +				memcpy(*output, ";", 1);
> +				*output += 1;
> +				*remaining -= 1;
> +			}
> +		}
> +		haystack = next_char;
> +	}
> +}
> +
> +static void kvp_get_gateway(void *buffer, size_t buffer_len)
> +{
> +	static const char needle[] = "default ";
> +	FILE *f;
> +	void *output = buffer;
> +	char *line = NULL;
> +	size_t alloc_size = 0, remaining = buffer_len - 1;
> +	ssize_t num_chars;
> +
> +	/* Show route information in a single line, for each address family */
> +	f = popen("ip --oneline -4 route show;ip --oneline -6 route show", "r");
> +	if (!f) {
> +		/* Convert buffer into C-String. */
> +		memcpy(output, "", 1);
> +		return;
> +	}
> +	while ((num_chars = getline(&line, &alloc_size, f)) > 0) {
> +		/* Skip short lines. */
> +		if (num_chars <= strlen(needle))
> +			continue;
> +		/* Skip lines without default route. */
> +		if (memcmp(line, needle, strlen(needle)))
> +			continue;
> +		/* Remove trailing newline to simplify further parsing. */
> +		if (line[num_chars - 1] == '\n')
> +			line[num_chars - 1] = '\0';
> +		/* Search routes after match. */
> +		kvp_extract_routes(line + strlen(needle), &output, &remaining);
> +	}
> +	/* Convert buffer into C-String. */
> +	memcpy(output, "", 1);
> +	free(line);
> +	pclose(f);
> +}
> +
>  static void kvp_get_ipconfig_info(char *if_name,
>  				 struct hv_kvp_ipaddr_value *buffer)
>  {
> @@ -685,30 +768,7 @@ static void kvp_get_ipconfig_info(char *if_name,
>  	char *p;
>  	FILE *file;
>  
> -	/*
> -	 * Get the address of default gateway (ipv4).
> -	 */
> -	sprintf(cmd, "%s %s", "ip route show dev", if_name);
> -	strcat(cmd, " | awk '/default/ {print $3 }'");
> -
> -	/*
> -	 * Execute the command to gather gateway info.
> -	 */
> -	kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
> -				(MAX_GATEWAY_SIZE * 2), INET_ADDRSTRLEN, 0);
> -
> -	/*
> -	 * Get the address of default gateway (ipv6).
> -	 */
> -	sprintf(cmd, "%s %s", "ip -f inet6  route show dev", if_name);
> -	strcat(cmd, " | awk '/default/ {print $3 }'");
> -
> -	/*
> -	 * Execute the command to gather gateway info (ipv6).
> -	 */
> -	kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
> -				(MAX_GATEWAY_SIZE * 2), INET6_ADDRSTRLEN, 1);
> -
> +	kvp_get_gateway(buffer->gate_way, sizeof(buffer->gate_way));
>  
>  	/*
>  	 * Gather the DNS state.
>