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