Merged from DEVEL. etharp prepared for queueing feature. DHCP fix.

CHANGELOG

@@ -5,8 +5,20 @@ TODO
 
 HISTORY
 
+(STABLE-0_6_6)
+
+  ++ Bug fixes:
+
+  * Fixed DHCP which did not include the IP address in DECLINE messages.
+
+  ++ Changes:
+
+  * etharp.c has been hauled over a bit.
+
 (STABLE-0_6_5)
 
+  ++ Bug fixes:
+
   * Fixed TCP bug induced by bad window resizing with unidirectional TCP traffic.
   * Packets sent from ARP queue had invalid source hardware address.
 

src/core/dhcp.c

@@ -192,6 +192,7 @@ static void dhcp_handle_offer(struct netif *netif)
     /* remember offered address */
     ip_addr_set(&dhcp->offered_ip_addr, (struct ip_addr *)&dhcp->msg_in->yiaddr);
     LWIP_DEBUGF(DHCP_DEBUG | DBG_STATE, ("dhcp_handle_offer(): offer for 0x%08lx\n", dhcp->offered_ip_addr.addr));
+
     dhcp_select(netif);
   }
 }
@@ -646,6 +647,9 @@ static err_t dhcp_decline(struct netif *netif)
     dhcp_option(dhcp, DHCP_OPTION_MAX_MSG_SIZE, DHCP_OPTION_MAX_MSG_SIZE_LEN);
     dhcp_option_short(dhcp, 576);
 
+    dhcp_option(dhcp, DHCP_OPTION_REQUESTED_IP, 4);
+    dhcp_option_long(dhcp, ntohl(dhcp->offered_ip_addr.addr));
+
     dhcp_option_trailer(dhcp);
     /* resize pbuf to reflect true size of options */
     pbuf_realloc(dhcp->p_out, sizeof(struct dhcp_msg) - DHCP_OPTIONS_LEN + dhcp->options_out_len);

src/core/memp.c

@@ -220,8 +220,6 @@ memp_malloc(memp_t type)
      ((u32_t)MEM_ALIGN((u8_t *)memp + sizeof(struct memp)) % MEM_ALIGNMENT) == 0);
 
     mem = MEM_ALIGN((u8_t *)memp + sizeof(struct memp));
