Re: [lwip-users] Patch for bug 23119

[Ken Smith]

On Fri, Mar 13, 2009 at 9:04 AM, address@hidden <address@hidden> wrote:
>
> Ken Smith schrieb:
>>
>> Also, in looking at my ethernet driver the way it interfaces with
>> lwIP, I noticed some meaningful differences between my implementation
>> and the reference implementation in CVS HEAD.
>
> The infinite loop is not a problem for the stack (I'd expect the thread waits 
> on a semaphore that is signaled by an RX interrupt). The real problem with 
> your code (I think that was what it was like before 1.3.0) is that you call 
> directly into the etharp module instead of doing this in the lwip_thread, 
> which leads to a concurrent access problem in the module.
>
> The new code calls netif->input for *all* accepted packets (with p->payload 
> pointing to the start of the packet/ethernet header). An ethernet netif 
> should be initialized with the correct flag and with tcpip_input as ->input 
> function. This way, all code is called from one thread only (the 
> tcpip_thread), other threads only use the 2 sequential APIs or 
> sys/pbuf/mem/memp functions.

I tried to convert my ethernetif.c to match the style of the example
that comes with 1.3.0 without the ethernetif_input thread and my
application was unable to do any networking.  My naive approach was
obviously not the answer.  I tried making just the switch statement in
ethernetif_input look like the 1.3.0 example and my application didn't
respond to ARP packets and wasn't able to make its first connection.
So it appears that, as I have it configured, I need to keep calling
etharp_arp_input from my ethernetif_input thread.  Here is what I have
now in ethernetif_input.

347         switch( htons( ethhdr->type ) )
348         {
349             /* IP packet? */
350             case ETHTYPE_IP:
351                 /* skip Ethernet header */
352                 pbuf_header( p, (s16_t)-sizeof(struct eth_hdr) );
353
354                 /* pass to network layer */
355                 if (xNetIf->input( p, xNetIf ) != ERR_OK)
356                 {
357                     pbuf_free(p);
358                     p = NULL;
359                 }
360                 break;
361
362             case ETHTYPE_ARP:
363                 portENTER_CRITICAL();
364                 /* pass p to ARP module */
365                 etharp_arp_input( xNetIf, ethernetif->ethaddr, p );
366                 portEXIT_CRITICAL();
367                 break;
368
369             default:
370                 pbuf_free( p );
371                 p = NULL;
372                 break;
373         }

...and in etharp.c, in ethernet_input, I have this...

1160     case ETHTYPE_ARP:
1161       /* pass p to ARP module */
1162       portENTER_CRITICAL();
1163       etharp_arp_input(netif, (struct eth_addr*)(netif->hwaddr), p);
1164       portEXIT_CRITICAL();
1165       break;

So all calls to etharp_arp_input are their own critical sections.
Will this avoid the problem of calling into the etharp module from
outside the tcpip_thread context?  Other comments welcome.

   Regards,
   Ken