Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Sun, 22 Oct 2023 22:37:46 -0700
From:   Shradha Gupta <shradhagupta@linux.microsoft.com>
To:     Ani Sinha <anisinha@redhat.com>
Cc:     "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
        Saurabh Sengar <ssengar@linux.microsoft.com>,
        linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hv/hv_kvp_daemon: Some small fixes for handling NM
 keyfiles
Message-ID: <20231023053746.GA11148@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <20231016133122.2419537-1-anisinha@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231016133122.2419537-1-anisinha@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

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>