net/: Fix some issues with regard to UDP broadcast handling. This is Bitbucket Issue #77. This commit tentatively closes the issues, subject to verification.

include/nuttx/net/ip.h

@@ -508,6 +508,46 @@ bool net_ipv6addr_maskcmp(const net_ipv6addr_t addr1,
                           const net_ipv6addr_t mask);
 #endif
 
+/****************************************************************************
+ * Name: net_ipv4addr_broadcast
+ *
+ * Description:
+ *    Mask out the network part of an IP address, given the address and
+ *    the netmask.
+ *
+ *    Example:
+ *
+ *     in_addr_t ipaddr;
+ *     in_addr_t netmask;
+ *     bool isbroadcast;
+ *
+ *     net_ipaddr(&netmask, 255,255,255,0);
+ *     net_ipaddr(&ipaddr, 192,16,1,255);
+ *     isbroadcast = net_ipv4addr_broadcast(ipaddr, netmask);
+ *
+ *   Will return isboadcast == true.
+ *
+ *     net_ipaddr(&ipaddr, 192,16,1,2);
+ *     isbroadcast = net_ipv4addr_broadcast(ipaddr, netmask);
+ *
+ *   Will return isboadcast == false.
+ *
+ *   NOTES:
+ *   1. This function does not check for the broadcast address
+ *      255.255.255.255.  That must be performed as a seperate check.
+ *   2. You must also separately check if the ipaddress lies on the sub-net
+ *      using, perhaps, net_ipv4addr_maskcmp().
+ *
+ * Input Parameters:
+ *   addr - The IPv4 address to check
+ *   mask - The network mask
+ *
+ ****************************************************************************/
+
+#define net_ipv4addr_broadcast(addr, mask) \
+   (((in_addr_t)(addr) & ~(in_addr_t)(mask)) == \
+    ((in_addr_t)(0xffffffff) & ~(in_addr_t)(mask)))
+
 /****************************************************************************
  * Name: net_ipv6addr_prefixcmp
  *
@@ -587,36 +627,6 @@ bool net_ipv6addr_maskcmp(const net_ipv6addr_t addr1,
 
 #define net_is_addr_linklocal(a) ((a)[0] == HTONS(0xfe80))
 
-/****************************************************************************
- * Name: net_ipaddr_mask
- *
- * Description:
- *    Mask out the network part of an IP address, given the address and
- *    the netmask.
- *
- *    Example:
- *
- *     in_addr_t ipaddr1, ipaddr2, netmask;
- *
- *     net_ipaddr(&ipaddr1, 192,16,1,2);
- *     net_ipaddr(&netmask, 255,255,255,0);
- *     net_ipaddr_mask(&ipaddr2, &ipaddr1, &netmask);
- *
- *   In the example above, the variable "ipaddr2" will contain the IP
- *   address 192.168.1.0.
- *
- * Input Parameters:
- *   dest Where the result is to be placed.
- *   src The IP address.
- *   mask The netmask.
- *
- ****************************************************************************/
-
-#define net_ipaddr_mask(dest, src, mask) \
-  do { \
-    (in_addr_t)(dest) = (in_addr_t)(src) & (in_addr_t)(mask); \
-  } while (0)
-
 #undef EXTERN
 #ifdef __cplusplus
 }

net/arp/arp_out.c

@@ -169,6 +169,7 @@ void arp_out(FAR struct net_driver_s *dev)
   if (net_ipv4addr_hdrcmp(pip->eh_destipaddr, g_broadcast_ipaddr))
     {
       memcpy(peth->dest, g_broadcast_ethaddr.ether_addr_octet, ETHER_ADDR_LEN);
+      goto finish_header;
     }
 
 #ifdef CONFIG_NET_IGMP
@@ -181,8 +182,8 @@ void arp_out(FAR struct net_driver_s *dev)
    *   addresses=0xff (ff00::/8.)
    */
 