-    /* initialize memp memory with zeroes */
-    memset(mem, 0, memp_sizes[type]);  
     return mem;
   } else {
     LWIP_DEBUGF(MEMP_DEBUG | 2, ("memp_malloc: out of memory in pool %d\n", type));

src/core/netif.c

@@ -39,7 +39,6 @@
 #include "lwip/opt.h"
 
 #include "lwip/def.h"
-#include "lwip/mem.h"
 #include "lwip/netif.h"
 #include "lwip/ip_addr.h"
 #include "lwip/tcp.h"
@@ -61,22 +60,15 @@ struct netif *netif_default = NULL;
  * @return netif, or NULL if failed.
  */
 struct netif *
-netif_add(struct ip_addr *ipaddr, struct ip_addr *netmask,
+netif_add(struct netif *netif, struct ip_addr *ipaddr, struct ip_addr *netmask,
   struct ip_addr *gw,
   void *state,
   err_t (* init)(struct netif *netif),
   err_t (* input)(struct pbuf *p, struct netif *netif))
 {
-  struct netif *netif;
   static int netifnum = 0;
 
-  /* allocate netif structure */
-  netif = mem_malloc(sizeof(struct netif));
-
-  if (netif == NULL) {
-    LWIP_DEBUGF(NETIF_DEBUG, ("netif_add(): out of memory for netif\n"));
-    return NULL;
-  }
+  
 #if LWIP_DHCP
   /* netif not under DHCP control by default */
   netif->dhcp = NULL;
@@ -90,7 +82,6 @@ netif_add(struct ip_addr *ipaddr, struct ip_addr *netmask,
 
   /* call user specified initialization function for netif */
   if (init(netif) != ERR_OK) {
-    mem_free(netif);
     return NULL;
   }
 
@@ -142,7 +133,6 @@ void netif_remove(struct netif * netif)
     /* reset default netif */
     netif_default = NULL;
   LWIP_DEBUGF( NETIF_DEBUG, ("netif_remove: removed netif\n") );
-  mem_free( netif );
 }
 
 struct netif *

src/core/pbuf.c

@@ -504,29 +504,33 @@ pbuf_header(struct pbuf *p, s16_t header_size)
 }
 
 /**
- * Free a pbuf (chain) from usage, de-allocate non-used head of chain.
+ * Dereference a pbuf (chain) and deallocate any no-longer-used
+ * pbufs at the head of this chain.
  *
  * Decrements the pbuf reference count. If it reaches
  * zero, the pbuf is deallocated.
  *
- * For a pbuf chain, this is repeated for each pbuf in the chain, up to the 
- * pbuf which has a non-zero reference count after decrementing. 
- * (This might be the whole chain.)
+ * For a pbuf chain, this is repeated for each pbuf in the chain,
+ * up to a pbuf which has a non-zero reference count after
+ * decrementing. (This might de-allocate the whole chain.)
  *
- * @param pbuf pbuf (chain) to be freed from one user.
+ * @param pbuf The pbuf (chain) to be dereferenced.
  *
- * @return the number of unreferenced pbufs that were de-allocated
+ * @return the number of pbufs that were de-allocated
  * from the head of the chain.
  *
- * @note May not be called on a packet queue.
+ * @note MUST NOT be called on a packet queue.
  * @note the reference counter of a pbuf equals the number of pointers
  * that refer to the pbuf (or into the pbuf).
  *
  * @internal examples:
  *
+ * Assuming existing chains a->b->c with the following reference
+ * counts, calling pbuf_free(a) results in:
+ * 
  * 1->2->3 becomes ...1->3
  * 3->3->3 becomes 2->3->3
- * 1->1->2 becomes ....->1
+ * 1->1->2 becomes ......1
  * 2->1->1 becomes 1->1->1
  * 1->1->1 becomes .......
  *
@@ -636,11 +640,12 @@ pbuf_ref(struct pbuf *p)
 
 /**
  * Concatenate two pbufs (each may be a pbuf chain) and take over
- * the reference of the tail pbuf.
+ * the caller's reference of the tail pbuf.
  * 
- *  @note The caller MAY NOT reference the tail pbuf afterwards.
+ * @note The caller MAY NOT reference the tail pbuf afterwards.
+ * Use pbuf_chain() for that purpose.
  * 
- *  @see pbuf_chain()
+ * @see pbuf_chain()
  */
 
 void
@@ -650,16 +655,14 @@ pbuf_cat(struct pbuf *h, struct pbuf *t)
 
   LWIP_ASSERT("h != NULL", h != NULL);
   LWIP_ASSERT("t != NULL", t != NULL);
-
-  if (t == NULL)
-    return;
+  if ((h == NULL) || (t == NULL)) return;
 
   /* proceed to last pbuf of chain */
   for (p = h; p->next != NULL; p = p->next) {
     /* add total length of second chain to all totals of first chain */
     p->tot_len += t->tot_len;
   }
-  /* p is last pbuf of first h chain */
+  /* { p is last pbuf of first h chain, p->next == NULL } */
   LWIP_ASSERT("p->tot_len == p->len (of last pbuf in chain)", p->tot_len == p->len);
   /* add total length of second chain to last pbuf total of first chain */
   p->tot_len += t->tot_len;
@@ -668,13 +671,15 @@ pbuf_cat(struct pbuf *h, struct pbuf *t)
 }
 
 /**
- * Chain two pbufs (or pbuf chains) together. They must belong to the same packet.
- * It's the same as pbuf_cat with the addition that it increases the reference count
- * of the tail.
+ * Chain two pbufs (or pbuf chains) together.
+ * 
+ * The caller MUST call pbuf_free(t) once it has stopped
+ * using it. Use pbuf_cat() instead if you no longer use t.
  * 
  * @param h head pbuf (chain)
  * @param t tail pbuf (chain)
- * @note May not be called on a packet queue.
+ * @note The pbufs MUST belong to the same packet.
+ * @note MAY NOT be called on a packet queue.
  *
  * The ->tot_len fields of all pbufs of the head chain are adjusted.
  * The ->next field of the last pbuf of the head chain is adjusted.
@@ -685,7 +690,7 @@ void
 pbuf_chain(struct pbuf *h, struct pbuf *t)
 {
   pbuf_cat(h, t);
-  /* t is now referenced to one more time */
+  /* t is now referenced by h */
   pbuf_ref(t);
   LWIP_DEBUGF(PBUF_DEBUG | DBG_FRESH | 2, ("pbuf_chain: %p references %p\n", (void *)h, (void *)t));
 }

src/core/tcp_out.c

@@ -133,7 +133,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     pcb->unsent != NULL);
   }
 
-  seg = NULL;
+  seg = useg = NULL;
   seglen = 0;
 
   /* First, break up the data into segments and tuck them together in
@@ -158,6 +158,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     }
     else {
       /* Attach the segment to the end of the queued segments. */
