IPv4/single: SAME70 EMAC buffer sizes (#568)

htibosch · Hein Tibosch · actions-user · web-flow · commit 9bd9d28ced6a · 2022-11-07T09:37:53.000-08:00
* Implemented Maty's solution

* Added a new statistic 'tx_write_fail'

* Uncrustify: triggered by comment.

* Increase NETWORK_BUFFER_SIZE in order to include the 'ipBUFFER_PADDING' bytes

* ICMP checksum calculated manually

* Uncrustify: triggered by comment.

* Update gmac_SAM.c

Co-authored-by: Hein Tibosch &lt;hein@htibosch.net&gt;
Co-authored-by: GitHub Action &lt;action@github.com&gt;
Co-authored-by: Aniruddha Kanhere &lt;60444055+AniruddhaKanhere@users.noreply.github.com&gt;
diff --git a/source/portable/NetworkInterface/DriverSAM/NetworkInterface.c b/source/portable/NetworkInterface/DriverSAM/NetworkInterface.c
@@ -71,12 +71,6 @@
 #define EMAC_IF_ERR_EVENT    4UL
 #define EMAC_IF_ALL_EVENT    ( EMAC_IF_RX_EVENT | EMAC_IF_TX_EVENT | EMAC_IF_ERR_EVENT )
 
-/* 1536 bytes is more than needed, 1524 would be enough.
- * But 1536 is a multiple of 32, which gives a great alignment for
- * cached memories. */
-
-#define NETWORK_BUFFER_SIZE    1536
-
 #ifndef EMAC_MAX_BLOCK_TIME_MS
 
 /* The task 'prvEMACHandlerTask()' will wake-up every 100 ms, to see
@@ -518,9 +512,10 @@ BaseType_t xNetworkInterfaceOutput( NetworkBufferDescriptor_t * const pxDescript
 
     pxSendPacket = ( IPPacket_t * ) pxDescriptor->pucEthernetBuffer;
 
-    if( ulTransmitSize > NETWORK_BUFFER_SIZE )
+    /* 'GMAC_TX_UNITSIZE' is the netto size of a transmission buffer. */
+    if( ulTransmitSize > GMAC_TX_UNITSIZE )
     {
-        ulTransmitSize = NETWORK_BUFFER_SIZE;
+        ulTransmitSize = GMAC_TX_UNITSIZE;
     }
 
     /* A do{}while(0) loop is introduced to allow the use of multiple break
@@ -566,7 +561,7 @@ BaseType_t xNetworkInterfaceOutput( NetworkBufferDescriptor_t * const pxDescript
             }
         #endif
 
-        ulResult = gmac_dev_write( &gs_gmac_dev, ( void * ) pxDescriptor->pucEthernetBuffer, pxDescriptor->xDataLength );
+        ulResult = gmac_dev_write( &gs_gmac_dev, ( void * ) pxDescriptor->pucEthernetBuffer, ulTransmitSize );
 
         if( ulResult != GMAC_OK )
         {
@@ -742,17 +737,26 @@ void vGMACGenerateChecksum( uint8_t * pucBuffer,
 {
     ProtocolPacket_t * xProtPacket = ( ProtocolPacket_t * ) pucBuffer;
 
-    if( xProtPacket->xTCPPacket.xEthernetHeader.usFrameType == ipIPv4_FRAME_TYPE )
+    /* The SAM4E has problems offloading checksums for transmission.
+     * The SAME70 does not set the CRC for ICMP packets (ping). */
+
+    if( xProtPacket->xICMPPacket.xEthernetHeader.usFrameType == ipIPv4_FRAME_TYPE )
     {
-        IPHeader_t * pxIPHeader = &( xProtPacket->xTCPPacket.xIPHeader );
+        #if ( SAME70 != 0 )
+            if( ( xProtPacket->xICMPPacket.xIPHeader.ucProtocol != ipPROTOCOL_UDP ) &&
+                ( xProtPacket->xICMPPacket.xIPHeader.ucProtocol != ipPROTOCOL_TCP ) )
+        #endif
+        {
+            IPHeader_t * pxIPHeader = &( xProtPacket->xTCPPacket.xIPHeader );
 
-        /* Calculate the IP header checksum. */
-        pxIPHeader->usHeaderChecksum = 0x00;
-        pxIPHeader->usHeaderChecksum = usGenerateChecksum( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ipSIZE_OF_IPv4_HEADER );
-        pxIPHeader->usHeaderChecksum = ~FreeRTOS_htons( pxIPHeader->usHeaderChecksum );
+            /* Calculate the IP header checksum. */
+            pxIPHeader->usHeaderChecksum = 0x00;
+            pxIPHeader->usHeaderChecksum = usGenerateChecksum( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ipSIZE_OF_IPv4_HEADER );
+            pxIPHeader->usHeaderChecksum = ~FreeRTOS_htons( pxIPHeader->usHeaderChecksum );
 
-        /* Calculate the TCP checksum for an outgoing packet. */
-        usGenerateProtocolChecksum( pucBuffer, uxLength, pdTRUE );
+            /* Calculate the TCP checksum for an outgoing packet. */
+            usGenerateProtocolChecksum( pucBuffer, uxLength, pdTRUE );
+        }
     }
 }
 /*-----------------------------------------------------------*/
