STMicroelectronics / STM32CubeF7

STM32Cube MCU Full Package for the STM32F7 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
320 stars 191 forks source link

HAL_ETH_ReadData() uses ethernet framelength as buffer length #92

Closed Maashau closed 4 months ago

Maashau commented 1 year ago

HAL_ETH_ReadData() uses ethernet framelength as buffer length which causes problems when using buffer sizes smaller than ethernet frame.

Following row sets full frame length as buffer length: https://github.com/STMicroelectronics/STM32CubeF7/blob/6a3b9bb8d09839f365ba83a6cf5823ae5f5226db/Drivers/STM32F7xx_HAL_Driver/Src/stm32f7xx_hal_eth.c#L1129

This causes RxDataLength to be incorrect as it is just incremented by buffer length: https://github.com/STMicroelectronics/STM32CubeF7/blob/6a3b9bb8d09839f365ba83a6cf5823ae5f5226db/Drivers/STM32F7xx_HAL_Driver/Src/stm32f7xx_hal_eth.c#L1150

The same buffLength is also given to rxLinkCallback as parameter instead of size of the latest buffer.

ASELSTM commented 1 year ago

ST Internal Reference: 152028

PhilippHaefele commented 7 months ago

@Maashau First of all thanks for sharing your input in public, so user can find it and get informed before a fix from ST is made available.

Did investigate to understand your bug report and wanted to share my results, as on a first look i was not understanding what happens.

I did assume that buffer size does mean ETH_RX_BUF_SIZE.

The issue (at my point of view) seems to be that the size of the last chunk on chained buffers (why it shouldn't matter when buffer size is equal or bigger then ethernet frame) is calculated wrong. This is the case because the FL / ETH_DMARXDESC_FL part of the RDES0 / DESC0 part returns the whole frame length including the CRC. Sentence from ref manual for this: "These bits indicate the byte length of the received frame that was transferred to host memory (including CRC)."
So it seems that there was a misunderstanding that this part of the register does return the buffer byte count or it was not foreseen that the ETH_RX_BUF_SIZE is smaller than ethernet frame size.

To my limited understanding of the ethernet hardware / code, the right bufflength should be easily calculated by subtracting of the already received bytes aka heth->RxDescList.RxDataLength.

@Maashau Are my assumptions and description the one that you had in mind when reporting this issue?

Maashau commented 7 months ago

@PhilippHaefele It has been long since I've last looked at this but I'll try to remember the specifics.

For each descriptor, buffLength is calculated. Each descriptor's buffLength is given to the rxLinkCallback and added to RxDataLength. For the last descriptor, buffLength is set to full frame length, making RxDataLength have size of (fullFrameLen * 2 - lastFrameLen). rxLinkCallback has to subtract all previous Lengths from the last descriptor if Length > ETH_RX_BUF_SIZE.

Hope this explains things bit better.

PhilippHaefele commented 7 months ago

@Maashau Thank you for your response. That seems to match my above description. Adapting the ethernetif code seems to be a reasonable workaround for now, but the issue should still be fixed properly in stm32f7xx_hal_eth.c by ST

RJMSTM commented 4 months ago

Fixed in 867c190