Hello,
using any function of lwIP before it completed its initialization usually ends in a crash. lwIP is initialized when its thread starts. So you have a race condition if you are using lwIP just after starting its thread. To prevent that you can pass a callback function to tcpip_init() which is called when the initialization of lwIP is completed.
The ChibiOS layer around lwIP doesn't allow to pass the callback function down to tcpip_init(). A small pacth fixes that.
Greetings
Frank
diff --git a/ChibiOS/os/various/lwip_bindings/lwipthread.c b/ChibiOS/os/various/lwip_bindings/lwipthread.c
index 38ef8cf..2961277 100644
--- a/ChibiOS/os/various/lwip_bindings/lwipthread.c
+++ b/ChibiOS/os/various/lwip_bindings/lwipthread.c
@@ -211,7 +211,7 @@ static err_t ethernetif_init(struct netif *netif) {
* @param[in] p pointer to a @p lwipthread_opts structure or @p NULL
* @return The function does not return.
*/
-msg_t lwip_thread(void *p) {
+msg_t lwip_thread(void *p, tcpip_init_done_fn tcpip_init_done, void *arg) {
EvTimer evt;
EventListener el0, el1;
struct ip_addr ip, gateway, netmask;
@@ -221,7 +221,7 @@ msg_t lwip_thread(void *p) {
chRegSetThreadName("lwipthread");
/* Initializes the thing.*/
- tcpip_init(NULL, NULL);
+ tcpip_init(tcpip_init_done, arg);
/* TCP/IP parameters, runtime or compile time.*/
if (p) {
diff --git a/ChibiOS/os/various/lwip_bindings/lwipthread.h b/ChibiOS/os/various/lwip_bindings/lwipthread.h
index fe4de28..1503dad 100644
--- a/ChibiOS/os/various/lwip_bindings/lwipthread.h
+++ b/ChibiOS/os/various/lwip_bindings/lwipthread.h
@@ -25,6 +25,7 @@
#define _LWIPTHREAD_H_
#include <lwip/opt.h>
+#include <lwip/tcpip.h>
/** @brief MAC thread priority.*/
#ifndef LWIP_THREAD_PRIORITY
@@ -121,7 +122,7 @@ extern WORKING_AREA(wa_lwip_thread, LWIP_THREAD_STACK_SIZE);
#ifdef __cplusplus
extern "C" {
#endif
- msg_t lwip_thread(void *p);
+ msg_t lwip_thread(void *p, tcpip_init_done_fn tcpip_init_done, void *arg);
#ifdef __cplusplus
}
#endif
lwIP start race condition (patch)
Moderators: RoccoMarco, lbednarz, utzig, tfAteba, barthess
- Giovanni
- Site Admin
- Posts: 14444
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1074 times
- Been thanked: 921 times
- Contact:
Re: lwIP start race condition (patch)
Hi,
I am not sure to understand the patch but thread functions cannot take more than one parameter.
The race condition can be avoided by starting lwip_thread at an higher priority level than all threads using lwIP functions. After initialization lwip_thread lowers its priority to LWIP_THREAD_PRIORITY.
Giovanni
I am not sure to understand the patch but thread functions cannot take more than one parameter.
The race condition can be avoided by starting lwip_thread at an higher priority level than all threads using lwIP functions. After initialization lwip_thread lowers its priority to LWIP_THREAD_PRIORITY.
Giovanni
Re: lwIP start race condition (patch)
Hello,
the lwIP thread should have the highest possible priority then but you still have to be careful that nothing you include starts at an equally high priority.
I didn't see that a thread function can only have one argument because I used the C++ API to create the thread.
A proper way to use that feature of lwIP would be to change the argument of the thread function to a pointer to a struct that includes a all three parameters from my patch.
Greetings
Frank
the lwIP thread should have the highest possible priority then but you still have to be careful that nothing you include starts at an equally high priority.
I didn't see that a thread function can only have one argument because I used the C++ API to create the thread.
A proper way to use that feature of lwIP would be to change the argument of the thread function to a pointer to a struct that includes a all three parameters from my patch.
Greetings
Frank
Re: lwIP start race condition (new patch)
Hello,
I have expanded the optional parameter of the lwip_thread() function. It now carries a function pointer and a parameter pointer which are passed to tcpip_init(). This should be compatible with all existing code.
Just starting lwip_thread with a high priority did not work when I used a hardware debugger.
Greetings
Frank
diff --git a/ChibiOS/os/various/lwip_bindings/lwipthread.c b/ChibiOS/os/various/lwip_bindings/lwipthread.c
index 38ef8cf..f2cfa28 100644
--- a/ChibiOS/os/various/lwip_bindings/lwipthread.c
+++ b/ChibiOS/os/various/lwip_bindings/lwipthread.c
@@ -217,15 +217,23 @@ msg_t lwip_thread(void *p) {
struct ip_addr ip, gateway, netmask;
static struct netif thisif;
static const MACConfig mac_config = {thisif.hwaddr};
+ tcpip_init_done_fn initfunc = NULL;
+ void *initfunc_arg = NULL;
+ struct lwipthread_opts *opts = p;
chRegSetThreadName("lwipthread");
+ /* Get initialization options.*/
+ if (opts) {
+ initfunc = opts->initfunc;
+ initfunc_arg = opts->initfunc_arg;
+ }
+
/* Initializes the thing.*/
- tcpip_init(NULL, NULL);
+ tcpip_init(initfunc, initfunc_arg);
/* TCP/IP parameters, runtime or compile time.*/
- if (p) {
- struct lwipthread_opts *opts = p;
+ if (opts) {
unsigned i;
for (i = 0; i < 6; i++)
diff --git a/ChibiOS/os/various/lwip_bindings/lwipthread.h b/ChibiOS/os/various/lwip_bindings/lwipthread.h
index fe4de28..8038be1 100644
--- a/ChibiOS/os/various/lwip_bindings/lwipthread.h
+++ b/ChibiOS/os/various/lwip_bindings/lwipthread.h
@@ -25,6 +25,7 @@
#define _LWIPTHREAD_H_
#include <lwip/opt.h>
+#include <lwip/tcpip.h>
/** @brief MAC thread priority.*/
#ifndef LWIP_THREAD_PRIORITY
@@ -110,10 +111,12 @@
* @brief Runtime TCP/IP settings.
*/
struct lwipthread_opts {
- uint8_t *macaddress;
- uint32_t address;
- uint32_t netmask;
- uint32_t gateway;
+ uint8_t *macaddress;
+ uint32_t address;
+ uint32_t netmask;
+ uint32_t gateway;
+ tcpip_init_done_fn initfunc;
+ void *initfunc_arg;
};
extern WORKING_AREA(wa_lwip_thread, LWIP_THREAD_STACK_SIZE);
I have expanded the optional parameter of the lwip_thread() function. It now carries a function pointer and a parameter pointer which are passed to tcpip_init(). This should be compatible with all existing code.
Just starting lwip_thread with a high priority did not work when I used a hardware debugger.
Greetings
Frank
diff --git a/ChibiOS/os/various/lwip_bindings/lwipthread.c b/ChibiOS/os/various/lwip_bindings/lwipthread.c
index 38ef8cf..f2cfa28 100644
--- a/ChibiOS/os/various/lwip_bindings/lwipthread.c
+++ b/ChibiOS/os/various/lwip_bindings/lwipthread.c
@@ -217,15 +217,23 @@ msg_t lwip_thread(void *p) {
struct ip_addr ip, gateway, netmask;
static struct netif thisif;
static const MACConfig mac_config = {thisif.hwaddr};
+ tcpip_init_done_fn initfunc = NULL;
+ void *initfunc_arg = NULL;
+ struct lwipthread_opts *opts = p;
chRegSetThreadName("lwipthread");
+ /* Get initialization options.*/
+ if (opts) {
+ initfunc = opts->initfunc;
+ initfunc_arg = opts->initfunc_arg;
+ }
+
/* Initializes the thing.*/
- tcpip_init(NULL, NULL);
+ tcpip_init(initfunc, initfunc_arg);
/* TCP/IP parameters, runtime or compile time.*/
- if (p) {
- struct lwipthread_opts *opts = p;
+ if (opts) {
unsigned i;
for (i = 0; i < 6; i++)
diff --git a/ChibiOS/os/various/lwip_bindings/lwipthread.h b/ChibiOS/os/various/lwip_bindings/lwipthread.h
index fe4de28..8038be1 100644
--- a/ChibiOS/os/various/lwip_bindings/lwipthread.h
+++ b/ChibiOS/os/various/lwip_bindings/lwipthread.h
@@ -25,6 +25,7 @@
#define _LWIPTHREAD_H_
#include <lwip/opt.h>
+#include <lwip/tcpip.h>
/** @brief MAC thread priority.*/
#ifndef LWIP_THREAD_PRIORITY
@@ -110,10 +111,12 @@
* @brief Runtime TCP/IP settings.
*/
struct lwipthread_opts {
- uint8_t *macaddress;
- uint32_t address;
- uint32_t netmask;
- uint32_t gateway;
+ uint8_t *macaddress;
+ uint32_t address;
+ uint32_t netmask;
+ uint32_t gateway;
+ tcpip_init_done_fn initfunc;
+ void *initfunc_arg;
};
extern WORKING_AREA(wa_lwip_thread, LWIP_THREAD_STACK_SIZE);
-
- Posts: 21
- Joined: Sat Nov 22, 2014 8:23 pm
Re: lwIP start race condition (patch)
Hello,
I am trying to implement your fix because I think this solves one of my problems, I have a delay now before starting to use the lwip otherwise I get into trouble as well.
One thing is a bit unclear to me about starting the thread.
Normally I start the thread like this:
But I can only pass one parameter to the thread from there which should be 3 now with a callback to my tcpinit done function.
The command only accepts parameter, so my question is how is this done to call your code with the thread?
Something like the argv[] way in Linux?
Cheers,
Gerard
I am trying to implement your fix because I think this solves one of my problems, I have a delay now before starting to use the lwip otherwise I get into trouble as well.
One thing is a bit unclear to me about starting the thread.
Normally I start the thread like this:
Code: Select all
chThdCreateFromHeap(&extramHeap, LWIP_THREAD_STACK_SIZE * 4, NORMALPRIO + 1,lwip_thread, NULL);
But I can only pass one parameter to the thread from there which should be 3 now with a callback to my tcpinit done function.
The command only accepts parameter, so my question is how is this done to call your code with the thread?
Something like the argv[] way in Linux?
Cheers,
Gerard
- Giovanni
- Site Admin
- Posts: 14444
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1074 times
- Been thanked: 921 times
- Contact:
Re: lwIP start race condition (patch)
Hi,
In 3.0 it is no more required to start the lwip thread. There is an initialization function to call, see the lwIP demo in 3.0.
Giovanni
In 3.0 it is no more required to start the lwip thread. There is an initialization function to call, see the lwIP demo in 3.0.
Giovanni
-
- Posts: 21
- Joined: Sat Nov 22, 2014 8:23 pm
Re: lwIP start race condition (patch)
Hi Giovanni,
We are still on the 2.6x version, is this possible there?
Or was this proposed patch for the 3.x version only?
Cheers,
Gerard
We are still on the 2.6x version, is this possible there?
Or was this proposed patch for the 3.x version only?
Cheers,
Gerard
- Giovanni
- Site Admin
- Posts: 14444
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1074 times
- Been thanked: 921 times
- Contact:
Re: lwIP start race condition (patch)
Hi,
Note that threads cannot have more than one parameter so I assume that the first patch hasn't really been tested.
2.6 and 3.0 are not much different regarding lwIP.
Giovanni
Note that threads cannot have more than one parameter so I assume that the first patch hasn't really been tested.
2.6 and 3.0 are not much different regarding lwIP.
Giovanni
-
- Posts: 21
- Joined: Sat Nov 22, 2014 8:23 pm
Re: lwIP start race condition (patch)
Hi Giovanni,
Thank you for your reply.
I fixed it by slightly modifying the patch with passing my own struct as the single parameter.
The callback from lwip works like a charm now.
Cheers,
Gerard
Thank you for your reply.
I fixed it by slightly modifying the patch with passing my own struct as the single parameter.
The callback from lwip works like a charm now.
Cheers,
Gerard
Re: lwIP start race condition (patch)
From experience with other similar software, there are two main use cases in terms of network initialisation, having called lwipInit():
a) Block all execution until the network stuff is ready
b) Start the network stuff running, then go away and do non-network stuff. Then wait for network initialisation to complete before starting on network-related bits.
Chibi implements the first case (subject to some caveats on thread priorities). This is simplest to understand and implement
I usually implement the second case; our network port isn't always used, and my impression is that net initialisation takes a lot longer to complete if there's no net cable connected. So my 'main' starts all the non-network threads, waits for net initialisation to complete, then sets up all the net-related threads.
I'm starting to think the best way to present the interface for the second use case. From my perspective a simple blocking call 'waitUntilNetworkready()' would suffice (probably implemented as a wait on semaphore in the underlying code). Is there need for more than this?
a) Block all execution until the network stuff is ready
b) Start the network stuff running, then go away and do non-network stuff. Then wait for network initialisation to complete before starting on network-related bits.
Chibi implements the first case (subject to some caveats on thread priorities). This is simplest to understand and implement
I usually implement the second case; our network port isn't always used, and my impression is that net initialisation takes a lot longer to complete if there's no net cable connected. So my 'main' starts all the non-network threads, waits for net initialisation to complete, then sets up all the net-related threads.
I'm starting to think the best way to present the interface for the second use case. From my perspective a simple blocking call 'waitUntilNetworkready()' would suffice (probably implemented as a wait on semaphore in the underlying code). Is there need for more than this?
Who is online
Users browsing this forum: No registered users and 12 guests