Hi Ani, This patch should be out for review by mid next week. Thanks and Regards, Shradha. On Sat, Feb 24, 2024 at 09:48:17PM +0530, Ani Sinha wrote: > On Fri, 5 Jan, 2024, 11:12 am Shradha Gupta, < > shradhagupta@xxxxxxxxxxxxxxxxxxx> wrote: > > > On Sat, Dec 23, 2023 at 01:35:26PM +0530, Ani Sinha wrote: > > > 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. > > Thanks for bringing this up Ani. I will try to fix this in a new patch > > (would get the new testcases added in our suite as well) > > > > Has there been any progress on this front? > > > >