@@ -773,7 +777,7 @@ static uint32_t prvEMACRxPoll( void )
          * descriptor then allocate one now. */
         if( ( pxNextNetworkBufferDescriptor == NULL ) && ( uxGetNumberOfFreeNetworkBuffers() > xMinDescriptorsToLeave ) )
         {
-            pxNextNetworkBufferDescriptor = pxGetNetworkBufferWithDescriptor( ipTOTAL_ETHERNET_FRAME_SIZE, xBlockTime );
+            pxNextNetworkBufferDescriptor = pxGetNetworkBufferWithDescriptor( GMAC_RX_UNITSIZE, xBlockTime );
         }
 
         if( pxNextNetworkBufferDescriptor != NULL )
@@ -789,7 +793,7 @@ static uint32_t prvEMACRxPoll( void )
         }
 
         /* Read the next packet from the hardware into pucUseBuffer. */
-        ulResult = gmac_dev_read( &gs_gmac_dev, pucUseBuffer, ipTOTAL_ETHERNET_FRAME_SIZE, &ulReceiveCount, &pucDMABuffer );
+        ulResult = gmac_dev_read( &gs_gmac_dev, pucUseBuffer, GMAC_RX_UNITSIZE, &ulReceiveCount, &pucDMABuffer );
 
         if( ( ulResult != GMAC_OK ) || ( ulReceiveCount == 0 ) )
         {
diff --git a/source/portable/NetworkInterface/DriverSAM/gmac_SAM.c b/source/portable/NetworkInterface/DriverSAM/gmac_SAM.c
@@ -111,17 +111,27 @@
  * See \ref gmac_quickstart.
  *
  * Driver for the GMAC (Ethernet Media Access Controller).
- * This file contains basic functions for the GMAC, with support for all modes, settings
- * and clock speeds.
+ * This file contains basic functions for the GMAC, with support for all modes,
+ * settings and clock speeds.
  *
  * \section dependencies Dependencies
  * This driver does not depend on other modules.
  *
  * @{
  */
 
-#define NETWORK_BUFFER_SIZE    1536
-
+/*
+ *     When BufferAllocation_1.c is used, the network buffer space
+ *     is declared statically as 'ucNetworkPackets[]'.
+ *     Like the DMA descriptors, this array is located in a non-cached area.
+ *     Here an example of the total size:
+ *
+ *         #define ipconfigNUM_NETWORK_BUFFER_DESCRIPTORS    24
+ *         #define GMAC_FRAME_LENTGH_MAX                     1536
+ *         Hidden space for back-pointer and IP-type         16
+ *
+ *     Total size: 24 * ( 1536 + 16 ) = 37248 bytes
+ */
 __attribute__( ( aligned( 32 ) ) )
 __attribute__( ( section( ".first_data" ) ) )
 uint8_t ucNetworkPackets[ ipconfigNUM_NETWORK_BUFFER_DESCRIPTORS * NETWORK_BUFFER_SIZE ];
@@ -688,11 +698,6 @@ uint32_t gmac_dev_read( gmac_device_t * p_gmac_dev,
     return GMAC_OK;
 }
 
-#if ( SAME70 == 0 )
-    extern void vGMACGenerateChecksum( uint8_t * apBuffer,
-                                       size_t uxLength );
-#endif
-
 /**
  * \brief Send ulLength bytes from pcFrom. This copies the buffer to one of the
  * GMAC Tx buffers, and then indicates to the GMAC that the buffer is ready.
@@ -749,13 +754,10 @@ uint32_t gmac_dev_write( gmac_device_t * p_gmac_dev,
                 memcpy( ( void * ) p_tx_td->addr, p_buffer, ul_size );
             }
         #endif /* ipconfigZERO_COPY_TX_DRIVER */
-        #if ( SAME70 == 0 )
-            {
-                #warning Is this a SAM4E?
-                /* Needs to be called for SAM4E series only. */
-                vGMACGenerateChecksum( ( uint8_t * ) p_tx_td->addr, ( size_t ) ul_size );
-            }
-        #endif
+        {
+            /* Needs to be called for SAM4E series only. */
+            vGMACGenerateChecksum( ( uint8_t * ) p_tx_td->addr, ( size_t ) ul_size );
+        }
     }
 
     /* Update transmit descriptor status */
diff --git a/source/portable/NetworkInterface/DriverSAM/gmac_SAM.h b/source/portable/NetworkInterface/DriverSAM/gmac_SAM.h
@@ -96,42 +96,45 @@
 
 /** The MAC can support frame lengths up to 1536 bytes */
     #define GMAC_FRAME_LENTGH_MAX      1536
+    #define GMAC_RX_UNITSIZE           GMAC_FRAME_LENTGH_MAX /**< Maximum size for RX buffer  */
+    #define GMAC_TX_UNITSIZE           GMAC_FRAME_LENTGH_MAX /**< Maximum size for TX buffer  */
 
-/*#define GMAC_RX_UNITSIZE            128     / **< Fixed size for RX buffer  * / */
-    #define GMAC_RX_UNITSIZE           1536 /**< Fixed size for RX buffer  */
+/* A network buffer starts with 10 hidden bytes (ipBUFFER_PADDING)
+ * in which a pointer is stored. Round up this extra size to a multiple of 16,
+ * in order to get well-aligned buffers. */
 
-/*#define GMAC_TX_UNITSIZE            1518    / **< Size for ETH frame length * / */
-    #define GMAC_TX_UNITSIZE           1536 /**< Size for ETH frame length */
+    #define BUFFER_PADDING           ( ( ipBUFFER_PADDING + 16U ) & ~0x0FU )
+    #define NETWORK_BUFFER_SIZE      ( GMAC_FRAME_LENTGH_MAX + BUFFER_PADDING )
 
 /** GMAC clock speed */
-    #define GMAC_MCK_SPEED_240MHZ      ( 240 * 1000 * 1000 )
-    #define GMAC_MCK_SPEED_160MHZ      ( 160 * 1000 * 1000 )
-    #define GMAC_MCK_SPEED_120MHZ      ( 120 * 1000 * 1000 )
-    #define GMAC_MCK_SPEED_80MHZ       ( 80 * 1000 * 1000 )
-    #define GMAC_MCK_SPEED_40MHZ       ( 40 * 1000 * 1000 )
-    #define GMAC_MCK_SPEED_20MHZ       ( 20 * 1000 * 1000 )
+    #define GMAC_MCK_SPEED_240MHZ    ( 240 * 1000 * 1000 )
+    #define GMAC_MCK_SPEED_160MHZ    ( 160 * 1000 * 1000 )
+    #define GMAC_MCK_SPEED_120MHZ    ( 120 * 1000 * 1000 )
+    #define GMAC_MCK_SPEED_80MHZ     ( 80 * 1000 * 1000 )
+    #define GMAC_MCK_SPEED_40MHZ     ( 40 * 1000 * 1000 )
+    #define GMAC_MCK_SPEED_20MHZ     ( 20 * 1000 * 1000 )
 
 /** GMAC maintain code default value*/
-    #define GMAC_MAN_CODE_VALUE        ( 10 )
+    #define GMAC_MAN_CODE_VALUE      ( 10 )
 
 /** GMAC maintain start of frame default value*/
-    #define GMAC_MAN_SOF_VALUE         ( 1 )
+    #define GMAC_MAN_SOF_VALUE       ( 1 )
 
 /** GMAC maintain read/write*/
-    #define GMAC_MAN_RW_TYPE           ( 2 )
+    #define GMAC_MAN_RW_TYPE         ( 2 )
 
 /** GMAC maintain read only*/
-    #define GMAC_MAN_READ_ONLY         ( 1 )
+    #define GMAC_MAN_READ_ONLY       ( 1 )
 
 /** GMAC address length */
-    #define GMAC_ADDR_LENGTH           ( 6 )
+    #define GMAC_ADDR_LENGTH         ( 6 )
 
 
-    #define GMAC_DUPLEX_HALF           0
-    #define GMAC_DUPLEX_FULL           1
+    #define GMAC_DUPLEX_HALF         0
+    #define GMAC_DUPLEX_FULL         1
 
-    #define GMAC_SPEED_10M             0
-    #define GMAC_SPEED_100M            1
+    #define GMAC_SPEED_10M           0
+    #define GMAC_SPEED_100M          1
 
 /**
  * \brief Return codes for GMAC APIs.
@@ -1536,6 +1539,11 @@
 
     void gmac_reset_tx_mem( gmac_device_t * p_dev );
 
+/* The SAM4E has problems offloading checksums for transmission.
+ * The SAME70 does not set the CRC for ICMP packets (ping). */
+    extern void vGMACGenerateChecksum( uint8_t * apBuffer,
+                                       size_t uxLength );
+
 /*/ @cond 0 */
 /**INDENT-OFF**/
     #ifdef __cplusplus
