> On 13-Oct-2023, at 3:06 PM, Ani Sinha <anisinha@xxxxxxxxxx> wrote: > > > >> On 09-Oct-2023, at 4:08 PM, Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx> wrote: >> >> Ifcfg config file support in NetworkManger is deprecated. This patch >> provides support for the new keyfile config format for connection >> profiles in NetworkManager. The patch modifies the hv_kvp_daemon code >> to generate the new network configuration in keyfile >> format(.ini-style format) along with a ifcfg format configuration. >> The ifcfg format configuration is also retained to support easy >> backward compatibility for distro vendors. These configurations are >> stored in temp files which are further translated using the >> hv_set_ifconfig.sh script. This script is implemented by individual >> distros based on the network management commands supported. >> For example, RHEL's implementation could be found here: >> https://gitlab.com/redhat/centos-stream/src/hyperv-daemons/-/blob/c9s/hv_set_ifconfig.sh >> Debian's implementation could be found here: >> https://github.com/endlessm/linux/blob/master/debian/cloud-tools/hv_set_ifconfig >> >> The next part of this support is to let the Distro vendors consume >> these modified implementations to the new configuration format. >> >> Tested-on: Rhel9(Hyper-V, Azure)(nm and ifcfg files verified) > > Was this patch tested with ipv6? We are seeing a mix of ipv6 and ipv4 addresses in ipv6 section: > > ]# cat eth0.nmconnection > > [connection] > autoconnect=yes > interface-name=eth0 > > [ethernet] > mac-address=00:15:5D:C4:63:45 > > [ipv6] > method=manual > address=1234:1234:1234:1234:0000:0000:0000:0113/120 > gateway=10.73.199.254 > dns=10.72.17.5 > > Gateway and dns should be in ipv6. > > Ipv4 dns addresses comes from /etc/resolv.conf and in our system: > > # /usr/libexec/hypervkvpd/hv_get_dns_info > 10.72.17.5 > 10.68.5.26 > > [root@vm-197-248 ~]# cat /etc/resolv.conf > # Generated by NetworkManager > search lab.eng.pek2.redhat.com > nameserver 10.72.17.5 > nameserver 10.68.5.26 > > So we should check if the addresses are indeed ipv6 or not and should not print it if !is_ipv4(). Ipv6 addresses may not be provisioned. > > I will post a patch fix when we complete our internal testing. > > >> Signed-off-by: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx> >> Reviewed-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx> >> --- >> Changes v7->v8 >> * Fix some filename variable names to avoid confusion >> * Add initialization of is_ipv6 variable >> * Add a few comments >> --- >> tools/hv/hv_kvp_daemon.c | 233 +++++++++++++++++++++++++++++++----- >> tools/hv/hv_set_ifconfig.sh | 39 +++++- >> 2 files changed, 235 insertions(+), 37 deletions(-) >> >> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c >> index 27f5e7dfc2f7..264eeb9c46a9 100644 >> --- a/tools/hv/hv_kvp_daemon.c >> +++ b/tools/hv/hv_kvp_daemon.c >> @@ -1171,12 +1171,79 @@ static int process_ip_string(FILE *f, char *ip_string, int type) >> return 0; >> } >> >> +/* >> + * Only IPv4 subnet strings needs to be converted to plen >> + * For IPv6 the subnet is already privided in plen format >> + */ >> +static int kvp_subnet_to_plen(char *subnet_addr_str) >> +{ >> + int plen = 0; >> + struct in_addr subnet_addr4; >> + >> + /* >> + * Convert subnet address to binary representation >> + */ >> + if (inet_pton(AF_INET, subnet_addr_str, &subnet_addr4) == 1) { >> + uint32_t subnet_mask = ntohl(subnet_addr4.s_addr); >> + >> + while (subnet_mask & 0x80000000) { >> + plen++; >> + subnet_mask <<= 1; >> + } >> + } else { >> + return -1; >> + } >> + >> + return plen; >> +} >> + >> +static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet, >> + int is_ipv6) >> +{ >> + char addr[INET6_ADDRSTRLEN]; >> + char subnet_addr[INET6_ADDRSTRLEN]; >> + int error, i = 0; > > This should be initialised to 1 Sorry we already do a ++i and not i++ so it should be fine. > > https://developer-old.gnome.org/NetworkManager/stable/nm-settings-keyfile.html#:~:text=File%20Format,%2Dsettings(5)). > > Also see below. > >> + int ip_offset = 0, subnet_offset = 0; >> + int plen; >> + >> + memset(addr, 0, sizeof(addr)); >> + memset(subnet_addr, 0, sizeof(subnet_addr)); >> + >> + while (parse_ip_val_buffer(ip_string, &ip_offset, addr, >> + (MAX_IP_ADDR_SIZE * 2)) && >> + parse_ip_val_buffer(subnet, >> + &subnet_offset, >> + subnet_addr, >> + (MAX_IP_ADDR_SIZE * >> + 2))) { >> + if (!is_ipv6) >> + plen = kvp_subnet_to_plen((char *)subnet_addr); >> + else >> + plen = atoi(subnet_addr); >> + >> + if (plen < 0) >> + return plen; >> + >> + error = fprintf(f, "address%d=%s/%d\n", ++i, (char *)addr, >> + plen); >> + if (error < 0) >> + return error; >> + >> + memset(addr, 0, sizeof(addr)); >> + memset(subnet_addr, 0, sizeof(subnet_addr)); >> + } >> + >> + return 0; >> +} >> + >> static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> { >> int error = 0; >> - char if_file[PATH_MAX]; >> - FILE *file; >> + char if_filename[PATH_MAX]; >> + char nm_filename[PATH_MAX]; >> + FILE *ifcfg_file, *nmfile; >> char cmd[PATH_MAX]; >> + int is_ipv6 = 0; >> char *mac_addr; >> int str_len; >> >> @@ -1197,7 +1264,7 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> * in a given distro to configure the interface and so are free >> * ignore information that may not be relevant. >> * >> - * Here is the format of the ip configuration file: >> + * Here is the ifcfg format of the ip configuration file: >> * >> * HWADDR=macaddr >> * DEVICE=interface name >> @@ -1220,6 +1287,32 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as >> * IPV6NETMASK. >> * >> + * Here is the keyfile format of the ip configuration file: >> + * >> + * [ethernet] >> + * mac-address=macaddr >> + * [connection] >> + * interface-name=interface name >> + * >> + * [ipv4] >> + * method=<protocol> (where <protocol> is "auto" if DHCP is configured >> + * or "manual" if no boot-time protocol should be used) >> + * >> + * address1=ipaddr1/plen >> + * address2=ipaddr2/plen >> + * >> + * gateway=gateway1;gateway2 >> + * >> + * dns=dns1;dns2 >> + * >> + * [ipv6] >> + * address1=ipaddr1/plen >> + * address2=ipaddr2/plen >> + * >> + * gateway=gateway1;gateway2 >> + * >> + * dns=dns1;dns2 >> + * >> * The host can specify multiple ipv4 and ipv6 addresses to be >> * configured for the interface. Furthermore, the configuration >> * needs to be persistent. A subsequent GET call on the interface >> @@ -1227,14 +1320,29 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> * call. >> */ >> >> - snprintf(if_file, sizeof(if_file), "%s%s%s", KVP_CONFIG_LOC, >> - "/ifcfg-", if_name); >> + /* >> + * We are populating both ifcfg and nmconnection files >> + */ >> + snprintf(if_filename, sizeof(if_filename), "%s%s%s", KVP_CONFIG_LOC, >> + "/ifcfg-", if_name); >> >> - file = fopen(if_file, "w"); >> + ifcfg_file = fopen(if_filename, "w"); >> >> - if (file == NULL) { >> + if (!ifcfg_file) { >> syslog(LOG_ERR, "Failed to open config file; error: %d %s", >> - errno, strerror(errno)); >> + errno, strerror(errno)); >> + return HV_E_FAIL; >> + } >> + >> + snprintf(nm_filename, sizeof(nm_filename), "%s%s%s%s", KVP_CONFIG_LOC, >> + "/", if_name, ".nmconnection"); >> + >> + nmfile = fopen(nm_filename, "w"); >> + >> + if (!nmfile) { >> + syslog(LOG_ERR, "Failed to open config file; error: %d %s", >> + errno, strerror(errno)); >> + fclose(ifcfg_file); >> return HV_E_FAIL; >> } >> >> @@ -1248,14 +1356,31 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> goto setval_error; >> } >> >> - error = kvp_write_file(file, "HWADDR", "", mac_addr); >> - free(mac_addr); >> + error = kvp_write_file(ifcfg_file, "HWADDR", "", mac_addr); >> + if (error < 0) >> + goto setmac_error; >> + >> + error = kvp_write_file(ifcfg_file, "DEVICE", "", if_name); >> + if (error < 0) >> + goto setmac_error; >> + >> + error = fprintf(nmfile, "\n[connection]\n"); >> + if (error < 0) >> + goto setmac_error; >> + >> + error = kvp_write_file(nmfile, "interface-name", "", if_name); >> if (error) >> - goto setval_error; >> + goto setmac_error; >> >> - error = kvp_write_file(file, "DEVICE", "", if_name); >> + error = fprintf(nmfile, "\n[ethernet]\n"); >> + if (error < 0) >> + goto setmac_error; >> + >> + error = kvp_write_file(nmfile, "mac-address", "", mac_addr); >> if (error) >> - goto setval_error; >> + goto setmac_error; >> + >> + free(mac_addr); >> >> /* >> * The dhcp_enabled flag is only for IPv4. In the case the host only >> @@ -1263,47 +1388,91 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> * proceed to parse and pass the IPv6 information to the >> * disto-specific script hv_set_ifconfig. >> */ >> + >> + /* >> + * First populate the ifcfg file format >> + */ >> if (new_val->dhcp_enabled) { >> - error = kvp_write_file(file, "BOOTPROTO", "", "dhcp"); >> + error = kvp_write_file(ifcfg_file, "BOOTPROTO", "", "dhcp"); >> if (error) >> goto setval_error; >> - >> } else { >> - error = kvp_write_file(file, "BOOTPROTO", "", "none"); >> + error = kvp_write_file(ifcfg_file, "BOOTPROTO", "", "none"); >> if (error) >> goto setval_error; >> } >> >> - /* >> - * Write the configuration for ipaddress, netmask, gateway and >> - * name servers. >> - */ >> - >> - error = process_ip_string(file, (char *)new_val->ip_addr, IPADDR); >> + error = process_ip_string(ifcfg_file, (char *)new_val->ip_addr, >> + IPADDR); >> if (error) >> goto setval_error; >> >> - error = process_ip_string(file, (char *)new_val->sub_net, NETMASK); >> + error = process_ip_string(ifcfg_file, (char *)new_val->sub_net, >> + NETMASK); >> if (error) >> goto setval_error; >> >> - error = process_ip_string(file, (char *)new_val->gate_way, GATEWAY); >> + error = process_ip_string(ifcfg_file, (char *)new_val->gate_way, >> + GATEWAY); >> if (error) >> goto setval_error; >> >> - error = process_ip_string(file, (char *)new_val->dns_addr, DNS); >> + error = process_ip_string(ifcfg_file, (char *)new_val->dns_addr, DNS); >> if (error) >> goto setval_error; >> >> - fclose(file); >> + if (new_val->addr_family == ADDR_FAMILY_IPV6) { > > I think we should have done something like > > new_val->addr_family & ADDR_FAMILY_IPV6 here. > >> + 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 >> + */ >> + >> + 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, is_ipv6); >> + if (error < 0) >> + goto setval_error; >> + >> + error = fprintf(nmfile, "gateway=%s\n", (char *)new_val->gate_way); >> + if (error < 0) >> + goto setval_error; >> + >> + error = fprintf(nmfile, "dns=%s\n", (char *)new_val->dns_addr); >> + if (error < 0) >> + goto setval_error; >> + >> + fclose(nmfile); >> + fclose(ifcfg_file); >> >> /* >> * Now that we have populated the configuration file, >> * invoke the external script to do its magic. >> */ >> >> - str_len = snprintf(cmd, sizeof(cmd), KVP_SCRIPTS_PATH "%s %s", >> - "hv_set_ifconfig", if_file); >> + str_len = snprintf(cmd, sizeof(cmd), KVP_SCRIPTS_PATH "%s %s %s", >> + "hv_set_ifconfig", if_filename, nm_filename); >> /* >> * This is a little overcautious, but it's necessary to suppress some >> * false warnings from gcc 8.0.1. >> @@ -1316,14 +1485,16 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) >> >> if (system(cmd)) { >> syslog(LOG_ERR, "Failed to execute cmd '%s'; error: %d %s", >> - cmd, errno, strerror(errno)); >> + cmd, errno, strerror(errno)); >> return HV_E_FAIL; >> } >> return 0; >> - >> +setmac_error: >> + free(mac_addr); >> setval_error: >> syslog(LOG_ERR, "Failed to write config file"); >> - fclose(file); >> + fclose(ifcfg_file); >> + fclose(nmfile); >> return error; >> } >> >> diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh >> index d10fe35b7f25..ae5a7a8249a2 100755 >> --- a/tools/hv/hv_set_ifconfig.sh >> +++ b/tools/hv/hv_set_ifconfig.sh >> @@ -18,12 +18,12 @@ >> # >> # This example script is based on a RHEL environment. >> # >> -# Here is the format of the ip configuration file: >> +# Here is the ifcfg format of the ip configuration file: >> # >> # HWADDR=macaddr >> # DEVICE=interface name >> # BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is configured >> -# or "none" if no boot-time protocol should be used) >> +# or "none" if no boot-time protocol should be used) >> # >> # IPADDR0=ipaddr1 >> # IPADDR1=ipaddr2 >> @@ -41,6 +41,32 @@ >> # tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as >> # IPV6NETMASK. >> # >> +# Here is the keyfile format of the ip configuration file: >> +# >> +# [ethernet] >> +# mac-address=macaddr >> +# [connection] >> +# interface-name=interface name >> +# >> +# [ipv4] >> +# method=<protocol> (where <protocol> is "auto" if DHCP is configured >> +# or "manual" if no boot-time protocol should be used) >> +# >> +# address1=ipaddr1/plen >> +# address=ipaddr2/plen > > address2 here? > >> +# >> +# gateway=gateway1;gateway2 >> +# >> +# dns=dns1; >> +# >> +# [ipv6] >> +# address1=ipaddr1/plen >> +# address2=ipaddr1/plen > > ipaddr2 ? > >> +# >> +# gateway=gateway1;gateway2 >> +# >> +# dns=dns1;dns2 >> +# >> # The host can specify multiple ipv4 and ipv6 addresses to be >> # configured for the interface. Furthermore, the configuration >> # needs to be persistent. A subsequent GET call on the interface >> @@ -48,18 +74,19 @@ >> # call. >> # >> >> - >> - >> echo "IPV6INIT=yes" >> $1 >> echo "NM_CONTROLLED=no" >> $1 >> echo "PEERDNS=yes" >> $1 >> echo "ONBOOT=yes" >> $1 >> >> - >> cp $1 /etc/sysconfig/network-scripts/ >> >> +chmod 600 $2 >> +interface=$(echo $2 | awk -F - '{ print $2 }') >> +filename="${2##*/}" >> + >> +sed '/\[connection\]/a autoconnect=true' $2 > /etc/NetworkManager/system-connections/${filename} >> >> -interface=$(echo $1 | awk -F - '{ print $2 }') >> >> /sbin/ifdown $interface 2>/dev/null >> /sbin/ifup $interface 2>/dev/null >> -- >> 2.34.1
