diff mbox series

net: tftp: don't call stop callback from UDP handler

Message ID 20240513062643.3589070-1-alexander.sverdlin@siemens.com
State Accepted
Commit 44340456475da953a30c5cc4fbe8a9bc013c8c0f
Delegated to: Ramon Fried
Headers show
Series net: tftp: don't call stop callback from UDP handler | expand

Commit Message

Sverdlin, Alexander May 13, 2024, 6:26 a.m. UTC 
From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Contrary to doc/develop/driver-model/ethernet.rst contract, eth_ops
.free_pkt can be called after .stop, there are several error paths in TFTP,
for instance:

eth_halt() <= tftp_handler() <= net_process_received_packet() <= eth_rx()
...
am65_cpsw_free_pkt() <= eth_rx()

Which results in (deliberately "tftpboot"ing non-existing file):

TFTP error: 'File not found' (1)
Not retrying...
am65_cpsw_nuss_port ethernet@8000000port@1: RX dma free_pkt failed -22

Avoid the DMA error message (and follow the documentation) by deferring
eth_halt() until net_loop() calls net_start_again() and only do
eth_halt_state_only() instead.

Fixes: aafda38fb266 ("Add error codes/handling for TFTP-server")
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 net/eth-uclass.c | 2 ++
 net/tftp.c       | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

This isn't a v2, but a completely different approach to
https://patchwork.ozlabs.org/project/uboot/patch/20240508183605.955341-1-alexander.sverdlin@siemens.com/

Comments

Tom Rini May 23, 2025, 10:26 p.m. UTC | #1 
On Mon, 13 May 2024 08:26:35 +0200, A. Sverdlin wrote:

> Contrary to doc/develop/driver-model/ethernet.rst contract, eth_ops
> .free_pkt can be called after .stop, there are several error paths in TFTP,
> for instance:
> 
> eth_halt() <= tftp_handler() <= net_process_received_packet() <= eth_rx()
> ...
> am65_cpsw_free_pkt() <= eth_rx()
> 
> [...]

Applied to u-boot/next, thanks!

[1/1] net: tftp: don't call stop callback from UDP handler
      commit: 44340456475da953a30c5cc4fbe8a9bc013c8c0f
diff mbox series

Patch

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 193218a328b..9863db59d68 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -450,6 +450,8 @@  int eth_rx(void)
 			eth_get_ops(current)->free_pkt(current, packet, ret);
 		if (ret <= 0)
 			break;
+		if (!eth_is_active(current))
+			break;
 	}
 	if (ret == -EAGAIN)
 		ret = 0;
diff --git a/net/tftp.c b/net/tftp.c
index 2e335413492..081014a1fde 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -649,7 +649,7 @@  static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
 
 		if (store_block(tftp_cur_block, pkt + 2, len)) {
-			eth_halt();
+			eth_halt_state_only();
 			net_set_state(NETLOOP_FAIL);
 			break;
 		}
@@ -678,7 +678,7 @@  static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		case TFTP_ERR_FILE_NOT_FOUND:
 		case TFTP_ERR_ACCESS_DENIED:
 			puts("Not retrying...\n");
-			eth_halt();
+			eth_halt_state_only();
 			net_set_state(NETLOOP_FAIL);
 			break;
 		case TFTP_ERR_UNDEFINED: