2024-03-01 12:56:30

by Shradha Gupta

[permalink] [raw]
Subject: [PATCH] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format

If the network configuration strings are passed as a combination of IPv and
IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
configuration format.
With these changes, the keyfile config generation logic scans through the
list twice to generate IPv4 and IPv6 sections for the configuration files
to handle this support.

Built-on: Rhel9
Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
Signed-off-by: Shradha Gupta <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 152 ++++++++++++++++++++++++++++-----------
1 file changed, 112 insertions(+), 40 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 318e2dad27e0..7e84e40b55fb 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -76,6 +76,11 @@ enum {
DNS
};

+enum {
+ IPV4 = 1,
+ IPV6
+};
+
static int in_hand_shake;

static char *os_name = "";
@@ -1171,6 +1176,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
return 0;
}

+int ip_version_check(const char *input_addr)
+{
+ struct in6_addr addr;
+
+ if (inet_pton(AF_INET, input_addr, &addr))
+ return IPV4;
+ else if (inet_pton(AF_INET6, input_addr, &addr))
+ return IPV6;
+ else
+ return -EINVAL;
+}
+
/*
* Only IPv4 subnet strings needs to be converted to plen
* For IPv6 the subnet is already privided in plen format
@@ -1197,14 +1214,56 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
return plen;
}

+static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
+ int ip_sec)
+{
+ char addr[INET6_ADDRSTRLEN], *output_str;
+ int ip_offset = 0, error, ip_ver;
+ char *param_name;
+
+ output_str = malloc(strlen(ip_string));
+
+ if (!output_str)
+ return 1;
+
+ output_str[0] = '\0';
+
+ if (type == DNS)
+ param_name = "dns";
+ else
+ param_name = "gateway";
+
+ while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
+ (MAX_IP_ADDR_SIZE * 2))) {
+ ip_ver = ip_version_check(addr);
+
+ if ((ip_ver == IPV4 && ip_sec == IPV4) ||
+ (ip_ver == IPV6 && ip_sec == IPV6)) {
+ strcat(output_str, addr);
+ strcat(output_str, ",");
+ } else {
+ continue;
+ }
+ }
+
+ if (strlen(output_str)) {
+ output_str[strlen(output_str) - 1] = '\0';
+ error = fprintf(f, "%s=%s\n", param_name, output_str);
+ if (error < 0)
+ return error;
+ }
+
+ return 0;
+}
+
static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
- int is_ipv6)
+ int ip_sec)
{
char addr[INET6_ADDRSTRLEN];
char subnet_addr[INET6_ADDRSTRLEN];
int error, i = 0;
int ip_offset = 0, subnet_offset = 0;
- int plen;
+ int plen, ip_ver;

memset(addr, 0, sizeof(addr));
memset(subnet_addr, 0, sizeof(subnet_addr));
@@ -1216,10 +1275,13 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
subnet_addr,
(MAX_IP_ADDR_SIZE *
2))) {
- if (!is_ipv6)
+ ip_ver = ip_version_check(addr);
+ if (ip_ver == IPV4 && ip_sec == IPV4)
plen = kvp_subnet_to_plen((char *)subnet_addr);
- else
+ else if (ip_ver == IPV6 && ip_sec == IPV6)
plen = atoi(subnet_addr);
+ else
+ continue;

if (plen < 0)
return plen;
@@ -1242,8 +1304,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
char if_filename[PATH_MAX];
char nm_filename[PATH_MAX];
FILE *ifcfg_file, *nmfile;
+ int ip_sections_count;
char cmd[PATH_MAX];
- int is_ipv6 = 0;
char *mac_addr;
int str_len;

@@ -1421,52 +1483,62 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
if (error)
goto setval_error;

- if (new_val->addr_family & ADDR_FAMILY_IPV6) {
- error = fprintf(nmfile, "\n[ipv6]\n");
- if (error < 0)
- goto setval_error;
- is_ipv6 = 1;
- } else {
- error = fprintf(nmfile, "\n[ipv4]\n");
- if (error < 0)
- goto setval_error;
- }
-
/*
- * Now we populate the keyfile format
+ * The keyfile format expects the IPv6 and IPv4 configuration in
+ * different sections. Therefore we iterate through the list twice,
+ * once to populate the IPv4 section and the next time for IPv6
*/
+ ip_sections_count = 1;
+ do {
+ if (ip_sections_count == 1) {
+ error = fprintf(nmfile, "\n[ipv4]\n");
+ if (error < 0)
+ goto setval_error;
+ } else {
+ error = fprintf(nmfile, "\n[ipv6]\n");
+ if (error < 0)
+ goto setval_error;
+ }

- if (new_val->dhcp_enabled) {
- error = kvp_write_file(nmfile, "method", "", "auto");
- if (error < 0)
- goto setval_error;
- } else {
- error = kvp_write_file(nmfile, "method", "", "manual");
+ /*
+ * Now we populate the keyfile format
+ */
+
+ if (new_val->dhcp_enabled) {
+ error = kvp_write_file(nmfile, "method", "", "auto");
+ if (error < 0)
+ goto setval_error;
+ } else {
+ error = kvp_write_file(nmfile, "method", "", "manual");
+ if (error < 0)
+ goto setval_error;
+ }
+
+ /*
+ * Write the configuration for ipaddress, netmask, gateway and
+ * name services
+ */
+ error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
+ (char *)new_val->sub_net,
+ ip_sections_count);
if (error < 0)
goto setval_error;
- }

