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

[David Lechner]

On 5/12/26 9:36 AM, Jerome Forissier wrote:
> 
> 
> 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,

I have prepared a revision that splits this into two functions so
the -1 is left untouched.

If I am understanding correctly, it sounds like I should add another
patch to make the return values of wget_do_request() consistent.

The docs for this function says:

 * Return:	zero on success, negative if failed

And there is one existing caller checking err < 0 (efi_bootmgr.c).
So I plan to replace all `return CMD_RET_FAILURE` with `return -1`.
Unless we should return errno codes?