On 3/12/2024 5:38 AM, Shradha Gupta wrote:
> If the network configuration strings are passed as a combination of IPv and
*IPv4*
> IPv6 addresses, the current KVP daemon doesnot handle it for the keyfile
*does not/doesn't*
> configuration format.
> With these changes, the keyfile config generation logic scans through the
> list twice to generate IPv4 and IPv6 sections for the configuration files
> to handle this support.
>
> Built-on: Rhel9
> Tested-on: Rhel9(IPv4 only, IPv6 only, IPv4 and IPv6 combination)
As mentioned by Jakub[1], what value does this information provide?
Please follow Haiyang's suggestion [2] and put SKU and test information, or just
skip it.
[1] https://lore.kernel.org/all/20240307072923.6cc8a2ba@xxxxxxxxxx/
[2] https://lore.kernel.org/all/DM6PR21MB14817597567C638DEF020FE3CA202@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> Signed-off-by: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v2
> * Use calloc to avoid initialization later
> * Return standard error codes
> * Free the output_str pointer on completion
> * Add out-of bound checks while writing to buffers
> ---
> tools/hv/hv_kvp_daemon.c | 173 +++++++++++++++++++++++++++++----------
> 1 file changed, 132 insertions(+), 41 deletions(-)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 318e2dad27e0..ae65be004eb1 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -76,6 +76,12 @@ enum {
> DNS
> };
>
> +enum {
> + IPV4 = 1,
> + IPV6,
> + IP_TYPE_MAX
> +};
> +
> static int in_hand_shake;
>
> static char *os_name = "";
> @@ -102,6 +108,7 @@ static struct utsname uts_buf;
>
> #define MAX_FILE_NAME 100
> #define ENTRIES_PER_BLOCK 50
> +#define MAX_IP_ENTRIES 64
Is this a limitation defined by hv_kvp? If so, is it possible it may change in a later
version? A comment would help here
>
> struct kvp_record {
> char key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
> @@ -1171,6 +1178,18 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
> return 0;
> }
>
> +int ip_version_check(const char *input_addr)
> +{
> + struct in6_addr addr;
> +
> + if (inet_pton(AF_INET, input_addr, &addr))
> + return IPV4;
> + else if (inet_pton(AF_INET6, input_addr, &addr))
> + return IPV6;
You can skip the else here...
> + else
> + return -EINVAL;
...and you can skip the else here as well and just return -EINVAL
> +}
> +
> /*
> * Only IPv4 subnet strings needs to be converted to plen
> * For IPv6 the subnet is already privided in plen format
> @@ -1197,14 +1216,71 @@ static int kvp_subnet_to_plen(char *subnet_addr_str)
> return plen;
> }
>
> +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> + int ip_sec)
> +{
> + char addr[INET6_ADDRSTRLEN], *output_str;
> + int ip_offset = 0, error = 0, ip_ver;
> + char *param_name;
> +
> + output_str = (char *)calloc(INET6_ADDRSTRLEN * MAX_IP_ENTRIES,
> + sizeof(char));
> +
> + if (!output_str)
> + return -ENOMEM;
> +
> + memset(addr, 0, sizeof(addr));
> +
> + if (type == DNS) {
> + param_name = "dns";
> + } else if (type == GATEWAY) {
> + param_name = "gateway";
> + } else {
> + error = -EINVAL;
> + goto cleanup;
> + }
> +
> + while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> + (MAX_IP_ADDR_SIZE * 2))) {
> + ip_ver = ip_version_check(addr);
> + if (ip_ver < 0)
> + continue;
> +
> + if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> + (ip_ver == IPV6 && ip_sec == IPV6)) {
> + if (((INET6_ADDRSTRLEN * MAX_IP_ENTRIES) - strlen(output_str)) >
> + (strlen(addr))) {
> + strcat(output_str, addr);
> + strcat(output_str, ",");
Prefer strncat() here
> + }
> + memset(addr, 0, sizeof(addr));
> +
> + } else {
> + memset(addr, 0, sizeof(addr));
> + continue;
> + }
> + }
> +
> + if (strlen(output_str)) {
> + output_str[strlen(output_str) - 1] = '\0';
> + error = fprintf(f, "%s=%s\n", param_name, output_str);
> + if (error < 0)
> + goto cleanup;
> + }
> +
> +cleanup:
> + free(output_str);
> + return error;
> +}
> +
> static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> - int is_ipv6)
> + int ip_sec)
> {
> char addr[INET6_ADDRSTRLEN];
> char subnet_addr[INET6_ADDRSTRLEN];
> int error, i = 0;
> int ip_offset = 0, subnet_offset = 0;
> - int plen;
> + int plen, ip_ver;
>
> memset(addr, 0, sizeof(addr));
> memset(subnet_addr, 0, sizeof(subnet_addr));
> @@ -1216,10 +1292,16 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
> subnet_addr,
> (MAX_IP_ADDR_SIZE *
> 2))) {
> - if (!is_ipv6)
> + ip_ver = ip_version_check(addr);
> + if (ip_ver < 0)
> + continue;
> +
> + if (ip_ver == IPV4 && ip_sec == IPV4)
> plen = kvp_subnet_to_plen((char *)subnet_addr);
> - else
> + else if (ip_ver == IPV6 && ip_sec == IPV6)
> plen = atoi(subnet_addr);
> + else
> + continue;
>
> if (plen < 0)
> return plen;
> @@ -1238,12 +1320,11 @@ static int process_ip_string_nm(FILE *f, char *ip_string, char *subnet,
>
> static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> {
> - int error = 0;
> + int error = 0, ip_type;
nit: Can we keep ip_ver through all the functions for consistency
<snip>
Thanks,
Easwar