- /*
- * Write the configuration for ipaddress, netmask, gateway and
- * name services
- */
- error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
- (char *)new_val->sub_net, is_ipv6);
- if (error < 0)
- goto setval_error;
-
- /* we do not want ipv4 addresses in ipv6 section and vice versa */
- if (is_ipv6 != is_ipv4((char *)new_val->gate_way)) {
- error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way);
+ error = process_dns_gateway_nm(nmfile,
+ (char *)new_val->gate_way,
+ GATEWAY, ip_sections_count);
if (error < 0)
goto setval_error;
- }

- if (is_ipv6 != is_ipv4((char *)new_val->dns_addr)) {
- error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr);
+ error = process_dns_gateway_nm(nmfile,
+ (char *)new_val->dns_addr, DNS,
+ ip_sections_count);
if (error < 0)
goto setval_error;
- }
+
+ ip_sections_count++;
+ } while (ip_sections_count <= 2);
+
fclose(nmfile);
fclose(ifcfg_file);

--
2.34.1



2024-03-04 12:21:35

by Ani Sinha

[permalink] [raw]
Subject: Re: [PATCH] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format



> On 01-Mar-2024, at 18:26, Shradha Gupta <[email protected]> wrote:
>
> If the network configuration strings are passed as a combination of IPv and
> IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
> configuration format.
> With these changes, the keyfile config generation logic scans through the
> list twice to generate IPv4 and IPv6 sections for the configuration files
> to handle this support.

We will test this patch internally and I will report back but after the following suggestions

>
> Built-on: Rhel9
> Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
> Signed-off-by: Shradha Gupta <[email protected]>
> ---
> tools/hv/hv_kvp_daemon.c | 152 ++++++++++++++++++++++++++++-----------
> 1 file changed, 112 insertions(+), 40 deletions(-)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 318e2dad27e0..7e84e40b55fb 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -76,6 +76,11 @@ enum {
> DNS
> };
>
> +enum {
> + IPV4 = 1,
> + IPV6,

How about adding IP_TYPE_MAX here? Then see below …

> +};
> +
> static int in_hand_shake;
>
> static char *os_name = "";
> @@ -1171,6 +1176,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
> return 0;
> }
>
> +int ip_version_check(const char *input_addr)
> +{
> + struct in6_addr addr;
> +
> + if (inet_pton(AF_INET, input_addr, &addr))
> + return IPV4;
> + else if (inet_pton(AF_INET6, input_addr, &addr))
> + return IPV6;
> + else
> + return -EINVAL;
> +}
> +
> /*
> * Only IPv4 subnet strings needs to be converted to plen
> * For IPv6 the subnet is already privided in plen format
> @@ -1197,14 +1214,56 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
> return plen;
> }
>
> +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> + int ip_sec)
> +{
> + char addr[INET6_ADDRSTRLEN], *output_str;
> + int ip_offset = 0, error, ip_ver;
> + char *param_name;
> +
> + output_str = malloc(strlen(ip_string));

Could we use calloc() here? Then ...

> +
> + if (!output_str)
> + return 1;
> +
> + output_str[0] = '\0';

We do not need to do this.

> +
> + if (type == DNS)
> + param_name = "dns";
> + else
> + param_name = "gateway";
> +
> + while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> + (MAX_IP_ADDR_SIZE * 2))) {
> + ip_ver = ip_version_check(addr);

Maybe assert here that ip_ver is != -EINVAL.

> +
> + if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> + (ip_ver == IPV6 && ip_sec == IPV6)) {
> + strcat(output_str, addr);
> + strcat(output_str, ",");
> + } else {
> + continue;
> + }
> + }
> +
> + if (strlen(output_str)) {
> + output_str[strlen(output_str) - 1] = '\0';
> + error = fprintf(f, "%s=%s\n", param_name, output_str);
> + if (error < 0)
> + return error;
> + }
> +
> + return 0;
> +}
> +
> static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> - int is_ipv6)
> + int ip_sec)
> {
> char addr[INET6_ADDRSTRLEN];
> char subnet_addr[INET6_ADDRSTRLEN];
> int error, i = 0;
> int ip_offset = 0, subnet_offset = 0;
> - int plen;
> + int plen, ip_ver;
>
> memset(addr, 0, sizeof(addr));
> memset(subnet_addr, 0, sizeof(subnet_addr));
> @@ -1216,10 +1275,13 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> subnet_addr,
> (MAX_IP_ADDR_SIZE *
> 2))) {
> - if (!is_ipv6)
> + ip_ver = ip_version_check(addr);
> + if (ip_ver == IPV4 && ip_sec == IPV4)
> plen = kvp_subnet_to_plen((char *)subnet_addr);
> - else
> + else if (ip_ver == IPV6 && ip_sec == IPV6)
> plen = atoi(subnet_addr);
> + else
> + continue;
>
> if (plen < 0)
> return plen;
> @@ -1242,8 +1304,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> char if_filename[PATH_MAX];
> char nm_filename[PATH_MAX];
> FILE *ifcfg_file, *nmfile;
> + int ip_sections_count;
> char cmd[PATH_MAX];
> - int is_ipv6 = 0;
> char *mac_addr;
> int str_len;
>
> @@ -1421,52 +1483,62 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> if (error)
> goto setval_error;
>
> - if (new_val->addr_family & ADDR_FAMILY_IPV6) {
> - error = fprintf(nmfile, "\n[ipv6]\n");
> - if (error < 0)
> - goto setval_error;
> - is_ipv6 = 1;
> - } else {
> - error = fprintf(nmfile, "\n[ipv4]\n");
> - if (error < 0)
> - goto setval_error;
> - }
> -
> /*
> - * Now we populate the keyfile format
> + * The keyfile format expects the IPv6 and IPv4 configuration in
> + * different sections. Therefore we iterate through the list twice,
> + * once to populate the IPv4 section and the next time for IPv6
> */
> + ip_sections_count = 1;

