buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27996, Thread-safety of tcp_timer_needed()

 
 

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

bug #27996: Thread-safety of tcp_timer_needed()

Submitted by:  Albert Bartel <albertb>
Submitted on:  Wed 11 Nov 2009 01:53:00 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Invalid
Privacy: PublicAssigned to: None
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

Wed 11 Nov 2009 03:58:35 PM UTC, comment #5:

> You say, that if it is called from a different thread that's the bug.
> So is it wrong to call ethernet_input() from the separate ethernetif thread?


Yes, in two ways:

1) ethernet_input() (as any other function from the core files - except for some pbuf/mem/memp functions) must not be called from any other thread than the tcpip_thread

2) the port should not call ethernet_input() but instead initialize the netif correctly with the netif_add() call, set the flag NETIF_FLAG_ETHARP in its _init() function (see example ethernetif.c) and then call netif->input() instead of ethernet_input() (also, see example ethernetif.c), which takes care of getting into the correct thread and caling ethernet_input().

As you see, the example ethernetif.c would have been a good guidance :-)

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 11 Nov 2009 03:27:48 PM UTC, comment #4:

Something should be calling tcpip_input() instead of ip_input() directly. That is most likely your bug.

Kieran Mansley <kieranm>
Project Administrator
Wed 11 Nov 2009 03:17:33 PM UTC, comment #3:

I have got an ATMEL AVR32 UC3 port for FreeRTOS. In this port the ethernetif.c creates a thread for ethernetif_input.
The thread function is:
static void ethernetif_input(void * pvParameters)
{
struct netif netif = (struct netif )pvParameters;
struct pbuf *p;

for( ;; )
{
do
{
/* move received packet into a new pbuf */
p = low_level_input( netif );
if( p == NULL )
{
/* No packet could be read. Wait a for an interrupt to
tell us there is more data available. */
vMACBWaitForInput(100);
}
}while( p == NULL );

if( ERR_OK != ethernet_input( p, netif ) )
{
pbuf_free(p);
p = NULL;
}
}
}

In this port, if a SYN arrives, ethernet_input(struct pbuf p, struct netif netif) is called.
Then ip_input(struct pbuf p, struct netif inp) is called,
then tcp_input(struct pbuf p, struct netif inp),
then tcp_listen_input(struct tcp_pcb_listen *pcb)
and there TCP_REG((pcbs, npcb)) is called,
that calls tcp_timer_needed().
So the timeout will be set only for this thread.
When I set a breakpoint in the tcpip_tcp_timer-function, I never get there, because the tcpip thread does not handle the timeout, because it is in the wrong thread.
After changing sys_timeout() to tcpip_timeout() I pass the breakpoint if a timeout occurrs.

You say, that if it is called from a different thread that's the bug. So is it wrong to call ethernet_input() from the separate ethernetif thread?

Albert Bartel <albertb>
Wed 11 Nov 2009 02:05:19 PM UTC, comment #2:

I don't think this is necessary because tcp_timer_needed() is only called from tcpip_thread. If not, that's the bug.

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 11 Nov 2009 01:56:07 PM UTC, comment #1:

Can you give a bit more detail about the problem you're seeing that this change fixes?

Kieran Mansley <kieranm>
Project Administrator
Wed 11 Nov 2009 01:53:00 PM UTC, original submission:

The function tcp_timer_needed(void) should not call

sys_timeout(TCP_TMR_INTERVAL, tcpip_tcp_timer, NULL);

directly, but tell the tcpip thread to set the timeout thread-safe by calling the existing function

tcpip_timeout(TCP_TMR_INTERVAL, tcpip_tcp_timer, NULL);

that sends TCPIP_MSG_TIMEOUT to the tcpip thread.

Albert Bartel <albertb>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by albertb (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 11 Nov 2009 03:58:35 PM UTCgoldsimonStatusNone=>Invalid
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup1