Merge pull request #1221 from evpopov/BCastAddressDetection_PR

Improves handling of IPv4 broadcast addresses.

Author: tony-josi-aws

source/FreeRTOS_ARP.c

@@ -893,10 +893,11 @@
 
         *( ppxEndPoint ) = NULL;
         ulAddressToLookup = *pulIPAddress;
-        pxEndPoint = FreeRTOS_FindEndPointOnIP_IPv4( ulAddressToLookup );
 
         if( xIsIPv4Loopback( ulAddressToLookup ) != 0 )
         {
+            pxEndPoint = FreeRTOS_FindEndPointOnIP_IPv4( ulAddressToLookup );
+
             if( pxEndPoint != NULL )
             {
                 /* For multi-cast, use the first IPv4 end-point. */
@@ -925,16 +926,15 @@
                 }
             }
         }
-        else if( ( FreeRTOS_htonl( ulAddressToLookup ) & 0xffU ) == 0xffU ) /* Is this a broadcast address like x.x.x.255 ? */
+        else if( xIsIPv4Broadcast( ulAddressToLookup, ppxEndPoint ) )
         {
             /* This is a broadcast so it uses the broadcast MAC address. */
             ( void ) memcpy( pxMACAddress->ucBytes, xBroadcastMACAddress.ucBytes, sizeof( MACAddress_t ) );
-            pxEndPoint = FreeRTOS_FindEndPointOnNetMask( ulAddressToLookup );
 
-            if( pxEndPoint != NULL )
-            {
-                *( ppxEndPoint ) = pxEndPoint;
-            }
+            /* Note that xIsIPv4Broadcast() already filled in ppxEndPoint with the corresponding endpoint
+             * or the first IPv4 endpoint in case ulAddressToLookup was FREERTOS_INADDR_BROADCAST.
+             * It is also safe to call xIsIPv4Broadcast() with a null pointer so no need to use
+             * the intermediary pxEndPoint */
 
             eReturn = eResolutionCacheHit;
         }

source/FreeRTOS_DHCP.c

@@ -1047,7 +1047,7 @@
                         const void * pvCopySource = &( pxSet->pucByte[ uxByteIndex ] );
                         ( void ) memcpy( pvCopyDest, pvCopySource, sizeof( pxSet->ulParameter ) );
 
-                        if( ( pxSet->ulParameter != FREERTOS_INADDR_ANY ) && ( pxSet->ulParameter != ipBROADCAST_IP_ADDRESS ) )
+                        if( ( pxSet->ulParameter != FREERTOS_INADDR_ANY ) && ( pxSet->ulParameter != FREERTOS_INADDR_BROADCAST ) )
                         {
                             EP_IPv4_SETTINGS.ulDNSServerAddresses[ uxTargetIndex ] = pxSet->ulParameter;
                             uxTargetIndex++;
@@ -1470,7 +1470,7 @@
                              pxEndPoint->xMACAddress.ucBytes, sizeof( MACAddress_t ) );
 
             /* Set the addressing. */
-            pxAddress->sin_address.ulIP_IPv4 = ipBROADCAST_IP_ADDRESS;
+            pxAddress->sin_address.ulIP_IPv4 = FREERTOS_INADDR_BROADCAST;
             pxAddress->sin_port = ( uint16_t ) dhcpSERVER_PORT_IPv4;
             pxAddress->sin_family = FREERTOS_AF_INET4;
         }
@@ -1676,7 +1676,7 @@
             EP_IPv4_SETTINGS.ulIPAddress = EP_DHCPData.ulOfferedIPAddress;
 
             /* Setting the 'local' broadcast address, something like 192.168.1.255' */
-            EP_IPv4_SETTINGS.ulBroadcastAddress = ( EP_DHCPData.ulOfferedIPAddress & EP_IPv4_SETTINGS.ulNetMask ) | ~EP_IPv4_SETTINGS.ulNetMask;
+            EP_IPv4_SETTINGS.ulBroadcastAddress = ( EP_DHCPData.ulOfferedIPAddress | ( ~EP_IPv4_SETTINGS.ulNetMask ) );
 
             /* Close socket to ensure packets don't queue on it. not needed anymore as DHCP failed. but still need timer for ARP testing. */
             prvCloseDHCPSocket( pxEndPoint );

source/FreeRTOS_DNS.c

@@ -1006,7 +1006,7 @@ const MACAddress_t xMDNS_MacAddressIPv6 = { { 0x33, 0x33, 0x00, 0x00, 0x00, 0xFB
                                 configASSERT( ucIndex < ipconfigENDPOINT_DNS_ADDRESS_COUNT );
                                 ulIPAddress = pxEndPoint->ipv4_settings.ulDNSServerAddresses[ ucIndex ];
 
-                                if( ( ulIPAddress != 0U ) && ( ulIPAddress != ipBROADCAST_IP_ADDRESS ) )
+                                if( ( ulIPAddress != 0U ) && ( ulIPAddress != FREERTOS_INADDR_BROADCAST ) )
                                 {
                                     pxAddress->sin_family = FREERTOS_AF_INET;
                                     pxAddress->sin_len = ( uint8_t ) sizeof( struct freertos_sockaddr );

source/FreeRTOS_IPv4.c

@@ -210,6 +210,65 @@ BaseType_t xIsIPv4Multicast( uint32_t ulIPAddress )
 }
 /*-----------------------------------------------------------*/
 
+/**
+ * @brief Checks if the IP address matches the global 255.255.255.255 broadcast address or
+ * the broadcast address for any of our IPv4 endpoints.
+ *
+ * @param[in] ulIPAddress The IP address being checked.
+ *
+ * @param[out] ppxEndPoint Pointer to an end-point where we store the endpoint whose broadcast address we matched. Or NULL if no IPv4 endpoints were found.
+ *
+ * @return pdTRUE if the IP address is a broadcast address or else, pdFALSE.
+ */
+BaseType_t xIsIPv4Broadcast( uint32_t ulIPAddress,
+                             struct xNetworkEndPoint ** ppxEndPoint )
+{
+    BaseType_t xIsBroadcast;
+    NetworkEndPoint_t * pxEndPoint;
+
+    /* Assign a default answer based on the "global" broadcast.	 This way
+     * we can still return the correct result even if there are no endpoints. */
+    xIsBroadcast = ( ulIPAddress == FREERTOS_INADDR_BROADCAST ) ? pdTRUE : pdFALSE;
+
+    for( pxEndPoint = FreeRTOS_FirstEndPoint( NULL );
+         pxEndPoint != NULL;
+         pxEndPoint = FreeRTOS_NextEndPoint( NULL, pxEndPoint ) )
+    {
+        #if ( ipconfigUSE_IPv6 == ipconfigENABLE )
+            /* Skip over any IPv6 endpoints. */
+            if( pxEndPoint->bits.bIPv6 == pdTRUE )
+            {
+                continue;
+            }
+        #endif /* ( ipconfigUSE_IPv6 == ipconfigENABLE ) */
+
+        /* If we already know if ulIPAddress is a broadcast,
+         * simply return this first matching IPv4 endpoint. */
+        if( xIsBroadcast == pdTRUE )
+        {
+            break;
+        }
+        else if( ulIPAddress == pxEndPoint->ipv4_settings.ulBroadcastAddress )
+        {
+            xIsBroadcast = pdTRUE;
+            break;
+        }
+    }
+
+    /* If the caller wants to know the corresponding endpoint, copy the result.
+     * Note that this may be null if ulIPAddress is 255.255.255.255 AND there are
+     * no IPv4 endpoints.
+     * Also, when ulIPAddress is 255.255.255.255, we will
+     * return the first IPv4 endpoint that we run across. */
+    if( xIsBroadcast && ( ppxEndPoint != NULL ) )
+    {
+        *ppxEndPoint = pxEndPoint;
+    }
+
+    return xIsBroadcast;
+}
+/*-----------------------------------------------------------*/
+
 /**
  * @brief Check if the packet is an illegal loopback packet.
  *
@@ -284,9 +343,11 @@ enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * cons
     {
         /* In systems with a very small amount of RAM, it might be advantageous
          * to have incoming messages checked earlier, by the network card driver.
-         * This method may decrease the usage of sparse network buffers. */
+         * This method may decrease the usage of scarce network buffers. */
         uint32_t ulDestinationIPAddress = pxIPHeader->ulDestinationIPAddress;
         uint32_t ulSourceIPAddress = pxIPHeader->ulSourceIPAddress;
+        /* Get a reference to the endpoint that the packet was assigned to during pxEasyFit() */
+        NetworkEndPoint_t * pxEndPoint = pxNetworkBuffer->pxEndPoint;
 
         /* Ensure that the incoming packet is not fragmented because the stack
          * doesn't not support IP fragmentation. All but the last fragment coming in will have their
@@ -318,16 +379,20 @@ enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * cons
             }
         }
         else if(
-            ( FreeRTOS_FindEndPointOnIP_IPv4( ulDestinationIPAddress ) == NULL ) &&
-            /* Is it an IPv4 broadcast address x.x.x.255 ? */
-            ( ( FreeRTOS_ntohl( ulDestinationIPAddress ) & 0xffU ) != 0xffU ) &&
+            /* Not destined for the assigned endpoint IPv4 address? */
+            ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulIPAddress ) &&
+            /* Also not an IPv4 broadcast address ? */
+            ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulBroadcastAddress ) &&
+            ( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) &&
+            /* And not an IPv4 multicast address ? */
             ( xIsIPv4Multicast( ulDestinationIPAddress ) == pdFALSE ) )
         {
             /* Packet is not for this node, release it */
             eReturn = eReleaseBuffer;
         }
         /* Is the source address correct? */
-        else if( ( FreeRTOS_ntohl( ulSourceIPAddress ) & 0xffU ) == 0xffU )
+        else if( ( ulSourceIPAddress == pxEndPoint->ipv4_settings.ulBroadcastAddress ) ||
+                 ( ulSourceIPAddress == FREERTOS_INADDR_BROADCAST ) )
         {
             /* The source address cannot be broadcast address. Replying to this
              * packet may cause network storms. Drop the packet. */
@@ -336,7 +401,7 @@ enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * cons
         else if( ( memcmp( xBroadcastMACAddress.ucBytes,
                            pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes,
                            sizeof( MACAddress_t ) ) == 0 ) &&
-                 ( ( FreeRTOS_ntohl( ulDestinationIPAddress ) & 0xffU ) != 0xffU ) )
+                 ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulBroadcastAddress ) && ( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) )
         {
             /* Ethernet address is a broadcast address, but the IP address is not a
              * broadcast address. */

source/include/FreeRTOS_IP_Private.h

@@ -258,8 +258,6 @@ struct xPacketSummary
     uint16_t usProtocolBytes;              /**< The total length of the protocol data. */
 };
 
-#define ipBROADCAST_IP_ADDRESS               0xffffffffU
-
 /* Offset into the Ethernet frame that is used to temporarily store information
  * on the fragmentation status of the packet being sent.  The value is important,
  * as it is past the location into which the destination address will get placed. */

source/include/FreeRTOS_IPv4.h

@@ -84,6 +84,10 @@ uint32_t FreeRTOS_GetIPAddress( void );
 /* Return pdTRUE if the IPv4 address is a multicast address. */
 BaseType_t xIsIPv4Multicast( uint32_t ulIPAddress );
 
+/* Return pdTRUE if the IPv4 address is a broadcast address. */
+BaseType_t xIsIPv4Broadcast( uint32_t ulIPAddress,
+                             struct xNetworkEndPoint ** ppxEndPoint );
+
 /* Return pdTRUE if the IPv4 address is a loopback address. */
 BaseType_t xIsIPv4Loopback( uint32_t ulAddress );
 

source/include/FreeRTOS_Sockets.h

@@ -150,9 +150,10 @@
     #if ( ipconfigUSE_TCP == 1 )
         #define FREERTOS_SO_SET_LOW_HIGH_WATER            ( 18 )
     #endif
-    #define FREERTOS_INADDR_ANY                           ( 0U ) /* The 0.0.0.0 IPv4 address. */
+    #define FREERTOS_INADDR_ANY                           ( 0U )           /* The 0.0.0.0 IPv4 address. */
+    #define FREERTOS_INADDR_BROADCAST                     ( 0xffffffffUL ) /* 255.255.255.255 is a special broadcast address that represents all host attached to the physical network. */
 
-    #if ( 0 )                                                    /* Not Used */
+    #if ( 0 )                                                              /* Not Used */
         #define FREERTOS_NOT_LAST_IN_FRAGMENTED_PACKET    ( 0x80 )
         #define FREERTOS_FRAGMENTED_PACKET                ( 0x40 )
     #endif

source/portable/NetworkInterface/STM32/CMakeLists.txt

@@ -24,7 +24,7 @@ target_sources( freertos_plus_tcp_network_if
       Drivers/H7/stm32h7xx_hal_eth.c>
 )
 
-target_include_directories( freertos_plus_tcp_network_if 
+target_include_directories( freertos_plus_tcp_network_if
   PUBLIC
     $<$<STREQUAL:${FREERTOS_PLUS_TCP_STM32_IF_DRIVER},F4>:
       Drivers/F4>