+      LWIP_ASSERT("useg != NULL", useg != NULL);
       useg->next = seg;
       useg = seg;
     }

src/include/lwip/dhcp.h

@@ -191,10 +191,10 @@ void dhcp_fine_tmr(void);
 #define DHCP_OPTION_MESSAGE_TYPE_LEN 1
 
 
-#define DHCP_OPTION_SERVER_ID 54 /* RFC 2131 9.7, server IP address */
-#define DHCP_OPTION_PARAMETER_REQUEST_LIST 55 /* RFC 2131 9.8, requested option types */
+#define DHCP_OPTION_SERVER_ID 54 /* RFC 2132 9.7, server IP address */
+#define DHCP_OPTION_PARAMETER_REQUEST_LIST 55 /* RFC 2132 9.8, requested option types */
 
-#define DHCP_OPTION_MAX_MSG_SIZE 57 /* RFC 2131 9.10, message size accepted >= 576 */
+#define DHCP_OPTION_MAX_MSG_SIZE 57 /* RFC 2132 9.10, message size accepted >= 576 */
 #define DHCP_OPTION_MAX_MSG_SIZE_LEN 2
 
 #define DHCP_OPTION_T1 58 /* T1 renewal time */

src/include/lwip/netif.h

@@ -115,7 +115,7 @@ extern struct netif *netif_default;
 /* netif_init() must be called first. */
 void netif_init(void);
 
