zephyr: support IPv6-only devices#116
Open
Damian-Nordic wants to merge 1 commit into
Open
Conversation
34c5dda to
f97d274
Compare
Contributor
|
@Damian-Nordic thanks for the patch! what do you think about just falling back on IPv6 if IPv4 is not enabled, instead of having explicit Kconfig for it? or is it preferable to have granular control over it? |
Do not require IPv4 support to communicate with the Memfault cloud. Devices that do not support IPv4 are still able to reach the cloud, either using the cloud's IPv6 address, or NAT64 service deployed on the intermediate router. Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
Author
|
@noahp Right, I thought more flexibility would be better, but I can't really think of a real world scenario for that, so I agree to keep it simple. Will update the PR. |
f97d274 to
a9930c4
Compare
Contributor
|
Hi @Damian-Nordic apologies for the radio silence, after some more testing, I ended up with this patch below. Would you mind giving it a test to make sure it works in your configuration/network? diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2bf73f36c1..b2ea918dd6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,17 @@ and this project adheres to
certificates in the SDK. This certificate is no longer needed to connect to
the Memfault cloud.
+### 🐛 Fixed
+
+- Zephyr:
+
+ - Fixed HTTP connections on IPv6-only and dual-stack networks. Previously,
+ `getaddrinfo` was hardcoded to `AF_INET`, causing failures on IPv6-only
+ networks. HTTP client code is updated to support any IPv4/IPv6
+ configuration. Thanks to [@DamianNordic](https://github.com/DamianNordic)
+ for providing the fix in
+ [#116](https://github.com/memfault/memfault-firmware-sdk/pull/116) 🎉!
+
### 📈 Added
- Zephyr:
diff --git a/ports/zephyr/common/memfault_platform_http.c b/ports/zephyr/common/memfault_platform_http.c
index bc29312151..ffa4756aa3 100644
--- a/ports/zephyr/common/memfault_platform_http.c
+++ b/ports/zephyr/common/memfault_platform_http.c
@@ -187,8 +187,13 @@ static bool prv_send_data(const void *data, size_t data_len, void *ctx) {
}
static int prv_getaddrinfo(struct zsock_addrinfo **res, const char *host, int port_num) {
+ // Use AF_UNSPEC when both stacks are enabled so the OS returns both A and AAAA records and we
+ // can try each in turn. Fall back to whichever single stack is compiled in.
+ const int ai_family = (IS_ENABLED(CONFIG_NET_IPV4) && IS_ENABLED(CONFIG_NET_IPV6)) ? AF_UNSPEC :
+ IS_ENABLED(CONFIG_NET_IPV6) ? AF_INET6 :
+ AF_INET;
struct zsock_addrinfo hints = {
- .ai_family = AF_INET,
+ .ai_family = ai_family,
.ai_socktype = SOCK_STREAM,
};
@@ -200,27 +205,27 @@ static int prv_getaddrinfo(struct zsock_addrinfo **res, const char *host, int po
MEMFAULT_LOG_ERROR("DNS lookup for %s failed: %d", host, rv);
} else {
#if CONFIG_MEMFAULT_LOG_LEVEL >= 4 // DBG
- struct sockaddr_in *addr = net_sin((*res)->ai_addr);
+ char addr_str[INET6_ADDRSTRLEN];
+ const int af = (*res)->ai_family;
+ const void *addr_ptr = (af == AF_INET) ? (const void *)&net_sin((*res)->ai_addr)->sin_addr :
+ (const void *)&net_sin6((*res)->ai_addr)->sin6_addr;
+ zsock_inet_ntop(af, addr_ptr, addr_str, sizeof(addr_str));
+ MEMFAULT_LOG_DEBUG("DNS lookup for %s = %s", host, addr_str);
#endif
- MEMFAULT_LOG_DEBUG("DNS lookup for %s = %d.%d.%d.%d", host, addr->sin_addr.s4_addr[0],
- addr->sin_addr.s4_addr[1], addr->sin_addr.s4_addr[2],
- addr->sin_addr.s4_addr[3]);
}
return rv;
}
-static int prv_create_socket(struct zsock_addrinfo **res, const char *host, int port_num) {
+// Creates a socket for a single addrinfo entry and binds it to the configured interface (if any).
+// Does not perform DNS resolution or connection.
+static int prv_create_socket_for_addr(struct zsock_addrinfo *addr) {
const int protocol = g_mflt_http_client_config.disable_tls ? IPPROTO_TCP : IPPROTO_TLS_1_2;
- int rv = prv_getaddrinfo(res, host, port_num);
- if (rv) {
- return rv;
- }
-
- int fd = zsock_socket((*res)->ai_family, (*res)->ai_socktype, protocol);
+ int fd = zsock_socket(addr->ai_family, addr->ai_socktype, protocol);
if (fd < 0) {
MEMFAULT_LOG_ERROR("Failed to open socket, errno=%d", errno);
+ return fd;
}
#if defined(CONFIG_MEMFAULT_HTTP_SOCKET_DISPATCH)
@@ -233,6 +238,7 @@ static int prv_create_socket(struct zsock_addrinfo **res, const char *host, int
static struct ifreq ifreq = { 0 };
strncpy(ifreq.ifr_name, s_mflt_http_net_interface_name, IFNAMSIZ);
+ int rv = 0;
#if defined(CONFIG_NET_INTERFACE_NAME)
// Use the name utilities if available to get the interface index for debug
rv = net_if_get_by_name(ifreq.ifr_name);
@@ -254,6 +260,15 @@ static int prv_create_socket(struct zsock_addrinfo **res, const char *host, int
return fd;
}
+static int prv_create_socket(struct zsock_addrinfo **res, const char *host, int port_num) {
+ int rv = prv_getaddrinfo(res, host, port_num);
+ if (rv) {
+ return rv;
+ }
+
+ return prv_create_socket_for_addr(*res);
+}
+
#if !defined(CONFIG_MEMFAULT_HTTP_DISABLE_TLS)
static int prv_configure_tls_socket(int sock_fd, const char *host) {
@@ -362,22 +377,36 @@ static bool prv_try_send(int sock_fd, const uint8_t *buf, size_t buf_len) {
}
static int prv_open_socket(struct zsock_addrinfo **res, const char *host, int port_num) {
- const int sock_fd = prv_create_socket(res, host, port_num);
- if (sock_fd < 0) {
+ int rv = prv_getaddrinfo(res, host, port_num);
+ if (rv != 0) {
return -1;
}
- int rv = prv_configure_socket(sock_fd, host);
- if (rv < 0) {
- zsock_close(sock_fd);
- return -1;
+ for (struct zsock_addrinfo *addr = *res; addr != NULL; addr = addr->ai_next) {
+ int fd = prv_create_socket_for_addr(addr);
+ if (fd < 0) {
+ continue;
+ }
+
+ rv = prv_configure_socket(fd, host);
+ if (rv < 0) {
+ zsock_close(fd);
+ continue;
+ }
+
+ rv = zsock_connect(fd, addr->ai_addr, addr->ai_addrlen);
+ if (rv < 0) {
+ MEMFAULT_LOG_DEBUG("Connect to %s (af=%d) failed, errno=%d, trying next address", host,
+ addr->ai_family, errno);
+ zsock_close(fd);
+ continue;
+ }
+
+ return fd;
}
- rv = prv_connect_socket(sock_fd, *res);
- if (rv < 0) {
- return -1;
- }
- return sock_fd;
+ MEMFAULT_LOG_ERROR("Failed to connect to %s on any address", host);
+ return -1;
}
//! Returns: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not require IPv4 support to communicate with the Memfault cloud. Devices that do not support IPv4 are still able to reach the cloud, either using the cloud's IPv6 address, or NAT64 service deployed on the intermediate router.