diff mbox series

[v2] net: recv(): return -EAGAIN instead of 0 when no cleanup is expected

Message ID 20241009094208.1212555-1-jerome.forissier@linaro.org
State Accepted
Commit 63150710e34aa9d5d7c45e142e70016b31af2c04
Headers show
Series [v2] net: recv(): return -EAGAIN instead of 0 when no cleanup is expected | expand

Commit Message

Jerome Forissier Oct. 9, 2024, 9:42 a.m. UTC 
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:

1. 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 [...]"

2. 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 -EAGAIN instead of 0 when no packet is
available and the driver doesn't expect free_pkt() to be called
subsequently.

[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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/net/eepro100.c | 2 +-
 drivers/net/rtl8139.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Tom Rini Oct. 27, 2024, 11:11 p.m. UTC | #1 
On Wed, 09 Oct 2024 11:42:08 +0200, Jerome Forissier wrote:

> 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:
> 
> [...]

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/drivers/net/eepro100.c b/drivers/net/eepro100.c
index d18a8d577ca..f64dbb7d6a1 100644
--- a/drivers/net/eepro100.c
+++ b/drivers/net/eepro100.c
@@ -678,7 +678,7 @@  static int eepro100_recv_common(struct eepro100_priv *priv, uchar **packetp)
 	status = le16_to_cpu(desc->status);
 
 	if (!(status & RFD_STATUS_C))
-		return 0;
+		return -EAGAIN;
 
 	/* Valid frame status. */
 	if (status & RFD_STATUS_OK) {
diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
index 2e0afad089f..5f4b1e2d3a0 100644
--- a/drivers/net/rtl8139.c
+++ b/drivers/net/rtl8139.c
@@ -433,7 +433,7 @@  static int rtl8139_recv_common(struct rtl8139_priv *priv, unsigned char *rxdata,
 	int length = 0;
 
 	if (inb(priv->ioaddr + RTL_REG_CHIPCMD) & RTL_REG_CHIPCMD_RXBUFEMPTY)
-		return 0;
+		return -EAGAIN;
 
 	priv->rxstatus = inw(priv->ioaddr + RTL_REG_INTRSTATUS);
 	/* See below for the rest of the interrupt acknowledges.  */