lwIP start race condition (patch)

ChibiOS public support forum for all topics not covered by a specific support forum.

Moderators: RoccoMarco, lbednarz, utzig, tfAteba, barthess

fjb
Posts: 11
Joined: Tue Dec 07, 2010 9:49 pm

lwIP start race condition (patch)

Postby fjb » Sun Jun 28, 2015 7:50 pm

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

User avatar
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)

Postby Giovanni » Sun Jun 28, 2015 9:22 pm

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

fjb
Posts: 11
Joined: Tue Dec 07, 2010 9:49 pm

Re: lwIP start race condition (patch)

Postby fjb » Sun Jun 28, 2015 10:30 pm

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

fjb
Posts: 11
Joined: Tue Dec 07, 2010 9:49 pm

Re: lwIP start race condition (new patch)

Postby fjb » Tue Jun 30, 2015 9:10 pm

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);

gvandenbosch
Posts: 21
Joined: Sat Nov 22, 2014 8:23 pm

Re: lwIP start race condition (patch)

Postby gvandenbosch » Fri Jul 24, 2015 9:11 am

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:

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

User avatar
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)

Postby Giovanni » Fri Jul 24, 2015 10:13 am

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

gvandenbosch
Posts: 21
Joined: Sat Nov 22, 2014 8:23 pm

Re: lwIP start race condition (patch)

Postby gvandenbosch » Fri Jul 24, 2015 11:52 am

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

User avatar
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)

Postby Giovanni » Fri Jul 24, 2015 12:36 pm

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

gvandenbosch
Posts: 21
Joined: Sat Nov 22, 2014 8:23 pm

Re: lwIP start race condition (patch)

Postby gvandenbosch » Fri Jul 24, 2015 6:46 pm

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

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: lwIP start race condition (patch)

Postby steved » Tue Jul 28, 2015 2:00 pm

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?


Return to “General Support”

Who is online

Users browsing this forum: No registered users and 12 guests