Skip to content

Commit 8956bbb

Browse files
shubhamdprojer
andauthored
[cherrypick][v1.1-branch] Cherry picking lwip related fixes (project-chip#32370)
* inet: ScopedLwIPLock for better safety and added locks at necessary places (project-chip#28655) * inet: scoped lwip locks for better safety and add at few more places * Do not static assert if LWIP_TCPIP_CORE_LOCKING is disabled * Add scope for locks * move out the error variable definition to the top * [ESP32] Enable LWIP_TCPIP_CORE_LOCKING by default and added static assert for the same (project-chip#28798) * [ESP32] Enable LWIP_TCPIP_CORE_LOCKING by default and acquire lwip locks when initializing route_hook * inet: static assert if LWIP_TCPIP_CORE_LOCKING is disabled Static assert is disabled for ASR and bouffalolab platforms * typo * UDPEndPointImplLwIP: Support LWIP_TCPIP_CORE_LOCKING=0 (project-chip#29057) Wrap calls to LwIP APIs in `tcpip_api_call()`, as required. When `LWIP_TCPIP_CORE_LOCKING` is enabled, this internally becomes `LOCK_TCPIP_CORE/UNLOCK_TCPIP_CORE` and when it isn't, it posts a message to the TCPIP task to run the function. Added CHIP stack locking to the UDP receive function. --------- Co-authored-by: Deomid Ryabkov <rojer@rojer.me>
1 parent 28733f1 commit 8956bbb

File tree

7 files changed

+217
-99
lines changed

7 files changed

+217
-99
lines changed

config/esp32/components/chip/Kconfig

+11
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,17 @@ menu "CHIP Core"
174174
help
175175
Enable this option to start a UDP Endpoint queue filter for mDNS Broadcast packets
176176

177+
config ENABLE_LWIP_THREAD_SAFETY
178+
bool "Enable LwIP Thread safety options"
179+
default y
180+
select LWIP_TCPIP_CORE_LOCKING
181+
select LWIP_CHECK_THREAD_SAFETY
182+
help
183+
CHIP SDK performs LwIP core locking before calling an LwIP API.
184+
To make the calls thread safe we have to enable LWIP_TCPIP_CORE_LOCKING.
185+
Here, we are also enabling LWIP_CHECK_THREAD_SAFETY which will assert when
186+
LwIP code gets called from any other context or without holding the LwIP lock.
187+
177188
endmenu # "Networking Options"
178189

179190
menu "System Options"

src/inet/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ static_library("inet") {
103103
]
104104

105105
if (chip_system_config_use_lwip) {
106+
sources += [ "EndPointStateLwIP.cpp" ]
106107
public_deps += [ "${chip_root}/src/lwip" ]
107108
}
108109

src/inet/EndPointStateLwIP.cpp

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
*
3+
* Copyright (c) 2023 Project CHIP Authors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
#include <inet/EndPointStateLwIP.h>
19+
20+
#include <lwip/sys.h>
21+
#include <lwip/tcpip.h>
22+
23+
#include <platform/LockTracker.h>
24+
25+
namespace chip {
26+
namespace Inet {
27+
28+
err_t EndPointStateLwIP::RunOnTCPIPRet(std::function<err_t()> fn)
29+
{
30+
assertChipStackLockedByCurrentThread();
31+
#if LWIP_TCPIP_CORE_LOCKING
32+
err_t err;
33+
LOCK_TCPIP_CORE();
34+
err = fn();
35+
UNLOCK_TCPIP_CORE();
36+
return err;
37+
#else
38+
// Post a message to the TCPIP task and wait for it to run.
39+
static sys_sem_t sTCPIPSem;
40+
static bool sTCPIPSemInited = false;
41+
if (!sTCPIPSemInited)
42+
{
43+
err_t err = sys_sem_new(&sTCPIPSem, 0);
44+
if (err != ERR_OK)
45+
{
46+
return err;
47+
}
48+
sTCPIPSemInited = true;
49+
}
50+
51+
// tcpip_callback takes a C function pointer, so we can't pass a capturing lambda to it.
52+
// Just store the state the function we pass to it needs in statics.
53+
// This should be safe, since that function will execute before we return and there is no
54+
// re-entry into this method.
55+
static std::function<err_t()> sTCPIPFunction;
56+
static err_t sTCPIPFunctionResult;
57+
VerifyOrDie(sTCPIPFunction == nullptr);
58+
59+
sTCPIPFunction = fn;
60+
const err_t err = tcpip_callback(
61+
[](void * aCtx) {
62+
sTCPIPFunctionResult = sTCPIPFunction();
63+
sys_sem_signal(&sTCPIPSem);
64+
},
65+
nullptr);
66+
if (err != ERR_OK)
67+
{
68+
return err;
69+
}
70+
sys_arch_sem_wait(&sTCPIPSem, 0);
71+
sTCPIPFunction = nullptr;
72+
return sTCPIPFunctionResult;
73+
#endif
74+
}
75+
76+
void EndPointStateLwIP::RunOnTCPIP(std::function<void()> fn)
77+
{
78+
VerifyOrDie(RunOnTCPIPRet([&fn]() {
79+
fn();
80+
return ERR_OK;
81+
}) == ERR_OK);
82+
}
83+
84+
} // namespace Inet
85+
} // namespace chip

src/inet/EndPointStateLwIP.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222

2323
#pragma once
2424

25-
#include <inet/EndPointBasis.h>
25+
#include <functional>
2626

27+
#include <inet/EndPointBasis.h>
2728
#include <inet/IPAddress.h>
2829

2930
struct udp_pcb;
@@ -46,6 +47,10 @@ class DLL_EXPORT EndPointStateLwIP
4647
UDP = 1,
4748
TCP = 2
4849
} mLwIPEndPointType;
50+
51+
// Synchronously runs a function within the TCPIP task's context.
52+
static void RunOnTCPIP(std::function<void()>);
53+
static err_t RunOnTCPIPRet(std::function<err_t()>);
4954
};
5055

5156
} // namespace Inet

src/inet/TCPEndPointImplLwIP.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@
3737
#include <lwip/tcp.h>
3838
#include <lwip/tcpip.h>
3939

40+
static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later");
41+
42+
#if !(CHIP_DEVICE_LAYER_TARGET_BL602 || CHIP_DEVICE_LAYER_TARGET_BL702 || CHIP_DEVICE_LAYER_TARGET_BL702L)
43+
// TODO: Update to use RunOnTCPIP.
44+
static_assert(LWIP_TCPIP_CORE_LOCKING, "CHIP requires config LWIP_TCPIP_CORE_LOCKING enabled");
45+
#endif
46+
4047
namespace chip {
4148
namespace Inet {
4249

0 commit comments

Comments
 (0)