-  else if (NTOHS(pip->eh_destipaddr[0]) >= 0xe000 &&
+  if (NTOHS(pip->eh_destipaddr[0]) >= 0xe000 &&
-           NTOHS(pip->eh_destipaddr[0]) <= 0xefff)
+      NTOHS(pip->eh_destipaddr[0]) <= 0xefff)
     {
       /* Build the well-known IPv4 IGMP Ethernet address.  The first
        * three bytes are fixed; the final three variable come from the
@@ -199,63 +200,76 @@ void arp_out(FAR struct net_driver_s *dev)
       peth->dest[3] = ip[2] & 0x7f;
       peth->dest[4] = ip[3];
       peth->dest[5] = ip[4];
+
+      goto finish_header;
     }
 #endif
-  else
-    {
-      /* Check if the destination address is on the local network. */
 
-      destipaddr = net_ip4addr_conv32(pip->eh_destipaddr);
+  /* Check if the destination address is on the local network. */
-      if (!net_ipv4addr_maskcmp(destipaddr, dev->d_ipaddr, dev->d_netmask))
+
-        {
+  destipaddr = net_ip4addr_conv32(pip->eh_destipaddr);
-          /* Destination address is not on the local network */
+  if (!net_ipv4addr_maskcmp(destipaddr, dev->d_ipaddr, dev->d_netmask))
+    {
+      /* Destination address is not on the local network */
 
 #ifdef CONFIG_NET_ROUTE
-          /* We have a routing table.. find the correct router to use in
+      /* We have a routing table.. find the correct router to use in
-           * this case (or, as a fall-back, use the device's default router
+       * this case (or, as a fall-back, use the device's default router
-           * address).  We will use the router IP address instead of the
+       * address).  We will use the router IP address instead of the
-           * destination address when determining the MAC address.
+       * destination address when determining the MAC address.
-           */
+       */
 
-          netdev_ipv4_router(dev, destipaddr, &ipaddr);
+      netdev_ipv4_router(dev, destipaddr, &ipaddr);
 #else
-          /* Use the device's default router IP address instead of the
+      /* Use the device's default router IP address instead of the
-           * destination address when determining the MAC address.
+       * destination address when determining the MAC address.
-           */
+       */
 
-          net_ipv4addr_copy(ipaddr, dev->d_draddr);
+      net_ipv4addr_copy(ipaddr, dev->d_draddr);
 #endif
-        }
-      else
-        {
-          /* Else, we use the destination IP address. */
-
-          net_ipv4addr_copy(ipaddr, destipaddr);
-        }
-
-      /* Check if we already have this destination address in the ARP table */
-
-      tabptr = arp_find(ipaddr);
-      if (!tabptr)
-        {
-           ninfo("ARP request for IP %08lx\n", (unsigned long)ipaddr);
-
-          /* The destination address was not in our ARP table, so we
-           * overwrite the IP packet with an ARP request.
-           */
-
-          arp_format(dev, ipaddr);
-          arp_dump(ARPBUF);
-          return;
-        }
-
-      /* Build an Ethernet header. */
-
-      memcpy(peth->dest, tabptr->at_ethaddr.ether_addr_octet, ETHER_ADDR_LEN);
     }
 
+  /* The destination address is on the local network.  Check if it is
+   * the sub-net broadcast address.
+   */
+
+  else if (net_ipv4addr_broadcast(destipaddr, dev->d_netmask))
+    {
+      /* Yes.. then we won't need to know the destination MAC address */
+
+      memcpy(peth->dest, g_broadcast_ethaddr.ether_addr_octet, ETHER_ADDR_LEN);
+      goto finish_header;
+    }
+  else
+    {
+      /* Else, we use the destination IP address. */
+
+      net_ipv4addr_copy(ipaddr, destipaddr);
+    }
+
+  /* Check if we already have this destination address in the ARP table */
+
+  tabptr = arp_find(ipaddr);
+  if (tabptr == NULL)
+    {
+      ninfo("ARP request for IP %08lx\n", (unsigned long)ipaddr);
+
+      /* The destination address was not in our ARP table, so we overwrite
+       * the IP packet with an ARP request.
+       */
+
+      arp_format(dev, ipaddr);
+      arp_dump(ARPBUF);
+      return;
+    }
+
+  /* Build an Ethernet header. */
+
+  memcpy(peth->dest, tabptr->at_ethaddr.ether_addr_octet, ETHER_ADDR_LEN);
+
   /* Finish populating the Ethernet header */
 
+finish_header:
   memcpy(peth->src, dev->d_mac.ether.ether_addr_octet, ETHER_ADDR_LEN);
   peth->type  = HTONS(ETHTYPE_IP);
   dev->d_len += ETH_HDRLEN;

net/arp/arp_send.c

@@ -250,7 +250,6 @@ int arp_send(in_addr_t ipaddr)
       /* Destination address is not on the local network */
 
 #ifdef CONFIG_NET_ROUTE
-
       /* We have a routing table.. find the correct router to use in
        * this case (or, as a fall-back, use the device's default router
        * address).  We will use the router IP address instead of the
@@ -268,6 +267,17 @@ int arp_send(in_addr_t ipaddr)
       ipaddr = dripaddr;
     }
 
+  /* The destination address is on the local network.  Check if it is
+   * the sub-net broadcast address.
+   */
+
+  else if (net_ipv4addr_broadcast(ipaddr, dev->d_netmask))
+    {
+      /* Yes.. We don't need to send the ARP request */
+
+      return OK;
+    }
+
   /* Allocate resources to receive a callback.  This and the following
    * initialization is performed with the network lock because we don't
    * want anything to happen until we are ready.

net/devif/ipv4_input.c

@@ -321,6 +321,7 @@ nullreturn:
 int ipv4_input(FAR struct net_driver_s *dev)
 {
   FAR struct ipv4_hdr_s *ipv4 = BUF;
+  in_addr_t destipaddr;
   uint16_t hdrlen;
   uint16_t iplen;
 
@@ -395,6 +396,10 @@ int ipv4_input(FAR struct net_driver_s *dev)
 #endif /* CONFIG_NET_TCP_REASSEMBLY */
     }
 
