2014-12-09 10:51:29

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH] 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(-)

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


2014-12-09 13:06:06

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work

Dexuan Cui <[email protected]> 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 <[email protected]>
> Signed-off-by: Dexuan Cui <[email protected]>
> ---
> 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

2014-12-10 07:34:59

by Dexuan Cui

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

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

2014-12-10 07:37:20

by Dexuan Cui

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



Thanks,
-- Dexuan

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

2014-12-10 09:07:59

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work

Dexuan Cui <[email protected]> writes:

> Thanks,
> -- Dexuan
>
>> -----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
>>
>> > -----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
>> > ......
>> > > + * 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

2014-12-10 10:03:24

by Dexuan Cui

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

> -----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
>
> Dexuan Cui <[email protected]> writes:
>
> > Thanks,
> > -- Dexuan
> >
> >> -----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
> >>
> >> > -----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
> >> > ......
> >> > > + * 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

2014-12-10 10:49:42

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH] tools: hv: kvp_daemon: make IPv6-only-injection work

Dexuan Cui <[email protected]> writes:

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

2014-12-10 18:42:09

by KY Srinivasan

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



> -----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
>
> Dexuan Cui <[email protected]> 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 <[email protected]>
> > Signed-off-by: Dexuan Cui <[email protected]>
> > ---
> > 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