-struct netif *netif_add(struct ip_addr *ipaddr, struct ip_addr *netmask,
+struct netif *netif_add(struct netif *netif, struct ip_addr *ipaddr, struct ip_addr *netmask,
       struct ip_addr *gw,
       void *state,
       err_t (* init)(struct netif *netif),

src/netif/etharp.c

@@ -42,6 +42,13 @@
  *
  */
 
+/**
+ * TODO:
+ * - pbufs should be sent from the queue once an ARP entry state
+ *   goes from PENDING to STABLE.
+ * - Non-PENDING entries MUST NOT have queued packets.
+ */
+
 /*
  * TODO:
  *
@@ -113,6 +120,9 @@ struct etharp_entry {
   struct eth_addr ethaddr;
   enum etharp_state state;
 #if ARP_QUEUEING
+  /** 
+   * Pointer to queue of pending outgoing packets on this ARP entry.
+   * Must be at most a single packet for now. */
   struct pbuf *p;
 #endif
   u8_t ctime;
@@ -121,22 +131,27 @@ struct etharp_entry {
 static const struct eth_addr ethbroadcast = {{0xff,0xff,0xff,0xff,0xff,0xff}};
 static struct etharp_entry arp_table[ARP_TABLE_SIZE];
 
-static struct pbuf *update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *ethaddr, u8_t flags);
+static s8_t find_arp_entry(void);
 #define ARP_INSERT_FLAG 1
-
+static struct pbuf *update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *ethaddr, u8_t flags);
+#if ARP_QUEUEING
+static struct pbuf *etharp_enqueue(s8_t i, struct pbuf *q);
+static u8_t etharp_dequeue(s8_t i);
+#endif
 /**
  * Initializes ARP module.
  */
 void
 etharp_init(void)
 {
-  u8_t i;
+  s8_t i;
   /* clear ARP entries */
   for(i = 0; i < ARP_TABLE_SIZE; ++i) {
     arp_table[i].state = ETHARP_STATE_EMPTY;
 #if ARP_QUEUEING
     arp_table[i].p = NULL;
 #endif
+    arp_table[i].ctime = 0;
   }
 }
 
@@ -149,22 +164,28 @@ etharp_init(void)
 void
 etharp_tmr(void)
 {
-  u8_t i;
+  s8_t i;
 
   LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer\n"));
   /* remove expired entries from the ARP table */
   for (i = 0; i < ARP_TABLE_SIZE; ++i) {
     arp_table[i].ctime++;
+    /* a resolved/stable entry? */
     if ((arp_table[i].state == ETHARP_STATE_STABLE) &&
+         /* entry has become old? */
         (arp_table[i].ctime >= ARP_MAXAGE)) {
       LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer: expired stable entry %u.\n", i));
       goto empty;
+    /* an unresolved/pending entry? */
     } else if ((arp_table[i].state == ETHARP_STATE_PENDING) &&
+         /* entry unresolved/pending for too long? */
         (arp_table[i].ctime >= ARP_MAXPENDING)) {
       LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer: expired pending entry %u.\n", i));
-  empty:      
+  empty:
+      /* empty old entry */      
       arp_table[i].state = ETHARP_STATE_EMPTY;
 #if ARP_QUEUEING
+      /* and empty packet queue */
       if (arp_table[i].p != NULL) {
         /* remove any queued packet */
         LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer: freeing entry %u, packet queue %p.\n", i, (void *)(arp_table[i].p)));
@@ -177,55 +198,125 @@ etharp_tmr(void)
 }
 
 /**
- * Return an empty ARP entry or, if the table is full, ARP_TABLE_SIZE if all
- * entries are pending, otherwise the oldest entry.
+ * Return an empty ARP entry (possibly recycling the oldest stable entry).
  *
- * @return The ARP entry index that is available, ARP_TABLE_SIZE if no usable
+ * @return The ARP entry index that is available, ERR_MEM if no usable
  * entry is found.
  */
-static u8_t
+static s8_t
 find_arp_entry(void)
 {
-  u8_t i, j, maxtime;
+  s8_t i, j;
+  u8_t maxtime = 0;
 
-  /* Try to find an unused entry in the ARP table. */
+  j = ARP_TABLE_SIZE;
+  /* search ARP table for an unused or old entry */
   for (i = 0; i < ARP_TABLE_SIZE; ++i) {
+  	/* empty entry? */
     if (arp_table[i].state == ETHARP_STATE_EMPTY) {
-      LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: found empty entry %u\n", i));
-      break;
+      LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: returning empty entry %u\n", i));
+      return i;
+  	/* stable entry? */
+    } else if (arp_table[i].state == ETHARP_STATE_STABLE) {
+      /* remember entry with oldest stable entry in j */
+      if (arp_table[i].ctime >= maxtime) maxtime = arp_table[j = i].ctime;
     }
   }
-
-  /* If no unused entry is found, we try to find the oldest entry and
-     throw it away. If all entries are new and have 0 ctime drop one  */
+  /* no empty entry found? */
   if (i == ARP_TABLE_SIZE) {
-    maxtime = 0;
-    j = ARP_TABLE_SIZE;
-    for (i = 0; i < ARP_TABLE_SIZE; ++i) {
-      /* remember entry with oldest stable entry in j*/
-      if ((arp_table[i].state == ETHARP_STATE_STABLE) &&
-#if ARP_QUEUEING /* do not want to re-use an entry with queued packets */
-      (arp_table[i].p == NULL) &&
+  	LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: found oldest stable entry %u\n", j));
+    /* fall-back to oldest stable */
+  	i = j;
+  }
+  /* no available entry found? */
+  if (i == ARP_TABLE_SIZE) {
+    LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: no replacable entry could be found\n"));
+    /* return failure */
+    return ERR_MEM;
+  }
+
+  /* clean up the recycled stable entry */
+  if (arp_table[i].state == ETHARP_STATE_STABLE) {
+#if ARP_QUEUEING
+    /* free packets on queue */
+    etharp_dequeue(i);
 #endif
-      (arp_table[i].ctime >= maxtime)) {
-        maxtime = arp_table[i].ctime;
-        j = i;
-      }
-    }
-    if (j != ARP_TABLE_SIZE) {
-      LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: found oldest stable entry %u\n", j));
-    } else {
-      LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: no replacable entry could be found\n"));
-    }
-    i = j;
+    LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("find_arp_entry: recycling oldest stable entry %u\n", i));
+    arp_table[i].state = ETHARP_STATE_EMPTY;
+    arp_table[i].ctime = 0;
   }
-  LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: returning %u, state %u\n", i, arp_table[i].state));
+  LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: returning %u\n", i));
   return i;
 }
 
+#if ARP_QUEUEING
+/*
+ * Enqueues a pbuf (chain) on an ARP entry.
+ * 
+ * Places the pbuf (chain) on the queue (if space allows). The
+ * caller may safely free the pbuf (chain) afterwards, as the
+ * pbufs will be referenced by the queue and copies are made of
+ * pbufs referencing external payloads.
+ * 
+ * @ i the ARP entry index
+ * @arg q the pbuf (chain) to be queued on the ARP entry
+ * 
+ * @return Returns the new head of queue of the ARP entry.
+ * 
+ */
+static struct pbuf *
+etharp_enqueue(s8_t i, struct pbuf *q)
+{
+  /* any pbuf to queue? */
+  if (q != NULL) {
+/* queue later packet over earliers? TODO: Implement multiple pbuf queue */
+#if ARP_QUEUE_FIRST == 0
+    /* remove any pbufs on queue */
+    u8_t deq = etharp_dequeue(i);
+    if (deq > 0) LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | 3, ("etharp_query: dequeued %u pbufs from ARP entry %u. Should not occur.\n", deq, i));
+#endif
+    /* packet can be queued? TODO: Implement multiple pbuf queue */
+    if (arp_table[i].p == NULL) {
+      /* copy any PBUF_REF referenced payloads into PBUF_RAM */
+      q = pbuf_take(q);
+      /* add pbuf to queue */
+      arp_table[i].p = q;
+      /* pbuf (chain) now queued, increase the reference count */
+      pbuf_ref(q);
+      LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | DBG_STATE, ("etharp_query: queued packet %p on ARP entry %u.\n", (void *)q, i));
+    }
+  }
+  return arp_table[i].p;
+}
+
+/**
+ * Dequeues any pbufs queued on an ARP entry
+ * 
+ * @return number of pbufs removed from the queue
+ * 
+ * TODO: decide what is a sensible return value?
+ */
+static u8_t
+etharp_dequeue(s8_t i)
+{
+  /* queued packets on a stable entry (work in progress) */
+  if (arp_table[i].p != NULL) {
+  	/* queue no longer references pbuf */
+  	pbuf_free(arp_table[i].p);
+    arp_table[i].p = NULL;
+    return 1;
+  } else {
+    return 0;
+  }
+}
+#endif
+
 /**
  * Update (or insert) a IP/MAC address pair in the ARP cache.
  *
+ * If a pending entry is resolved, any queued packets will be sent
+ * at this point.
+ * 
  * @param ipaddr IP address of the inserted ARP entry.
  * @param ethaddr Ethernet address of the inserted ARP entry.
  * @param flags Defines behaviour:
@@ -240,7 +331,7 @@ find_arp_entry(void)
 static struct pbuf *
 update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *ethaddr, u8_t flags)
 {
-  u8_t i, k;
+  s8_t i, k;
   LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | 3, ("update_arp_entry()\n"));
   LWIP_ASSERT("netif->hwaddr_len != 0", netif->hwaddr_len != 0);
   LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: %u.%u.%u.%u - %02x:%02x:%02x:%02x:%02x:%02x\n", ip4_addr1(ipaddr), ip4_addr2(ipaddr), ip4_addr3(ipaddr), ip4_addr4(ipaddr),
@@ -264,7 +355,7 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
         arp_table[i].state = ETHARP_STATE_STABLE;
         /* fall-through to next if */
       }
