patchlwIP - A Lightweight TCP/IP stack - Patches: patch #8737, tcp_timer_needed should use...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #8737: tcp_timer_needed should use tcpip_timeout

Submitter:  Robert Szewczyk <robszewczyk>
Submitted:  Fri 11 Sep 2015 05:13:33 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Invalid Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Fri 11 Sep 2015 09:31:07 PM UTC, comment #6: 

There are two more things worth bringing up:

  • Even with the warnings in rawapi.txt about thread safety, a programmer might lull themselves into a (false) sense of security by thinking: "they warned us about the thread safety, but they provided these helpful LOCK_TCPIP_CORE/UNLOCK_TCPIP_CORE, so I'll use those in the 'right' places and it will be great".  That's the path we pursued.  The particular problem with tcpip timers was exposed when we tuned the system to minimize the wakeups.
  • Some functions (as we've discovered recently) are not in the standard API, and, yet, they do call into tcp_* methods -- the particular function that came to bite us recently was the dhcp_release, which may ultimately close a connection.  Perhaps the "proper" solution here would have also been to go through tcpip_callback.


Robert Szewczyk <robszewczyk>
Fri 11 Sep 2015 08:09:55 PM UTC, comment #5: 

Confession time.

I must admit I did it wrong as well. When I got hired at my current job
they were already using lwIP in some products (which were already on
market at this time). Well, they used the lwIP AVR32 port from a very
very -VERY- old Atmel Framework they never took the time to upgrade and
used lwIP 1.3.0 while 1.4.x was released for quite a while. They didn't
understand lwIP RAW API wasn't thread safe (well, thread safety is still
quite an obscure concept) and used the RAW API everywhere. It worked
because we are mostly using UDP, and the UDP FSM doesn't suffer much
from thread safety issues (yes, that's obvious). They didn't even
understand the pbuf chained concept and were stuck to "lwIP doesn't
support packets over > XXX bytes, the packet is corrupted and we only
get a truncated packet if we send more than XXX bytes". (I KID NOT YOU).

I just followed what they did until it hit my head that we were doing
things wrong… terribly wrong… in already sold products for a while…
yikes.

Maybe we should just add in CAPITALS in main RAW API functions
(udp_bind(), udp_send(), tcp_write(), tcp_listent(), …) comment just
above them that they are not thread safe, at least developers who care
about thread safety would be warned the first time they want to use
them.

Sylvain

Sylvain Rochet <gradator>
Group Member
Fri 11 Sep 2015 07:45:11 PM UTC, comment #4: 


>> then something is wrong with your application.
>> The raw API must not be used from a different thread.
>
> What's the recommendation or alternative?


Reading doc/rawapi.txt or lwip.wikia.com! I know that's not a popular answer... (-> "using one of the api headers' functions or using tcpip_callback to get you into the right thread" is probably what you expected)

However, I would have excpeted this question from a newbie, but I'm honestly surprised to read it from someone knowing lwip since more than 2 years...

It would be nice if someone could tell me how to improve documentation (or asserts in the code?) to help users to better understand the basic threading requirements of lwip. This kind of misunderstanding keeps popping up too often.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 11 Sep 2015 07:38:47 PM UTC, comment #3: 

Netconn and socket API, see lwip/doc/rawapi.txt

Sylvain Rochet <gradator>
Group Member
Fri 11 Sep 2015 07:18:31 PM UTC, comment #2: 


> then something is wrong with your application. The raw API must not be used from a different thread.


What's the recommendation or alternative?

Grant Erickson <marathon96>
Fri 11 Sep 2015 05:40:58 PM UTC, comment #1: 


> When tcp_connect is called from a different thread


then something is wrong with your application. The raw API must not be used from a different thread.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 11 Sep 2015 05:13:33 PM UTC, original submission:  

When tcp_connect is called from a different thread, it is possible that the tcp timers do not get started properly.  This happens when there are no TCP timers active and the SYN packet is lost (or not responded to). Previous code would simply adjust the timer queue, but not fix the LwIP wakeup schedule; in the absence of active timers and incoming packets, there would be nothing to wakeup the LwIP thread to adjust the wakeup to reflect the new state of the timer queue.  This patch replaces the direct call to sys_timeout with a tcpip_timeout call that wakes up the LwIP thread.

Robert Szewczyk <robszewczyk>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gradator (Posted a comment)
  • -email is unavailable- added by marathon96 (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by robszewczyk (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-09-11 goldsimon StatusNone Invalid
        Open/ClosedOpen Closed
    2015-09-11 robszewczyk Attached File- Added 0001-tcp_timer_needed-should-use-tcpip_timeout.patch, #34861

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code