linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] tools/hv: async name resolution in kvp_daemon
@ 2021-03-19 14:41 Olaf Hering
  2021-03-19 14:46 ` Olaf Hering
  0 siblings, 1 reply; 5+ messages in thread
From: Olaf Hering @ 2021-03-19 14:41 UTC (permalink / raw)
  To: linux-hyperv, linux-kernel
  Cc: Olaf Hering, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu

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>
---
v2:
 resend, the thread aims for success.

 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 1e6fd6ca513b..3951b927aa3d 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;
 
-	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;
+	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;
 
-	error = getaddrinfo(buffer, NULL, &hints, &info);
-	if (error != 0) {
-		snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
-			error, gai_strerror(error));
+	if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
+		tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
+	*str_ptr = tmp;
+
+out:
+	pthread_exit(NULL);
+}
+
+static void kvp_obtain_domain_name(char **str_ptr)
+{
+	pthread_t t;
+
+	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[])
@@ -1404,11 +1431,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");
@@ -1573,7 +1596,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 related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tools/hv: async name resolution in kvp_daemon
  2021-03-19 14:41 [PATCH v2] tools/hv: async name resolution in kvp_daemon Olaf Hering
@ 2021-03-19 14:46 ` Olaf Hering
  0 siblings, 0 replies; 5+ messages in thread
From: Olaf Hering @ 2021-03-19 14:46 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu
  Cc: linux-hyperv, linux-kernel

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

Am Fri, 19 Mar 2021 15:41:44 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> FullyQualifiedDomainName

I think in the past I did asked MSFT what the host side really expects. Maybe this time there will be an answer.

Why would the host expect a FQDN from a VM? Why would it care about DNS layout of the network within the VM?

Basically my copy of hv_kvp_daemon just sends `uname -n` to the host. This is more correct. This does not waste any network resources. This, up to now, led to no complains.

So, what is the purpose of this API?


Olaf

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

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

* Re: [PATCH v2] tools/hv: async name resolution in kvp_daemon
  2019-11-13 15:54 Olaf Hering
  2019-11-13 16:00 ` Olaf Hering
@ 2020-01-03 13:31 ` Olaf Hering
  1 sibling, 0 replies; 5+ messages in thread
From: Olaf Hering @ 2020-01-03 13:31 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list:Hyper-V CORE AND DRIVERS, open list

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

Am Wed, 13 Nov 2019 16:54:00 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> Adjust the code to resolve the current hostname in a separate thread.

Are you now finally able to compile the patch?
Did you find some documentation about the host-side API of the "FullyQualifiedDomainName" request?
Any further comments about the "aim for success" approach?


Thanks,
Olaf

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

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

* Re: [PATCH v2] tools/hv: async name resolution in kvp_daemon
  2019-11-13 15:54 Olaf Hering
@ 2019-11-13 16:00 ` Olaf Hering
  2020-01-03 13:31 ` Olaf Hering
  1 sibling, 0 replies; 5+ messages in thread
From: Olaf Hering @ 2019-11-13 16:00 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger
  Cc: Sasha Levin, open list:Hyper-V CORE AND DRIVERS, open list

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

Am Wed, 13 Nov 2019 16:54:00 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> During this time all "FullyQualifiedDomainName" requests will be
> answered with an empty string.

> +			strcpy(key_value, full_domain_name ? : "");
>  			strcpy(key_name, "FullyQualifiedDomainName");


KY, Haiyang, Stephen,

please check if the host side can actually handle such temporary, empty string.
If the tools cache the first result, the implemented approach can not work.
If that is indeed the case, is there some sort of -ERETRY in the protocol?


Olaf

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

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

* [PATCH v2] tools/hv: async name resolution in kvp_daemon
@ 2019-11-13 15:54 Olaf Hering
  2019-11-13 16:00 ` Olaf Hering
  2020-01-03 13:31 ` Olaf Hering
  0 siblings, 2 replies; 5+ messages in thread
From: Olaf Hering @ 2019-11-13 15:54 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list:Hyper-V CORE AND DRIVERS, open list
  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>
---

v2:
- link with -pthread instead of -lpthread
- consider some errors from getaddrinfo fatal, log them via syslog
- update also hostname in the loop

 tools/hv/Makefile        |  2 ++
 tools/hv/hv_kvp_daemon.c | 83 ++++++++++++++++++++++++++++++++++--------------
 2 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/tools/hv/Makefile b/tools/hv/Makefile
index b57143d9459c..9bbab96ac06d 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 += -pthread
+
 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..b930506a9632 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,67 @@ 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;
 
-	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;
+	do {
+		if (gethostname(hostname, sizeof(hostname) - 1) < 0)
+			goto out;
 
-	error = getaddrinfo(buffer, NULL, &hints, &info);
-	if (error != 0) {
-		snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
-			error, gai_strerror(error));
+		ret = getaddrinfo(hostname, NULL, &hints, &info);
+		switch (ret) {
+		case 0:
+			break;
+		case EAI_BADFLAGS:
+		case EAI_MEMORY:
+		case EAI_OVERFLOW:
+		case EAI_SOCKTYPE:
+		case EAI_SYSTEM:
+			/* Permanent failure */
+			syslog(LOG_ERR, "getaddrinfo failed: %d %s",
+				ret, gai_strerror(ret));
+			goto out;
+		default:
+			/* Temporary failure, aim for success. */
+			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;
+
+out:
+	pthread_exit(NULL);
+}
+
+static void kvp_obtain_domain_name(char **str_ptr)
+{
+	pthread_t t;
+
+	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 +1453,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 +1608,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 related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-19 14:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 14:41 [PATCH v2] tools/hv: async name resolution in kvp_daemon Olaf Hering
2021-03-19 14:46 ` Olaf Hering
  -- strict thread matches above, loose matches on Subject: below --
2019-11-13 15:54 Olaf Hering
2019-11-13 16:00 ` Olaf Hering
2020-01-03 13:31 ` Olaf Hering

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).