Ugh, this is ugly! How about,

ip_type = IPV4 ;


> + do {
> + if (ip_sections_count == 1) {
> + error = fprintf(nmfile, "\n[ipv4]\n");
> + if (error < 0)
> + goto setval_error;
> + } else {
> + error = fprintf(nmfile, "\n[ipv6]\n");
> + if (error < 0)
> + goto setval_error;
> + }
>
> - if (new_val->dhcp_enabled) {
> - error = kvp_write_file(nmfile, "method", "", "auto");
> - if (error < 0)
> - goto setval_error;
> - } else {
> - error = kvp_write_file(nmfile, "method", "", "manual");
> + /*
> + * Now we populate the keyfile format
> + */
> +
> + if (new_val->dhcp_enabled) {
> + error = kvp_write_file(nmfile, "method", "", "auto");
> + if (error < 0)
> + goto setval_error;
> + } else {
> + error = kvp_write_file(nmfile, "method", "", "manual");
> + if (error < 0)
> + goto setval_error;
> + }
> +
> + /*
> + * Write the configuration for ipaddress, netmask, gateway and
> + * name services
> + */
> + error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> + (char *)new_val->sub_net,
> + ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
>
> - /*
> - * Write the configuration for ipaddress, netmask, gateway and
> - * name services
> - */
> - error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> - (char *)new_val->sub_net, is_ipv6);
> - if (error < 0)
> - goto setval_error;
> -
> - /* we do not want ipv4 addresses in ipv6 section and vice versa */
> - if (is_ipv6 != is_ipv4((char *)new_val->gate_way)) {
> - error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way);
> + error = process_dns_gateway_nm(nmfile,
> + (char *)new_val->gate_way,
> + GATEWAY, ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
>
> - if (is_ipv6 != is_ipv4((char *)new_val->dns_addr)) {
> - error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr);
> + error = process_dns_gateway_nm(nmfile,
> + (char *)new_val->dns_addr, DNS,
> + ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
> +
> + ip_sections_count++;

ip_type++ ;


> + } while (ip_sections_count < 2);

while (ip_type < IP_TYPE_MAX)

> +
> fclose(nmfile);
> fclose(ifcfg_file);
>
> --
> 2.34.1
>


2024-03-06 08:25:13

by Ani Sinha

[permalink] [raw]
Subject: Re: [PATCH] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format



> On 01-Mar-2024, at 18:26, Shradha Gupta <[email protected]> wrote:
>
> If the network configuration strings are passed as a combination of IPv and
> IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
> configuration format.
> With these changes, the keyfile config generation logic scans through the
> list twice to generate IPv4 and IPv6 sections for the configuration files
> to handle this support.

We tried to test this but seems some memory corruption is happening.
Some more feedbacks follows ...

>
> Built-on: Rhel9
> Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
> Signed-off-by: Shradha Gupta <[email protected]>
> ---
> tools/hv/hv_kvp_daemon.c | 152 ++++++++++++++++++++++++++++-----------
> 1 file changed, 112 insertions(+), 40 deletions(-)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 318e2dad27e0..7e84e40b55fb 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -76,6 +76,11 @@ enum {
> DNS
> };
>
> +enum {
> + IPV4 = 1,
> + IPV6
> +};
> +
> static int in_hand_shake;
>
> static char *os_name = "";
> @@ -1171,6 +1176,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
> return 0;
> }
>
> +int ip_version_check(const char *input_addr)
> +{
> + struct in6_addr addr;
> +
> + if (inet_pton(AF_INET, input_addr, &addr))
> + return IPV4;
> + else if (inet_pton(AF_INET6, input_addr, &addr))
> + return IPV6;
> + else
> + return -EINVAL;
> +}
> +
> /*
> * Only IPv4 subnet strings needs to be converted to plen
> * For IPv6 the subnet is already privided in plen format
> @@ -1197,14 +1214,56 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
> return plen;
> }
>
> +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> + int ip_sec)
> +{
> + char addr[INET6_ADDRSTRLEN], *output_str;
> + int ip_offset = 0, error, ip_ver;
> + char *param_name;
> +
> + output_str = malloc(strlen(ip_string));
> +
> + if (!output_str)
> + return 1;

Shouldn’t you be returning -ENOMEM or some such here?

> +
> + output_str[0] = '\0';
> +
> + if (type == DNS)
> + param_name = "dns";
> + else

Please be more explicit here and check for type == GATEWAY

> + param_name = "gateway";

Else, if neither, it should return error.

> +
> + while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> + (MAX_IP_ADDR_SIZE * 2))) {
> + ip_ver = ip_version_check(addr);
> +
> + if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> + (ip_ver == IPV6 && ip_sec == IPV6)) {
> + strcat(output_str, addr);
> + strcat(output_str, ",");

I prefer strncat() here.

> + } else {
> + continue;
> + }
> + }
> +
> + if (strlen(output_str)) {
> + output_str[strlen(output_str) - 1] = '\0';
> + error = fprintf(f, "%s=%s\n", param_name, output_str);

Please do not forget to free output_str.

> + if (error < 0)
> + return error;
> + }
> +
> + return 0;
> +}
> +
> static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> - int is_ipv6)
> + int ip_sec)
> {
> char addr[INET6_ADDRSTRLEN];
> char subnet_addr[INET6_ADDRSTRLEN];
> int error, i = 0;
> int ip_offset = 0, subnet_offset = 0;
> - int plen;
> + int plen, ip_ver;
>
> memset(addr, 0, sizeof(addr));
> memset(subnet_addr, 0, sizeof(subnet_addr));
> @@ -1216,10 +1275,13 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> subnet_addr,
> (MAX_IP_ADDR_SIZE *
> 2))) {
> - if (!is_ipv6)
> + ip_ver = ip_version_check(addr);
> + if (ip_ver == IPV4 && ip_sec == IPV4)
> plen = kvp_subnet_to_plen((char *)subnet_addr);
> - else
> + else if (ip_ver == IPV6 && ip_sec == IPV6)
> plen = atoi(subnet_addr);
> + else
> + continue;
>
> if (plen < 0)
> return plen;
> @@ -1242,8 +1304,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> char if_filename[PATH_MAX];
> char nm_filename[PATH_MAX];
> FILE *ifcfg_file, *nmfile;
> + int ip_sections_count;
> char cmd[PATH_MAX];
> - int is_ipv6 = 0;
> char *mac_addr;
> int str_len;
>
> @@ -1421,52 +1483,62 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> if (error)
> goto setval_error;
>
> - if (new_val->addr_family & ADDR_FAMILY_IPV6) {
> - error = fprintf(nmfile, "\n[ipv6]\n");
> - if (error < 0)
> - goto setval_error;
> - is_ipv6 = 1;
> - } else {
> - error = fprintf(nmfile, "\n[ipv4]\n");
> - if (error < 0)
> - goto setval_error;
> - }
> -
> /*
> - * Now we populate the keyfile format
> + * The keyfile format expects the IPv6 and IPv4 configuration in
> + * different sections. Therefore we iterate through the list twice,
> + * once to populate the IPv4 section and the next time for IPv6
> */
> + ip_sections_count = 1;
> + do {
> + if (ip_sections_count == 1) {
> + error = fprintf(nmfile, "\n[ipv4]\n");
> + if (error < 0)
> + goto setval_error;
> + } else {
> + error = fprintf(nmfile, "\n[ipv6]\n");
> + if (error < 0)
> + goto setval_error;
> + }
>
> - if (new_val->dhcp_enabled) {
> - error = kvp_write_file(nmfile, "method", "", "auto");
> - if (error < 0)
> - goto setval_error;
> - } else {
> - error = kvp_write_file(nmfile, "method", "", "manual");
> + /*
> + * Now we populate the keyfile format
> + */
> +
> + if (new_val->dhcp_enabled) {
> + error = kvp_write_file(nmfile, "method", "", "auto");
> + if (error < 0)
> + goto setval_error;
> + } else {
> + error = kvp_write_file(nmfile, "method", "", "manual");
> + if (error < 0)
> + goto setval_error;
> + }
> +
> + /*
> + * Write the configuration for ipaddress, netmask, gateway and
> + * name services
> + */
> + error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> + (char *)new_val->sub_net,
> + ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
>
> - /*
> - * Write the configuration for ipaddress, netmask, gateway and
> - * name services
> - */
> - error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> - (char *)new_val->sub_net, is_ipv6);
> - if (error < 0)
> - goto setval_error;
> -
> - /* we do not want ipv4 addresses in ipv6 section and vice versa */
> - if (is_ipv6 != is_ipv4((char *)new_val->gate_way)) {
> - error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way);
> + error = process_dns_gateway_nm(nmfile,
> + (char *)new_val->gate_way,
> + GATEWAY, ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
>
> - if (is_ipv6 != is_ipv4((char *)new_val->dns_addr)) {
> - error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr);
> + error = process_dns_gateway_nm(nmfile,
> + (char *)new_val->dns_addr, DNS,
> + ip_sections_count);
> if (error < 0)

But you return +ve from this function so this will never be true.

> goto setval_error;
> - }
> +
> + ip_sections_count++;
> + } while (ip_sections_count <= 2);
> +
> fclose(nmfile);
> fclose(ifcfg_file);
>
> --
> 2.34.1
>


2024-03-06 09:30:07

by Shradha Gupta

[permalink] [raw]
Subject: Re: [PATCH] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format

Thanks Ani, will send out another version soon with these fixes.

On Wed, Mar 06, 2024 at 01:54:36PM +0530, Ani Sinha wrote:
>
>
> > On 01-Mar-2024, at 18:26, Shradha Gupta <[email protected]> wrote:
> >
> > If the network configuration strings are passed as a combination of IPv and
> > IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
> > configuration format.
> > With these changes, the keyfile config generation logic scans through the
> > list twice to generate IPv4 and IPv6 sections for the configuration files
> > to handle this support.
>
> We tried to test this but seems some memory corruption is happening.
> Some more feedbacks follows ...
>
> >
> > Built-on: Rhel9
> > Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
> > Signed-off-by: Shradha Gupta <[email protected]>
> > ---
> > tools/hv/hv_kvp_daemon.c | 152 ++++++++++++++++++++++++++++-----------
> > 1 file changed, 112 insertions(+), 40 deletions(-)
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index 318e2dad27e0..7e84e40b55fb 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -76,6 +76,11 @@ enum {
> > DNS
> > };
> >
> > +enum {
> > + IPV4 = 1,
> > + IPV6
> > +};
> > +
> > static int in_hand_shake;
> >
> > static char *os_name = "";
> > @@ -1171,6 +1176,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
> > return 0;
> > }
> >
> > +int ip_version_check(const char *input_addr)
> > +{
> > + struct in6_addr addr;
> > +
> > + if (inet_pton(AF_INET, input_addr, &addr))
> > + return IPV4;
> > + else if (inet_pton(AF_INET6, input_addr, &addr))
> > + return IPV6;
> > + else
> > + return -EINVAL;
> > +}
> > +
> > /*
> > * Only IPv4 subnet strings needs to be converted to plen
> > * For IPv6 the subnet is already privided in plen format
> > @@ -1197,14 +1214,56 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
> > return plen;
> > }
> >
> > +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> > + int ip_sec)
> > +{
> > + char addr[INET6_ADDRSTRLEN], *output_str;
> > + int ip_offset = 0, error, ip_ver;
> > + char *param_name;
> > +
> > + output_str = malloc(strlen(ip_string));
> > +
> > + if (!output_str)
> > + return 1;
>
> Shouldn???t you be returning -ENOMEM or some such here?
>
> > +
> > + output_str[0] = '\0';
> > +
> > + if (type == DNS)
> > + param_name = "dns";
> > + else
>
> Please be more explicit here and check for type == GATEWAY
>
> > + param_name = "gateway";
>
> Else, if neither, it should return error.
>
> > +
> > + while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> > + (MAX_IP_ADDR_SIZE * 2))) {
> > + ip_ver = ip_version_check(addr);
> > +
> > + if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> > + (ip_ver == IPV6 && ip_sec == IPV6)) {
> > + strcat(output_str, addr);
> > + strcat(output_str, ",");
>
> I prefer strncat() here.
>
> > + } else {
> > + continue;
> > + }
> > + }
> > +
> > + if (strlen(output_str)) {
> > + output_str[strlen(output_str) - 1] = '\0';
> > + error = fprintf(f, "%s=%s\n", param_name, output_str);
>
> Please do not forget to free output_str.
>
> > + if (error < 0)
> > + return error;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> > - int is_ipv6)
> > + int ip_sec)
> > {
> > char addr[INET6_ADDRSTRLEN];
> > char subnet_addr[INET6_ADDRSTRLEN];
> > int error, i = 0;
> > int ip_offset = 0, subnet_offset = 0;
> > - int plen;
> > + int plen, ip_ver;
> >
> > memset(addr, 0, sizeof(addr));
> > memset(subnet_addr, 0, sizeof(subnet_addr));
> > @@ -1216,10 +1275,13 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> > subnet_addr,
> > (MAX_IP_ADDR_SIZE *
> > 2))) {
> > - if (!is_ipv6)
> > + ip_ver = ip_version_check(addr);
> > + if (ip_ver == IPV4 && ip_sec == IPV4)
> > plen = kvp_subnet_to_plen((char *)subnet_addr);
> > - else
> > + else if (ip_ver == IPV6 && ip_sec == IPV6)
> > plen = atoi(subnet_addr);
> > + else
> > + continue;
> >
> > if (plen < 0)
> > return plen;
> > @@ -1242,8 +1304,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> > char if_filename[PATH_MAX];
> > char nm_filename[PATH_MAX];
> > FILE *ifcfg_file, *nmfile;
> > + int ip_sections_count;
> > char cmd[PATH_MAX];
> > - int is_ipv6 = 0;
> > char *mac_addr;
> > int str_len;
> >
> > @@ -1421,52 +1483,62 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> > if (error)
> > goto setval_error;
> >
> > - if (new_val->addr_family & ADDR_FAMILY_IPV6) {
> > - error = fprintf(nmfile, "\n[ipv6]\n");
> > - if (error < 0)
> > - goto setval_error;
> > - is_ipv6 = 1;
> > - } else {
> > - error = fprintf(nmfile, "\n[ipv4]\n");
> > - if (error < 0)
> > - goto setval_error;
> > - }
> > -
> > /*
> > - * Now we populate the keyfile format
> > + * The keyfile format expects the IPv6 and IPv4 configuration in
> > + * different sections. Therefore we iterate through the list twice,
> > + * once to populate the IPv4 section and the next time for IPv6
> > */
> > + ip_sections_count = 1;
> > + do {
> > + if (ip_sections_count == 1) {
> > + error = fprintf(nmfile, "\n[ipv4]\n");
> > + if (error < 0)
> > + goto setval_error;
> > + } else {
> > + error = fprintf(nmfile, "\n[ipv6]\n");
> > + if (error < 0)
> > + goto setval_error;
> > + }
> >
> > - if (new_val->dhcp_enabled) {
> > - error = kvp_write_file(nmfile, "method", "", "auto");
> > - if (error < 0)
> > - goto setval_error;
> > - } else {
> > - error = kvp_write_file(nmfile, "method", "", "manual");
> > + /*
> > + * Now we populate the keyfile format
> > + */
> > +
> > + if (new_val->dhcp_enabled) {
> > + error = kvp_write_file(nmfile, "method", "", "auto");
> > + if (error < 0)
> > + goto setval_error;
> > + } else {
> > + error = kvp_write_file(nmfile, "method", "", "manual");
> > + if (error < 0)
> > + goto setval_error;
> > + }
> > +
> > + /*
> > + * Write the configuration for ipaddress, netmask, gateway and
> > + * name services
> > + */
> > + error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> > + (char *)new_val->sub_net,
> > + ip_sections_count);
> > if (error < 0)
> > goto setval_error;
> > - }
> >
> > - /*
> > - * Write the configuration for ipaddress, netmask, gateway and
> > - * name services
> > - */
> > - error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> > - (char *)new_val->sub_net, is_ipv6);
> > - if (error < 0)
> > - goto setval_error;
> > -
> > - /* we do not want ipv4 addresses in ipv6 section and vice versa */
> > - if (is_ipv6 != is_ipv4((char *)new_val->gate_way)) {
> > - error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way);
> > + error = process_dns_gateway_nm(nmfile,
> > + (char *)new_val->gate_way,
> > + GATEWAY, ip_sections_count);
> > if (error < 0)
> > goto setval_error;
> > - }
> >
> > - if (is_ipv6 != is_ipv4((char *)new_val->dns_addr)) {
> > - error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr);
> > + error = process_dns_gateway_nm(nmfile,
> > + (char *)new_val->dns_addr, DNS,
> > + ip_sections_count);
> > if (error < 0)
>
> But you return +ve from this function so this will never be true.
>
> > goto setval_error;
> > - }
> > +
> > + ip_sections_count++;
> > + } while (ip_sections_count <= 2);
> > +
> > fclose(nmfile);
> > fclose(ifcfg_file);
> >
> > --
> > 2.34.1
> >

