STMicroelectronics / stm32h7xx_hal_driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32H7 series.
BSD 3-Clause "New" or "Revised" License
97 stars 41 forks source link

Ethernet reception is inoperable if the buffers' size is small #15

Closed kernelcr closed 2 years ago

kernelcr commented 2 years ago

Describe the set-up ST STM32H743ZI STM32CubeH7 HAL Driver v1.10.0 STM32CubeMX 6.3.0 IAR EWARM 9.10.2 LwIP 1.4.1

Describe the bug Ethernet reception via HAL driver discards packets and lock-ups if Rx buffers' size is less than size of the packets. For example, I set ETH_InitTypeDef::RxBuffLen as 256 and try to receive UDP packets with 512 bytes of payload. I suppose that an issue is in HAL_ETH_IsRxDataAvailable in this block of code, and if I remove it reception works fine:

  /* Build Descriptors if an incomplete Packet is received */
  if(appdesccnt > 0U)
  {
    dmarxdesclist->CurRxDesc = descidx;
    dmarxdesclist->FirstAppDesc = firstappdescidx;
    descidx = firstappdescidx;
    dmarxdesc = (ETH_DMADescTypeDef *)dmarxdesclist->RxDesc[descidx];

    for(descscancnt = 0; descscancnt < appdesccnt; descscancnt++)
    {
      WRITE_REG(dmarxdesc->DESC0, dmarxdesc->BackupAddr0);
      WRITE_REG(dmarxdesc->DESC3, ETH_DMARXNDESCRF_BUF1V);

      if (READ_REG(dmarxdesc->BackupAddr1) != ((uint32_t)RESET))
      {
        WRITE_REG(dmarxdesc->DESC2, dmarxdesc->BackupAddr1);
        SET_BIT(dmarxdesc->DESC3, ETH_DMARXNDESCRF_BUF2V);
      }

      SET_BIT(dmarxdesc->DESC3, ETH_DMARXNDESCRF_OWN);

      if(dmarxdesclist->ItMode != ((uint32_t)RESET))
      {
        SET_BIT(dmarxdesc->DESC3, ETH_DMARXNDESCRF_IOC);
      }
      if(descscancnt < (appdesccnt - 1U))
      {
        /* Increment rx descriptor index */
        INCR_RX_DESC_INDEX(descidx, 1U);
        /* Get descriptor address */
        dmarxdesc = (ETH_DMADescTypeDef *)dmarxdesclist->RxDesc[descidx];
      }
    }

    /* Set the Tail pointer address to the last rx descriptor hold by the app */
    WRITE_REG(heth->Instance->DMACRDTPR, (uint32_t)dmarxdesc);
  }

I don' t know the point of this code. I make the polling in while(1), so in my opinion in some calls to HAL_ETH_IsRxDataAvailable it is normally to have part of the packet (in one or several buffers) without last part that is not transferred to Rx buffer yet. Maybe I should poll it in another way, but I didn't find any statement forbidding to do it in while(1). So in my situation this code just discards such packets for some reason. Moreover, as I suppose, last part of the packet then arrives to the buffer, but HAL_ETH_IsRxDataAvailable doesn't see FD now, so it doesn't change firstappdescidx from 0 to something and zeroing dmarxdesclist->FirstAppDesc. After finding LD function returns 1, therefore I should call HAL_ETH_BuildRxDescriptors, that iterates beginning from incorrect (zero) position in FirstAppDesc and really handled descriptors are left with OWN bit resetted. When ETH_DMACCARXDR moves to these descriptors, DMA will go to suspend state. Because of absence of LD bit for these descriptors HAL_ETH_IsRxDataAvailable never returns 1 again.

Note. Current implementation of eternetif.c by Cube uses zero-copy Rx buffers' pool, so they need to be at MTU size. I replaced this pool with a real copy to lwip pool as I need to have Rx buffers small. With MTU size problematic code never take place. Note 2. If you want to check a correct approach to implementation of HAL_ETH_IsRxDataAvailable, you may look at ETH_CheckFrameReceived in STM32F4x7 Ethernet Driver, that doesn't have presented issue.

ALABSTM commented 2 years ago

Hi @kernelcr,

Thank you for this detailed report. We just made available a fully reworked ETH HAL driver via pull-request #16. The point you raised should have been addressed. You can use this new driver for your applications. Please feel free to provide your feedback here.

Please note also that this new ETH HAL driver breaks the compatibility with the previous one.

With regards,

Arni1115 commented 2 years ago

Could you provide example of ethernetif_input (using LwIP and FreeRTOS) function with for reworked ETH HAL driver?

ALABSTM commented 2 years ago

Hi @Arni1115 and @kernelcr,

I hope you are fine. Applications using the reworked ETH HAL driver have been published via pull-request STM32CubeH7#195. You will find updated ethernetif.c and ethernetif.h files. I hope this is what you are looking for.

With regards,

PS: All the best for the new year 2022!