Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v1] tools/hv: async name resolution in kvp_daemon
@ 2019-10-24 14:49 Olaf Hering
  2019-10-28 16:17 ` Sasha Levin
  2019-11-07 13:39 ` Vitaly Kuznetsov
  0 siblings, 2 replies; 12+ messages in thread
From: Olaf Hering @ 2019-10-24 14:49 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	linux-hyperv, linux-kernel
  Cc: Olaf Hering

The hostname is resolved just once since commit 58125210ab3b ("Tools:
hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
responds within the timeout limits to requests from the host.

If for some reason getaddrinfo fails, the string returned by the
"FullyQualifiedDomainName" request contains some error string, which is
then used by tools on the host side.

Adjust the code to resolve the current hostname in a separate thread.
This thread loops until getaddrinfo returns success. During this time
all "FullyQualifiedDomainName" requests will be answered with an empty
string.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/hv/Makefile        |  2 ++
 tools/hv/hv_kvp_daemon.c | 69 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/tools/hv/Makefile b/tools/hv/Makefile
index b57143d9459c..3b5481015a84 100644
--- a/tools/hv/Makefile
+++ b/tools/hv/Makefile
@@ -22,6 +22,8 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
 
 ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
 
+$(OUTPUT)hv_kvp_daemon: LDFLAGS += -lpthread
+
 all: $(ALL_PROGRAMS)
 
 export srctree OUTPUT CC LD CFLAGS
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index e9ef4ca6a655..22cf1c4dbf5c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -41,6 +41,7 @@
 #include <net/if.h>
 #include <limits.h>
 #include <getopt.h>
+#include <pthread.h>
 
 /*
  * KVP protocol: The user mode component first registers with the
@@ -85,7 +86,7 @@ static char *processor_arch;
 static char *os_build;
 static char *os_version;
 static char *lic_version = "Unknown version";
-static char full_domain_name[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
+static char *full_domain_name;
 static struct utsname uts_buf;
 
 /*
@@ -1327,27 +1328,53 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 	return error;
 }
 
-
-static void
-kvp_get_domain_name(char *buffer, int length)
+/*
+ * Async retrival of Fully Qualified Domain Name because getaddrinfo takes an
+ * unpredictable amount of time to finish.
+ */
+static void *kvp_getaddrinfo(void *p)
 {
-	struct addrinfo	hints, *info ;
-	int error = 0;
+	char *tmp, **str_ptr = (char **)p;
+	char hostname[HOST_NAME_MAX + 1];
+	struct addrinfo	*info, hints = {
+		.ai_family = AF_INET, /* Get only ipv4 addrinfo. */
+		.ai_socktype = SOCK_STREAM,
+		.ai_flags = AI_CANONNAME,
+	};
+	int ret;
+
+	if (gethostname(hostname, sizeof(hostname) - 1) < 0)
+		goto out;
+
+	do {
+		ret = getaddrinfo(hostname, NULL, &hints, &info);
+		if (ret)
+			sleep(1);
+	} while (ret);
+
+	ret = asprintf(&tmp, "%s", info->ai_canonname);
+	freeaddrinfo(info);
+	if (ret <= 0)
+		goto out;
+
+	if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
+		tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
+	*str_ptr = tmp;
 
-	gethostname(buffer, length);
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_family = AF_INET; /*Get only ipv4 addrinfo. */
-	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_flags = AI_CANONNAME;
+out:
+	pthread_exit(NULL);
+}
+
+static void kvp_obtain_domain_name(char **str_ptr)
+{
+	pthread_t t;
 
-	error = getaddrinfo(buffer, NULL, &hints, &info);
-	if (error != 0) {
-		snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
-			error, gai_strerror(error));
+	if (pthread_create(&t, NULL, kvp_getaddrinfo, str_ptr)) {
+		syslog(LOG_ERR, "pthread_create failed; error: %d %s",
+			errno, strerror(errno));
 		return;
 	}
-	snprintf(buffer, length, "%s", info->ai_canonname);
-	freeaddrinfo(info);
+	pthread_detach(t);
 }
 
 void print_usage(char *argv[])
@@ -1412,11 +1439,7 @@ int main(int argc, char *argv[])
 	 * Retrieve OS release information.
 	 */
 	kvp_get_os_info();
-	/*
-	 * Cache Fully Qualified Domain Name because getaddrinfo takes an
-	 * unpredictable amount of time to finish.
-	 */
-	kvp_get_domain_name(full_domain_name, sizeof(full_domain_name));
+	kvp_obtain_domain_name(&full_domain_name);
 
 	if (kvp_file_init()) {
 		syslog(LOG_ERR, "Failed to initialize the pools");
@@ -1571,7 +1594,7 @@ int main(int argc, char *argv[])
 
 		switch (hv_msg->body.kvp_enum_data.index) {
 		case FullyQualifiedDomainName:
-			strcpy(key_value, full_domain_name);
+			strcpy(key_value, full_domain_name ? : "");
 			strcpy(key_name, "FullyQualifiedDomainName");
 			break;
 		case IntegrationServicesVersion:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-10-24 14:49 [PATCH v1] tools/hv: async name resolution in kvp_daemon Olaf Hering
@ 2019-10-28 16:17 ` Sasha Levin
  2019-10-28 17:49   ` Olaf Hering
  2019-11-07 13:39 ` Vitaly Kuznetsov
  1 sibling, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2019-10-28 16:17 UTC (permalink / raw)
  To: Olaf Hering
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	open list:Hyper-V CORE AND DRIVERS, open list

On Thu, Oct 24, 2019 at 04:49:43PM +0200, Olaf Hering wrote:
>The hostname is resolved just once since commit 58125210ab3b ("Tools:
>hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
>responds within the timeout limits to requests from the host.
>
>If for some reason getaddrinfo fails, the string returned by the
>"FullyQualifiedDomainName" request contains some error string, which is
>then used by tools on the host side.
>
>Adjust the code to resolve the current hostname in a separate thread.
>This thread loops until getaddrinfo returns success. During this time
>all "FullyQualifiedDomainName" requests will be answered with an empty
>string.
>
>Signed-off-by: Olaf Hering <olaf@aepfle.de>

Thank for the patch Olaf. However, it seems to break build for me:

/usr/bin/ld: hv_kvp_daemon-in.o: in function `kvp_obtain_domain_name':
/home/sasha/linux/tools/hv/hv_kvp_daemon.c:1372: undefined reference to `pthread_create'
/usr/bin/ld: /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1377: undefined reference to `pthread_detach'
collect2: error: ld returned 1 exit status

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-10-28 16:17 ` Sasha Levin
@ 2019-10-28 17:49   ` Olaf Hering
  2019-11-01 20:17     ` Olaf Hering
  2019-11-02  4:18     ` Sasha Levin
  0 siblings, 2 replies; 12+ messages in thread
From: Olaf Hering @ 2019-10-28 17:49 UTC (permalink / raw)
  To: Sasha Levin
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	open list:Hyper-V CORE AND DRIVERS, open list

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

Am Mon, 28 Oct 2019 12:17:54 -0400
schrieb Sasha Levin <sashal@kernel.org>:

> undefined reference to `pthread_create'

Does "make V=1 -C tools/hv" work for you?
This is what I use.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-10-28 17:49   ` Olaf Hering
@ 2019-11-01 20:17     ` Olaf Hering
  2019-11-02  4:18     ` Sasha Levin
  1 sibling, 0 replies; 12+ messages in thread
From: Olaf Hering @ 2019-11-01 20:17 UTC (permalink / raw)
  To: Sasha Levin
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	open list:Hyper-V CORE AND DRIVERS, open list

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]

Am Mon, 28 Oct 2019 18:49:55 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> Am Mon, 28 Oct 2019 12:17:54 -0400
> schrieb Sasha Levin <sashal@kernel.org>:
> > undefined reference to `pthread_create'  
> Does "make V=1 -C tools/hv" work for you?

Did you have a chance to check why the patch fails for you?


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-10-28 17:49   ` Olaf Hering
  2019-11-01 20:17     ` Olaf Hering
@ 2019-11-02  4:18     ` Sasha Levin
  2019-11-11 15:55       ` Olaf Hering
  1 sibling, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2019-11-02  4:18 UTC (permalink / raw)
  To: Olaf Hering
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	open list:Hyper-V CORE AND DRIVERS, open list

On Mon, Oct 28, 2019 at 06:49:55PM +0100, Olaf Hering wrote:
>Am Mon, 28 Oct 2019 12:17:54 -0400
>schrieb Sasha Levin <sashal@kernel.org>:
>
>> undefined reference to `pthread_create'
>
>Does "make V=1 -C tools/hv" work for you?
>This is what I use.

It works before I apply this patch, but breaks after:

$ make V=1 -C tools/hv
make: Entering directory '/home/sasha/linux/tools/hv'
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_kvp_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
  gcc -Wp,-MD,./.hv_kvp_daemon.o.d -Wp,-MT,hv_kvp_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_kvp_daemon.o hv_kvp_daemon.c
   ld -r -o hv_kvp_daemon-in.o  hv_kvp_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude  hv_kvp_daemon-in.o -o hv_kvp_daemon
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_vss_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
  gcc -Wp,-MD,./.hv_vss_daemon.o.d -Wp,-MT,hv_vss_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_vss_daemon.o hv_vss_daemon.c
   ld -r -o hv_vss_daemon-in.o  hv_vss_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude  hv_vss_daemon-in.o -o hv_vss_daemon
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_fcopy_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
  gcc -Wp,-MD,./.hv_fcopy_daemon.o.d -Wp,-MT,hv_fcopy_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_fcopy_daemon.o hv_fcopy_daemon.c
   ld -r -o hv_fcopy_daemon-in.o  hv_fcopy_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude  hv_fcopy_daemon-in.o -o hv_fcopy_daemon
make: Leaving directory '/home/sasha/linux/tools/hv'
$ git am ~/incoming/_PATCH_v1_tools-hv_async_name_resolution_in_kvp_daemon.patch
Applying: tools/hv: async name resolution in kvp_daemon
$ make V=1 -C tools/hv
make: Entering directory '/home/sasha/linux/tools/hv'
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_kvp_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
  gcc -Wp,-MD,./.hv_kvp_daemon.o.d -Wp,-MT,hv_kvp_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_kvp_daemon.o hv_kvp_daemon.c
   ld -r -o hv_kvp_daemon-in.o  hv_kvp_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude -lpthread hv_kvp_daemon-in.o -o hv_kvp_daemon
/usr/bin/ld: hv_kvp_daemon-in.o: in function `kvp_obtain_domain_name':
/home/sasha/linux/tools/hv/hv_kvp_daemon.c:1372: undefined reference to `pthread_create'
/usr/bin/ld: /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1377: undefined reference to `pthread_detach'
collect2: error: ld returned 1 exit status
make: *** [Makefile:36: hv_kvp_daemon] Error 1
make: Leaving directory '/home/sasha/linux/tools/hv'

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-10-24 14:49 [PATCH v1] tools/hv: async name resolution in kvp_daemon Olaf Hering
  2019-10-28 16:17 ` Sasha Levin
@ 2019-11-07 13:39 ` Vitaly Kuznetsov
  2019-11-07 13:48   ` Olaf Hering
  1 sibling, 1 reply; 12+ messages in thread
From: Vitaly Kuznetsov @ 2019-11-07 13:39 UTC (permalink / raw)
  To: Olaf Hering, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Sasha Levin, open list\:Hyper-V CORE AND DRIVERS, open list
  Cc: Olaf Hering

Olaf Hering <olaf@aepfle.de> writes:

> The hostname is resolved just once since commit 58125210ab3b ("Tools:
> hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
> responds within the timeout limits to requests from the host.
>
> If for some reason getaddrinfo fails, the string returned by the
> "FullyQualifiedDomainName" request contains some error string, which is
> then used by tools on the host side.
>
> Adjust the code to resolve the current hostname in a separate thread.
> This thread loops until getaddrinfo returns success. During this time
> all "FullyQualifiedDomainName" requests will be answered with an empty
> string.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/hv/Makefile        |  2 ++
>  tools/hv/hv_kvp_daemon.c | 69 ++++++++++++++++++++++++++++++++----------------
>  2 files changed, 48 insertions(+), 23 deletions(-)
>
> diff --git a/tools/hv/Makefile b/tools/hv/Makefile
> index b57143d9459c..3b5481015a84 100644
> --- a/tools/hv/Makefile
> +++ b/tools/hv/Makefile
> @@ -22,6 +22,8 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
>  
>  ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
>  
> +$(OUTPUT)hv_kvp_daemon: LDFLAGS += -lpthread
> +
>  all: $(ALL_PROGRAMS)
>  
>  export srctree OUTPUT CC LD CFLAGS
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index e9ef4ca6a655..22cf1c4dbf5c 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -41,6 +41,7 @@
>  #include <net/if.h>
>  #include <limits.h>
>  #include <getopt.h>
> +#include <pthread.h>
>  
>  /*
>   * KVP protocol: The user mode component first registers with the
> @@ -85,7 +86,7 @@ static char *processor_arch;
>  static char *os_build;
>  static char *os_version;
>  static char *lic_version = "Unknown version";
> -static char full_domain_name[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
> +static char *full_domain_name;
>  static struct utsname uts_buf;
>  
>  /*
> @@ -1327,27 +1328,53 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
>  	return error;
>  }
>  
> -
> -static void
> -kvp_get_domain_name(char *buffer, int length)
> +/*
> + * Async retrival of Fully Qualified Domain Name because getaddrinfo takes an
> + * unpredictable amount of time to finish.
> + */
> +static void *kvp_getaddrinfo(void *p)
>  {
> -	struct addrinfo	hints, *info ;
> -	int error = 0;
> +	char *tmp, **str_ptr = (char **)p;
> +	char hostname[HOST_NAME_MAX + 1];
> +	struct addrinfo	*info, hints = {
> +		.ai_family = AF_INET, /* Get only ipv4 addrinfo. */
> +		.ai_socktype = SOCK_STREAM,
> +		.ai_flags = AI_CANONNAME,
> +	};
> +	int ret;
> +
> +	if (gethostname(hostname, sizeof(hostname) - 1) < 0)
> +		goto out;
> +
> +	do {
> +		ret = getaddrinfo(hostname, NULL, &hints, &info);
> +		if (ret)
> +			sleep(1);

Is it only EAI_AGAIN or do you see any other return values which justify
the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
be infinitely looping with EAI_FAIL.

> +	} while (ret);
> +
> +	ret = asprintf(&tmp, "%s", info->ai_canonname);
> +	freeaddrinfo(info);
> +	if (ret <= 0)
> +		goto out;
> +
> +	if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
> +		tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
> +	*str_ptr = tmp;
>  
> -	gethostname(buffer, length);
> -	memset(&hints, 0, sizeof(hints));
> -	hints.ai_family = AF_INET; /*Get only ipv4 addrinfo. */
> -	hints.ai_socktype = SOCK_STREAM;
> -	hints.ai_flags = AI_CANONNAME;
> +out:
> +	pthread_exit(NULL);
> +}
> +
> +static void kvp_obtain_domain_name(char **str_ptr)
> +{
> +	pthread_t t;
>  
> -	error = getaddrinfo(buffer, NULL, &hints, &info);
> -	if (error != 0) {
> -		snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
> -			error, gai_strerror(error));
> +	if (pthread_create(&t, NULL, kvp_getaddrinfo, str_ptr)) {
> +		syslog(LOG_ERR, "pthread_create failed; error: %d %s",
> +			errno, strerror(errno));
>  		return;
>  	}
> -	snprintf(buffer, length, "%s", info->ai_canonname);
> -	freeaddrinfo(info);
> +	pthread_detach(t);

I think this should be complemented with pthread_cancel/pthread_join
before exiting main().

>  }
>  
>  void print_usage(char *argv[])
> @@ -1412,11 +1439,7 @@ int main(int argc, char *argv[])
>  	 * Retrieve OS release information.
>  	 */
>  	kvp_get_os_info();
> -	/*
> -	 * Cache Fully Qualified Domain Name because getaddrinfo takes an
> -	 * unpredictable amount of time to finish.
> -	 */
> -	kvp_get_domain_name(full_domain_name, sizeof(full_domain_name));
> +	kvp_obtain_domain_name(&full_domain_name);
>  
>  	if (kvp_file_init()) {
>  		syslog(LOG_ERR, "Failed to initialize the pools");
> @@ -1571,7 +1594,7 @@ int main(int argc, char *argv[])
>  
>  		switch (hv_msg->body.kvp_enum_data.index) {
>  		case FullyQualifiedDomainName:
> -			strcpy(key_value, full_domain_name);
> +			strcpy(key_value, full_domain_name ? : "");
>  			strcpy(key_name, "FullyQualifiedDomainName");
>  			break;
>  		case IntegrationServicesVersion:

-- 
Vitaly


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-11-07 13:39 ` Vitaly Kuznetsov
@ 2019-11-07 13:48   ` Olaf Hering
  2019-11-07 14:15     ` Vitaly Kuznetsov
  0 siblings, 1 reply; 12+ messages in thread
From: Olaf Hering @ 2019-11-07 13:48 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list\:Hyper-V CORE AND DRIVERS, open list

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

Am Thu, 07 Nov 2019 14:39:11 +0100
schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:

> Olaf Hering <olaf@aepfle.de> writes:

> Is it only EAI_AGAIN or do you see any other return values which justify
> the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
> be infinitely looping with EAI_FAIL.

I currently do not have a setup that reproduces the failure.
I think if this thread loops forever, so be it.

The report I have shows "getaddrinfo failed: 0xfffffffe Name or service not known" on the host side.
And that went away within the VM once "networking was fixed", whatever this means.
But hv_kvp_daemon would report the error string forever.

> > +	pthread_detach(t);  
> I think this should be complemented with pthread_cancel/pthread_join
> before exiting main().

If the thread is detached, it is exactly that: detached. Why do you think the main thread should wait for the detached thread?

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-11-07 13:48   ` Olaf Hering
@ 2019-11-07 14:15     ` Vitaly Kuznetsov
  2019-11-07 14:20       ` Olaf Hering
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Kuznetsov @ 2019-11-07 14:15 UTC (permalink / raw)
  To: Olaf Hering
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list\:Hyper-V CORE AND DRIVERS, open list

Olaf Hering <olaf@aepfle.de> writes:

> Am Thu, 07 Nov 2019 14:39:11 +0100
> schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:
>
>> Olaf Hering <olaf@aepfle.de> writes:
>
>> Is it only EAI_AGAIN or do you see any other return values which justify
>> the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
>> be infinitely looping with EAI_FAIL.
>
> I currently do not have a setup that reproduces the failure.
> I think if this thread loops forever, so be it.
>
> The report I have shows "getaddrinfo failed: 0xfffffffe Name or service not known" on the host side.
> And that went away within the VM once "networking was fixed", whatever this means.
> But hv_kvp_daemon would report the error string forever.

Looping forever with a permanent error is pretty unusual...

>
>> > +	pthread_detach(t);  
>> I think this should be complemented with pthread_cancel/pthread_join
>> before exiting main().
>
> If the thread is detached, it is exactly that: detached. Why do you think the main thread should wait for the detached thread?

Ah, my bad: you actually can't join a detached thread, scratch my
comment.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-11-07 14:15     ` Vitaly Kuznetsov
@ 2019-11-07 14:20       ` Olaf Hering
  2019-11-07 14:44         ` Vitaly Kuznetsov
  0 siblings, 1 reply; 12+ messages in thread
From: Olaf Hering @ 2019-11-07 14:20 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list\:Hyper-V CORE AND DRIVERS, open list

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

Am Thu, 07 Nov 2019 15:15:45 +0100
schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:

> Looping forever with a permanent error is pretty unusual...

That might be true, but how would you detect a permanent error?
Just because the DNS server is gone for one hour does not mean it will be gone forever.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-11-07 14:20       ` Olaf Hering
@ 2019-11-07 14:44         ` Vitaly Kuznetsov
  2019-11-13 15:39           ` Olaf Hering
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Kuznetsov @ 2019-11-07 14:44 UTC (permalink / raw)
  To: Olaf Hering
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list\:Hyper-V CORE AND DRIVERS, open list

Olaf Hering <olaf@aepfle.de> writes:

> Am Thu, 07 Nov 2019 15:15:45 +0100
> schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:
>
>> Looping forever with a permanent error is pretty unusual...
>
> That might be true, but how would you detect a permanent error?
> Just because the DNS server is gone for one hour does not mean it will be gone forever.

'man 3 getaddrinfo' lists the following:

       EAI_ADDRFAMILY
       EAI_AGAIN
       EAI_BADFLAGS
       EAI_FAIL
       EAI_FAMILY
       EAI_MEMORY
       EAI_NODATA
       EAI_NONAME
       EAI_SERVICE
       EAI_SOCKTYPE
       EAI_SYSTEM

I *think* what you're aiming at is EAI_AGAIN and EAI_FAIL, the rest
should probably terminate the resolver thread (e.g. AF_INET is
unsupported or something like that).

-- 
Vitaly


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-11-02  4:18     ` Sasha Levin
@ 2019-11-11 15:55       ` Olaf Hering
  0 siblings, 0 replies; 12+ messages in thread
From: Olaf Hering @ 2019-11-11 15:55 UTC (permalink / raw)
  To: Sasha Levin
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	open list:Hyper-V CORE AND DRIVERS, open list

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Am Sat, 2 Nov 2019 00:18:56 -0400
schrieb Sasha Levin <sashal@kernel.org>:

> make[1]: Leaving directory '/home/sasha/linux/tools/hv'
> gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude -lpthread hv_kvp_daemon-in.o -o hv_kvp_daemon
> /usr/bin/ld: hv_kvp_daemon-in.o: in function `kvp_obtain_domain_name':
> /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1372: undefined reference to `pthread_create'
> /usr/bin/ld: /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1377: undefined reference to `pthread_detach'

Is perhaps '-pthread' instead of -lpthread' required, as indicated by pthread_create(3)?
Not sure why it happens to work for me. But I will make this change for the upcoming v2.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
  2019-11-07 14:44         ` Vitaly Kuznetsov
@ 2019-11-13 15:39           ` Olaf Hering
  0 siblings, 0 replies; 12+ messages in thread
From: Olaf Hering @ 2019-11-13 15:39 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list\:Hyper-V CORE AND DRIVERS, open list

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

Am Thu, 07 Nov 2019 15:44:27 +0100
schrieb Vitaly Kuznetsov <vkuznets@redhat.com>:

> I *think* what you're aiming at is EAI_AGAIN and EAI_FAIL, the rest
> should probably terminate the resolver thread (e.g. AF_INET is
> unsupported or something like that).

The thread aims for success. Resolving of the hostname can take some time.
Maybe the network is not fully functional when the thread starts.
Maybe the VM admin does further tweaks while the thread is running.
IMO there is no downside to wait for success.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 14:49 [PATCH v1] tools/hv: async name resolution in kvp_daemon Olaf Hering
2019-10-28 16:17 ` Sasha Levin
2019-10-28 17:49   ` Olaf Hering
2019-11-01 20:17     ` Olaf Hering
2019-11-02  4:18     ` Sasha Levin
2019-11-11 15:55       ` Olaf Hering
2019-11-07 13:39 ` Vitaly Kuznetsov
2019-11-07 13:48   ` Olaf Hering
2019-11-07 14:15     ` Vitaly Kuznetsov
2019-11-07 14:20       ` Olaf Hering
2019-11-07 14:44         ` Vitaly Kuznetsov
2019-11-13 15:39           ` Olaf Hering

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git