tools: hv: kvp_daemon: make IPv6-only-injection work

PATCH Superseded Dexuan Cui 3 participants 8 messages Dec 9, 2014 – Dec 10, 2014 Hyper-V/Azure core and drivers

Share & tools

Dexuan Cui

author maintainer

2014-12-09 10:51 UTC #0 lore raw

PATCH 1/1 tools: hv: kvp_daemon: make IPv6-only-injection work

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 <[email protected]>

Signed-off-by Dexuan Cui <[email protected]>

---
 tools/hv/hv_kvp_daemon.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

tools/hv/hv_kvp_daemon.c +31 ~~-16~~

index 6a6432a..6ef6c04 100644
@@ -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);

Signature · 1 line

1.9.1

Vitaly Kuznetsov

2014-12-09 13:06 UTC #1 lore raw

Quoted 76 lines · Dexuan Cui

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.

Tested in Ubuntu 14.10 and RHEL7.

Cc K. Y. Srinivasan <[email protected]>

Signed-off-by Dexuan Cui <[email protected]>

---
 tools/hv/hv_kvp_daemon.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

tools/hv/hv_kvp_daemon.c +11 ~~-16~~

index 6a6432a..6ef6c04 100644
@@ -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)

tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) · line 1352

Quoted 10 lines

tools/hv/hv_kvp_daemon.c +10 -0

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

Quoted 12 lines

tools/hv/hv_kvp_daemon.c +10 -0

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

Signature · 1 line

  Vitaly

Dexuan Cui

author maintainer

2014-12-10 07:34 UTC #2 lore raw

Quoted 19 lines

-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, December 9, 2014 21:06 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
......
Review
Quoted 1 line
(quoted diff) +1 -0
+	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
team.
Review
Quoted 3 lines
(quoted diff) +3 -0
+	 *
+	 * In the case the host only injects 1 IPv6 address:
+	 * new_val->dhcp_enabled is true, but we can't pass
BOOTPROTO=dhcp to
Review
Quoted 3 lines
(quoted diff) +3 -0
+	 * 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

Quoted 11 lines

Review

Quoted 8 lines

(quoted diff) +8 -0

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

Quoted 3 lines

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.

Quoted 2 lines

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 :-)

Quoted 5 lines

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.

Quoted 5 lines

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:

tools/hv/hv_kvp_daemon.c +1 -3

index 22b0764..53fdaad 100644
@@ -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 Kuznetsov

2014-12-10 10:49 UTC #3 lore raw

Quoted 42 lines · Dexuan Cui

Quoted 19 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, December 9, 2014 21:06 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
......
Review
Quoted 1 line
(quoted diff) +1 -0
+	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
team.
Review
Quoted 3 lines
(quoted diff) +3 -0
+	 *
+	 * In the case the host only injects 1 IPv6 address:
+	 * new_val->dhcp_enabled is true, but we can't pass
BOOTPROTO=dhcp to
Review
Quoted 3 lines
(quoted diff) +3 -0
+	 * 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
Quoted 11 lines
Review
Quoted 8 lines
(quoted diff) +8 -0
+	 * 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.

Quoted 1 line

I'm trying to find out the cause.

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

Quoted 11 lines

Quoted 4 lines
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.
Quoted 2 lines
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.

Quoted 36 lines

Quoted 6 lines
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.
Quoted 5 lines
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:
tools/hv/hv_kvp_daemon.c +1 -3
index 22b0764..53fdaad 100644
@@ -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

Signature · 1 line

  Vitaly

Dexuan Cui

author maintainer

2014-12-10 07:37 UTC #4 lore raw

In reply to Vitaly Kuznetsov

Thanks,
-- Dexuan

Quoted 38 lines

-----Original Message-----
From: Dexuan Cui
Sent: Wednesday, December 10, 2014 15:34 PM
To: 'Vitaly Kuznetsov'
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
Quoted 19 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, December 9, 2014 21:06 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
......
Review
Quoted 1 line
(quoted diff) +1 -0
+	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
team.
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 *
+	 * In the case the host only injects 1 IPv6 address:
+	 * new_val->dhcp_enabled is true, but we can't pass
BOOTPROTO=dhcp to
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 * 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
Quoted 10 lines
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) · line 1352
Quoted 8 lines
tools/hv/hv_kvp_daemon.c +8 -0
+	 * 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

Quoted 8 lines

"BOOTPROTO=dhcp"
Quoted 1 line
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

Vitaly Kuznetsov

2014-12-10 09:07 UTC #5 lore raw

Quoted 43 lines · Dexuan Cui

Thanks,
-- Dexuan
Quoted 40 lines
-----Original Message-----
From: Dexuan Cui
Sent: Wednesday, December 10, 2014 15:34 PM
To: 'Vitaly Kuznetsov'
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
Quoted 19 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, December 9, 2014 21:06 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
......
Review
Quoted 1 line
(quoted diff) +1 -0
+	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
team.
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 *
+	 * In the case the host only injects 1 IPv6 address:
+	 * new_val->dhcp_enabled is true, but we can't pass
BOOTPROTO=dhcp to
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 * 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
Quoted 10 lines
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) · line 1352
Quoted 8 lines
tools/hv/hv_kvp_daemon.c +8 -0
+	 * 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"
Quoted 1 line
works for me with both RHEL7 and Fedora21.