-      /* stable entry? (possible just marked to become stable) */
+      /* stable entry? (possibly just marked to become stable) */
       if (arp_table[i].state == ETHARP_STATE_STABLE) {
 #if ARP_QUEUEING
         struct pbuf *p;
@@ -277,12 +368,25 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
         }
         /* reset time stamp */
         arp_table[i].ctime = 0;
+/* this is where we will send out queued packets! */
 #if ARP_QUEUEING
+        /* get the first packet on the queue (if any) */
         p = arp_table[i].p;
         /* queued packet present? */
-        if (p != NULL) {
-          /* NULL attached buffer immediately */
-          arp_table[i].p = NULL;
+        while (p != NULL) {
+          struct pbuf *q, *n;
+          /* search for second packet on queue (n) */
+          q = p;
+          while (q->tot_len > q->len) {
+          	/* proceed to next pbuf of this packet */
+          	LWIP_ASSERT("q->next ! NULL", q->next != NULL);
+          	q = q->next;
+          }
+          /* { q = last pbuf of first packet, q->tot_len = q->len } */
+          n = q->next;
+          /* { n = first pbuf of 2nd packet, or NULL if no 2nd packet } */
+          /* terminate the first packet pbuf chain */
+          q->next = NULL;
           /* fill-in Ethernet header */
           ethhdr = p->payload;
           for (k = 0; k < netif->hwaddr_len; ++k) {
@@ -290,12 +394,16 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
             ethhdr->src.addr[k] = netif->hwaddr[k];
           }
           ethhdr->type = htons(ETHTYPE_IP);
-          LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: sending queued IP packet.\n"));
+          LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: sending queued IP packet %p.\n",(void *)p));
           /* send the queued IP packet */
           netif->linkoutput(netif, p);
           /* free the queued IP packet */
           pbuf_free(p);
+          /* proceed to next packet on queue */
+          p = n;
         }
+        /* NULL attached buffer*/
+        arp_table[i].p = NULL;
 #endif
         return NULL;
       }
@@ -312,21 +420,10 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
     LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: adding entry to table\n"));
     /* find an empty or old entry. */
     i = find_arp_entry();
