On 10/9/24 03:51, Simon Glass wrote:
On Thu, 3 Oct 2024 at 09:23, Jerome Forissier jerome.forissier@linaro.org wrote:
Note: patch posted separately [0].
[0] http://patchwork.ozlabs.org/project/uboot/patch/20240927142038.879037-1-jero...
Some drivers do not behave properly when free_pkt() is called with a length of zero. It is an issue I observed when developing the lwIP series [1] (see "QEMU CI tests for r2dplus_i82557c, r2dplus_rtl8139" in the change log) and which I fixed incorrectly by not calling free_pkt() when recv() returns 0. That turned out to be wrong for two reasons:
- The DM documentation [2] clearly requires it:
"The **recv** function polls for availability of a new packet. [...] If there is an error [...], return 0 if you require the packet to be cleaned up normally, or a negative error code otherwise (cleanup not necessary or already done).
If **free_pkt** is defined, U-Boot will call it after a received packet has been processed [...]. free_pkt() will be called after recv(), for the same packet [...]"
The imx8mp_evk platform will fail with OOM errors if free_pkt() is not called after recv() returns 0:
u-boot=> tftp 192.168.0.16:50M Using ethernet@30be0000 device TFTP from server 192.168.0.16; our IP address is 192.168.0.48 Filename '50M'. Load address: 0x40480000 Loading: #######################fecmxc_recv: error allocating packetp fecmxc_recv: error allocating packetp fecmxc_recv: error allocating packetp ...
Therefore, make recv() return -EINVAL instead of 0 when no packet is available and the driver doesn't expect free_pkt() to be called subsequently.
Do you mean -EAGAIN ? Otherwise, it seems like this comment relates to a different patch.
Yes, good catch. I will fix the description and resend patch [1] as v2.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240927142038.879037-1-jer...
[1] https://lists.denx.de/pipermail/u-boot/2024-August/562861.html [2] doc/develop/driver-model/ethernet.rst
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org
drivers/net/eepro100.c | 2 +- drivers/net/rtl8139.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Thanks,