public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
@ 2014-12-09 12:01 Dexuan Cui
  2014-12-09 13:05 ` Vitaly Kuznetsov
  0 siblings, 1 reply; 8+ messages in thread
From: Dexuan Cui @ 2014-12-09 12:01 UTC (permalink / raw)
  To: gregkh, linux-kernel, driverdev-devel, olaf, apw, jasowang, kys; +Cc: haiyangz

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@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 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.
+	 */
+	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);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-09 12:01 [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work Dexuan Cui
@ 2014-12-09 13:05 ` Vitaly Kuznetsov
  2014-12-10  7:34   ` Dexuan Cui
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2014-12-09 13:05 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: gregkh, linux-kernel, driverdev-devel, olaf, apw, jasowang, kys,
	haiyangz

Dexuan Cui <decui@microsoft.com> 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@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>  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.


> +	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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-09 13:05 ` Vitaly Kuznetsov
@ 2014-12-10  7:34   ` Dexuan Cui
  2014-12-10  9:06     ` Vitaly Kuznetsov
  2014-12-10  7:36   ` Dexuan Cui
  2014-12-10 18:42   ` KY Srinivasan
  2 siblings, 1 reply; 8+ messages in thread
From: Dexuan Cui @ 2014-12-10  7:34 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, KY Srinivasan,
	Haiyang Zhang

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Tuesday, December 9, 2014 21:06 PM
> To: Dexuan Cui
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
>  ......
> > +	 * 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.
It doesn't work in my side. :-(
Running 'ifup eth0' shows some errors(I use "set -x")
...
+ /sbin/dhclient -H localhost -1 -q -lf /var/lib/dhclient/dhclient--eth0.lease -pf /var/run/dhclient-eth0.pid eth0
grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
done.

I'm trying to find out the cause.

> 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,
I agree.

> different distros can treat setting differently. I think it was wrong to
> stick to this format in kvp daemon from very beginning.
We can also think the current format used in kvp daemon is already
distro-agnostic -- it just happens to look like the style of network config file
used in RHEL :-)

> 
> 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
Yeah, this is exactly what we already have today.

> 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
OK, so the patch would be 1-line only:

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 22b0764..53fdaad 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -1314,10 +1314,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
                        goto setval_error;

                /*
-                * We are done!.
+                * We are not done... TODO: add comment here.
                 */
-               goto setval_done;
-
        } else {
                error = kvp_write_file(file, "BOOTPROTO", "", "none");
                if (error)

I'll send out a v2 after I resolve the "grep ... Permission dinied" issue.

Thanks,
-- Dexuan

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-09 13:05 ` Vitaly Kuznetsov
  2014-12-10  7:34   ` Dexuan Cui
@ 2014-12-10  7:36   ` Dexuan Cui
  2014-12-10  9:07     ` Vitaly Kuznetsov
  2014-12-10 18:42   ` KY Srinivasan
  2 siblings, 1 reply; 8+ messages in thread
From: Dexuan Cui @ 2014-12-10  7:36 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, KY Srinivasan,
	Haiyang Zhang



Thanks,
-- Dexuan

> -----Original Message-----
> From: Dexuan Cui
> Sent: Wednesday, December 10, 2014 15:34 PM
> To: 'Vitaly Kuznetsov'
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> Subject: RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
> 
> > -----Original Message-----
> > From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> > Sent: Tuesday, December 9, 2014 21:06 PM
> > To: Dexuan Cui
> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> > jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> > Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
> >  ......
> > > +	 * 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.
> It doesn't work in my side. :-(
> Running 'ifup eth0' shows some errors(I use "set -x")
> ...
> + /sbin/dhclient -H localhost -1 -q -lf /var/lib/dhclient/dhclient--eth0.lease -pf
> /var/run/dhclient-eth0.pid eth0
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
BTW, I run with root, and
'chown 777 /etc/sysconfig/network-scripts/ifcfg-eth0" doesn't help.

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-10  7:34   ` Dexuan Cui
@ 2014-12-10  9:06     ` Vitaly Kuznetsov
  0 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2014-12-10  9:06 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, KY Srinivasan,
	Haiyang Zhang

Dexuan Cui <decui@microsoft.com> writes:

>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Tuesday, December 9, 2014 21:06 PM
>> To: Dexuan Cui
>> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
>> Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
>>  ......
>> > +	 * 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.
> It doesn't work in my side. :-(
> Running 'ifup eth0' shows some errors(I use "set -x")
> ...
> + /sbin/dhclient -H localhost -1 -q -lf /var/lib/dhclient/dhclient--eth0.lease -pf /var/run/dhclient-eth0.pid eth0
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> done.
>
> I'm trying to find out the cause.

Selinux? You can try 'setenforce 0' to figure this out.

>
>> 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,
> I agree.
>
>> different distros can treat setting differently. I think it was wrong to
>> stick to this format in kvp daemon from very beginning.
> We can also think the current format used in kvp daemon is already
> distro-agnostic -- it just happens to look like the style of network config file
> used in RHEL :-)

Yes, it is already there and I don't see any point in changing it.

>
>> 
>> 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
> Yeah, this is exactly what we already have today.
>
>> 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
> OK, so the patch would be 1-line only:
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 22b0764..53fdaad 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -1314,10 +1314,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
>                         goto setval_error;
>
>                 /*
> -                * We are done!.
> +                * We are not done... TODO: add comment here.
>                  */
> -               goto setval_done;
> -
>         } else {
>                 error = kvp_write_file(file, "BOOTPROTO", "", "none");
>                 if (error)
>
> I'll send out a v2 after I resolve the "grep ... Permission dinied" issue.
>
> Thanks,
> -- Dexuan

-- 
  Vitaly

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-10  7:36   ` Dexuan Cui
@ 2014-12-10  9:07     ` Vitaly Kuznetsov
  2014-12-10 10:02       ` Dexuan Cui
  0 siblings, 1 reply; 8+ messages in thread
From: Vitaly Kuznetsov @ 2014-12-10  9:07 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, KY Srinivasan,
	Haiyang Zhang

Dexuan Cui <decui@microsoft.com> writes:

> Thanks,
> -- Dexuan
>
>> -----Original Message-----
>> From: Dexuan Cui
>> Sent: Wednesday, December 10, 2014 15:34 PM
>> To: 'Vitaly Kuznetsov'
>> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
>> Subject: RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
>> 
>> > -----Original Message-----
>> > From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> > Sent: Tuesday, December 9, 2014 21:06 PM
>> > To: Dexuan Cui
>> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
>> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> > jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
>> > Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
>> >  ......
>> > > +	 * 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.
>> It doesn't work in my side. :-(
>> Running 'ifup eth0' shows some errors(I use "set -x")
>> ...
>> + /sbin/dhclient -H localhost -1 -q -lf /var/lib/dhclient/dhclient--eth0.lease -pf
>> /var/run/dhclient-eth0.pid eth0
>> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> BTW, I run with root, and
> 'chown 777 /etc/sysconfig/network-scripts/ifcfg-eth0" doesn't help.
>

s,chown,chmod, :-) But it won't help in case of SELinux mislabeling.

