RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> Sent: Tuesday, December 9, 2014 5:06 AM
> To: Dexuan Cui
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; driverdev-
> devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; apw@xxxxxxxxxxxxx;
> jasowang@xxxxxxxxxx; KY Srinivasan; Haiyang Zhang
> Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
> 
> Dexuan Cui <decui@xxxxxxxxxxxxx> writes:
> 
> > Currently IPv6-only-injection doesn't work because the daemon doesn't
> > parse any IPv6 information at all once it finds the dhcp_enabled flag is true.
> >
> > But according to the Hyper-v host team, the flag is only for IPv4.
> > In the case the host only injects 1 IPv6 address, the dhcp flag is
> > true, but we shouldn't ignore the IPv6 address and we should pass
> > BOOTPROTO=none to the distro-specific script hv_set_ipconfig.
> >
> > Tested in Ubuntu 14.10 and RHEL7.
> >
> > Cc: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> > Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> > ---
> >  tools/hv/hv_kvp_daemon.c | 47
> > +++++++++++++++++++++++++++++++----------------
> >  1 file changed, 31 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index
> > 6a6432a..6ef6c04 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -1145,6 +1145,9 @@ static int kvp_write_file(FILE *f, char *s1,
> > char *s2, char *s3)  }
> >
> > +/* How many ipv6 addresses the host is trying to inject? */ static
> > +int num_ipv6_injected;
> > +
> >  static int process_ip_string(FILE *f, char *ip_string, int type)  {
> >  	int error = 0;
> > @@ -1190,6 +1193,7 @@ static int process_ip_string(FILE *f, char
> *ip_string, int type)
> >  			switch (type) {
> >  			case IPADDR:
> >  				snprintf(str, sizeof(str), "%s", "IPV6ADDR");
> > +				num_ipv6_injected++;
> >  				break;
> >  			case NETMASK:
> >  				snprintf(str, sizeof(str), "%s",
> "IPV6NETMASK"); @@ -1308,27
> > +1312,12 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
> >  	if (error)
> >  		goto setval_error;
> >
> > -	if (new_val->dhcp_enabled) {
> > -		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
> > -		if (error)
> > -			goto setval_error;
> > -
> > -		/*
> > -		 * We are done!.
> > -		 */
> > -		goto setval_done;
> > -
> > -	} else {
> > -		error = kvp_write_file(file, "BOOTPROTO", "", "none");
> > -		if (error)
> > -			goto setval_error;
> > -	}
> > -
> >  	/*
> >  	 * Write the configuration for ipaddress, netmask, gateway and
> >  	 * name servers.
> >  	 */
> >
> > +	num_ipv6_injected = 0;
> >  	error = process_ip_string(file, (char *)new_val->ip_addr, IPADDR);
> >  	if (error)
> >  		goto setval_error;
> > @@ -1345,6 +1334,32 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
> >  	if (error)
> >  		goto setval_error;
> >
> > +	/*
> > +	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
> team.
> > +	 *
> > +	 * In the case the host only injects 1 IPv6 address:
> > +	 * new_val->dhcp_enabled is true, but we can't pass
> BOOTPROTO=dhcp to
> > +	 * the script hv_set_ifconfig, because in some distros (like RHEL7)
> > +	 * BOOTPROTO=dhcp has a special meaning in the config file (e.g.,
> > +	 * /etc/sysconfig/network-scripts/ifcfg-eth0): the network init
> program
> > +	 * ignores any static IP addr information once there is
> > +	 * BOOTPROTO=dhcp; as a result, IPv6-only injection can't work.
> > +	 *
> > +	 * In the case of IPv6-only injection, BOOTPROTO=dhcp doesn't
> affect
> > +	 * Ubuntu because it's ignored by the Ubuntu version of
> > +	 * hv_set_ifconfig and it doesn't seem to have special meaning in
> > +	 * Ubuntu.
> > +	 */
> 
> I just checked and adding "IPV6ADDR=something" when
> "BOOTPROTO=dhcp"
> works for me with both RHEL7 and Fedora21.
> 
> Other than that I think bringing distribution specifics into kernel.git is not a
> good idea. /etc/sysconfig/network-scripts/ifcfg-* format is distro-specific
> and not all Linux distros support it. Moreover, different distros can treat
> setting differently. I think it was wrong to stick to this format in kvp daemon
> from very beginning.
> 
> As a solution I would suggest doing the following: kvp daemon writes all
> received request details in distro-agnostic format in some temporary place
> and then calls distro-specific script to set things up. Actually, we already have
> such script: tools/hv/hv_set_ifconfig.sh
> 
> As for this bug I propose the following: remove skipping all IPADDR/MASK/...
> settings in case of "BOOTPROTO=dhcp" and let distro-specific script deal with
> the rest.

Vitaly, I agree that we should not have Distro specific code in the upstream kernel.
The idea was to create a file in the guest that reflected the data we got from the host
and have Distro specific scripts process this file. The current scripts under /hv/tools were
meant to be just example scripts that Distro vendors could use as a starting point.
I just chose to use RHEL for writing these example scripts.

K. Y
> 
> 
> > +	if (new_val->dhcp_enabled && num_ipv6_injected == 0) {
> > +		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
> > +		if (error)
> > +			goto setval_error;
> > +	} else {
> > +		error = kvp_write_file(file, "BOOTPROTO", "", "none");
> > +		if (error)
> > +			goto setval_error;
> > +	}
> > +
> >  setval_done:
> >  	fclose(file);
> 
> --
>   Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/