diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index ee259a3244b4ca..aef77a9941826c 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -174,6 +174,17 @@ menu "CHIP Core" help Enable this option to start a UDP Endpoint queue filter for mDNS Broadcast packets + config ENABLE_LWIP_THREAD_SAFETY + bool "Enable LwIP Thread safety options" + default y + select LWIP_TCPIP_CORE_LOCKING + select LWIP_CHECK_THREAD_SAFETY + help + CHIP SDK performs LwIP core locking before calling an LwIP API. + To make the calls thread safe we have to enable LWIP_TCPIP_CORE_LOCKING. + Here, we are also enabling LWIP_CHECK_THREAD_SAFETY which will assert when + LwIP code gets called from any other context or without holding the LwIP lock. + endmenu # "Networking Options" menu "System Options" diff --git a/src/inet/BUILD.gn b/src/inet/BUILD.gn index f8f81f94e3c44a..1ba8ea5687e2e3 100644 --- a/src/inet/BUILD.gn +++ b/src/inet/BUILD.gn @@ -103,6 +103,7 @@ static_library("inet") { ] if (chip_system_config_use_lwip) { + sources += [ "EndPointStateLwIP.cpp" ] public_deps += [ "${chip_root}/src/lwip" ] } diff --git a/src/inet/EndPointStateLwIP.cpp b/src/inet/EndPointStateLwIP.cpp new file mode 100644 index 00000000000000..34fba243353701 --- /dev/null +++ b/src/inet/EndPointStateLwIP.cpp @@ -0,0 +1,85 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +#include + +namespace chip { +namespace Inet { + +err_t EndPointStateLwIP::RunOnTCPIPRet(std::function fn) +{ + assertChipStackLockedByCurrentThread(); +#if LWIP_TCPIP_CORE_LOCKING + err_t err; + LOCK_TCPIP_CORE(); + err = fn(); + UNLOCK_TCPIP_CORE(); + return err; +#else + // Post a message to the TCPIP task and wait for it to run. + static sys_sem_t sTCPIPSem; + static bool sTCPIPSemInited = false; + if (!sTCPIPSemInited) + { + err_t err = sys_sem_new(&sTCPIPSem, 0); + if (err != ERR_OK) + { + return err; + } + sTCPIPSemInited = true; + } + + // tcpip_callback takes a C function pointer, so we can't pass a capturing lambda to it. + // Just store the state the function we pass to it needs in statics. + // This should be safe, since that function will execute before we return and there is no + // re-entry into this method. + static std::function sTCPIPFunction; + static err_t sTCPIPFunctionResult; + VerifyOrDie(sTCPIPFunction == nullptr); + + sTCPIPFunction = fn; + const err_t err = tcpip_callback( + [](void * aCtx) { + sTCPIPFunctionResult = sTCPIPFunction(); + sys_sem_signal(&sTCPIPSem); + }, + nullptr); + if (err != ERR_OK) + { + return err; + } + sys_arch_sem_wait(&sTCPIPSem, 0); + sTCPIPFunction = nullptr; + return sTCPIPFunctionResult; +#endif +} + +void EndPointStateLwIP::RunOnTCPIP(std::function fn) +{ + VerifyOrDie(RunOnTCPIPRet([&fn]() { + fn(); + return ERR_OK; + }) == ERR_OK); +} + +} // namespace Inet +} // namespace chip diff --git a/src/inet/EndPointStateLwIP.h b/src/inet/EndPointStateLwIP.h index d42f613931484e..ddc862d65ef386 100644 --- a/src/inet/EndPointStateLwIP.h +++ b/src/inet/EndPointStateLwIP.h @@ -22,8 +22,9 @@ #pragma once -#include +#include +#include #include struct udp_pcb; @@ -46,6 +47,10 @@ class DLL_EXPORT EndPointStateLwIP UDP = 1, TCP = 2 } mLwIPEndPointType; + + // Synchronously runs a function within the TCPIP task's context. + static void RunOnTCPIP(std::function); + static err_t RunOnTCPIPRet(std::function); }; } // namespace Inet diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 757b7d8d5c95de..a80253643864b8 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -37,6 +37,13 @@ #include #include +static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later"); + +#if !(CHIP_DEVICE_LAYER_TARGET_BL602 || CHIP_DEVICE_LAYER_TARGET_BL702 || CHIP_DEVICE_LAYER_TARGET_BL702L) +// TODO: Update to use RunOnTCPIP. +static_assert(LWIP_TCPIP_CORE_LOCKING, "CHIP requires config LWIP_TCPIP_CORE_LOCKING enabled"); +#endif + namespace chip { namespace Inet { diff --git a/src/inet/UDPEndPointImplLwIP.cpp b/src/inet/UDPEndPointImplLwIP.cpp index 7ec594127b0f14..f808bf5ed9bb1c 100644 --- a/src/inet/UDPEndPointImplLwIP.cpp +++ b/src/inet/UDPEndPointImplLwIP.cpp @@ -36,7 +36,6 @@ #include #include #include -#include #include static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later"); @@ -67,9 +66,6 @@ EndpointQueueFilter * UDPEndPointImplLwIP::sQueueFilter = nullptr; CHIP_ERROR UDPEndPointImplLwIP::BindImpl(IPAddressType addressType, const IPAddress & address, uint16_t port, InterfaceId interfaceId) { - // Lock LwIP stack - LOCK_TCPIP_CORE(); - // Make sure we have the appropriate type of PCB. CHIP_ERROR res = GetPCB(addressType); @@ -82,7 +78,7 @@ CHIP_ERROR UDPEndPointImplLwIP::BindImpl(IPAddressType addressType, const IPAddr if (res == CHIP_NO_ERROR) { - res = chip::System::MapErrorLwIP(udp_bind(mUDP, &ipAddr, port)); + res = chip::System::MapErrorLwIP(RunOnTCPIPRet([this, &ipAddr, port]() { return udp_bind(mUDP, &ipAddr, port); })); } if (res == CHIP_NO_ERROR) @@ -90,18 +86,12 @@ CHIP_ERROR UDPEndPointImplLwIP::BindImpl(IPAddressType addressType, const IPAddr res = LwIPBindInterface(mUDP, interfaceId); } - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - return res; } CHIP_ERROR UDPEndPointImplLwIP::BindInterfaceImpl(IPAddressType addrType, InterfaceId intfId) { - // A lock is required because the LwIP thread may be referring to intf_filter, - // while this code running in the Inet application is potentially modifying it. // NOTE: this only supports LwIP interfaces whose number is no bigger than 9. - LOCK_TCPIP_CORE(); // Make sure we have the appropriate type of PCB. CHIP_ERROR err = GetPCB(addrType); @@ -110,9 +100,6 @@ CHIP_ERROR UDPEndPointImplLwIP::BindInterfaceImpl(IPAddressType addrType, Interf { err = LwIPBindInterface(mUDP, intfId); } - - UNLOCK_TCPIP_CORE(); - return err; } @@ -128,14 +115,16 @@ CHIP_ERROR UDPEndPointImplLwIP::LwIPBindInterface(struct udp_pcb * aUDP, Interfa } } - udp_bind_netif(aUDP, netifp); + RunOnTCPIP([aUDP, netifp]() { udp_bind_netif(aUDP, netifp); }); return CHIP_NO_ERROR; } InterfaceId UDPEndPointImplLwIP::GetBoundInterface() const { #if HAVE_LWIP_UDP_BIND_NETIF - return InterfaceId(netif_get_by_index(mUDP->netif_idx)); + struct netif * netif; + RunOnTCPIP([this, &netif]() { netif = netif_get_by_index(mUDP->netif_idx); }); + return InterfaceId(netif); #else return InterfaceId(mUDP->intf_filter); #endif @@ -148,19 +137,14 @@ uint16_t UDPEndPointImplLwIP::GetBoundPort() const CHIP_ERROR UDPEndPointImplLwIP::ListenImpl() { - // Lock LwIP stack - LOCK_TCPIP_CORE(); - - udp_recv(mUDP, LwIPReceiveUDPMessage, this); - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - + RunOnTCPIP([this]() { udp_recv(mUDP, LwIPReceiveUDPMessage, this); }); return CHIP_NO_ERROR; } CHIP_ERROR UDPEndPointImplLwIP::SendMsgImpl(const IPPacketInfo * pktInfo, System::PacketBufferHandle && msg) { + assertChipStackLockedByCurrentThread(); + const IPAddress & destAddr = pktInfo->DestAddress; if (!msg.HasSoleOwnership()) @@ -174,14 +158,13 @@ CHIP_ERROR UDPEndPointImplLwIP::SendMsgImpl(const IPPacketInfo * pktInfo, System VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY); } - // Lock LwIP stack - LOCK_TCPIP_CORE(); + CHIP_ERROR res = CHIP_NO_ERROR; + err_t lwipErr = ERR_VAL; // Make sure we have the appropriate type of PCB based on the destination address. - CHIP_ERROR res = GetPCB(destAddr.Type()); + res = GetPCB(destAddr.Type()); if (res != CHIP_NO_ERROR) { - UNLOCK_TCPIP_CORE(); return res; } @@ -191,7 +174,6 @@ CHIP_ERROR UDPEndPointImplLwIP::SendMsgImpl(const IPPacketInfo * pktInfo, System // If a source address has been specified, temporarily override the local_ip of the PCB. // This results in LwIP using the given address being as the source address for the generated // packet, as if the PCB had been bound to that address. - err_t lwipErr = ERR_VAL; const IPAddress & srcAddr = pktInfo->SrcAddress; const uint16_t & destPort = pktInfo->DestPort; const InterfaceId & intfId = pktInfo->Interface; @@ -207,21 +189,20 @@ CHIP_ERROR UDPEndPointImplLwIP::SendMsgImpl(const IPPacketInfo * pktInfo, System ip_addr_copy(mUDP->local_ip, lwipSrcAddr); } - if (intfId.IsPresent()) - { - lwipErr = udp_sendto_if(mUDP, System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(msg), &lwipDestAddr, destPort, - intfId.GetPlatformInterface()); - } - else - { - lwipErr = udp_sendto(mUDP, System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(msg), &lwipDestAddr, destPort); - } + lwipErr = RunOnTCPIPRet([this, &intfId, &msg, &lwipDestAddr, destPort]() { + if (intfId.IsPresent()) + { + return udp_sendto_if(mUDP, System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(msg), &lwipDestAddr, destPort, + intfId.GetPlatformInterface()); + } + else + { + return udp_sendto(mUDP, System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(msg), &lwipDestAddr, destPort); + } + }); ip_addr_copy(mUDP->local_ip, boundAddr); - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - if (lwipErr != ERR_OK) { res = chip::System::MapErrorLwIP(lwipErr); @@ -232,37 +213,33 @@ CHIP_ERROR UDPEndPointImplLwIP::SendMsgImpl(const IPPacketInfo * pktInfo, System void UDPEndPointImplLwIP::CloseImpl() { - - // Lock LwIP stack - LOCK_TCPIP_CORE(); + assertChipStackLockedByCurrentThread(); // Since UDP PCB is released synchronously here, but UDP endpoint itself might have to wait // for destruction asynchronously, there could be more allocated UDP endpoints than UDP PCBs. - if (mUDP != nullptr) + if (mUDP == nullptr) { - udp_remove(mUDP); - mUDP = nullptr; - mLwIPEndPointType = LwIPEndPointType::Unknown; - - // If there is a UDPEndPointImplLwIP::LwIPReceiveUDPMessage - // event pending in the event queue (SystemLayer::ScheduleLambda), we - // schedule a release call to the end of the queue, to ensure that the - // queued pointer to UDPEndPointImplLwIP is not dangling. - if (mDelayReleaseCount != 0) + return; + } + RunOnTCPIP([this]() { udp_remove(mUDP); }); + mUDP = nullptr; + mLwIPEndPointType = LwIPEndPointType::Unknown; + + // If there is a UDPEndPointImplLwIP::LwIPReceiveUDPMessage + // event pending in the event queue (SystemLayer::ScheduleLambda), we + // schedule a release call to the end of the queue, to ensure that the + // queued pointer to UDPEndPointImplLwIP is not dangling. + if (mDelayReleaseCount != 0) + { + Retain(); + CHIP_ERROR err = GetSystemLayer().ScheduleLambda([this] { Release(); }); + if (err != CHIP_NO_ERROR) { - Retain(); - CHIP_ERROR err = GetSystemLayer().ScheduleLambda([this] { Release(); }); - if (err != CHIP_NO_ERROR) - { - ChipLogError(Inet, "Unable to schedule lambda: %" CHIP_ERROR_FORMAT, err.Format()); - // There is nothing we can do here, accept the chance of racing - Release(); - } + ChipLogError(Inet, "Unable to schedule lambda: %" CHIP_ERROR_FORMAT, err.Format()); + // There is nothing we can do here, accept the chance of racing + Release(); } } - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); } void UDPEndPointImplLwIP::Free() @@ -303,7 +280,7 @@ void UDPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && msg, CHIP_ERROR UDPEndPointImplLwIP::GetPCB(IPAddressType addrType) { - // IMPORTANT: This method MUST be called with the LwIP stack LOCKED! + assertChipStackLockedByCurrentThread(); // If a PCB hasn't been allocated yet... if (mUDP == nullptr) @@ -311,12 +288,12 @@ CHIP_ERROR UDPEndPointImplLwIP::GetPCB(IPAddressType addrType) // Allocate a PCB of the appropriate type. if (addrType == IPAddressType::kIPv6) { - mUDP = udp_new_ip_type(IPADDR_TYPE_V6); + RunOnTCPIP([this]() { mUDP = udp_new_ip_type(IPADDR_TYPE_V6); }); } #if INET_CONFIG_ENABLE_IPV4 else if (addrType == IPAddressType::kIPv4) { - mUDP = udp_new_ip_type(IPADDR_TYPE_V4); + RunOnTCPIP([this]() { mUDP = udp_new_ip_type(IPADDR_TYPE_V4); }); } #endif // INET_CONFIG_ENABLE_IPV4 else @@ -471,23 +448,26 @@ CHIP_ERROR UDPEndPointImplLwIP::IPv4JoinLeaveMulticastGroupImpl(InterfaceId aInt { #if LWIP_IPV4 && LWIP_IGMP const ip4_addr_t lIPv4Address = aAddress.ToIPv4(); - err_t lStatus; - + struct netif * lNetif = nullptr; if (aInterfaceId.IsPresent()) { - - struct netif * const lNetif = FindNetifFromInterfaceId(aInterfaceId); + lNetif = FindNetifFromInterfaceId(aInterfaceId); VerifyOrReturnError(lNetif != nullptr, INET_ERROR_UNKNOWN_INTERFACE); - - lStatus = join ? igmp_joingroup_netif(lNetif, &lIPv4Address) // - : igmp_leavegroup_netif(lNetif, &lIPv4Address); - } - else - { - lStatus = join ? igmp_joingroup(IP4_ADDR_ANY4, &lIPv4Address) // - : igmp_leavegroup(IP4_ADDR_ANY4, &lIPv4Address); } + err_t lStatus = RunOnTCPIPRet([lNetif, &lIPv4Address, join]() { + if (lNetif != nullptr) + { + return join ? igmp_joingroup_netif(lNetif, &lIPv4Address) // + : igmp_leavegroup_netif(lNetif, &lIPv4Address); + } + else + { + return join ? igmp_joingroup(IP4_ADDR_ANY4, &lIPv4Address) // + : igmp_leavegroup(IP4_ADDR_ANY4, &lIPv4Address); + } + }); + if (lStatus == ERR_MEM) { return CHIP_ERROR_NO_MEMORY; @@ -503,20 +483,26 @@ CHIP_ERROR UDPEndPointImplLwIP::IPv6JoinLeaveMulticastGroupImpl(InterfaceId aInt { #ifdef HAVE_IPV6_MULTICAST const ip6_addr_t lIPv6Address = aAddress.ToIPv6(); - err_t lStatus; + struct netif * lNetif = nullptr; if (aInterfaceId.IsPresent()) { - struct netif * const lNetif = FindNetifFromInterfaceId(aInterfaceId); + lNetif = FindNetifFromInterfaceId(aInterfaceId); VerifyOrReturnError(lNetif != nullptr, INET_ERROR_UNKNOWN_INTERFACE); - lStatus = join ? mld6_joingroup_netif(lNetif, &lIPv6Address) // - : mld6_leavegroup_netif(lNetif, &lIPv6Address); - } - else - { - lStatus = join ? mld6_joingroup(IP6_ADDR_ANY6, &lIPv6Address) // - : mld6_leavegroup(IP6_ADDR_ANY6, &lIPv6Address); } + err_t lStatus = RunOnTCPIPRet([lNetif, &lIPv6Address, join]() { + if (lNetif != nullptr) + { + return join ? mld6_joingroup_netif(lNetif, &lIPv6Address) // + : mld6_leavegroup_netif(lNetif, &lIPv6Address); + } + else + { + return join ? mld6_joingroup(IP6_ADDR_ANY6, &lIPv6Address) // + : mld6_leavegroup(IP6_ADDR_ANY6, &lIPv6Address); + } + }); + if (lStatus == ERR_MEM) { return CHIP_ERROR_NO_MEMORY; @@ -532,18 +518,20 @@ struct netif * UDPEndPointImplLwIP::FindNetifFromInterfaceId(InterfaceId aInterf { struct netif * lRetval = nullptr; + RunOnTCPIP([aInterfaceId, &lRetval]() { #if defined(NETIF_FOREACH) - NETIF_FOREACH(lRetval) - { - if (lRetval == aInterfaceId.GetPlatformInterface()) + NETIF_FOREACH(lRetval) { - break; + if (lRetval == aInterfaceId.GetPlatformInterface()) + { + break; + } } - } #else // defined(NETIF_FOREACH) - for (lRetval = netif_list; lRetval != nullptr && lRetval != aInterfaceId.GetPlatformInterface(); lRetval = lRetval->next) - ; + for (lRetval = netif_list; lRetval != nullptr && lRetval != aInterfaceId.GetPlatformInterface(); lRetval = lRetval->next) + ; #endif // defined(NETIF_FOREACH) + }); return (lRetval); } diff --git a/src/platform/ESP32/route_hook/ESP32RouteHook.c b/src/platform/ESP32/route_hook/ESP32RouteHook.c index df19adf1de13e8..f19cf59ebf90b2 100644 --- a/src/platform/ESP32/route_hook/ESP32RouteHook.c +++ b/src/platform/ESP32/route_hook/ESP32RouteHook.c @@ -17,10 +17,12 @@ #include "lwip/icmp6.h" #include "lwip/mld6.h" #include "lwip/netif.h" +#include "lwip/opt.h" #include "lwip/prot/icmp6.h" #include "lwip/prot/ip6.h" #include "lwip/prot/nd6.h" #include "lwip/raw.h" +#include "lwip/tcpip.h" #define HOPLIM_MAX 255 #define PIO_FLAG_ON_LINK (1 << 7) @@ -155,25 +157,41 @@ esp_err_t esp_route_hook_init(esp_netif_t * netif) esp_err_t ret = ESP_OK; ESP_RETURN_ON_FALSE(netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface"); + + LOCK_TCPIP_CORE(); + int netif_idx = esp_netif_get_netif_impl_index(netif); if (netif_idx < 0 || netif_idx > UINT8_MAX) { + UNLOCK_TCPIP_CORE(); return ESP_ERR_INVALID_SIZE; } lwip_netif = netif_get_by_index((uint8_t) netif_idx); - ESP_RETURN_ON_FALSE(lwip_netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface"); + + if (lwip_netif == NULL) + { + UNLOCK_TCPIP_CORE(); + ESP_LOGE(TAG, "Invalid network interface"); + return ESP_ERR_INVALID_ARG; + } for (esp_route_hook_t * iter = s_hooks; iter != NULL; iter = iter->next) { if (iter->netif == lwip_netif) { + UNLOCK_TCPIP_CORE(); ESP_LOGI(TAG, "Hook already installed on netif, skip..."); return ESP_OK; } } hook = (esp_route_hook_t *) malloc(sizeof(esp_route_hook_t)); - ESP_RETURN_ON_FALSE(hook != NULL, ESP_ERR_NO_MEM, TAG, "Cannot allocate hook"); + if (hook == NULL) + { + UNLOCK_TCPIP_CORE(); + ESP_LOGE(TAG, "Cannot allocate hook"); + return ESP_ERR_NO_MEM; + } ESP_GOTO_ON_FALSE(mld6_joingroup_netif(lwip_netif, ip_2_ip6(&router_group)) == ESP_OK, ESP_FAIL, exit, TAG, "Failed to join multicast group"); @@ -189,6 +207,9 @@ esp_err_t esp_route_hook_init(esp_netif_t * netif) s_hooks = hook; exit: + + UNLOCK_TCPIP_CORE(); + if (ret != ESP_OK && hook != NULL) { free(hook);