2024-03-06 09:58:21

by Ani Sinha

[permalink] [raw]
Subject: Re: [PATCH] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format



> On 01-Mar-2024, at 18:26, Shradha Gupta <[email protected]> wrote:
>
> If the network configuration strings are passed as a combination of IPv and
> IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
> configuration format.
> With these changes, the keyfile config generation logic scans through the
> list twice to generate IPv4 and IPv6 sections for the configuration files
> to handle this support.
>
> Built-on: Rhel9
> Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
> Signed-off-by: Shradha Gupta <[email protected]>
> ---
> tools/hv/hv_kvp_daemon.c | 152 ++++++++++++++++++++++++++++-----------
> 1 file changed, 112 insertions(+), 40 deletions(-)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 318e2dad27e0..7e84e40b55fb 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -76,6 +76,11 @@ enum {
> DNS
> };
>
> +enum {
> + IPV4 = 1,
> + IPV6
> +};
> +
> static int in_hand_shake;
>
> static char *os_name = "";
> @@ -1171,6 +1176,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
> return 0;
> }
>
> +int ip_version_check(const char *input_addr)
> +{
> + struct in6_addr addr;
> +
> + if (inet_pton(AF_INET, input_addr, &addr))
> + return IPV4;
> + else if (inet_pton(AF_INET6, input_addr, &addr))
> + return IPV6;
> + else
> + return -EINVAL;
> +}
> +
> /*
> * Only IPv4 subnet strings needs to be converted to plen
> * For IPv6 the subnet is already privided in plen format
> @@ -1197,14 +1214,56 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
> return plen;
> }
>
> +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> + int ip_sec)
> +{
> + char addr[INET6_ADDRSTRLEN], *output_str;
> + int ip_offset = 0, error, ip_ver;
> + char *param_name;
> +
> + output_str = malloc(strlen(ip_string));
> +
> + if (!output_str)
> + return 1;
> +
> + output_str[0] = '\0';
> +
> + if (type == DNS)
> + param_name = "dns";
> + else
> + param_name = "gateway";
> +
> + while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> + (MAX_IP_ADDR_SIZE * 2))) {
> + ip_ver = ip_version_check(addr);
> +
> + if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> + (ip_ver == IPV6 && ip_sec == IPV6)) {
> + strcat(output_str, addr);
> + strcat(output_str, ",");

We need to check if we are not going out of bounds here. So existing length of output_str + length of addr + 1 should be < strlen(ip_string) which is the length of the buffer. See parse_ip_val_buffer() how it does out of bounds check.

> + } else {
> + continue;
> + }
> + }
> +
> + if (strlen(output_str)) {
> + output_str[strlen(output_str) - 1] = '\0';
> + error = fprintf(f, "%s=%s\n", param_name, output_str);
> + if (error < 0)
> + return error;
> + }
> +
> + return 0;
> +}
> +
> static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> - int is_ipv6)
> + int ip_sec)
> {
> char addr[INET6_ADDRSTRLEN];
> char subnet_addr[INET6_ADDRSTRLEN];
> int error, i = 0;
> int ip_offset = 0, subnet_offset = 0;
> - int plen;
> + int plen, ip_ver;
>
> memset(addr, 0, sizeof(addr));
> memset(subnet_addr, 0, sizeof(subnet_addr));
> @@ -1216,10 +1275,13 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> subnet_addr,
> (MAX_IP_ADDR_SIZE *
> 2))) {
> - if (!is_ipv6)
> + ip_ver = ip_version_check(addr);
> + if (ip_ver == IPV4 && ip_sec == IPV4)
> plen = kvp_subnet_to_plen((char *)subnet_addr);
> - else
> + else if (ip_ver == IPV6 && ip_sec == IPV6)
> plen = atoi(subnet_addr);
> + else
> + continue;
>
> if (plen < 0)
> return plen;
> @@ -1242,8 +1304,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> char if_filename[PATH_MAX];
> char nm_filename[PATH_MAX];
> FILE *ifcfg_file, *nmfile;
> + int ip_sections_count;
> char cmd[PATH_MAX];
> - int is_ipv6 = 0;
> char *mac_addr;
> int str_len;
>
> @@ -1421,52 +1483,62 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> if (error)
> goto setval_error;
>
> - if (new_val->addr_family & ADDR_FAMILY_IPV6) {
> - error = fprintf(nmfile, "\n[ipv6]\n");
> - if (error < 0)
> - goto setval_error;
> - is_ipv6 = 1;
> - } else {
> - error = fprintf(nmfile, "\n[ipv4]\n");
> - if (error < 0)
> - goto setval_error;
> - }
> -
> /*
> - * Now we populate the keyfile format
> + * The keyfile format expects the IPv6 and IPv4 configuration in
> + * different sections. Therefore we iterate through the list twice,
> + * once to populate the IPv4 section and the next time for IPv6
> */
> + ip_sections_count = 1;
> + do {
> + if (ip_sections_count == 1) {
> + error = fprintf(nmfile, "\n[ipv4]\n");
> + if (error < 0)
> + goto setval_error;
> + } else {
> + error = fprintf(nmfile, "\n[ipv6]\n");
> + if (error < 0)
> + goto setval_error;
> + }
>
> - if (new_val->dhcp_enabled) {
> - error = kvp_write_file(nmfile, "method", "", "auto");
> - if (error < 0)
> - goto setval_error;
> - } else {
> - error = kvp_write_file(nmfile, "method", "", "manual");
> + /*
> + * Now we populate the keyfile format
> + */
> +
> + if (new_val->dhcp_enabled) {
> + error = kvp_write_file(nmfile, "method", "", "auto");
> + if (error < 0)
> + goto setval_error;
> + } else {
> + error = kvp_write_file(nmfile, "method", "", "manual");
> + if (error < 0)
> + goto setval_error;
> + }
> +
> + /*
> + * Write the configuration for ipaddress, netmask, gateway and
> + * name services
> + */
> + error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> + (char *)new_val->sub_net,
> + ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
>
> - /*
> - * Write the configuration for ipaddress, netmask, gateway and
> - * name services
> - */
> - error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> - (char *)new_val->sub_net, is_ipv6);
> - if (error < 0)
> - goto setval_error;
> -
> - /* we do not want ipv4 addresses in ipv6 section and vice versa */
> - if (is_ipv6 != is_ipv4((char *)new_val->gate_way)) {
> - error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way);
> + error = process_dns_gateway_nm(nmfile,
> + (char *)new_val->gate_way,
> + GATEWAY, ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
>
> - if (is_ipv6 != is_ipv4((char *)new_val->dns_addr)) {
> - error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr);
> + error = process_dns_gateway_nm(nmfile,
> + (char *)new_val->dns_addr, DNS,
> + ip_sections_count);
> if (error < 0)
> goto setval_error;
> - }
> +
> + ip_sections_count++;
> + } while (ip_sections_count <= 2);
> +
> fclose(nmfile);
> fclose(ifcfg_file);
>
> --
> 2.34.1
>


