On 5/13/26 2:57 AM, Jerome Forissier wrote: > > > On 12/05/2026 23:48, David Lechner wrote: >> Add a introduce net_lwip_eth_stop() function and use that to stop the >> network interface after each command that uses the network. >> >> This makes the behavior the same as the legacy net code and avoids >> potential issues with the network interface being left in an active >> state after a command finishes. >> >> Signed-off-by: David Lechner <[email protected]> >> --- >> cmd/lwip/ping.c | 10 ++++++++-- >> cmd/lwip/sntp.c | 10 ++++++++-- >> include/net-lwip.h | 1 + >> net/lwip/dhcp.c | 15 +++++++++++---- >> net/lwip/dns.c | 7 ++++++- >> net/lwip/net-lwip.c | 5 +++++ >> net/lwip/nfs.c | 4 ++++ >> net/lwip/tftp.c | 4 ++++ >> net/lwip/wget.c | 5 ++++- >> 9 files changed, 51 insertions(+), 10 deletions(-) >> >> diff --git a/cmd/lwip/ping.c b/cmd/lwip/ping.c >> index fc4cf7bde5f..836d1d8287f 100644 >> --- a/cmd/lwip/ping.c >> +++ b/cmd/lwip/ping.c >> @@ -163,6 +163,7 @@ static int ping_loop(struct udevice *udev, const >> ip_addr_t *addr) >> int do_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) >> { >> ip_addr_t addr; >> + int ret = CMD_RET_FAILURE; >> >> if (argc < 2) >> return CMD_RET_USAGE; >> @@ -176,8 +177,13 @@ restart: >> if (net_start_again() == 0) >> goto restart; >> else >> - return CMD_RET_FAILURE; >> + goto out; >> } >> >> - return CMD_RET_SUCCESS; >> + ret = CMD_RET_SUCCESS; >> + >> +out: >> + net_lwip_eth_stop(); >> + >> + return ret; >> } >> diff --git a/cmd/lwip/sntp.c b/cmd/lwip/sntp.c >> index 608345c873b..5fa400b104a 100644 >> --- a/cmd/lwip/sntp.c >> +++ b/cmd/lwip/sntp.c >> @@ -101,6 +101,7 @@ int do_sntp(struct cmd_tbl *cmdtp, int flag, int argc, >> char *const argv[]) >> ip_addr_t *srvip; >> char *server; >> ip_addr_t ipaddr; >> + int ret = CMD_RET_FAILURE; >> >> switch (argc) { >> case 1: >> @@ -127,7 +128,12 @@ int do_sntp(struct cmd_tbl *cmdtp, int flag, int argc, >> char *const argv[]) >> return CMD_RET_FAILURE; >> >> if (sntp_loop(eth_get_dev(), srvip) < 0) >> - return CMD_RET_FAILURE; >> + goto out; >> + >> + ret = CMD_RET_SUCCESS; >> + >> +out: >> + net_lwip_eth_stop(); >> >> - return CMD_RET_SUCCESS; >> + return ret; >> } >> diff --git a/include/net-lwip.h b/include/net-lwip.h >> index 20cb0992cce..5d0627eb271 100644 >> --- a/include/net-lwip.h >> +++ b/include/net-lwip.h >> @@ -35,6 +35,7 @@ int eth_init_state_only(void); /* Set active state */ >> >> int net_lwip_dns_init(void); >> int net_lwip_eth_start(void); >> +void net_lwip_eth_stop(void); >> struct netif *net_lwip_new_netif(struct udevice *udev); >> struct netif *net_lwip_new_netif_noip(struct udevice *udev); >> void net_lwip_remove_netif(struct netif *netif); >> diff --git a/net/lwip/dhcp.c b/net/lwip/dhcp.c >> index acdf601d7eb..18dc36ae7ca 100644 >> --- a/net/lwip/dhcp.c >> +++ b/net/lwip/dhcp.c >> @@ -138,18 +138,25 @@ int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc, >> char *const argv[]) >> dev = eth_get_dev(); >> if (!dev) { >> log_err("No network device\n"); >> - return CMD_RET_FAILURE; >> + ret = CMD_RET_FAILURE; >> + goto out; >> } >> >> ret = dhcp_loop(dev); >> if (ret) >> - return ret; >> + goto out; >> >> if (argc > 1) { >> struct cmd_tbl cmdtp = {}; >> >> - return do_tftpb(&cmdtp, 0, argc, argv); >> + ret = do_tftpb(&cmdtp, 0, argc, argv); >> + goto out; > > do_tftpb() already takes care of net_lwip_eth_stop() so this should > probably be: > > + return do_tftpb(&cmdtp, 0, argc, argv); > > But this also raises the qestion of the double call to > net_lwip_eth_start(): > > do_dhcp() > net_lwip_eth_start() > do_tftpb() > net_lwip_eth_start() > > How about adding a static counter to better deal with the nested case? > > static int net_lwip_eth_started; > > int net_lwip_eth_start(void) > { > int ret; > > if (net_lwip_eth_started++ > 0) > return 0; > > ret = eth_init(); > if (ret) { > net_lwip_eth_started--; > return ret; > } > > return 0; > } > > void net_lwip_eth_stop(void) > { > if (!net_lwip_eth_started) > return; > > if (--net_lwip_eth_started) > return; > > eth_halt(); > } > > Thanks,
Yes, I like this idea. There is also an odd case in do_ping() that we will need to look at though. It currently would not work with reference counting since net_lwip_eth_start() can be called multiple times on retries.
