Some updates on "gratuitious ARP" from RFC3220. Cleanup of some code and comments.

src/core/netif.c

@@ -198,6 +198,14 @@ netif_set_ipaddr(struct netif *netif, struct ip_addr *ipaddr)
   }
 #endif
   ip_addr_set(&(netif->ip_addr), ipaddr);
+#if 0 /* only allowed for Ethernet interfaces TODO: how can we check? */
+  /** For Ethernet network interfaces, we would like to send a
+   *  "gratuitous ARP"; this is an ARP packet sent by a node in order
+   *  to spontaneously cause other nodes to update an entry in their
+   *  ARP cache. From RFC 3220 "IP Mobility Support for IPv4" section 4.6.
+   */ 
+  etharp_query(netif, ipaddr, NULL);
+#endif
   LWIP_DEBUGF(NETIF_DEBUG | DBG_TRACE | DBG_STATE | 3, ("netif: IP address of interface %c%c set to %u.%u.%u.%u\n",
     netif->name[0], netif->name[1],
     (unsigned int)(ntohl(netif->ip_addr.addr) >> 24 & 0xff),

src/include/lwip/netif.h

@@ -52,7 +52,8 @@
 
 /** whether the network interface is 'up'. this is
  * a software flag used to control whether this network
- * interface is enabled and processes traffic */
+ * interface is enabled and processes traffic.
+ * TODO: who should act on this flag, lwIP stack or driver?? */
 #define NETIF_FLAG_UP 0x1U
 /** if set, the netif has broadcast capability */
 #define NETIF_FLAG_BROADCAST 0x2U
@@ -61,35 +62,36 @@
 /** if set, the interface is configured using DHCP */
 #define NETIF_FLAG_DHCP 0x08U
 /** if set, the interface has an active link
- *  (set by the interface) */
+ *  (set by the network interface driver) */
 #define NETIF_FLAG_LINK_UP 0x10U
 
-/** generic data structure used for all lwIP network interfaces */
+/** Generic data structure used for all lwIP network interfaces.
+ *  The following fields should be filled in by the initialization
+ *  function for the device driver: hwaddr_len, hwaddr[], mtu, flags */
+
 struct netif {
   /** pointer to next in linked list */
   struct netif *next;
-  /** The following fields should be filled in by the
+
-      initialization function for the device driver. */
-  
   /** IP address configuration in network byte order */
   struct ip_addr ip_addr;
   struct ip_addr netmask;
   struct ip_addr gw;
 
   /** This function is called by the network device driver
-      to pass a packet up the TCP/IP stack. */
+   *  to pass a packet up the TCP/IP stack. */
   err_t (* input)(struct pbuf *p, struct netif *inp);
   /** This function is called by the IP module when it wants
-      to send a packet on the interface. This function typically
+   *  to send a packet on the interface. This function typically
-      first resolves the hardware address, then sends the packet. */
+   *  first resolves the hardware address, then sends the packet. */
   err_t (* output)(struct netif *netif, struct pbuf *p,
        struct ip_addr *ipaddr);
   /** This function is called by the ARP module when it wants
-      to send a packet on the interface. This function outputs
+   *  to send a packet on the interface. This function outputs
-      the pbuf as-is on the link medium. */
+   *  the pbuf as-is on the link medium. */
   err_t (* linkoutput)(struct netif *netif, struct pbuf *p);
   /** This field can be set by the device driver and could point
-      to state information for the device. */
+   *  to state information for the device. */
   void *state;
 #if LWIP_DHCP
   /** the DHCP client state information for this netif */
@@ -101,12 +103,12 @@ struct netif {
   unsigned char hwaddr[NETIF_MAX_HWADDR_LEN];
   /** maximum transfer unit (in bytes) */
   u16_t mtu;
+  /** flags (see NETIF_FLAG_ above) */
+  u8_t flags;
   /** descriptive abbreviation */
   char name[2];
   /** number of this interface */
   u8_t num;
-  /** NETIF_FLAG_* */
-  u8_t flags;
 };
 
 /** The list of network interfaces. */

src/netif/etharp.c

@@ -6,8 +6,10 @@
  * to a physical address when sending a packet, and the second part answers
  * requests from other machines for our physical address.
  *
- * This implementation complies with RFC 826 (Ethernet ARP) and supports
+ * This implementation complies with RFC 826 (Ethernet ARP). It supports
- * Gratuitious ARP from RFC3220 (IP Mobility Support for IPv4) section 4.6.
+ * Gratuitious ARP from RFC3220 (IP Mobility Support for IPv4) section 4.6
+ * if an interface calls etharp_query(our_netif, its_ip_addr, NULL) upon
+ * address change.
  */
 
 /*
@@ -40,49 +42,8 @@
  *
  * This file is part of the lwIP TCP/IP stack.
  *
- * Author: Adam Dunkels <adam@sics.se>
- *
  */
 
-/**
- * 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:
- *
-RFC 3220 4.6          IP Mobility Support for IPv4          January 2002
-
-      -  A Gratuitous ARP [45] is an ARP packet sent by a node in order
-         to spontaneously cause other nodes to update an entry in their
-         ARP cache.  A gratuitous ARP MAY use either an ARP Request or
-         an ARP Reply packet.  In either case, the ARP Sender Protocol
-         Address and ARP Target Protocol Address are both set to the IP
-         address of the cache entry to be updated, and the ARP Sender
-         Hardware Address is set to the link-layer address to which this
-         cache entry should be updated.  When using an ARP Reply packet,
-         the Target Hardware Address is also set to the link-layer
-         address to which this cache entry should be updated (this field
-         is not used in an ARP Request packet).
-
-         In either case, for a gratuitous ARP, the ARP packet MUST be
-         transmitted as a local broadcast packet on the local link.  As
-         specified in [36], any node receiving any ARP packet (Request
-         or Reply) MUST update its local ARP cache with the Sender
-         Protocol and Hardware Addresses in the ARP packet, if the
-         receiving node has an entry for that IP address already in its
-         ARP cache.  This requirement in the ARP protocol applies even
-         for ARP Request packets, and for ARP Reply packets that do not
-         match any ARP Request transmitted by the receiving node [36].
-*
-  My suggestion would be to send a ARP request for our newly obtained
-  address upon configuration of an Ethernet interface.
-
-*/
-
 #include "lwip/opt.h"
 #include "lwip/inet.h"
 #include "netif/etharp.h"
@@ -242,13 +203,13 @@ find_arp_entry(void)
     return ERR_MEM;
   }
 
-  /* clean up the recycled stable entry */
+  /* clean up the oldest stable entry (to be recycled) */
   if (arp_table[i].state == ETHARP_STATE_STABLE) {
 #if ARP_QUEUEING
     /* and empty the packet queue */
     if (arp_table[i].p != NULL) {
-      /* remove all queued packets */
       LWIP_DEBUGF(ETHARP_DEBUG, ("find_arp_entry: freeing entry %u, packet queue %p.\n", i, (void *)(arp_table[i].p)));
+      /* remove all queued packets */
       pbuf_free(arp_table[i].p);
       arp_table[i].p = NULL;
     }
@@ -292,12 +253,11 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
     LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: will not add 0.0.0.0 to ARP cache\n"));
     return NULL;
   }
-  /* Walk through the ARP mapping table and try to find an entry to
+  /* Walk through the ARP mapping table and try to find an entry to update.
-  update. If none is found, the IP -> MAC address mapping is
+   * If none is found, a new IP -> MAC address mapping is inserted. */
-  inserted in the ARP table. */
   for (i = 0; i < ARP_TABLE_SIZE; ++i) {
     /* Check if the source IP address of the incoming packet matches
-    the IP address in this ARP table entry. */
+     * the IP address in this ARP table entry. */
     if (arp_table[i].state != ETHARP_STATE_EMPTY &&
     	ip_addr_cmp(ipaddr, &arp_table[i].ipaddr)) {
       /* pending entry? */
@@ -307,12 +267,8 @@ 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? (possibly just marked to become stable) */
+      /* stable entry? (possibly just marked stable) */
       if (arp_table[i].state == ETHARP_STATE_STABLE) {
-#if ARP_QUEUEING
-        struct pbuf *p;
-        struct eth_hdr *ethhdr;
-#endif
         LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("update_arp_entry: updating stable entry %u\n", i));
         /* An old entry found, update this and return. */
         for (k = 0; k < netif->hwaddr_len; ++k) {
@@ -324,11 +280,13 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e
 #if ARP_QUEUEING
         while (arp_table[i].p != NULL) {
           /* get the first packet on the queue (if any) */
-          p = arp_table[i].p;
+          struct pbuf *p = arp_table[i].p;
+          /* Ethernet header */
+          struct eth_hdr *ethhdr = p->payload;;
           /* remember (and reference) remainder of queue */
+          /* note: this will also terminate the p pbuf chain */
           arp_table[i].p = pbuf_dequeue(p);
           /* fill-in Ethernet header */
-          ethhdr = p->payload;
           for (k = 0; k < netif->hwaddr_len; ++k) {
             ethhdr->dest.addr[k] = ethaddr->addr[k];
             ethhdr->src.addr[k] = netif->hwaddr[k];
@@ -521,7 +479,7 @@ etharp_arp_input(struct netif *netif, struct eth_addr *ethaddr, struct pbuf *p)
     }
     break;
   case ARP_REPLY:
-    /* ARP reply. We insert or update the ARP table later. */
+    /* ARP reply. We already updated the ARP cache earlier. */
     LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_arp_input: incoming ARP reply\n"));
 #if (LWIP_DHCP && DHCP_DOES_ARP_CHECK)
     /* DHCP wants to know about ARP replies to our wanna-have-address */
@@ -696,20 +654,24 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
   err_t result = ERR_OK;
   s8_t i;
   u8_t perform_arp_request = 1;
-  /* prevent 'unused argument' warning if ARP_QUEUEING == 0 */
+  /* prevent 'unused argument' compiler warning if ARP_QUEUEING == 0 */
   (void)q;
   srcaddr = (struct eth_addr *)netif->hwaddr;
   /* bail out if this IP address is pending */
   for (i = 0; i < ARP_TABLE_SIZE; ++i) {
+    /* valid ARP cache entry with matching IP address? */
     if (arp_table[i].state != ETHARP_STATE_EMPTY &&
     	ip_addr_cmp(ipaddr, &arp_table[i].ipaddr)) {
+      /* pending entry? */
       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 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 */
+        /* { i != ARP_TABLE_SIZE } */
         break;
       }
+      /* stable entry? */
       else if (arp_table[i].state == ETHARP_STATE_STABLE) {
         LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE | DBG_STATE, ("etharp_query: requested IP already stable as entry %u\n", i));
         /* User wishes to queue a packet on a stable entry (or does she want to send
@@ -720,6 +682,7 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
          * immediately. I chose to implement the former approach.
          */
         perform_arp_request = (q?1:0);
+        /* { i != ARP_TABLE_SIZE } */
         break;
       }
     }
@@ -739,9 +702,11 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q)
     ip_addr_set(&arp_table[i].ipaddr, ipaddr);
   }
   /* { i is now valid } */
-#if ARP_QUEUEING /* queue packet (even on a stable entry, see above) */
+#if ARP_QUEUEING /* queue the given q packet */
   /* copy any PBUF_REF referenced payloads into PBUF_RAM */
+  /* (the caller assumes the referenced payload can be freed) */
   q = pbuf_take(q);
+  /* queue packet (even on a stable entry, see above) */
   pbuf_queue(arp_table[i].p, q);
 #endif
   /* ARP request? */