From a88e9b814663c83f1206668c9dc8b973e164f4f4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 10 Mar 2026 00:02:26 -1000 Subject: [PATCH 1/9] [socket] Fix RP2040 TCP race condition between lwip callbacks and main loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On RP2040 (Pico W), arduino-pico sets PICO_CYW43_ARCH_THREADSAFE_BACKGROUND=1, which means lwip callbacks (recv_fn, accept_fn, err_fn) run from a PendSV interrupt — not the main loop. This allows them to preempt read(), write(), close(), and accept() at any point, causing race conditions on shared state like the rx_buf_ pbuf chain. The most critical race: recv_fn calls pbuf_cat(rx_buf_, pb) while read() is freeing nodes in the same chain, leading to use-after-free and lwip's "Creating an infinite loop" assertion panic. This is the root cause of #10681. Fix: implement RP2040's LwIPLock (previously a no-op) to call cyw43_arch_lwip_begin/end, which acquires the pico-sdk async_context recursive mutex. Add LWIP_LOCK() guards to all main-loop lwip API call sites in the socket layer. On ESP8266, lwip callbacks run cooperatively from the main loop, so LwIPLock remains a no-op. Closes #10681 --- esphome/components/rp2040/helpers.cpp | 16 ++++++- .../components/socket/lwip_raw_tcp_impl.cpp | 47 +++++++++++++++++++ esphome/components/socket/lwip_raw_tcp_impl.h | 6 +++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/esphome/components/rp2040/helpers.cpp b/esphome/components/rp2040/helpers.cpp index 30b40a723a..4191c2164a 100644 --- a/esphome/components/rp2040/helpers.cpp +++ b/esphome/components/rp2040/helpers.cpp @@ -7,6 +7,7 @@ #if defined(USE_WIFI) #include +#include // For cyw43_arch_lwip_begin/end (LwIPLock) #endif #include #include @@ -44,9 +45,22 @@ void Mutex::unlock() {} IRAM_ATTR InterruptLock::InterruptLock() { state_ = save_and_disable_interrupts(); } IRAM_ATTR InterruptLock::~InterruptLock() { restore_interrupts(state_); } -// RP2040 doesn't support lwIP core locking, so this is a no-op +// On RP2040 (Pico W), arduino-pico sets PICO_CYW43_ARCH_THREADSAFE_BACKGROUND=1. +// This means lwip callbacks run from a low-priority user IRQ context, not the +// main loop (see low_priority_irq_handler() in pico-sdk +// async_context_threadsafe_background.c). cyw43_arch_lwip_begin/end acquires the +// async_context recursive mutex to prevent IRQ callbacks from firing during +// critical sections. See esphome#10681. +// +// When CYW43 is not available (non-WiFi RP2040 boards), this is a no-op since +// there's no network stack and no lwip callbacks to race with. +#if defined(USE_WIFI) +LwIPLock::LwIPLock() { cyw43_arch_lwip_begin(); } +LwIPLock::~LwIPLock() { cyw43_arch_lwip_end(); } +#else LwIPLock::LwIPLock() {} LwIPLock::~LwIPLock() {} +#endif void get_mac_address_raw(uint8_t *mac) { // NOLINT(readability-non-const-parameter) #ifdef USE_WIFI diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 445a57809d..b1ea45b82a 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -111,6 +111,24 @@ void socket_wake() { } #endif +// ---- LWIP thread safety ---- +// +// On RP2040 (Pico W), arduino-pico sets PICO_CYW43_ARCH_THREADSAFE_BACKGROUND=1. +// This means lwip callbacks (recv_fn, accept_fn, err_fn) run from a low-priority +// user IRQ context, not the main loop (see low_priority_irq_handler() in pico-sdk +// async_context_threadsafe_background.c). They can preempt main-loop code at any point. +// +// Without locking, this causes race conditions between recv_fn and read() on the +// shared rx_buf_ pbuf chain — recv_fn calls pbuf_cat() while read() is freeing +// nodes, leading to use-after-free and infinite-loop crashes. See esphome#10681. +// +// On ESP8266, lwip callbacks run from the SYS context which cooperates with user +// code (CONT context) — they never preempt each other, so no locking is needed. +// +// esphome::LwIPLock is the platform-provided RAII guard (see helpers.h/helpers.cpp). +// On RP2040, it acquires cyw43_arch_lwip_begin/end. On ESP8266, it's a no-op. +#define LWIP_LOCK() esphome::LwIPLock lwip_lock_guard // NOLINT + static const char *const TAG = "socket.lwip"; // set to 1 to enable verbose lwip logging @@ -123,6 +141,7 @@ static const char *const TAG = "socket.lwip"; // ---- LWIPRawCommon methods ---- LWIPRawCommon::~LWIPRawCommon() { + LWIP_LOCK(); if (this->pcb_ != nullptr) { LWIP_LOG("tcp_abort(%p)", this->pcb_); tcp_abort(this->pcb_); @@ -131,6 +150,7 @@ LWIPRawCommon::~LWIPRawCommon() { } int LWIPRawCommon::bind(const struct sockaddr *name, socklen_t addrlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -196,6 +216,7 @@ int LWIPRawCommon::bind(const struct sockaddr *name, socklen_t addrlen) { } int LWIPRawCommon::close() { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -214,6 +235,7 @@ int LWIPRawCommon::close() { } int LWIPRawCommon::shutdown(int how) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -240,6 +262,7 @@ int LWIPRawCommon::shutdown(int how) { } int LWIPRawCommon::getpeername(struct sockaddr *name, socklen_t *addrlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -252,6 +275,7 @@ int LWIPRawCommon::getpeername(struct sockaddr *name, socklen_t *addrlen) { } int LWIPRawCommon::getsockname(struct sockaddr *name, socklen_t *addrlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -284,6 +308,7 @@ size_t LWIPRawCommon::getsockname_to(std::span buf) { } int LWIPRawCommon::getsockopt(int level, int optname, void *optval, socklen_t *optlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -318,6 +343,7 @@ int LWIPRawCommon::getsockopt(int level, int optname, void *optval, socklen_t *o } int LWIPRawCommon::setsockopt(int level, int optname, const void *optval, socklen_t optlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -388,6 +414,7 @@ int LWIPRawCommon::ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *n // ---- LWIPRawImpl methods ---- LWIPRawImpl::~LWIPRawImpl() { + LWIP_LOCK(); // Free any received pbufs that LWIP transferred ownership of via recv_fn. // tcp_abort() in the base destructor won't free these since LWIP considers // ownership transferred once the recv callback accepts them. @@ -399,6 +426,7 @@ LWIPRawImpl::~LWIPRawImpl() { } void LWIPRawImpl::init() { + LWIP_LOCK(); LWIP_LOG("init(%p)", this->pcb_); tcp_arg(this->pcb_, this); tcp_recv(this->pcb_, LWIPRawImpl::s_recv_fn); @@ -406,6 +434,9 @@ void LWIPRawImpl::init() { } void LWIPRawImpl::s_err_fn(void *arg, err_t err) { + // Called by lwip core which already holds the async_context lock on RP2040. + // No LWIP_LOCK() needed — acquiring it would be redundant (recursive mutex). + // // "If a connection is aborted because of an error, the application is alerted of this event by // the err callback." // pcb is already freed when this callback is called @@ -422,6 +453,7 @@ err_t LWIPRawImpl::s_recv_fn(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, er } err_t LWIPRawImpl::recv_fn(struct pbuf *pb, err_t err) { + // Called by lwip core which already holds the async_context lock on RP2040. LWIP_LOG("recv(pb=%p err=%d)", pb, err); if (err != 0) { // "An error code if there has been an error receiving Only return ERR_ABRT if you have @@ -448,6 +480,7 @@ err_t LWIPRawImpl::recv_fn(struct pbuf *pb, err_t err) { } ssize_t LWIPRawImpl::read(void *buf, size_t len) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -525,6 +558,7 @@ ssize_t LWIPRawImpl::readv(const struct iovec *iov, int iovcnt) { } ssize_t LWIPRawImpl::internal_write_(const void *buf, size_t len) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; @@ -557,6 +591,11 @@ ssize_t LWIPRawImpl::internal_write_(const void *buf, size_t len) { } int LWIPRawImpl::internal_output_() { + LWIP_LOCK(); + if (this->pcb_ == nullptr) { + errno = ECONNRESET; + return -1; + } LWIP_LOG("tcp_output(%p)", this->pcb_); err_t err = tcp_output(this->pcb_); if (err == ERR_ABRT) { @@ -621,6 +660,7 @@ ssize_t LWIPRawImpl::writev(const struct iovec *iov, int iovcnt) { // ---- LWIPRawListenImpl methods ---- LWIPRawListenImpl::~LWIPRawListenImpl() { + LWIP_LOCK(); // Listen PCBs must use tcp_close(), not tcp_abort(). // tcp_abandon() asserts pcb->state != LISTEN and would access // fields that don't exist in the smaller tcp_pcb_listen struct. @@ -632,6 +672,7 @@ LWIPRawListenImpl::~LWIPRawListenImpl() { } void LWIPRawListenImpl::init() { + LWIP_LOCK(); LWIP_LOG("init(%p)", this->pcb_); tcp_arg(this->pcb_, this); tcp_accept(this->pcb_, LWIPRawListenImpl::s_accept_fn); @@ -639,6 +680,7 @@ void LWIPRawListenImpl::init() { } void LWIPRawListenImpl::s_err_fn(void *arg, err_t err) { + // Called by lwip core which already holds the async_context lock on RP2040. auto *arg_this = reinterpret_cast(arg); ESP_LOGVV(TAG, "socket %p: err(err=%d)", arg_this, err); arg_this->pcb_ = nullptr; @@ -650,6 +692,7 @@ err_t LWIPRawListenImpl::s_accept_fn(void *arg, struct tcp_pcb *newpcb, err_t er } std::unique_ptr LWIPRawListenImpl::accept(struct sockaddr *addr, socklen_t *addrlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return nullptr; @@ -674,6 +717,7 @@ std::unique_ptr LWIPRawListenImpl::accept(struct sockaddr *addr, so } int LWIPRawListenImpl::listen(int backlog) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -699,6 +743,7 @@ int LWIPRawListenImpl::listen(int backlog) { } err_t LWIPRawListenImpl::accept_fn_(struct tcp_pcb *newpcb, err_t err) { + // Called by lwip core which already holds the async_context lock on RP2040. LWIP_LOG("accept(newpcb=%p err=%d)", newpcb, err); if (err != ERR_OK || newpcb == nullptr) { // "An error code if there has been an error accepting. Only return ERR_ABRT if you have @@ -766,6 +811,8 @@ std::unique_ptr socket_listen_loop_monitored(int domain, int type, return socket_listen(domain, type, protocol); } +#undef LWIP_LOCK + } // namespace esphome::socket #endif // USE_SOCKET_IMPL_LWIP_TCP diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index c171e0537f..5b2c11cfe2 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -95,8 +95,13 @@ class LWIPRawImpl : public LWIPRawCommon { errno = ENOSYS; return -1; } + // Intentionally unlocked — this is a polling check called every loop iteration. + // A stale read at worst delays processing by one loop tick; the actual I/O in + // read() holds the lwip lock and re-checks properly. See esphome#10681. bool ready() const { return this->rx_buf_ != nullptr || this->rx_closed_ || this->pcb_ == nullptr; } + // No lock needed — only called during setup before callbacks are registered. + // A stale pcb_ read is benign (returns ECONNRESET, which the caller handles). int setblocking(bool blocking) { if (this->pcb_ == nullptr) { errno = ECONNRESET; @@ -134,6 +139,7 @@ class LWIPRawListenImpl : public LWIPRawCommon { void init(); + // Intentionally unlocked — polling check, see LWIPRawImpl::ready() comment. bool ready() const { return this->accepted_socket_count_ > 0; } std::unique_ptr accept(struct sockaddr *addr, socklen_t *addrlen); From c182c0c74f549eeccaf34355b8e319e0ca031851 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 10 Mar 2026 00:53:01 -1000 Subject: [PATCH 2/9] [socket] Hold lwip lock for entire readv/writev scatter-gather operation Avoid repeated lock acquire/release cycles per iovec element. The recursive mutex re-entry in inner calls is nearly free (counter bump), while the outer lock prevents the expensive IRQ disable/enable on each iteration. --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index b1ea45b82a..cabf546a27 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -540,6 +540,7 @@ ssize_t LWIPRawImpl::read(void *buf, size_t len) { } ssize_t LWIPRawImpl::readv(const struct iovec *iov, int iovcnt) { + LWIP_LOCK(); // Hold for entire scatter-gather operation ssize_t ret = 0; for (int i = 0; i < iovcnt; i++) { ssize_t err = this->read(reinterpret_cast(iov[i].iov_base), iov[i].iov_len); @@ -631,6 +632,7 @@ ssize_t LWIPRawImpl::write(const void *buf, size_t len) { } ssize_t LWIPRawImpl::writev(const struct iovec *iov, int iovcnt) { + LWIP_LOCK(); // Hold for entire scatter-gather operation ssize_t written = 0; for (int i = 0; i < iovcnt; i++) { ssize_t err = this->internal_write_(reinterpret_cast(iov[i].iov_base), iov[i].iov_len); From cc05bf3ed22decdc82bcaf24ae6b9224634a8963 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 10 Mar 2026 00:55:34 -1000 Subject: [PATCH 3/9] [socket] Add LWIP_LOCK to socket factory functions tcp_new() is an lwip core API call that must be bracketed with the lwip lock on RP2040 per pico-sdk docs. Add LWIP_LOCK() to socket() and socket_listen() factory functions. --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index cabf546a27..0c0d64d198 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -781,6 +781,7 @@ std::unique_ptr socket(int domain, int type, int protocol) { errno = EPROTOTYPE; return nullptr; } + LWIP_LOCK(); auto *pcb = tcp_new(); if (pcb == nullptr) return nullptr; @@ -800,6 +801,7 @@ std::unique_ptr socket_listen(int domain, int type, int protocol) errno = EPROTOTYPE; return nullptr; } + LWIP_LOCK(); auto *pcb = tcp_new(); if (pcb == nullptr) return nullptr; From 81d12fd14ae95ebe2ab9bc0935d307bda7cebd9e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 10 Mar 2026 00:57:36 -1000 Subject: [PATCH 4/9] [socket] Hold lwip lock for entire write() operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same pattern as writev — write() calls internal_write_() then internal_output_(), each acquiring the lock separately. Hold the lock at the outer scope so inner calls just bump the recursion counter. --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 0c0d64d198..d7fa6a2694 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -616,6 +616,7 @@ int LWIPRawImpl::internal_output_() { } ssize_t LWIPRawImpl::write(const void *buf, size_t len) { + LWIP_LOCK(); // Hold for write + optional output ssize_t written = this->internal_write_(buf, len); if (written == -1) return -1; From 49ba08cec98bb2ac8e95ab67dbc1fe6f67a05542 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 Mar 2026 23:20:05 -1000 Subject: [PATCH 5/9] [socket] Add lwip raw UDP socket implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add native UDP support to the lwip raw TCP socket layer used by ESP8266 and RP2040, eliminating the need for Arduino WiFiUDP fallback. Two new classes: - LWIPRawUDPImpl: send-only UDP (8 bytes overhead) - LWIPRawUDPRecvImpl: send+recv with fixed-size ring buffer (no heap allocation in recv callback) Factory functions: socket_udp(), socket_udp_recv(), socket_ip_udp(), socket_ip_udp_recv() with UDPSocket/UDPRecvSocket type aliases. Additive only — no consumer migration in this PR. --- esphome/components/socket/headers.h | 10 + .../components/socket/lwip_raw_tcp_impl.cpp | 448 ++++++++++++++++-- esphome/components/socket/lwip_raw_tcp_impl.h | 86 ++++ esphome/components/socket/socket.cpp | 26 + esphome/components/socket/socket.h | 19 +- 5 files changed, 549 insertions(+), 40 deletions(-) diff --git a/esphome/components/socket/headers.h b/esphome/components/socket/headers.h index 16e4d23d3b..9ee3873c33 100644 --- a/esphome/components/socket/headers.h +++ b/esphome/components/socket/headers.h @@ -20,6 +20,16 @@ #define IPPROTO_IP 0 #define IPPROTO_TCP 6 +#define IPPROTO_UDP 17 + +#define IP_ADD_MEMBERSHIP 3 +#define IP_DROP_MEMBERSHIP 4 + +// NOLINTNEXTLINE(readability-identifier-naming) +struct ip_mreq { + struct in_addr imr_multiaddr; + struct in_addr imr_interface; +}; #if LWIP_IPV6 #define AF_INET6 10 diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index d7fa6a2694..26d62cd298 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -9,6 +9,10 @@ #include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "lwip/igmp.h" +#include "lwip/pbuf.h" +#include "lwip/udp.h" + #ifdef USE_ESP8266 #include // For esp_schedule() #elif defined(USE_RP2040) @@ -138,6 +142,48 @@ static const char *const TAG = "socket.lwip"; #define LWIP_LOG(msg, ...) #endif +// ---- Shared helpers ---- + +/// Convert lwip ip_addr_t + host-order port to sockaddr, based on the socket's address family. +/// Shared by both TCP (LWIPRawCommon) and UDP (LWIPRawUDPImpl) implementations. +static int lwip_ip_to_sockaddr(sa_family_t family, const ip_addr_t *ip, uint16_t port_host, struct sockaddr *name, + socklen_t *addrlen) { + if (family == AF_INET) { + if (*addrlen < sizeof(struct sockaddr_in)) { + errno = EINVAL; + return -1; + } + auto *addr = reinterpret_cast(name); + addr->sin_family = AF_INET; + *addrlen = addr->sin_len = sizeof(struct sockaddr_in); + addr->sin_port = htons(port_host); + inet_addr_from_ip4addr(&addr->sin_addr, ip_2_ip4(ip)); + return 0; + } +#if LWIP_IPV6 + if (family == AF_INET6) { + if (*addrlen < sizeof(struct sockaddr_in6)) { + errno = EINVAL; + return -1; + } + auto *addr = reinterpret_cast(name); + addr->sin6_family = AF_INET6; + *addrlen = addr->sin6_len = sizeof(struct sockaddr_in6); + addr->sin6_port = htons(port_host); + // AF_INET6 sockets may receive IPv4 packets; convert to IPv4-mapped IPv6. + if (IP_IS_V4(ip)) { + ip_addr_t mapped; + ip4_2_ipv4_mapped_ipv6(ip_2_ip6(&mapped), ip_2_ip4(ip)); + inet6_addr_from_ip6addr(&addr->sin6_addr, ip_2_ip6(&mapped)); + } else { + inet6_addr_from_ip6addr(&addr->sin6_addr, ip_2_ip6(ip)); + } + return 0; + } +#endif + return -1; +} + // ---- LWIPRawCommon methods ---- LWIPRawCommon::~LWIPRawCommon() { @@ -372,43 +418,8 @@ int LWIPRawCommon::setsockopt(int level, int optname, const void *optval, sockle } int LWIPRawCommon::ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *name, socklen_t *addrlen) { - if (this->family_ == AF_INET) { - if (*addrlen < sizeof(struct sockaddr_in)) { - errno = EINVAL; - return -1; - } - - struct sockaddr_in *addr = reinterpret_cast(name); - addr->sin_family = AF_INET; - *addrlen = addr->sin_len = sizeof(struct sockaddr_in); - addr->sin_port = port; - inet_addr_from_ip4addr(&addr->sin_addr, ip_2_ip4(ip)); - return 0; - } -#if LWIP_IPV6 - else if (this->family_ == AF_INET6) { - if (*addrlen < sizeof(struct sockaddr_in6)) { - errno = EINVAL; - return -1; - } - - struct sockaddr_in6 *addr = reinterpret_cast(name); - addr->sin6_family = AF_INET6; - *addrlen = addr->sin6_len = sizeof(struct sockaddr_in6); - addr->sin6_port = port; - - // AF_INET6 sockets are bound to IPv4 as well, so we may encounter IPv4 addresses that must be converted to IPv6. - if (IP_IS_V4(ip)) { - ip_addr_t mapped; - ip4_2_ipv4_mapped_ipv6(ip_2_ip6(&mapped), ip_2_ip4(ip)); - inet6_addr_from_ip6addr(&addr->sin6_addr, ip_2_ip6(&mapped)); - } else { - inet6_addr_from_ip6addr(&addr->sin6_addr, ip_2_ip6(ip)); - } - return 0; - } -#endif - return -1; + // TCP pcb stores port in network byte order; convert to host order for the shared helper + return lwip_ip_to_sockaddr(this->family_, ip, ntohs(port), name, addrlen); } // ---- LWIPRawImpl methods ---- @@ -774,11 +785,350 @@ err_t LWIPRawListenImpl::accept_fn_(struct tcp_pcb *newpcb, err_t err) { return ERR_OK; } +// ---- LWIPRawUDPImpl (send-only) methods ---- + +LWIPRawUDPImpl::LWIPRawUDPImpl(sa_family_t family) : family_(family) { +#if LWIP_IPV6 + this->pcb_ = udp_new_ip_type(family == AF_INET6 ? IPADDR_TYPE_ANY : IPADDR_TYPE_V4); +#else + this->pcb_ = udp_new(); +#endif +} + +LWIPRawUDPImpl::~LWIPRawUDPImpl() { + if (this->pcb_ != nullptr) { + udp_remove(this->pcb_); + this->pcb_ = nullptr; + } +} + +int LWIPRawUDPImpl::bind_internal_(const struct sockaddr *name, socklen_t addrlen) { + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } + if (name == nullptr) { + errno = EINVAL; + return -1; + } + ip_addr_t ip; + uint16_t port; + if (!sockaddr_to_lwip(name, addrlen, &ip, &port)) { + errno = EINVAL; + return -1; + } +#if LWIP_IPV6 + // For bind, use IPADDR_TYPE_ANY on IPv6 sockets to accept both IPv4 and IPv6 + // packets (dual-stack). sockaddr_to_lwip uses IPADDR_TYPE_V6 which is correct + // for sendto destinations but too restrictive for bind. + if (this->family_ == AF_INET6) { + ip.type = IPADDR_TYPE_ANY; + } +#endif + err_t err = udp_bind(this->pcb_, &ip, port); + if (err == ERR_USE) { + errno = EADDRINUSE; + return -1; + } + if (err == ERR_VAL) { + errno = EINVAL; + return -1; + } + if (err != ERR_OK) { + errno = EIO; + return -1; + } + return 0; +} + +int LWIPRawUDPImpl::bind(const struct sockaddr *name, socklen_t addrlen) { return this->bind_internal_(name, addrlen); } + +int LWIPRawUDPImpl::close() { + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } + udp_remove(this->pcb_); + this->pcb_ = nullptr; + return 0; +} + +bool LWIPRawUDPImpl::sockaddr_to_lwip(const struct sockaddr *addr, socklen_t addrlen, ip_addr_t *ip, uint16_t *port) { + if (addrlen < sizeof(sa_family_t)) + return false; +#if LWIP_IPV6 + if (addr->sa_family == AF_INET) { + if (addrlen < sizeof(sockaddr_in)) + return false; + auto *addr4 = reinterpret_cast(addr); + *port = ntohs(addr4->sin_port); + ip->type = IPADDR_TYPE_V4; + ip->u_addr.ip4.addr = addr4->sin_addr.s_addr; + return true; + } + if (addr->sa_family == AF_INET6) { + if (addrlen < sizeof(sockaddr_in6)) + return false; + auto *addr6 = reinterpret_cast(addr); + *port = ntohs(addr6->sin6_port); + ip->type = IPADDR_TYPE_V6; + memcpy(&ip->u_addr.ip6.addr, &addr6->sin6_addr.un.u8_addr, 16); + return true; + } +#else + if (addr->sa_family == AF_INET) { + if (addrlen < sizeof(sockaddr_in)) + return false; + auto *addr4 = reinterpret_cast(addr); + *port = ntohs(addr4->sin_port); + ip->addr = addr4->sin_addr.s_addr; + return true; + } +#endif + return false; +} + +int LWIPRawUDPImpl::ip2sockaddr_(const ip_addr_t *ip, uint16_t port, struct sockaddr *name, socklen_t *addrlen) { + // UDP recv callback provides port in host byte order + return lwip_ip_to_sockaddr(this->family_, ip, port, name, addrlen); +} + +ssize_t LWIPRawUDPImpl::sendto(const void *buf, size_t len, int flags, const struct sockaddr *dest_addr, + socklen_t addrlen) { + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } + if (buf == nullptr || dest_addr == nullptr) { + errno = EINVAL; + return -1; + } + + // pbuf_alloc takes u16_t length; reject oversized packets + if (len > UINT16_MAX) { + errno = EMSGSIZE; + return -1; + } + + ip_addr_t dst_ip; + uint16_t dst_port; + if (!sockaddr_to_lwip(dest_addr, addrlen, &dst_ip, &dst_port)) { + errno = EINVAL; + return -1; + } + + // Allocate pbuf and copy data + struct pbuf *pb = pbuf_alloc(PBUF_TRANSPORT, (uint16_t) len, PBUF_RAM); + if (pb == nullptr) { + errno = ENOMEM; + return -1; + } + memcpy(pb->payload, buf, len); + + err_t err = udp_sendto(this->pcb_, pb, &dst_ip, dst_port); + pbuf_free(pb); + + if (err != ERR_OK) { + errno = err == ERR_MEM ? ENOMEM : EIO; + return -1; + } + return (ssize_t) len; +} + +int LWIPRawUDPImpl::setsockopt(int level, int optname, const void *optval, socklen_t optlen) { + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } + if (level == SOL_SOCKET && optname == SO_REUSEADDR) { + // lwip raw UDP doesn't enforce port exclusivity the same way, + // but we accept this silently for compatibility + return 0; + } + if (level == SOL_SOCKET && optname == SO_BROADCAST) { + if (optval == nullptr || optlen < sizeof(int)) { + errno = EINVAL; + return -1; + } + int val = *reinterpret_cast(optval); + if (val) { + ip_set_option(this->pcb_, SOF_BROADCAST); + } else { + ip_reset_option(this->pcb_, SOF_BROADCAST); + } + return 0; + } + if (level == IPPROTO_IP && optname == IP_ADD_MEMBERSHIP) { + if (optval == nullptr || optlen < sizeof(struct ip_mreq)) { + errno = EINVAL; + return -1; + } + auto *mreq = reinterpret_cast(optval); + ip4_addr_t multiaddr; + multiaddr.addr = mreq->imr_multiaddr.s_addr; + ip4_addr_t ifaddr; + ifaddr.addr = mreq->imr_interface.s_addr; + err_t err = igmp_joingroup(&ifaddr, &multiaddr); + if (err != ERR_OK) { + errno = EIO; + return -1; + } + return 0; + } + if (level == IPPROTO_IP && optname == IP_DROP_MEMBERSHIP) { + if (optval == nullptr || optlen < sizeof(struct ip_mreq)) { + errno = EINVAL; + return -1; + } + auto *mreq = reinterpret_cast(optval); + ip4_addr_t multiaddr; + multiaddr.addr = mreq->imr_multiaddr.s_addr; + ip4_addr_t ifaddr; + ifaddr.addr = mreq->imr_interface.s_addr; + err_t err = igmp_leavegroup(&ifaddr, &multiaddr); + if (err != ERR_OK) { + errno = EIO; + return -1; + } + return 0; + } + errno = ENOPROTOOPT; + return -1; +} + +int LWIPRawUDPImpl::getsockopt(int level, int optname, void *optval, socklen_t *optlen) { + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } + if (level == SOL_SOCKET && optname == SO_REUSEADDR) { + if (optval == nullptr || optlen == nullptr || *optlen < sizeof(int)) { + errno = EINVAL; + return -1; + } + *reinterpret_cast(optval) = 1; + *optlen = sizeof(int); + return 0; + } + errno = ENOPROTOOPT; + return -1; +} + +int LWIPRawUDPImpl::setblocking(bool blocking) { + if (blocking) { + // blocking operation not supported on raw lwip + errno = EINVAL; + return -1; + } + return 0; +} + +// ---- LWIPRawUDPRecvImpl methods ---- + +LWIPRawUDPRecvImpl::~LWIPRawUDPRecvImpl() { + // Flush rx queue and unregister callback before base destructor removes pcb + if (this->pcb_ != nullptr) + this->close(); +} + +int LWIPRawUDPRecvImpl::close() { + // Unregister recv callback before removing pcb + if (this->pcb_ != nullptr) { + udp_recv(this->pcb_, nullptr, nullptr); + } + // Flush any queued rx packets + while (this->rx_read_idx_ != this->rx_write_idx_) { + auto &pkt = this->rx_queue_[this->rx_read_idx_]; + if (pkt.pb != nullptr) { + pbuf_free(pkt.pb); + pkt.pb = nullptr; + } + this->rx_read_idx_ = (this->rx_read_idx_ + 1) & UDP_RX_MASK; + } + // close() returns EBADF if already closed, which is fine from destructor + return LWIPRawUDPImpl::close(); +} + +int LWIPRawUDPRecvImpl::bind(const struct sockaddr *name, socklen_t addrlen) { + int ret = this->bind_internal_(name, addrlen); + if (ret != 0) + return ret; + // Register recv callback now that we're bound and ready to receive + udp_recv(this->pcb_, LWIPRawUDPRecvImpl::s_recv_fn, this); + return 0; +} + +ssize_t LWIPRawUDPRecvImpl::read(void *buf, size_t len) { return this->recvfrom(buf, len, nullptr, nullptr); } + +ssize_t LWIPRawUDPRecvImpl::recvfrom(void *buf, size_t len, struct sockaddr *src_addr, socklen_t *addrlen) { + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } + if (buf == nullptr && len > 0) { + errno = EINVAL; + return -1; + } + if (this->rx_read_idx_ == this->rx_write_idx_) { + errno = EWOULDBLOCK; + return -1; + } + + auto &pkt = this->rx_queue_[this->rx_read_idx_]; + size_t pkt_len = pkt.pb->tot_len; + size_t copy_len = std::min(len, pkt_len); + + // Copy data from pbuf chain + pbuf_copy_partial(pkt.pb, buf, copy_len, 0); + + // Fill in source address if requested + if (src_addr != nullptr && addrlen != nullptr) { + this->ip2sockaddr_(&pkt.src_addr, pkt.src_port, src_addr, addrlen); + } + + // Free the pbuf and advance the read pointer — must be last, + // as this publishes the slot to the producer (recv callback). + pbuf_free(pkt.pb); + pkt.pb = nullptr; + this->rx_read_idx_ = (this->rx_read_idx_ + 1) & UDP_RX_MASK; + + return (ssize_t) copy_len; +} + +void LWIPRawUDPRecvImpl::s_recv_fn(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, u16_t port) { + auto *self = reinterpret_cast(arg); + self->recv_fn_(p, addr, port); +} + +void LWIPRawUDPRecvImpl::recv_fn_(struct pbuf *p, const ip_addr_t *addr, u16_t port) { + if (p == nullptr) + return; + + // Check if queue is full (next write position would collide with read position) + uint8_t next_write = (this->rx_write_idx_ + 1) & UDP_RX_MASK; + if (next_write == this->rx_read_idx_) { + // Drop packet — queue full + pbuf_free(p); + return; + } + + // Enqueue the packet — write data first, then publish by advancing write index. + auto &slot = this->rx_queue_[this->rx_write_idx_]; + slot.pb = p; + slot.src_addr = *addr; + slot.src_port = port; + this->rx_write_idx_ = next_write; + +#if defined(USE_ESP8266) || defined(USE_RP2040) + socket_wake(); +#endif +} + // ---- Factory functions ---- std::unique_ptr socket(int domain, int type, int protocol) { if (type != SOCK_STREAM) { - ESP_LOGE(TAG, "UDP sockets not supported on this platform, use WiFiUDP"); + ESP_LOGE(TAG, "Use socket_udp() for UDP sockets on this platform"); errno = EPROTOTYPE; return nullptr; } @@ -796,9 +1146,29 @@ std::unique_ptr socket_loop_monitored(int domain, int type, int protocol return socket(domain, type, protocol); } +std::unique_ptr socket_udp(int domain, int protocol) { + (void) protocol; // Raw lwip UDP ignores protocol; kept for API compatibility + auto sock = make_unique((sa_family_t) domain); + if (!sock->is_valid()) { + errno = ENOMEM; + return nullptr; + } + return sock; +} + +std::unique_ptr socket_udp_recv(int domain, int protocol) { + (void) protocol; // Raw lwip UDP ignores protocol; kept for API compatibility + auto sock = make_unique((sa_family_t) domain); + if (!sock->is_valid()) { + errno = ENOMEM; + return nullptr; + } + return sock; +} + std::unique_ptr socket_listen(int domain, int type, int protocol) { if (type != SOCK_STREAM) { - ESP_LOGE(TAG, "UDP sockets not supported on this platform, use WiFiUDP"); + ESP_LOGE(TAG, "Use socket_udp() for UDP sockets on this platform"); errno = EPROTOTYPE; return nullptr; } diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index 5b2c11cfe2..79ba82d438 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -15,6 +15,7 @@ #include "lwip/netif.h" #include "lwip/opt.h" #include "lwip/tcp.h" +#include "lwip/udp.h" namespace esphome::socket { @@ -201,6 +202,91 @@ class LWIPRawListenImpl : public LWIPRawCommon { uint8_t accepted_socket_count_ = 0; // Number of sockets currently in queue }; +/// Send-only UDP socket implementation for LWIP raw API. +/// Non-virtual, concrete type. Uses lwip/udp.h raw API. +/// No receive capability — use LWIPRawUDPRecvImpl for sockets that need to receive. +class LWIPRawUDPImpl { + public: + LWIPRawUDPImpl(sa_family_t family); + ~LWIPRawUDPImpl(); + LWIPRawUDPImpl(const LWIPRawUDPImpl &) = delete; + LWIPRawUDPImpl &operator=(const LWIPRawUDPImpl &) = delete; + + int bind(const struct sockaddr *name, socklen_t addrlen); + int close(); + + /// Send a UDP packet to the specified destination. + ssize_t sendto(const void *buf, size_t len, int flags, const struct sockaddr *dest_addr, socklen_t addrlen); + + int setsockopt(int level, int optname, const void *optval, socklen_t optlen); + int getsockopt(int level, int optname, void *optval, socklen_t *optlen); + + int setblocking(bool blocking); + + bool is_valid() const { return this->pcb_ != nullptr; } + bool ready() const { return false; } + int get_fd() const { return -1; } + + protected: + /// Convert sockaddr to lwip ip_addr_t and port. + static bool sockaddr_to_lwip(const struct sockaddr *addr, socklen_t addrlen, ip_addr_t *ip, uint16_t *port); + /// Convert lwip ip_addr_t and port to sockaddr. + int ip2sockaddr_(const ip_addr_t *ip, uint16_t port, struct sockaddr *name, socklen_t *addrlen); + + /// Shared bind logic — parses sockaddr and calls udp_bind. + int bind_internal_(const struct sockaddr *name, socklen_t addrlen); + + struct udp_pcb *pcb_{nullptr}; + sa_family_t family_{0}; +}; + +/// UDP socket with receive support for LWIP raw API. +/// Extends LWIPRawUDPImpl with a fixed-size ring buffer for incoming packets. +/// The recv callback is registered on bind(). +class LWIPRawUDPRecvImpl : public LWIPRawUDPImpl { + public: + using LWIPRawUDPImpl::LWIPRawUDPImpl; + ~LWIPRawUDPRecvImpl(); + + /// Close the socket, flushing any queued rx packets first. + int close(); + + /// Bind and register the recv callback for incoming packets. + int bind(const struct sockaddr *name, socklen_t addrlen); + + /// Read the next queued packet, discarding source address info. + /// If buf is smaller than the packet, data is silently truncated (returns bytes copied). + ssize_t read(void *buf, size_t len); + /// Read the next queued packet and return the source address. + /// If buf is smaller than the packet, data is silently truncated (returns bytes copied). + ssize_t recvfrom(void *buf, size_t len, struct sockaddr *src_addr, socklen_t *addrlen); + + /// Returns true if there are packets available to read. + bool ready() const { return this->rx_read_idx_ != this->rx_write_idx_; } + + protected: + static void s_recv_fn(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, u16_t port); + void recv_fn_(struct pbuf *p, const ip_addr_t *addr, u16_t port); + + /// Lock-free SPSC ring buffer for received UDP packets. + /// Producer (recv callback, possibly IRQ context on RP2040) writes rx_write_idx_. + /// Consumer (main loop) writes rx_read_idx_. + /// No shared read-modify-write — safe without locking. + /// One slot is reserved to distinguish full from empty, giving 3 usable slots. + /// No heap allocation in the recv callback — packets are dropped if the queue is full. + static constexpr uint8_t UDP_RX_QUEUE_SIZE = 4; // Must be power of 2 + static constexpr uint8_t UDP_RX_MASK = UDP_RX_QUEUE_SIZE - 1; + static_assert((UDP_RX_QUEUE_SIZE & UDP_RX_MASK) == 0, "UDP_RX_QUEUE_SIZE must be power of 2"); + struct UDPRxPacket { + struct pbuf *pb{nullptr}; + ip_addr_t src_addr{}; + uint16_t src_port{0}; + }; + std::array rx_queue_{}; + volatile uint8_t rx_read_idx_{0}; ///< Written by consumer (main loop), read by producer + volatile uint8_t rx_write_idx_{0}; ///< Written by producer (recv callback), read by consumer +}; + } // namespace esphome::socket #endif // USE_SOCKET_IMPL_LWIP_TCP diff --git a/esphome/components/socket/socket.cpp b/esphome/components/socket/socket.cpp index bfb6ae8e13..d039c5436e 100644 --- a/esphome/components/socket/socket.cpp +++ b/esphome/components/socket/socket.cpp @@ -101,6 +101,32 @@ std::unique_ptr socket_ip_loop_monitored(int type, int protocol) { } #endif +#if !defined(USE_SOCKET_IMPL_LWIP_TCP) +// BSD and LWIP_SOCKETS: UDPSocket == UDPRecvSocket == Socket, so these just delegate. +std::unique_ptr socket_udp(int domain, int protocol) { + return esphome::socket::socket(domain, SOCK_DGRAM, protocol); +} +std::unique_ptr socket_udp_recv(int domain, int protocol) { + return esphome::socket::socket(domain, SOCK_DGRAM, protocol); +} +#endif + +std::unique_ptr socket_ip_udp(int protocol) { +#if USE_NETWORK_IPV6 + return socket_udp(AF_INET6, protocol); +#else + return socket_udp(AF_INET, protocol); +#endif +} + +std::unique_ptr socket_ip_udp_recv(int protocol) { +#if USE_NETWORK_IPV6 + return socket_udp_recv(AF_INET6, protocol); +#else + return socket_udp_recv(AF_INET, protocol); +#endif +} + socklen_t set_sockaddr(struct sockaddr *addr, socklen_t addrlen, const char *ip_address, uint16_t port) { #if USE_NETWORK_IPV6 if (strchr(ip_address, ':') != nullptr) { diff --git a/esphome/components/socket/socket.h b/esphome/components/socket/socket.h index a21bd64730..c790719816 100644 --- a/esphome/components/socket/socket.h +++ b/esphome/components/socket/socket.h @@ -27,17 +27,24 @@ namespace esphome::socket { // Type aliases — only one implementation is active per build. // Socket is the concrete type for connected sockets. // ListenSocket is the concrete type for listening/server sockets. -// On BSD and LWIP_SOCKETS, both aliases resolve to the same type. +// UDPSocket is the concrete type for UDP sockets. +// On BSD and LWIP_SOCKETS, all aliases resolve to the same type. // On LWIP_TCP, they are different types (no virtual dispatch between them). #ifdef USE_SOCKET_IMPL_BSD_SOCKETS using Socket = BSDSocketImpl; using ListenSocket = BSDSocketImpl; +using UDPSocket = BSDSocketImpl; +using UDPRecvSocket = BSDSocketImpl; #elif defined(USE_SOCKET_IMPL_LWIP_SOCKETS) using Socket = LwIPSocketImpl; using ListenSocket = LwIPSocketImpl; +using UDPSocket = LwIPSocketImpl; +using UDPRecvSocket = LwIPSocketImpl; #elif defined(USE_SOCKET_IMPL_LWIP_TCP) using Socket = LWIPRawImpl; using ListenSocket = LWIPRawListenImpl; +using UDPSocket = LWIPRawUDPImpl; +using UDPRecvSocket = LWIPRawUDPRecvImpl; #endif #ifdef USE_LWIP_FAST_SELECT @@ -68,6 +75,16 @@ std::unique_ptr socket(int domain, int type, int protocol); /// Create a socket in the newest available IP domain (IPv6 or IPv4) of the given type and protocol. std::unique_ptr socket_ip(int type, int protocol); +/// Create a send-only UDP socket of the given domain and protocol. +std::unique_ptr socket_udp(int domain, int protocol); +/// Create a send-only UDP socket in the newest available IP domain. +std::unique_ptr socket_ip_udp(int protocol); + +/// Create a UDP socket with receive support of the given domain and protocol. +std::unique_ptr socket_udp_recv(int domain, int protocol); +/// Create a UDP socket with receive support in the newest available IP domain. +std::unique_ptr socket_ip_udp_recv(int protocol); + /// Create a socket and monitor it for data in the main loop. /// Like socket() but also registers the socket with the Application's select() loop. /// WARNING: These functions are NOT thread-safe. They must only be called from the main loop From f54756ae2dcae30f91b4a5bb03d2eae55849f552 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 Mar 2026 23:56:37 -1000 Subject: [PATCH 6/9] [socket] Address review comments: doc comments and (void) flags - Document port_host byte order convention in lwip_ip_to_sockaddr - Note intentional method hiding in LWIPRawUDPRecvImpl - Note recvfrom truncation differs from POSIX MSG_TRUNC - Add (void) flags in sendto to clarify flags are ignored Co-Authored-By: J. Nick Koston --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 4 ++++ esphome/components/socket/lwip_raw_tcp_impl.h | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 26d62cd298..159bc08a3f 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -145,6 +145,9 @@ static const char *const TAG = "socket.lwip"; // ---- Shared helpers ---- /// Convert lwip ip_addr_t + host-order port to sockaddr, based on the socket's address family. +/// @param port_host Port in host byte order. TCP callers must convert from network order first +/// (tcp_pcb stores ports in network byte order); UDP callers can pass directly +/// (lwip udp_recv callback provides port in host byte order). /// Shared by both TCP (LWIPRawCommon) and UDP (LWIPRawUDPImpl) implementations. static int lwip_ip_to_sockaddr(sa_family_t family, const ip_addr_t *ip, uint16_t port_host, struct sockaddr *name, socklen_t *addrlen) { @@ -895,6 +898,7 @@ int LWIPRawUDPImpl::ip2sockaddr_(const ip_addr_t *ip, uint16_t port, struct sock ssize_t LWIPRawUDPImpl::sendto(const void *buf, size_t len, int flags, const struct sockaddr *dest_addr, socklen_t addrlen) { + (void) flags; // Flags (MSG_DONTWAIT, etc.) are ignored; raw lwip is always non-blocking if (this->pcb_ == nullptr) { errno = EBADF; return -1; diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index 79ba82d438..f6cf136e9a 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -243,6 +243,11 @@ class LWIPRawUDPImpl { /// UDP socket with receive support for LWIP raw API. /// Extends LWIPRawUDPImpl with a fixed-size ring buffer for incoming packets. /// The recv callback is registered on bind(). +/// +/// Note: close() and bind() intentionally hide the base class methods to add +/// recv callback registration/cleanup. This is safe because these classes are +/// never used polymorphically (no virtual dispatch) — callers always use the +/// concrete LWIPRawUDPRecvImpl type via the UDPRecvSocket alias. class LWIPRawUDPRecvImpl : public LWIPRawUDPImpl { public: using LWIPRawUDPImpl::LWIPRawUDPImpl; @@ -256,9 +261,11 @@ class LWIPRawUDPRecvImpl : public LWIPRawUDPImpl { /// Read the next queued packet, discarding source address info. /// If buf is smaller than the packet, data is silently truncated (returns bytes copied). + /// Note: unlike POSIX MSG_TRUNC, this does not return the original packet length on truncation. ssize_t read(void *buf, size_t len); /// Read the next queued packet and return the source address. /// If buf is smaller than the packet, data is silently truncated (returns bytes copied). + /// Note: unlike POSIX MSG_TRUNC, this does not return the original packet length on truncation. ssize_t recvfrom(void *buf, size_t len, struct sockaddr *src_addr, socklen_t *addrlen); /// Returns true if there are packets available to read. From fccfab8083872656f62ae256f4cf9b288d4eb14e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 10 Mar 2026 01:15:57 -1000 Subject: [PATCH 7/9] [socket] Switch UDP rx queue from lock-free SPSC to count-based with LWIP_LOCK With the lwip lock infrastructure in place from the TCP race fix, the lock-free SPSC ring buffer's wasted slot is no longer needed. Switch to a simple rx_count_ approach that uses all 4 queue slots. Also add LWIP_LOCK() to all UDP methods that call lwip APIs (bind, close, sendto, setsockopt, getsockopt, recvfrom, factories). --- .../components/socket/lwip_raw_tcp_impl.cpp | 33 +++++++++++++------ esphome/components/socket/lwip_raw_tcp_impl.h | 18 +++++----- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 159bc08a3f..27313b95e6 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -799,6 +799,7 @@ LWIPRawUDPImpl::LWIPRawUDPImpl(sa_family_t family) : family_(family) { } LWIPRawUDPImpl::~LWIPRawUDPImpl() { + LWIP_LOCK(); if (this->pcb_ != nullptr) { udp_remove(this->pcb_); this->pcb_ = nullptr; @@ -806,6 +807,7 @@ LWIPRawUDPImpl::~LWIPRawUDPImpl() { } int LWIPRawUDPImpl::bind_internal_(const struct sockaddr *name, socklen_t addrlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -847,6 +849,7 @@ int LWIPRawUDPImpl::bind_internal_(const struct sockaddr *name, socklen_t addrle int LWIPRawUDPImpl::bind(const struct sockaddr *name, socklen_t addrlen) { return this->bind_internal_(name, addrlen); } int LWIPRawUDPImpl::close() { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -899,6 +902,7 @@ int LWIPRawUDPImpl::ip2sockaddr_(const ip_addr_t *ip, uint16_t port, struct sock ssize_t LWIPRawUDPImpl::sendto(const void *buf, size_t len, int flags, const struct sockaddr *dest_addr, socklen_t addrlen) { (void) flags; // Flags (MSG_DONTWAIT, etc.) are ignored; raw lwip is always non-blocking + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -940,6 +944,7 @@ ssize_t LWIPRawUDPImpl::sendto(const void *buf, size_t len, int flags, const str } int LWIPRawUDPImpl::setsockopt(int level, int optname, const void *optval, socklen_t optlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -1001,6 +1006,7 @@ int LWIPRawUDPImpl::setsockopt(int level, int optname, const void *optval, sockl } int LWIPRawUDPImpl::getsockopt(int level, int optname, void *optval, socklen_t *optlen) { + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = EBADF; return -1; @@ -1036,24 +1042,27 @@ LWIPRawUDPRecvImpl::~LWIPRawUDPRecvImpl() { } int LWIPRawUDPRecvImpl::close() { + LWIP_LOCK(); // Unregister recv callback before removing pcb if (this->pcb_ != nullptr) { udp_recv(this->pcb_, nullptr, nullptr); } // Flush any queued rx packets - while (this->rx_read_idx_ != this->rx_write_idx_) { + while (this->rx_count_ > 0) { auto &pkt = this->rx_queue_[this->rx_read_idx_]; if (pkt.pb != nullptr) { pbuf_free(pkt.pb); pkt.pb = nullptr; } this->rx_read_idx_ = (this->rx_read_idx_ + 1) & UDP_RX_MASK; + this->rx_count_--; } // close() returns EBADF if already closed, which is fine from destructor return LWIPRawUDPImpl::close(); } int LWIPRawUDPRecvImpl::bind(const struct sockaddr *name, socklen_t addrlen) { + LWIP_LOCK(); int ret = this->bind_internal_(name, addrlen); if (ret != 0) return ret; @@ -1073,7 +1082,8 @@ ssize_t LWIPRawUDPRecvImpl::recvfrom(void *buf, size_t len, struct sockaddr *src errno = EINVAL; return -1; } - if (this->rx_read_idx_ == this->rx_write_idx_) { + LWIP_LOCK(); + if (this->rx_count_ == 0) { errno = EWOULDBLOCK; return -1; } @@ -1090,11 +1100,11 @@ ssize_t LWIPRawUDPRecvImpl::recvfrom(void *buf, size_t len, struct sockaddr *src this->ip2sockaddr_(&pkt.src_addr, pkt.src_port, src_addr, addrlen); } - // Free the pbuf and advance the read pointer — must be last, - // as this publishes the slot to the producer (recv callback). + // Free the pbuf and advance the read pointer pbuf_free(pkt.pb); pkt.pb = nullptr; this->rx_read_idx_ = (this->rx_read_idx_ + 1) & UDP_RX_MASK; + this->rx_count_--; return (ssize_t) copy_len; } @@ -1104,24 +1114,25 @@ void LWIPRawUDPRecvImpl::s_recv_fn(void *arg, struct udp_pcb *pcb, struct pbuf * self->recv_fn_(p, addr, port); } +// Called by lwip core which already holds the async_context lock on RP2040. void LWIPRawUDPRecvImpl::recv_fn_(struct pbuf *p, const ip_addr_t *addr, u16_t port) { if (p == nullptr) return; - // Check if queue is full (next write position would collide with read position) - uint8_t next_write = (this->rx_write_idx_ + 1) & UDP_RX_MASK; - if (next_write == this->rx_read_idx_) { + // Check if queue is full + if (this->rx_count_ >= UDP_RX_QUEUE_SIZE) { // Drop packet — queue full pbuf_free(p); return; } - // Enqueue the packet — write data first, then publish by advancing write index. - auto &slot = this->rx_queue_[this->rx_write_idx_]; + // Enqueue the packet + uint8_t write_idx = (this->rx_read_idx_ + this->rx_count_) & UDP_RX_MASK; + auto &slot = this->rx_queue_[write_idx]; slot.pb = p; slot.src_addr = *addr; slot.src_port = port; - this->rx_write_idx_ = next_write; + this->rx_count_++; #if defined(USE_ESP8266) || defined(USE_RP2040) socket_wake(); @@ -1152,6 +1163,7 @@ std::unique_ptr socket_loop_monitored(int domain, int type, int protocol std::unique_ptr socket_udp(int domain, int protocol) { (void) protocol; // Raw lwip UDP ignores protocol; kept for API compatibility + LWIP_LOCK(); auto sock = make_unique((sa_family_t) domain); if (!sock->is_valid()) { errno = ENOMEM; @@ -1162,6 +1174,7 @@ std::unique_ptr socket_udp(int domain, int protocol) { std::unique_ptr socket_udp_recv(int domain, int protocol) { (void) protocol; // Raw lwip UDP ignores protocol; kept for API compatibility + LWIP_LOCK(); auto sock = make_unique((sa_family_t) domain); if (!sock->is_valid()) { errno = ENOMEM; diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index f6cf136e9a..4255611a8f 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -269,19 +269,19 @@ class LWIPRawUDPRecvImpl : public LWIPRawUDPImpl { ssize_t recvfrom(void *buf, size_t len, struct sockaddr *src_addr, socklen_t *addrlen); /// Returns true if there are packets available to read. - bool ready() const { return this->rx_read_idx_ != this->rx_write_idx_; } + /// Intentionally unlocked — same rationale as LWIPRawImpl::ready(). + bool ready() const { return this->rx_count_ > 0; } protected: static void s_recv_fn(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, u16_t port); void recv_fn_(struct pbuf *p, const ip_addr_t *addr, u16_t port); - /// Lock-free SPSC ring buffer for received UDP packets. - /// Producer (recv callback, possibly IRQ context on RP2040) writes rx_write_idx_. - /// Consumer (main loop) writes rx_read_idx_. - /// No shared read-modify-write — safe without locking. - /// One slot is reserved to distinguish full from empty, giving 3 usable slots. + /// Ring buffer for received UDP packets. + /// Both producer (recv callback) and consumer (main loop) are serialized by the + /// lwip lock — the callback runs under lwip core lock, and consumer methods hold + /// LWIP_LOCK(). All 4 slots are usable (no wasted slot for full/empty distinction). /// No heap allocation in the recv callback — packets are dropped if the queue is full. - static constexpr uint8_t UDP_RX_QUEUE_SIZE = 4; // Must be power of 2 + static constexpr uint8_t UDP_RX_QUEUE_SIZE = 4; static constexpr uint8_t UDP_RX_MASK = UDP_RX_QUEUE_SIZE - 1; static_assert((UDP_RX_QUEUE_SIZE & UDP_RX_MASK) == 0, "UDP_RX_QUEUE_SIZE must be power of 2"); struct UDPRxPacket { @@ -290,8 +290,8 @@ class LWIPRawUDPRecvImpl : public LWIPRawUDPImpl { uint16_t src_port{0}; }; std::array rx_queue_{}; - volatile uint8_t rx_read_idx_{0}; ///< Written by consumer (main loop), read by producer - volatile uint8_t rx_write_idx_{0}; ///< Written by producer (recv callback), read by consumer + uint8_t rx_read_idx_{0}; + uint8_t rx_count_{0}; }; } // namespace esphome::socket From 1131af169039bf0e080322462675e273a25dc392 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 10 Mar 2026 23:59:25 -1000 Subject: [PATCH 8/9] safety --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index eba8fa407f..f716f28e6f 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -1162,15 +1162,15 @@ int LWIPRawUDPRecvImpl::bind(const struct sockaddr *name, socklen_t addrlen) { ssize_t LWIPRawUDPRecvImpl::read(void *buf, size_t len) { return this->recvfrom(buf, len, nullptr, nullptr); } ssize_t LWIPRawUDPRecvImpl::recvfrom(void *buf, size_t len, struct sockaddr *src_addr, socklen_t *addrlen) { - if (this->pcb_ == nullptr) { - errno = EBADF; - return -1; - } if (buf == nullptr && len > 0) { errno = EINVAL; return -1; } LWIP_LOCK(); + if (this->pcb_ == nullptr) { + errno = EBADF; + return -1; + } if (this->rx_count_ == 0) { errno = EWOULDBLOCK; return -1; @@ -1202,7 +1202,9 @@ void LWIPRawUDPRecvImpl::s_recv_fn(void *arg, struct udp_pcb *pcb, struct pbuf * self->recv_fn_(p, addr, port); } -// Called by lwip core which already holds the async_context lock on RP2040. +// LWIP CALLBACK — runs from IRQ context on RP2040 (low-priority user IRQ). +// No heap allocation allowed — malloc is not IRQ-safe (see #14687). +// No LWIP_LOCK() needed — lwip core already holds the async_context lock. void LWIPRawUDPRecvImpl::recv_fn_(struct pbuf *p, const ip_addr_t *addr, u16_t port) { if (p == nullptr) return; From a0e162912cfb3595211faca81cd3eeb0c1d348d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Mar 2026 00:00:52 -1000 Subject: [PATCH 9/9] safety --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index f716f28e6f..af27531f44 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -879,6 +879,7 @@ err_t LWIPRawListenImpl::accept_fn_(struct tcp_pcb *newpcb, err_t err) { // ---- LWIPRawUDPImpl (send-only) methods ---- LWIPRawUDPImpl::LWIPRawUDPImpl(sa_family_t family) : family_(family) { + LWIP_LOCK(); #if LWIP_IPV6 this->pcb_ = udp_new_ip_type(family == AF_INET6 ? IPADDR_TYPE_ANY : IPADDR_TYPE_V4); #else