+  /* Get the destination IP address in a friendlier form */
+
+  destipaddr = net_ip4addr_conv32(ipv4->destipaddr);
+
 #if defined(CONFIG_NET_BROADCAST) && defined(NET_UDP_HAVE_STACK)
   /* If IP broadcast support is configured, we check for a broadcast
    * UDP packet, which may be destined to us (even if there is no IP
@@ -403,8 +408,7 @@ int ipv4_input(FAR struct net_driver_s *dev)
    */
 
   if (ipv4->proto == IP_PROTO_UDP &&
-      net_ipv4addr_cmp(net_ip4addr_conv32(ipv4->destipaddr),
+      net_ipv4addr_cmp(destipaddr, INADDR_BROADCAST))
-                       INADDR_BROADCAST))
     {
 #ifdef CONFIG_NET_IPFORWARD_BROADCAST
       /* Forward broadcast packets */
@@ -413,74 +417,90 @@ int ipv4_input(FAR struct net_driver_s *dev)
 #endif
       return udp_ipv4_input(dev);
     }
-
-  /* In other cases, the device must be assigned a non-zero IP address. */
-
   else
 #endif
 #ifdef CONFIG_NET_ICMP
+  /* In other cases, the device must be assigned a non-zero IP address. */
+
   if (net_ipv4addr_cmp(dev->d_ipaddr, INADDR_ANY))
     {
       nwarn("WARNING: No IP address assigned\n");
       goto drop;
     }
-
-  /* Check if the packet is destined for our IP address */
   else
 #endif
-    {
+#if defined(CONFIG_NET_BROADCAST) && defined(NET_UDP_HAVE_STACK)
-      /* Check if the packet is destined for our IP address. */
+  /* The address is not the broadcast address and we have been assigned a
+   * address.  So there is also the possibility that the destination address
+   * is a sub-net broadcast address which we will need to handle just as for
+   * the broadcast address above.
+   */
 
-      if (!net_ipv4addr_cmp(net_ip4addr_conv32(ipv4->destipaddr),
+  if (ipv4->proto == IP_PROTO_UDP &&
-                            dev->d_ipaddr))
+      net_ipv4addr_maskcmp(destipaddr, dev->d_ipaddr, dev->d_netmask) &&
-        {
+      net_ipv4addr_broadcast(destipaddr, dev->d_netmask))
-          /* Check for an IPv4 IGMP group address */
+    {
+#ifdef CONFIG_NET_IPFORWARD_BROADCAST
+      /* Forward broadcast packets */
+
+      ipv4_forward_broadcast(dev, ipv4);
+#endif
+      return udp_ipv4_input(dev);
+    }
+  else
+#endif
+  /* Check if the packet is destined for our IP address. */
+
+  if (!net_ipv4addr_cmp(destipaddr, dev->d_ipaddr))
+    {
+      /* No.. This is not our IP address. Check for an IPv4 IGMP group
+       * address
+       */
 
 #ifdef CONFIG_NET_IGMP
-          in_addr_t destip = net_ip4addr_conv32(ipv4->destipaddr);
+      in_addr_t destip = net_ip4addr_conv32(ipv4->destipaddr);
-          if (igmp_grpfind(dev, &destip) != NULL)
+      if (igmp_grpfind(dev, &destip) != NULL)
-            {
+        {
 #ifdef CONFIG_NET_IPFORWARD_BROADCAST
-              /* Forward multicast packets */
+          /* Forward multicast packets */
 
-              ipv4_forward_broadcast(dev, ipv4);
+          ipv4_forward_broadcast(dev, ipv4);
 #endif
+        }
+      else
+#endif
+        {
+          /* No.. The packet is not destined for us. */
+
+#ifdef CONFIG_NET_IPFORWARD
+          /* Try to forward the packet */
+
+          int ret = ipv4_forward(dev, ipv4);
+          if (ret >= 0)
+            {
+              /* The packet was forwarded.  Return success; d_len will
+               * be set appropriately by the forwarding logic:  Cleared
+               * if the packet is forward via anoother device or non-
+               * zero if it will be forwarded by the same device that
+               * it was received on.
+               */
+
+              return OK;
             }
           else
 #endif
             {
-              /* No.. The packet is not destined for us. */
+              /* Not destined for us and not forwardable... Drop the
+               * packet.
+               */
 
-#ifdef CONFIG_NET_IPFORWARD
+              nwarn("WARNING: Not destined for us; not forwardable... "
-              /* Try to forward the packet */
+                    "Dropping!\n");
-
-              int ret = ipv4_forward(dev, ipv4);
-              if (ret >= 0)
-                {
-                  /* The packet was forwarded.  Return success; d_len will
-                   * be set appropriately by the forwarding logic:  Cleared
-                   * if the packet is forward via anoother device or non-
-                   * zero if it will be forwarded by the same device that
-                   * it was received on.
-                   */
-
-                  return OK;
-                }
-              else
-#endif
-                {
-                  /* Not destined for us and not forwardable... Drop the
-                   * packet.
-                   */
-
-                  nwarn("WARNING: Not destined for us; not forwardable... "
-                        "Dropping!\n");
 
 #ifdef CONFIG_NET_STATISTICS
-                  g_netstats.ipv4.drop++;
+              g_netstats.ipv4.drop++;
 #endif
-                  goto drop;
+              goto drop;
-                }
             }
         }
     }