[PATCH] net: lwip: introduce net_lwip_eth_stop() function

[Jerome Forissier]

On 12/05/2026 13:41, Simon Glass wrote:
> Hi David,
> 
> On Mon, 11 May 2026 at 15:48, David Lechner <dlechner at baylibre.com> wrote:
>>
>> On 5/11/26 1:08 PM, Simon Glass wrote:
>>> Hi David,
>>>
>>> On 2026-05-09T17:36:26, David Lechner <dlechner at baylibre.com> wrote:
>>>> net: lwip: introduce net_lwip_eth_stop() function
>>>>
>>>> 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 <dlechner at baylibre.com>
>>>>
>>>> cmd/lwip/ping.c     | 10 ++++++++--
>>>>  cmd/lwip/sntp.c     | 10 ++++++++--
>>>>  include/net-lwip.h  |  1 +
>>>>  net/lwip/dhcp.c     | 15 +++++++++++----
>>>>  net/lwip/dns.c      |  6 +++++-
>>>>  net/lwip/net-lwip.c |  6 +++++-
>>>>  net/lwip/nfs.c      |  4 ++++
>>>>  net/lwip/tftp.c     |  4 ++++
>>>>  net/lwip/wget.c     | 34 ++++++++++++++++++++++------------
>>>>  9 files changed, 68 insertions(+), 22 deletions(-)
>>>
>>>> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
>>>> @@ -302,6 +302,7 @@ int wget_do_request(ulong dst_addr, char *uri)
>>>>       struct udevice *udev;
>>>>       struct netif *netif;
>>>>       struct wget_ctx ctx;
>>>> +     int ret = CMD_RET_USAGE;
>>>>       char *path;
>>>>       bool is_https;
>>>
>>> Initialising ret to CMD_RET_USAGE changes error paths:
>>>
>>>    netif = net_lwip_new_netif(udev);
>>>    if (!netif)
>>>         goto out_stop;          /* ret is still CMD_RET_USAGE */
>>>
>>>    if (!tls_allocator.arg) {
>>>         log_err(...);
>>>         goto out_remove_netif;  /* ret is still CMD_RET_USAGE */
>>>    }
>>>
>>> The fall-through after 'Certificate verification failed' also lands on
>>> out_remove_netif with ret unchanged. These paths previously returned
>>> -1; now they return CMD_RET_USAGE (1), so the command framework prints
>>> usage for a network/TLS failure.
>>
>> The code I am looking at says:
>>
>> enum command_ret_t {
>>         CMD_RET_SUCCESS,        /* 0 = Success */
>>         CMD_RET_FAILURE,        /* 1 = Failure */
>>         CMD_RET_USAGE = -1,     /* Failure, please report 'usage' error */
>> };
>>
>> So I don't think these changed. CMD_RET_USAGE is -1.
>>
>> Happy to init it -1 though as you suggest if the name doesn't make
>> sense here. That is might be why it was -1 before instead of using
>> the macro anyway.
> 
> Ah yes I missed that this is a pre-existing bug. We should return
> usage only when parsing is wrong, not when something fails.

+1
 
> So what you have here is fine and actually makes the bug more obvious
> in the code, which is good.

Yeah :)
 
> Reviewed-by: Simon Glass <sjg at chromium.org>

Reviewed-by: Jerome Forissier <jerome.forissier at arm.com>

Thanks,
-- 
Jerome

>>
>>>
>>> wget_do_request() isn't a command handler either, so CMD_RET_USAGE is
>>> doubly out of place. Please initialise ret to -1 and set ret =
>>> CMD_RET_FAILURE explicitly only in the paths that previously returned
>>> that.
>>>
>>> It might make sense to split this up function (perhaps in a prior
>>> patch) to try to make the error handling easier?
>>>
>> Sure. Is there a common naming pattern for that in U-Boot. I would
>> call the inner function __wget_do_request(). But I know opinions on
>> such things can vary widely. :-)
> 
> If you do take this on, I would say better to rename it, e.g.
> handle_request() - strictly speaking we are supposed to avoid leading
> underscores, since they indicate symbols used by the
> compiler/toolchain, particularly the double underscore. In practice
> you will see use of a leading underscore in the code.
> 
> Separately, I am taking a look at more use of getopt(), which would
> make parsing more regular, but unfortunately adds code size.
> 
> Regards,
> Simon