Re: [PATCH] hv/hv_kvp_daemon: Some small fixes for handling NM keyfiles

[Ani Sinha <anisinha@redhat.com>]

> On 23-Oct-2023, at 11:07 AM, Shradha Gupta <shradhagupta@linux.microsoft.com> wrote:
> 
> On Mon, Oct 16, 2023 at 07:01:22PM +0530, Ani Sinha wrote:
>> Some small fixes:
>> - lets make sure we are not adding ipv4 addresses in ipv6 section in
>>   keyfile and vice versa.
>> - ADDR_FAMILY_IPV6 is a bit in addr_family. Test that bit instead of
>>   checking the whole value of addr_family.
>> - Some trivial fixes in hv_set_ifconfig.sh.
>> 
>> These fixes are proposed after doing some internal testing at Red Hat.
>> 
>> CC: Shradha Gupta <shradhagupta@linux.microsoft.com>
>> CC: Saurabh Sengar <ssengar@linux.microsoft.com>
>> Fixes: 42999c904612 ("hv/hv_kvp_daemon:Support for keyfile based connection profile")
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>> ---
>> tools/hv/hv_kvp_daemon.c    | 20 ++++++++++++--------
>> tools/hv/hv_set_ifconfig.sh |  4 ++--
>> 2 files changed, 14 insertions(+), 10 deletions(-)
>> 
>> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
>> index 264eeb9c46a9..318e2dad27e0 100644
>> --- a/tools/hv/hv_kvp_daemon.c
>> +++ b/tools/hv/hv_kvp_daemon.c
>> @@ -1421,7 +1421,7 @@ 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) {
>> +	if (new_val->addr_family & ADDR_FAMILY_IPV6) {
>> 		error = fprintf(nmfile, "\n[ipv6]\n");
>> 		if (error < 0)
>> 			goto setval_error;
>> @@ -1455,14 +1455,18 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
>> 	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;
>> +	/* 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);
>> +		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);
>> +		if (error < 0)
>> +			goto setval_error;
>> +	}
>> 	fclose(nmfile);
>> 	fclose(ifcfg_file);
>> 
>> diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
>> index ae5a7a8249a2..440a91b35823 100755
>> --- a/tools/hv/hv_set_ifconfig.sh
>> +++ b/tools/hv/hv_set_ifconfig.sh
>> @@ -53,7 +53,7 @@
>> #                       or "manual" if no boot-time protocol should be used)
>> #
>> # address1=ipaddr1/plen
>> -# address=ipaddr2/plen
>> +# address2=ipaddr2/plen
>> #
>> # gateway=gateway1;gateway2
>> #
>> @@ -61,7 +61,7 @@
>> #
>> # [ipv6]
>> # address1=ipaddr1/plen
>> -# address2=ipaddr1/plen
>> +# address2=ipaddr2/plen
>> #
>> # gateway=gateway1;gateway2
>> #
>> -- 
>> 2.39.2
> Reviewed-by: Shradha Gupta <Shradhagupta@linux.microsoft.com>

Ping. Can anyone please queue this?
>