[bug #27996] Thread-safety of tcp_timer_needed()

[Oleg Tyshev]

URL:
  <http://savannah.nongnu.org/bugs/?27996>

                 Summary: Thread-safety of tcp_timer_needed()
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: albertb
            Submitted on: Mi 11 Nov 2009 13:53:00 GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release:
            lwIP version: CVS Head

    _______________________________________________________

Details:

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.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27996>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Oleg Tyshev]

Follow-up Comment #1, bug #27996 (project lwip):

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



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27996>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Oleg Tyshev]

Follow-up Comment #2, bug #27996 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27996>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Oleg Tyshev]

Follow-up Comment #3, bug #27996 (project lwip):

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?

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27996>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Oleg Tyshev]

Follow-up Comment #4, bug #27996 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27996>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Oleg Tyshev]

Update of bug #27996 (project lwip):

                  Status:                    None => Invalid                
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up 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 :-)

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27996>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel