On Sat, Dec 23, 2023 at 12:43 PM Ani Sinha <anisinha@xxxxxxxxxx> wrote: > > On Fri, Oct 13, 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: > > There is also another issue which is kind of a design problem that > existed from the get go but now is exposed since keyfile support was > added. > Imagine we configure both ipv6 and ipv4 and some interfaces have ipv4 > addresses and some have ipv6. > getifaddres() call in kvp_get_ip_info() will return a linked list per > interface. The code sets ip_buffer->addr_family based on the address > family of the address set for the interface. We use this to determine > which section in the keyfile to use, ipv6 or ipv4. However, once we > make this decision, we are locked in. The issue here is that > kvp_process_ip_address() that extracts the IP addresses concatenate > the addresses in a single buffer separating the IPs with ";". Thus > across interfaces, the buffer can contain both ipv4 and ipv6 addresses > separated by ";" if both v4 and v6 are configured. This is problematic > as the addr_family can be either ipv4 or ipv6 but not both. > Essentially, we can have a situation that for a single addr_family in > hv_kvp_ipaddr_value struct, the ip_addr member can be a buffer > containing both ipv6 and ipv4 addresses. Notice that > process_ip_string() handles this by iterating through the string and > for each ip extracted, it individually determines if the IP is a v6 or > a v4 and adds "IPV6ADDR" or "IPADDR" to the ifcfg file accordingly. > process_ip_string_nm() does not do that and solely makes the > determination based on is_ipv6 values which is based on a single > addr_family value above. Thus, it cannot possibly know whether the > specific IP address extracted from the string is a v4 or v6. Unlike > for ifcfg files, fir nm keyfiles, we need to add v4 and v6 addresses > in specific sections and we cannot mix the two. So we need to make two > passes. One for v4 and one for v6 and then add IPs in the respective > sections. > > This issue needs to be looked into and unless it's resolved, we cannot > support both ipv4 and ipv6 addresses at the same time. In the short term, we should probably do this to avoid the mismatch : diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 318e2dad27e0..b77c8edfe663 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1216,6 +1216,9 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet, subnet_addr, (MAX_IP_ADDR_SIZE * 2))) { + if (is_ipv6 == is_ipv4((char*) addr)) + continue; + if (!is_ipv6) plen = kvp_subnet_to_plen((char *)subnet_addr); else But this is really a short term hack.