-    if (i == ARP_TABLE_SIZE) {
+    if (i == ERR_MEM) {
       LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: no available entry found\n"));
       return NULL;
     }
-    /* see if find_arp_entry() gave us an old stable, or empty entry to re-use */
-    if (arp_table[i].state == ETHARP_STATE_STABLE) {
-      LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: overwriting old stable entry %u\n", i));
-      /* stable entries should have no queued packets (TODO: allow later) */
-#if ARP_QUEUEING
-      LWIP_ASSERT("update_arp_entry: arp_table[i].p == NULL", arp_table[i].p == NULL);
-#endif
-    } else {
-      LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | DBG_STATE, ("update_arp_entry: filling empty entry %u with state %u\n", i, arp_table[i].state));
-      LWIP_ASSERT("update_arp_entry: arp_table[i].state == ETHARP_STATE_EMPTY", arp_table[i].state == ETHARP_STATE_EMPTY);
-    }
     /* set IP address */
     ip_addr_set(&arp_table[i].ipaddr, ipaddr);
     /* set Ethernet hardware address */
@@ -350,7 +447,7 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
 }
 
 /**
- * Updates the ARP table using the given packet.
+ * Updates the ARP table using the given IP packet.
  *
  * Uses the incoming IP packet's source address to update the
  * ARP cache for the local network. The function does not alter
@@ -512,10 +609,10 @@ etharp_arp_input(struct netif *netif, struct eth_addr *ethaddr, struct pbuf *p)
  * This ARP request is returned as a pbuf, which should be sent by
  * the caller.
  *
- * If ARP failed to allocate resources, NULL is returned.
- *
  * A returned non-NULL packet should be sent by the caller.
  *
+ * If ARP failed to allocate resources, NULL is returned.
+ *
  * @param netif The lwIP network interface which the IP packet will be sent on.
  * @param ipaddr The IP address of the packet destination.
  * @param pbuf The pbuf(s) containing the IP packet to be sent.
@@ -528,9 +625,9 @@ etharp_output(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
 {
   struct eth_addr *dest, *srcaddr, mcastaddr;
   struct eth_hdr *ethhdr;
-  u8_t i;
+  s8_t i;
 
-  /* Make room for Ethernet header. */
+  /* make room for Ethernet header */
   if (pbuf_header(q, sizeof(struct eth_hdr)) != 0) {
     /* The pbuf_header() call shouldn't fail, and we'll just bail
     out if it does.. */
@@ -539,9 +636,6 @@ etharp_output(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
     return NULL;
   }
 
-  /* obtain source Ethernet address of the given interface */
-  srcaddr = (struct eth_addr *)netif->hwaddr;
-
   /* assume unresolved Ethernet address */
   dest = NULL;
   /* Construct Ethernet header. Start with looking up deciding which
@@ -569,19 +663,20 @@ etharp_output(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
   }
   /* destination IP address is an IP unicast address */
   else {
-    /* destination IP network address not on local network? */
-    /* this occurs if the packet is routed to the default gateway on this interface */
+    /* destination IP network address not on local network?
+     * IP layer wants us to forward to the default gateway */
     if (!ip_addr_maskcmp(ipaddr, &(netif->ip_addr), &(netif->netmask))) {
-      /* gateway available? */
+      /* interface has default gateway? */
       if (netif->gw.addr != 0)
       {
-        /* use the gateway IP address */
+        /* route to default gateway IP address */
         ipaddr = &(netif->gw);
       }
       /* no gateway available? */
       else
       {
         /* IP destination address outside local network, but no gateway available */
+        /* { packet is discarded } */
         return NULL;
       }
     }
@@ -600,6 +695,7 @@ etharp_output(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
       /* ARP query for the IP address, submit this IP packet for queueing */
       /* TODO: How do we handle netif->ipaddr == ipaddr? */
       etharp_query(netif, ipaddr, q);
+      /* { packet was queued (ERR_OK), or discarded } */
       /* return nothing */
       return NULL;
     }
@@ -612,8 +708,11 @@ etharp_output(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
 
   /* destination Ethernet address known */
   if (dest != NULL) {
-    /* A valid IP->MAC address mapping was found, so we construct the
-    Ethernet header for the outgoing packet. */
+    /* obtain source Ethernet address of the given interface */
+    srcaddr = (struct eth_addr *)netif->hwaddr;
+
+    /* A valid IP->MAC address mapping was found, fill in the
+     * Ethernet header for the outgoing packet */
     ethhdr = q->payload;
 
     for(i = 0; i < netif->hwaddr_len; i++) {
@@ -654,9 +753,8 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
 {
   struct eth_addr *srcaddr;
   struct etharp_hdr *hdr;
-  struct pbuf *p;
   err_t result = ERR_OK;
-  u8_t i;
+  s8_t i;
   u8_t perform_arp_request = 1;
   /* prevent 'unused argument' warning if ARP_QUEUEING == 0 */
   (void)q;
@@ -666,7 +764,7 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
     if (ip_addr_cmp(ipaddr, &arp_table[i].ipaddr)) {
       if (arp_table[i].state == ETHARP_STATE_PENDING) {
         LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | DBG_STATE, ("etharp_query: requested IP already pending as entry %u\n", i));
-        /* break out of for-loop, user may wish to queue a packet on a stable entry */
+        /* break out of for-loop, user may wish to queue a packet on a pending entry */
         /* TODO: we will issue a new ARP request, which should not occur too often */
         /* we might want to run a faster timer on ARP to limit this */
         break;
@@ -683,56 +781,26 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
   /* queried address not yet in ARP table? */
   if (i == ARP_TABLE_SIZE) {
     LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_query: IP address not found in ARP table\n"));
-    /* find an available entry */
+    /* find an available (unused or old) entry */
     i = find_arp_entry();
     /* bail out if no ARP entries are available */
-    if (i == ARP_TABLE_SIZE) {
-      LWIP_DEBUGF(ETHARP_DEBUG | 2, ("etharp_query: no more ARP entries available.\n"));
+    if (i == ERR_MEM) {
+      LWIP_DEBUGF(ETHARP_DEBUG | 2, ("etharp_query: no more ARP entries available. Should seldom occur.\n"));
       return ERR_MEM;
     }
-    /* we will now recycle entry i */
-    LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_query: created ARP table entry %u.\n", i));
     /* i is available, create ARP entry */
-    ip_addr_set(&arp_table[i].ipaddr, ipaddr);
-    arp_table[i].ctime = 0;
     arp_table[i].state = ETHARP_STATE_PENDING;
+    ip_addr_set(&arp_table[i].ipaddr, ipaddr);
+  /* queried address was already in ARP table */
+  } else {
 #if ARP_QUEUEING
-    /* free queued packet, as entry is now invalidated */
-    if (arp_table[i].p != NULL) {
-      pbuf_free(arp_table[i].p);
-      arp_table[i].p = NULL;
-      LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | 3, ("etharp_query: dropped packet on ARP queue. Should not occur.\n"));
-    }
+    etharp_enqueue(i, q);
 #endif
   }
-#if ARP_QUEUEING
-  /* any pbuf to queue and queue is empty? */
-  if (q != NULL) {
-/* yield later packets over older packets? */
-#if ARP_QUEUE_FIRST == 0
-    /* earlier queued packet on this entry? */
-    if (arp_table[i].p != NULL) {
-      pbuf_free(arp_table[i].p);
-      arp_table[i].p = NULL;
-      LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | 3, ("etharp_query: dropped packet on ARP queue. Should not occur.\n"));
-      /* fall-through into next if */
-    }
-#endif
-    /* packet can be queued? */
-    if (arp_table[i].p == NULL) {
-      /* copy PBUF_REF referenced payloads into PBUF_RAM */
-      q = pbuf_take(q);
-      /* remember pbuf to queue, if any */
-      arp_table[i].p = q;
-      /* pbufs are queued, increase the reference count */
-      pbuf_ref(q);
-      LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | DBG_STATE, ("etharp_query: queued packet %p on ARP entry %u.\n", (void *)q, i));
-    }
-  }
-#endif
   /* ARP request? */
   if (perform_arp_request)
   {
+    struct pbuf *p;
     /* allocate a pbuf for the outgoing ARP request packet */
     p = pbuf_alloc(PBUF_LINK, sizeof(struct etharp_hdr), PBUF_RAM);
     /* could allocate pbuf? */

src/netif/ppp/ppp.c

@@ -153,7 +153,7 @@ typedef struct PPPControl_s {
     struct vjcompress vjComp;           /* Van Jabobsen compression header. */
 #endif
 
-    struct netif *netif;
+    struct netif netif;
 
     struct ppp_addrs addrs;
 
@@ -296,7 +296,6 @@ void pppInit(void)
 
     for (i = 0; i < NUM_PPP; i++) {
         pppControl[i].openFlag = 0;
-        pppControl[i].netif = NULL;
 
 		subnetMask = htonl(0xffffff00);
     
@@ -537,14 +536,14 @@ static struct pbuf *pppAppend(u_char c, struct pbuf *nb, ext_accm *outACCM)
 /* Send a packet on the given connection. */
 static err_t pppifOutput(struct netif *netif, struct pbuf *pb, struct ip_addr *ipaddr)
 {
-	int pd = (int)netif->state;
-	u_short protocol = PPP_IP;
+    int pd = (int)netif->state;
+    u_short protocol = PPP_IP;
     PPPControl *pc = &pppControl[pd];
     u_int fcsOut = PPP_INITFCS;
     struct pbuf *headMB = NULL, *tailMB = NULL, *p;
     u_char c;
 
-	(void)ipaddr;
+    (void)ipaddr;
 
     /* Validate parameters. */
     /* We let any protocol value go through - it can't hurt us
@@ -1003,12 +1002,10 @@ int sifup(int pd)
         st = 0;
         PPPDEBUG((LOG_WARNING, "sifup[%d]: bad parms\n", pd));
     } else {
-		if(pc->netif)
-			netif_remove(pc->netif);
-		pc->netif = netif_add(&pc->addrs.our_ipaddr, &pc->addrs.netmask, &pc->addrs.his_ipaddr, (void *)pd, pppifNetifInit, ip_input);
-		if(pc->netif) {
-        	pc->if_up = 1;
-        	pc->errCode = PPPERR_NONE;
+		netif_remove(&pc->netif);
+		if (netif_add(&pc->netif, &pc->addrs.our_ipaddr, &pc->addrs.netmask, &pc->addrs.his_ipaddr, (void *)pd, pppifNetifInit, ip_input)) {
+        		pc->if_up = 1;
+        		pc->errCode = PPPERR_NONE;
 
 			PPPDEBUG((LOG_DEBUG, "sifup: unit %d: linkStatusCB=%lx errCode=%d\n", pd, pc->linkStatusCB, pc->errCode));
 			if(pc->linkStatusCB)
@@ -1046,12 +1043,10 @@ int sifdown(int pd)
         PPPDEBUG((LOG_WARNING, "sifdown[%d]: bad parms\n", pd));
     } else {
         pc->if_up = 0;
-		if(pc->netif)
-			netif_remove(pc->netif);
-		pc->netif = NULL;
-		PPPDEBUG((LOG_DEBUG, "sifdown: unit %d: linkStatusCB=%lx errCode=%d\n", pd, pc->linkStatusCB, pc->errCode));
-		if(pc->linkStatusCB)
-			pc->linkStatusCB(pc->linkStatusCtx, PPPERR_CONNECT, NULL);
+	netif_remove(&pc->netif);
+	PPPDEBUG((LOG_DEBUG, "sifdown: unit %d: linkStatusCB=%lx errCode=%d\n", pd, pc->linkStatusCB, pc->errCode));
+	if(pc->linkStatusCB)
+		pc->linkStatusCB(pc->linkStatusCtx, PPPERR_CONNECT, NULL);
 	}
     return st;
 }
@@ -1126,7 +1121,7 @@ int sifdefaultroute(int pd, u32_t l, u32_t g)
         st = 0;
         PPPDEBUG((LOG_WARNING, "sifup[%d]: bad parms\n", pd));
     } else {
-		netif_set_default(pc->netif);
+		netif_set_default(&pc->netif);
     }
 
     /* TODO: check how PPP handled the netMask, previously not set by ipSetDefault */
@@ -1324,7 +1319,7 @@ static void pppInput(void *arg)
          * pass the result to IP.
          */
         if (vj_uncompress_tcp(&nb, &pppControl[pd].vjComp) >= 0) {
-            pppControl[pd].netif->input(nb, pppControl[pd].netif);
+            pppControl[pd].netif.input(nb, &pppControl[pd].netif);
 			return;
         }
 	/* Something's wrong so drop it. */
@@ -1342,7 +1337,7 @@ static void pppInput(void *arg)
          * the packet to IP.
          */
         if (vj_uncompress_uncomp(nb, &pppControl[pd].vjComp) >= 0) {
-            pppControl[pd].netif->input(nb, pppControl[pd].netif);
+            pppControl[pd].netif.input(nb, &pppControl[pd].netif);
 			return;
         }
 	/* Something's wrong so drop it. */
@@ -1356,7 +1351,7 @@ static void pppInput(void *arg)
 	break;
     case PPP_IP:            /* Internet Protocol */
         PPPDEBUG((LOG_INFO, "pppInput[%d]: ip in pbuf len=%d\n", pd, nb->len));
-        pppControl[pd].netif->input(nb, pppControl[pd].netif);
+        pppControl[pd].netif.input(nb, &pppControl[pd].netif);
 		return;
     default:
 	{