2024-03-11 03:37:51

by Shradha Gupta

[permalink] [raw]
Subject: Re: [PATCH] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format

Thanks, will incorporate all these changes in next version

Regards,
Shradha
On Wed, Mar 06, 2024 at 02:57:12PM +0530, Ani Sinha wrote:
>
>
> > On 01-Mar-2024, at 18:26, Shradha Gupta <[email protected]> wrote:
> >
> > If the network configuration strings are passed as a combination of IPv and
> > IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
> > configuration format.
> > With these changes, the keyfile config generation logic scans through the
> > list twice to generate IPv4 and IPv6 sections for the configuration files
> > to handle this support.
> >
> > Built-on: Rhel9
> > Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
> > Signed-off-by: Shradha Gupta <[email protected]>
> > ---
> > tools/hv/hv_kvp_daemon.c | 152 ++++++++++++++++++++++++++++-----------
> > 1 file changed, 112 insertions(+), 40 deletions(-)
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index 318e2dad27e0..7e84e40b55fb 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -76,6 +76,11 @@ enum {
> > DNS
> > };
> >
> > +enum {
> > + IPV4 = 1,
> > + IPV6
> > +};
> > +
> > static int in_hand_shake;
> >
> > static char *os_name = "";
> > @@ -1171,6 +1176,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
> > return 0;
> > }
> >
> > +int ip_version_check(const char *input_addr)
> > +{
> > + struct in6_addr addr;
> > +
> > + if (inet_pton(AF_INET, input_addr, &addr))
> > + return IPV4;
> > + else if (inet_pton(AF_INET6, input_addr, &addr))
> > + return IPV6;
> > + else
> > + return -EINVAL;
> > +}
> > +
> > /*
> > * Only IPv4 subnet strings needs to be converted to plen
> > * For IPv6 the subnet is already privided in plen format
> > @@ -1197,14 +1214,56 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
> > return plen;
> > }
> >
> > +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> > + int ip_sec)
> > +{
> > + char addr[INET6_ADDRSTRLEN], *output_str;
> > + int ip_offset = 0, error, ip_ver;
> > + char *param_name;
> > +
> > + output_str = malloc(strlen(ip_string));
> > +
> > + if (!output_str)
> > + return 1;
> > +
> > + output_str[0] = '\0';
> > +
> > + if (type == DNS)
> > + param_name = "dns";
> > + else
> > + param_name = "gateway";
> > +
> > + while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> > + (MAX_IP_ADDR_SIZE * 2))) {
> > + ip_ver = ip_version_check(addr);
> > +
> > + if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> > + (ip_ver == IPV6 && ip_sec == IPV6)) {
> > + strcat(output_str, addr);
> > + strcat(output_str, ",");
>
> We need to check if we are not going out of bounds here. So existing length of output_str + length of addr + 1 should be < strlen(ip_string) which is the length of the buffer. See parse_ip_val_buffer() how it does out of bounds check.
>
> > + } else {
> > + continue;
> > + }
> > + }
> > +
> > + if (strlen(output_str)) {
> > + output_str[strlen(output_str) - 1] = '\0';
> > + error = fprintf(f, "%s=%s\n", param_name, output_str);
> > + if (error < 0)
> > + return error;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> > - int is_ipv6)
> > + int ip_sec)
> > {
> > char addr[INET6_ADDRSTRLEN];
> > char subnet_addr[INET6_ADDRSTRLEN];
> > int error, i = 0;
> > int ip_offset = 0, subnet_offset = 0;
> > - int plen;
> > + int plen, ip_ver;
> >
> > memset(addr, 0, sizeof(addr));
> > memset(subnet_addr, 0, sizeof(subnet_addr));
> > @@ -1216,10 +1275,13 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> > subnet_addr,
> > (MAX_IP_ADDR_SIZE *
> > 2))) {
> > - if (!is_ipv6)
> > + ip_ver = ip_version_check(addr);
> > + if (ip_ver == IPV4 && ip_sec == IPV4)
> > plen = kvp_subnet_to_plen((char *)subnet_addr);
> > - else
> > + else if (ip_ver == IPV6 && ip_sec == IPV6)
> > plen = atoi(subnet_addr);
> > + else
> > + continue;
> >
> > if (plen < 0)
> > return plen;
> > @@ -1242,8 +1304,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> > char if_filename[PATH_MAX];
> > char nm_filename[PATH_MAX];
> > FILE *ifcfg_file, *nmfile;
> > + int ip_sections_count;
> > char cmd[PATH_MAX];
> > - int is_ipv6 = 0;
> > char *mac_addr;
> > int str_len;
> >
> > @@ -1421,52 +1483,62 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> > if (error)
> > goto setval_error;
> >
> > - if (new_val->addr_family & ADDR_FAMILY_IPV6) {
> > - error = fprintf(nmfile, "\n[ipv6]\n");
> > - if (error < 0)
> > - goto setval_error;
> > - is_ipv6 = 1;
> > - } else {
> > - error = fprintf(nmfile, "\n[ipv4]\n");
> > - if (error < 0)
> > - goto setval_error;
> > - }
> > -
> > /*
> > - * Now we populate the keyfile format
> > + * The keyfile format expects the IPv6 and IPv4 configuration in
> > + * different sections. Therefore we iterate through the list twice,
> > + * once to populate the IPv4 section and the next time for IPv6
> > */
> > + ip_sections_count = 1;
> > + do {
> > + if (ip_sections_count == 1) {
> > + error = fprintf(nmfile, "\n[ipv4]\n");
> > + if (error < 0)
> > + goto setval_error;
> > + } else {
> > + error = fprintf(nmfile, "\n[ipv6]\n");
> > + if (error < 0)
> > + goto setval_error;
> > + }
> >
> > - if (new_val->dhcp_enabled) {
> > - error = kvp_write_file(nmfile, "method", "", "auto");
> > - if (error < 0)
> > - goto setval_error;
> > - } else {
> > - error = kvp_write_file(nmfile, "method", "", "manual");
> > + /*
> > + * Now we populate the keyfile format
> > + */
> > +
> > + if (new_val->dhcp_enabled) {
> > + error = kvp_write_file(nmfile, "method", "", "auto");
> > + if (error < 0)
> > + goto setval_error;
> > + } else {
> > + error = kvp_write_file(nmfile, "method", "", "manual");
> > + if (error < 0)
> > + goto setval_error;
> > + }
> > +
> > + /*
> > + * Write the configuration for ipaddress, netmask, gateway and
> > + * name services
> > + */
> > + error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> > + (char *)new_val->sub_net,
> > + ip_sections_count);
> > if (error < 0)
> > goto setval_error;
> > - }
> >
> > - /*
> > - * Write the configuration for ipaddress, netmask, gateway and
> > - * name services
> > - */
> > - error = process_ip_string_nm(nmfile, (char *)new_val->ip_addr,
> > - (char *)new_val->sub_net, is_ipv6);
> > - if (error < 0)
> > - goto setval_error;
> > -
> > - /* we do not want ipv4 addresses in ipv6 section and vice versa */
> > - if (is_ipv6 != is_ipv4((char *)new_val->gate_way)) {
> > - error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way);
> > + error = process_dns_gateway_nm(nmfile,
> > + (char *)new_val->gate_way,
> > + GATEWAY, ip_sections_count);
> > if (error < 0)
> > goto setval_error;
> > - }
> >
> > - if (is_ipv6 != is_ipv4((char *)new_val->dns_addr)) {
> > - error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr);
> > + error = process_dns_gateway_nm(nmfile,
> > + (char *)new_val->dns_addr, DNS,
> > + ip_sections_count);
> > if (error < 0)
> > goto setval_error;
> > - }
> > +
> > + ip_sections_count++;
> > + } while (ip_sections_count <= 2);
> > +
> > fclose(nmfile);
> > fclose(ifcfg_file);
> >
> > --
> > 2.34.1
> >