Quoted 9 lines

Quoted 6 lines
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.

Quoted 2 lines

Thanks,
-- Dexuan

Signature · 1 line

  Vitaly

Dexuan Cui

author maintainer

2014-12-10 10:03 UTC #6 lore raw

Quoted 68 lines

-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Wednesday, December 10, 2014 17:08 PM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
Quoted 11 lines · Dexuan Cui
Thanks,
-- Dexuan
Quoted 8 lines
-----Original Message-----
From: Dexuan Cui
Sent: Wednesday, December 10, 2014 15:34 PM
To: 'Vitaly Kuznetsov'
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: RE: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection
work
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, December 9, 2014 21:06 PM
To: Dexuan Cui
Cc: [email protected]; [email protected];
driverdev-
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
[email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection
work
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
......
Review
Quoted 1 line
(quoted diff) +1 -0
+	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
team.
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 *
+	 * In the case the host only injects 1 IPv6 address:
+	 * new_val->dhcp_enabled is true, but we can't pass
BOOTPROTO=dhcp to
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 * 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
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) · line 1352
Quoted 8 lines
tools/hv/hv_kvp_daemon.c +8 -0
+	 * 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
Quoted 6 lines
Quoted 6 lines
"BOOTPROTO=dhcp"
Quoted 1 line
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
Quoted 5 lines
Quoted 2 lines
/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.

Quoted 7 lines

s,chown,chmod, :-) But it won't help in case of SELinux mislabeling.
Quoted 2 lines
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

KY Srinivasan

2014-12-10 18:42 UTC #7 lore raw

In reply to Vitaly Kuznetsov

Quoted 119 lines

-----Original Message-----
From: Vitaly Kuznetsov [mailto:[email protected]]
Sent: Tuesday, December 9, 2014 5:06 AM
To: Dexuan Cui
Cc: [email protected]; [email protected]; driverdev- [email protected]; [email protected]; [email protected]; [email protected]; KY Srinivasan; Haiyang Zhang
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work
tools/hv/hv_kvp_daemon.c
Quoted 18 lines · Dexuan Cui
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 <[email protected]>
Signed-off-by Dexuan Cui <[email protected]>
---
 tools/hv/hv_kvp_daemon.c | 47
+++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)
tools/hv/hv_kvp_daemon.c +0 -0
index
tools/hv/hv_kvp_daemon.c static int process_ip_string(FILE *f, char
Quoted 12 lines
6a6432a..6ef6c04 100644
tools/hv/hv_kvp_daemon.c +3 -0
@@ -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)
tools/hv/hv_kvp_daemon.c static int process_ip_string(FILE *f, char *ip_string, int type)
Quoted 7 lines
tools/hv/hv_kvp_daemon.c +1 -0
 			switch (type) {
 			case IPADDR:
 				snprintf(str, sizeof(str), "%s", "IPV6ADDR");
+				num_ipv6_injected++;
 				break;
 			case NETMASK:
 				snprintf(str, sizeof(str), "%s",
"IPV6NETMASK"); @@ -1308,27
Quoted 1 line
+1312,12 @@ static int kvp_set_ip_info(char *if_name, struct
hv_kvp_ipaddr_value *new_val)
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct
Quoted 29 lines
tools/hv/hv_kvp_daemon.c +1 ~~-16~~
 	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)
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 5 lines
tools/hv/hv_kvp_daemon.c +2 -0
 	if (error)
 		goto setval_error;
 
+	/*
+	 * Here "dhcp_enabled" is only for IPv4 according to Hyper-V host
team.
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 *
+	 * In the case the host only injects 1 IPv6 address:
+	 * new_val->dhcp_enabled is true, but we can't pass
BOOTPROTO=dhcp to
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 3 lines
tools/hv/hv_kvp_daemon.c +3 -0
+	 * 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
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
Quoted 4 lines
tools/hv/hv_kvp_daemon.c +4 -0
+	 * 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
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) · line 1352
Quoted 4 lines
tools/hv/hv_kvp_daemon.c +4 -0
+	 * 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.

Quoted 8 lines

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

Quoted 17 lines

Quoted 4 lines
tools/hv/hv_kvp_daemon.c +2 -0
+	if (new_val->dhcp_enabled && num_ipv6_injected == 0) {
+		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
tools/hv/hv_kvp_daemon.c static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) · line 1364
Quoted 10 lines
tools/hv/hv_kvp_daemon.c +8 -0
+		if (error)
+			goto setval_error;
+	} else {
+		error = kvp_write_file(file, "BOOTPROTO", "", "none");
+		if (error)
+			goto setval_error;
+	}
+
 setval_done:
 	fclose(file);
--
Vitaly