Skip to content

Commit a4a8f60

Browse files
Stability fix for IRQ-based Ethernet
Under heavy load it is possible for a "packet available" interrupt to happen while another LWIP call is underway. In that case it is not safe to inject a packet at IRQ time. When that happens, now set a flag and disable IRQs and wait until the LWIP call is done. When the call finishes, it will re-enable IRQs and the IRQ callback will be called again, but this time the inlwip flag will be unset and the ethernet packet inject safe. I've run over 9000 W5100::begin/WiFiCliet read fortune/W5100::::end stress (no delays anywhere) cycles without incident. W/o this patch, LWIP state got corrupted in a matter of minutes.
1 parent 5bfbd7e commit a4a8f60

File tree

4 files changed

+19
-2
lines changed

4 files changed

+19
-2
lines changed

cores/rp2040/lwip_wrap.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@
3939
//auto_init_recursive_mutex(__lwipMutex); // Only for case with no Ethernet or PicoW, but still doing LWIP (PPP?)
4040
recursive_mutex_t __lwipMutex;
4141

42+
// When we have a GPIO IRQ for packet reception, the IntfDev will check if we're already doing lwip.
43+
// If so, it'll disable the IRQ and flag that we need to re-enable it as soon as the current LWIP call ends
44+
volatile int __inLWIP = 0;
45+
volatile bool __needsIRQEN = false;
46+
4247
extern "C" {
4348

4449
extern void __lwip(__lwip_op op, void *req, bool fromISR = false);

cores/rp2040/lwip_wrap.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ extern void ethernet_arch_lwip_gpio_unmask() __attribute__((weak));
3838
//auto_init_recursive_mutex(__lwipMutex); // Only for case with no Ethernet or PicoW, but still doing LWIP (PPP?)
3939
extern recursive_mutex_t __lwipMutex;
4040

41+
// When we have a GPIO IRQ for packet reception, the IntfDev will check if we're already doing lwip.
42+
// If so, it'll disable the IRQ and flag that we need to re-enable it as soon as the current LWIP call ends
43+
extern volatile int __inLWIP;
44+
extern volatile bool __needsIRQEN;
4145

4246
// LWIPMutex is a no-op under FreeRTOS because we wrap all calls and just send a request message to
4347
// the LWIP task. No locking needed, many people can call LWIP in parallel but the messages will be
@@ -49,6 +53,7 @@ class LWIPMutex {
4953
public:
5054
LWIPMutex() {
5155
#if !defined(__FREERTOS)
56+
__inLWIP++;
5257
if (ethernet_arch_lwip_begin) {
5358
ethernet_arch_lwip_begin();
5459
} else {
@@ -64,6 +69,11 @@ class LWIPMutex {
6469
} else {
6570
recursive_mutex_exit(&__lwipMutex);
6671
}
72+
__inLWIP--;
73+
if (__needsIRQEN && !__inLWIP) {
74+
__needsIRQEN = false;
75+
ethernet_arch_lwip_gpio_unmask();
76+
}
6777
#endif
6878
}
6979
};

libraries/lwIP_Ethernet/src/LwipEthernet.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ static async_context_t *_context = nullptr;
5858

5959
bool __ethernetContextInitted = false;
6060

61-
6261
// Theoretically support multiple interfaces
6362
static std::map<int, std::function<void(void)>> _handlePacketList;
6463

libraries/lwIP_Ethernet/src/LwipIntfDev.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ enum EthernetLinkStatus {
7575

7676
#include <lwip_wrap.h>
7777

78-
7978
template<class RawDev>
8079
class LwipIntfDev: public LwipIntf, public RawDev {
8180
public:
@@ -496,6 +495,10 @@ template<class RawDev>
496495
void LwipIntfDev<RawDev>::_irq(void *param) {
497496
LwipIntfDev *d = static_cast<LwipIntfDev*>(param);
498497
ethernet_arch_lwip_gpio_mask(); // Disable other IRQs until we're done processing this one
498+
if (__inLWIP) {
499+
__needsIRQEN = true;
500+
return; // We'll get this same IRQ once the current LWIP call is done
501+
}
499502
lwip_callback(_lwipCallback, param, &d->_irqBuffer);
500503
}
501504

0 commit comments

Comments
 (0)