[lwip-users] Some Feedback on Update to 1.3.0 from 1.1.0

[Caglar Akyuz]

Hi,

I've updated my project lwip version from 1.1.0 to 1.3.0(stable) lately. First 
of 
all, thanks for the latest updates, especially for the documentation. I think
current wiki is outstanding in contents. (lwIP was always)

Secondly I want give some feedback about my porting experience. I'm presenting
my results at the end of the mail. I don't know if they are correct or not, 
they 
are just for your information.

Best regards,
Caglar 

_____________________________________________________________________________________

1) In file ip.c, in function ip_input there is:

    if (iphdr_hlen > p->len)
    LWIP_DEBUGF(IP_DEBUG | 2, ("IP header (len %"U16_F") does not fit in first 
pbuf (len %"U16_F"), IP packet dropped.\n",
                               iphdr_hlen, p->len));
    if (iphdr_len > p->tot_len)
    LWIP_DEBUGF(IP_DEBUG | 2, ("IP (len %"U16_F") is longer than pbuf (len 
%"U16_F"), "
                               "IP packet dropped.\n",
                               iphdr_len, p->tot_len));


When debugging is not enabled, gcc gives some warning about empty if statements.
So I added "#ifdef LWIP_DEBUG" to this section. There may be something better, 
though.

2) Currently in debug.h LWIP_ERROR is defined as:

#ifndef LWIP_ERROR
#define LWIP_ERROR(m,e,h) do { if (!(e)) { LWIP_PLATFORM_ASSERT(m); h;}} 
while(0)
#endif /* LWIP_ERROR */

I think this is spelled wrong and it is incomplete:

#ifndef LWIP_NOERROR
#define LWIP_ERROR(m,e,h) do { if (!(e)) { LWIP_PLATFORM_ASSERT(m); h;}} 
while(0)
#else
#define LWIP_ERROR(m,e,h)
#endif /* LWIP_ERROR */

3) Wiki mentions SYS_STATS is obsolete and should be removed from the lwipopts.h
but I think it is used. 

4) In the skeleton ethernetif driver, ethernetif.c, in function 
ethernetif_input 
there is section like:

  switch (htons(ethhdr->type)) {
  /* IP or ARP packet? */
  case ETHTYPE_IP:
  case ETHTYPE_ARP:
#if PPPOE_SUPPORT
  /* PPPoE packet? */
  case ETHTYPE_PPPOEDISC:
  case ETHTYPE_PPPOE:
#endif /* PPPOE_SUPPORT */
    /* full packet send to tcpip_thread to process */
    if (netif->input(p, netif)!=ERR_OK)
     { LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_input: IP input error\n"));
       pbuf_free(p);
       p = NULL;
     }
    break;

  default:
    pbuf_free(p);
    p = NULL;
    break;
  }

So first I implemented using this block as:

                switch( htons( ethhdr->type ) )
                {
                        /* IP packet? */
                        case ETHTYPE_IP:
                                /* update ARP table */
                                etharp_ip_input( xNetIf, p );
                
                                /* skip Ethernet header */
                                pbuf_header( p, (s16_t)-sizeof(struct eth_hdr) 
);
                
                                /* pass to network layer */
                                xNetIf->input( p, xNetIf );
                                break;
                
                        case ETHTYPE_ARP:
                                /* pass p to ARP module */
                                etharp_arp_input( xNetIf, ethernetif->ethaddr, 
p );
                                break;
                
                        default:
                                pbuf_free( p );
                                p = NULL;
                                break;
                }

But the problem with this is, when LWIP_ARP is defined and packet type
is ETHTYPE_IP, tcpip_input function is automatically checking mac address, 
skipping ethernet header and calling ip_input. So header is skipped twice 
and stack just don't work. So I added a "#ifdef LWIP_ARP" to this block. In 
this case I call xNetIf->input( p, xNetIf ) directly without skipping header
as seen in the following code and everything works ok. Am I doing something 
wrong here?

                #if LWIP_ARP
                        /* 
                         * If arp is used, we can directly call
                         * network layer input because it checks
                         * for the arp internally
                         */
                        xNetIf->input( p, xNetIf );
                #else
                                switch( htons( ethhdr->type ) )
                {
                        /* IP packet? */
                        case ETHTYPE_IP:
                                /* update ARP table */
                                etharp_ip_input( xNetIf, p );
                
                                /* skip Ethernet header */
                                pbuf_header( p, (s16_t)-sizeof(struct eth_hdr) 
);
                
                                /* pass to network layer */
                                xNetIf->input( p, xNetIf );
                                break;
                
                        case ETHTYPE_ARP:
                                /* pass p to ARP module */
                                etharp_arp_input( xNetIf, ethernetif->ethaddr, 
p );
                                break;
                
                        default:
                                pbuf_free( p );
                                p = NULL;
                                break;
                }
                #endif
        }