From e86be05f742566e3c2fc090838115b339bcd34a4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 5 Sep 2026 13:45:12 +0200 Subject: [PATCH] Share the lwIP error mapping and address conversion, keep poll_connect in one place, and pin the socket layout --- .../components/async_tcp/async_tcp_socket.cpp | 3 +- .../components/socket/lwip_raw_tcp_impl.cpp | 122 +++++++----------- esphome/components/socket/lwip_raw_tcp_impl.h | 5 + esphome/components/socket/socket.cpp | 2 +- esphome/components/socket/socket.h | 4 + esphome/components/udp/udp_component.cpp | 10 +- 6 files changed, 62 insertions(+), 84 deletions(-) diff --git a/esphome/components/async_tcp/async_tcp_socket.cpp b/esphome/components/async_tcp/async_tcp_socket.cpp index 9af09da14e..e6e0ccd888 100644 --- a/esphome/components/async_tcp/async_tcp_socket.cpp +++ b/esphome/components/async_tcp/async_tcp_socket.cpp @@ -6,7 +6,6 @@ #include "esphome/components/network/util.h" #include "esphome/core/log.h" #include -#include namespace esphome::async_tcp { @@ -47,7 +46,7 @@ bool AsyncClient::connect(const char *host, uint16_t port) { // connect()/read() would otherwise stall the whole loop const int saved_errno = errno; ESP_LOGE(TAG, "Failed to set nonblocking: errno %d", saved_errno); - socket_.reset(); + close(); if (error_cb_) error_cb_(error_arg_, this, saved_errno); return false; diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index cca251e87c..b6c37af336 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -48,8 +48,33 @@ static const char *const TAG = "socket"; #ifdef USE_ESP8266 // optimistic_yield() rate limit in microseconds of CONT time; cheap when hot. static constexpr uint32_t ESP8266_YIELD_INTERVAL_US = 1000; +// Let SYS run so queued WiFi traffic reaches lwip; CONT and SYS are cooperative +static inline void yield_to_sys() { optimistic_yield(ESP8266_YIELD_INTERVAL_US); } +#else +static inline void yield_to_sys() {} #endif +// errno for a failed tcp_* call +static int lwip_err_to_errno(err_t err) { + switch (err) { + case ERR_MEM: + return ENOMEM; + case ERR_BUF: + return EAGAIN; // no free local port + case ERR_RTE: + return EHOSTUNREACH; // no route or no address yet + case ERR_VAL: + case ERR_ARG: + return EINVAL; + case ERR_USE: + return EADDRINUSE; + case ERR_ISCONN: + return EISCONN; + default: + return EIO; + } +} + // set to 1 to enable verbose lwip logging #if 0 // NOLINT(readability-avoid-unconditional-preprocessor-if) #define LWIP_LOG(msg, ...) ESP_LOGVV(TAG, "socket %p: " msg, this, ##__VA_ARGS__) @@ -108,17 +133,6 @@ bool LWIPRawCommon::sockaddr2ip_(const struct sockaddr *name, socklen_t addrlen, return false; } #if LWIP_IPV6 - if (this->family_ == AF_INET) { - if (addrlen < sizeof(sockaddr_in)) { - errno = EINVAL; - return false; - } - auto *addr4 = reinterpret_cast(name); - *port = ntohs(addr4->sin_port); - ip->type = IPADDR_TYPE_V4; - ip->u_addr.ip4.addr = addr4->sin_addr.s_addr; - return true; - } if (this->family_ == AF_INET6) { if (addrlen < sizeof(sockaddr_in6)) { errno = EINVAL; @@ -126,23 +140,20 @@ bool LWIPRawCommon::sockaddr2ip_(const struct sockaddr *name, socklen_t addrlen, } auto *addr6 = reinterpret_cast(name); *port = ntohs(addr6->sin6_port); + inet6_addr_to_ip6addr(ip_2_ip6(ip), &addr6->sin6_addr); // ANY lets bind() accept both families; connect() picks the concrete type - ip->type = IPADDR_TYPE_ANY; - memcpy(&ip->u_addr.ip6.addr, &addr6->sin6_addr.un.u8_addr, 16); + IP_SET_TYPE_VAL(*ip, IPADDR_TYPE_ANY); return true; } - errno = EINVAL; - return false; -#else +#endif if (this->family_ != AF_INET || addrlen < sizeof(sockaddr_in)) { errno = EINVAL; return false; } auto *addr4 = reinterpret_cast(name); *port = ntohs(addr4->sin_port); - ip->addr = addr4->sin_addr.s_addr; + ip_addr_set_ip4_u32(ip, addr4->sin_addr.s_addr); return true; -#endif } int LWIPRawCommon::bind(const struct sockaddr *name, socklen_t addrlen) { @@ -158,19 +169,9 @@ int LWIPRawCommon::bind(const struct sockaddr *name, socklen_t addrlen) { } LWIP_LOG("tcp_bind(%p ip=%s port=%u)", this->pcb_, ipaddr_ntoa(&ip), port); err_t err = tcp_bind(this->pcb_, &ip, port); - if (err == ERR_USE) { - LWIP_LOG(" -> err ERR_USE"); - errno = EADDRINUSE; - return -1; - } - if (err == ERR_VAL) { - LWIP_LOG(" -> err ERR_VAL"); - errno = EINVAL; - return -1; - } if (err != ERR_OK) { LWIP_LOG(" -> err %d", err); - errno = EIO; + errno = lwip_err_to_errno(err); return -1; } return 0; @@ -187,7 +188,7 @@ int LWIPRawCommon::close() { this->pcb_ = nullptr; if (err != ERR_OK) { LWIP_LOG(" -> err %d", err); - errno = err == ERR_MEM ? ENOMEM : EIO; + errno = lwip_err_to_errno(err); return -1; } return 0; @@ -214,7 +215,7 @@ int LWIPRawCommon::shutdown(int how) { err_t err = tcp_shutdown(this->pcb_, shut_rx, shut_tx); if (err != ERR_OK) { LWIP_LOG(" -> err %d", err); - errno = err == ERR_MEM ? ENOMEM : EIO; + errno = lwip_err_to_errno(err); return -1; } return 0; @@ -445,10 +446,10 @@ void LWIPRawImpl::s_err_fn(void *arg, err_t err) { } err_t LWIPRawImpl::s_connected_fn(void *arg, struct tcp_pcb *pcb, err_t err) { - // LWIP CALLBACK — same constraints as s_err_fn. lwip always passes ERR_OK - // here; a failed connect arrives through s_err_fn instead. + // LWIP CALLBACK — same constraints as s_err_fn. err is always ERR_OK; a + // failed connect arrives through s_err_fn instead. auto *arg_this = reinterpret_cast(arg); - arg_this->connect_err_ = err == ERR_OK ? 0 : ECONNRESET; + arg_this->connect_err_ = 0; esphome::wake_loop_any_context(); return ERR_OK; } @@ -482,45 +483,24 @@ int LWIPRawImpl::connect(const struct sockaddr *addr, socklen_t addrlen) { #endif LWIP_LOG("tcp_connect(%p ip=%s port=%u)", this->pcb_, ipaddr_ntoa(&ip), port); err_t err = tcp_connect(this->pcb_, &ip, port, LWIPRawImpl::s_connected_fn); - switch (err) { - case ERR_OK: - this->connect_err_ = EINPROGRESS; - errno = EINPROGRESS; - return -1; - case ERR_RTE: - errno = EHOSTUNREACH; // no route or no address yet; callers retry - break; - case ERR_USE: - errno = EADDRINUSE; - break; - case ERR_ISCONN: - errno = EISCONN; - break; - case ERR_BUF: - errno = EAGAIN; // no free local port - break; - case ERR_MEM: - errno = ENOMEM; - break; - default: - errno = EINVAL; - break; + if (err != ERR_OK) { + LWIP_LOG(" -> err %d", err); + errno = lwip_err_to_errno(err); + return -1; } - LWIP_LOG(" -> err %d", err); + this->connect_err_ = EINPROGRESS; + errno = EINPROGRESS; return -1; } ConnectPollResult LWIPRawImpl::poll_connect(int &err_out) const { - // pcb_ first: s_err_fn records the reason before it clears the pcb + // pcb_ first; see the ordering note on the declaration if (this->pcb_ == nullptr) { err_out = this->connect_err_ == 0 || this->connect_err_ == EINPROGRESS ? ECONNRESET : this->connect_err_; return ConnectPollResult::CONNECT_POLL_ERROR; } if (this->connect_err_ == EINPROGRESS) { -#ifdef USE_ESP8266 - // Let SYS process the SYN-ACK between polls; see read() - optimistic_yield(ESP8266_YIELD_INTERVAL_US); -#endif + yield_to_sys(); // so the SYN-ACK is processed between polls return ConnectPollResult::CONNECT_POLL_PENDING; } if (this->connect_err_ != 0) { @@ -530,8 +510,6 @@ ConnectPollResult LWIPRawImpl::poll_connect(int &err_out) const { return ConnectPollResult::CONNECT_POLL_CONNECTED; } -ConnectPollResult poll_connect(Socket &sock, int &err_out) { return sock.poll_connect(err_out); } - err_t LWIPRawImpl::s_recv_fn(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err) { auto *arg_this = reinterpret_cast(arg); return arg_this->recv_fn(pb, err); @@ -644,14 +622,12 @@ ssize_t LWIPRawImpl::read_locked_(void *buf, size_t len) { } ssize_t LWIPRawImpl::read(void *buf, size_t len) { -#ifdef USE_ESP8266 - // Would block: yield to SYS so queued WiFi RX reaches lwip and this read - // may succeed. Without this, inbound segments can sit unprocessed for - // seconds while the main loop polls (CONT/SYS are cooperative on ESP8266). + // Would block: let queued WiFi RX reach lwip first so this read may + // succeed; otherwise inbound segments can sit unprocessed for seconds + // while the main loop polls if (this->waiting_for_data_()) { - optimistic_yield(ESP8266_YIELD_INTERVAL_US); + yield_to_sys(); } -#endif // See waiting_for_data_() for safety of unlocked reads. if (this->recv_timeout_cs_ > 0 && this->waiting_for_data_()) { this->wait_for_data_(); @@ -740,12 +716,10 @@ int LWIPRawImpl::internal_output_() { return -1; } } -#ifdef USE_ESP8266 // Flushed: yield to SYS so the queued segments reach the WiFi driver // instead of waiting seconds for an unrelated SYS slot. Callers only get // here after a successful tcp_write, so idle paths never yield. - optimistic_yield(ESP8266_YIELD_INTERVAL_US); -#endif + yield_to_sys(); return 0; } diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index 13665d8d34..7028723056 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -68,6 +68,9 @@ class LWIPRawCommon { static_assert(EINPROGRESS < 256 && ECONNREFUSED < 256 && ECONNRESET < 256 && ETIMEDOUT < 256, "connect_err_ stores errno values in a byte"); }; +// The connect state must stay inside the padding: no socket, listening or +// accepted, pays RAM for it +static_assert(sizeof(LWIPRawCommon) == sizeof(struct tcp_pcb *) + 4, "LWIPRawCommon grew past one word of flags"); /// Connected socket implementation for LWIP raw TCP. /// No virtual methods — callers always use the concrete type. @@ -154,6 +157,8 @@ class LWIPRawImpl : public LWIPRawCommon { size_t rx_buf_offset_ = 0; bool rx_closed_ = false; }; +static_assert(sizeof(LWIPRawImpl) == sizeof(LWIPRawCommon) + sizeof(pbuf *) + sizeof(size_t) + sizeof(void *), + "LWIPRawImpl layout changed"); /// Listening socket implementation for LWIP raw TCP. /// Separate from LWIPRawImpl — no virtual dispatch needed. diff --git a/esphome/components/socket/socket.cpp b/esphome/components/socket/socket.cpp index 9ed6230c57..d539222843 100644 --- a/esphome/components/socket/socket.cpp +++ b/esphome/components/socket/socket.cpp @@ -226,7 +226,7 @@ ConnectPollResult poll_connect(Socket &sock, int &err_out) { err_out = errno; return ConnectPollResult::CONNECT_POLL_ERROR; } - if (ret == 0 || !FD_ISSET(fd, &writefds)) { + if (ret == 0) { return ConnectPollResult::CONNECT_POLL_PENDING; } int error = 0; diff --git a/esphome/components/socket/socket.h b/esphome/components/socket/socket.h index 2aaf9ba3f0..1e834948c3 100644 --- a/esphome/components/socket/socket.h +++ b/esphome/components/socket/socket.h @@ -149,7 +149,11 @@ socklen_t set_sockaddr_any(struct sockaddr *addr, socklen_t addrlen, uint16_t po /// CONNECT_POLL_ERROR, err_out holds the socket's SO_ERROR (or errno when the /// poll itself failed) on fd based implementations, and the failure recorded /// by the lwip callbacks on the raw lwip implementation. +#ifdef USE_SOCKET_IMPL_LWIP_TCP +inline ConnectPollResult poll_connect(Socket &sock, int &err_out) { return sock.poll_connect(err_out); } +#else ConnectPollResult poll_connect(Socket &sock, int &err_out); +#endif /// Format sockaddr into caller-provided buffer, returns length written (excluding null) size_t format_sockaddr_to(const struct sockaddr *addr_ptr, socklen_t len, std::span buf); diff --git a/esphome/components/udp/udp_component.cpp b/esphome/components/udp/udp_component.cpp index 9fca3ca6dc..f662cba4cf 100644 --- a/esphome/components/udp/udp_component.cpp +++ b/esphome/components/udp/udp_component.cpp @@ -15,14 +15,12 @@ void UDPComponent::setup() { struct sockaddr saddr {}; if (socket::set_sockaddr(&saddr, sizeof(saddr), address, this->broadcast_port_) == 0) { ESP_LOGW(TAG, "Invalid address %s", address); + // A dropped address silently receives nothing; surface the misconfiguration + this->status_set_warning(LOG_STR("invalid address")); continue; } this->sockaddrs_.push_back(saddr); } - if (this->sockaddrs_.size() != this->addresses_.size()) { - // A dropped address silently receives nothing; surface the misconfiguration - this->status_set_warning(LOG_STR("invalid address")); - } // set up broadcast socket if (this->should_broadcast_) { this->broadcast_socket_ = socket::socket(AF_INET, SOCK_DGRAM, IPPROTO_IP); @@ -103,13 +101,11 @@ void UDPComponent::setup() { auto ipaddr = IPAddress(); if (!ipaddr.fromString(address)) { ESP_LOGW(TAG, "Invalid address %s", address); + this->status_set_warning(LOG_STR("invalid address")); continue; } this->ipaddrs_.push_back(ipaddr); } - if (this->ipaddrs_.size() != this->addresses_.size()) { - this->status_set_warning(LOG_STR("invalid address")); - } if (this->should_listen_) this->udp_client_.begin(this->listen_port_); #endif