> Thanks,
> -- Dexuan

-- 
  Vitaly

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-10  9:07     ` Vitaly Kuznetsov
@ 2014-12-10 10:02       ` Dexuan Cui
  0 siblings, 0 replies; 8+ messages in thread
From: Dexuan Cui @ 2014-12-10 10:02 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, KY Srinivasan,
	Haiyang Zhang

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Wednesday, December 10, 2014 17:08 PM
> To: Dexuan Cui
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
> 
> Dexuan Cui <decui@microsoft.com> writes:
> 
> > Thanks,
> > -- Dexuan
> >
> >> -----Original Message-----
> >> From: Dexuan Cui
> >> Sent: Wednesday, December 10, 2014 15:34 PM
> >> To: 'Vitaly Kuznetsov'
> >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> >> Subject: RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection
> work
> >>
> >> > -----Original Message-----
> >> > From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> >> > Sent: Tuesday, December 9, 2014 21:06 PM
> >> > To: Dexuan Cui
> >> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> driverdev-
> >> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> > jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> >> > Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection
> work
> >> >  ......
> >> > > +	 * 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.
> >> It doesn't work in my side. :-(
> >> Running 'ifup eth0' shows some errors(I use "set -x")
> >> ...
> >> + /sbin/dhclient -H localhost -1 -q -lf /var/lib/dhclient/dhclient--eth0.lease
> -pf
> >> /var/run/dhclient-eth0.pid eth0
> >> grep: /etc/sysconfig/network-scripts/ifcfg-eth0: Permission dinied.
> > BTW, I run with root, and
> > 'chown 777 /etc/sysconfig/network-scripts/ifcfg-eth0" doesn't help.
> >
> 
> s,chown,chmod, :-) But it won't help in case of SELinux mislabeling.
> 
> > Thanks,
> > -- Dexuan
> 
> --
>   Vitaly

Hi Vitally,
I think you're correct:
BOOTPROTO=dhcp + "no IPADDR" +  IPV6ADDR in RHEL7's 
/etc/sysconfig/network-scripts/ifcfg-eth0 works for me too.

My previous "grep: ... ifcfg-eth0: Permission denied" issue can't repro now.
Maybe it's because I messed up the config file somehow(?).

I'll send out a v2 patch according to your suggestion.

Thanks,
-- Dexuan 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
  2014-12-09 13:05 ` Vitaly Kuznetsov
  2014-12-10  7:34   ` Dexuan Cui
  2014-12-10  7:36   ` Dexuan Cui
@ 2014-12-10 18:42   ` KY Srinivasan
  2 siblings, 0 replies; 8+ messages in thread
From: KY Srinivasan @ 2014-12-10 18:42 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Dexuan Cui
  Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, Haiyang Zhang



> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Tuesday, December 9, 2014 5:06 AM
> To: Dexuan Cui
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan; Haiyang Zhang
> Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
> 
> Dexuan Cui <decui@microsoft.com> 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@microsoft.com>
> > Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-12-10 18:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-09 12:01 [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work Dexuan Cui
2014-12-09 13:05 ` Vitaly Kuznetsov
2014-12-10  7:34   ` Dexuan Cui
2014-12-10  9:06     ` Vitaly Kuznetsov
2014-12-10  7:36   ` Dexuan Cui
2014-12-10  9:07     ` Vitaly Kuznetsov
2014-12-10 10:02       ` Dexuan Cui
2014-12-10 18:42   ` KY Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox