[PATCH v2 1/5] net: lwip: call sys_check_timeouts and schedule on rx

[Jerome Forissier]

On 6/9/25 19:35, Tim Harvey wrote:
> On Tue, Jun 3, 2025 at 5:04 PM Tim Harvey <tharvey at gateworks.com> wrote:
>>
>> On Sun, Jun 1, 2025 at 2:07 PM Heinrich Schuchardt
>> <heinrich.schuchardt at canonical.com> wrote:
>>>
>>> On 5/30/25 17:38, Tim Harvey wrote:
>>>> Call schedule() in net_lwip_rx() to service U-Boot tasks and
>>>> actions during packet rx.
>>>>
>>>> As a cleanup also move sys_check_timeouts() here and remove it from the
>>>> functions that call net_lwip_rx().
>>>>
>>>> This resolves the issue of an active watchdog resetting the board on
>>>> long network activities.
>>>>
>>>> Suggested-by: Jerome Forissier <jerome.forissier at linaro.org>
>>>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>>>> ---
>>>> v2:
>>>>   - remove duplication of sys_check_timeouts() from functions that call
>>>>     net_lwip_rx()
>>>> ---
>>>>   net/lwip/dhcp.c     | 1 -
>>>>   net/lwip/dns.c      | 1 -
>>>>   net/lwip/net-lwip.c | 7 +++++++
>>>>   net/lwip/ping.c     | 1 -
>>>>   net/lwip/tftp.c     | 1 -
>>>>   net/lwip/wget.c     | 1 -
>>>>   6 files changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/net/lwip/dhcp.c b/net/lwip/dhcp.c
>>>> index 92bd7067a7fb..b79b746db390 100644
>>>> --- a/net/lwip/dhcp.c
>>>> +++ b/net/lwip/dhcp.c
>>>> @@ -57,7 +57,6 @@ static int dhcp_loop(struct udevice *udev)
>>>>       /* Wait for DHCP to complete */
>>>>       do {
>>>>               net_lwip_rx(udev, netif);
>>>> -             sys_check_timeouts();
>>>>               bound = dhcp_supplied_address(netif);
>>>>               if (bound)
>>>>                       break;
>>>> diff --git a/net/lwip/dns.c b/net/lwip/dns.c
>>>> index 19172ac959ac..738f8bf3196b 100644
>>>> --- a/net/lwip/dns.c
>>>> +++ b/net/lwip/dns.c
>>>> @@ -91,7 +91,6 @@ static int dns_loop(struct udevice *udev, const char *name, const char *var)
>>>>                       net_lwip_rx(udev, netif);
>>>>                       if (dns_cb_arg.done)
>>>>                               break;
>>>> -                     sys_check_timeouts();
>>>>                       if (ctrlc()) {
>>>>                               printf("\nAbort\n");
>>>>                               break;
>>>> diff --git a/net/lwip/net-lwip.c b/net/lwip/net-lwip.c
>>>> index f05c4cd3f64f..e8d4cc542ed8 100644
>>>> --- a/net/lwip/net-lwip.c
>>>> +++ b/net/lwip/net-lwip.c
>>>> @@ -13,8 +13,10 @@
>>>>   #include <lwip/etharp.h>
>>>>   #include <lwip/init.h>
>>>>   #include <lwip/prot/etharp.h>
>>>> +#include <lwip/timeouts.h>
>>>>   #include <net.h>
>>>>   #include <timer.h>
>>>> +#include <u-boot/schedule.h>
>>>>
>>>>   /* xx:xx:xx:xx:xx:xx\0 */
>>>>   #define MAC_ADDR_STRLEN 18
>>>> @@ -284,6 +286,11 @@ int net_lwip_rx(struct udevice *udev, struct netif *netif)
>>>>       int len;
>>>>       int i;
>>>>
>>>> +     /* lwIP timers */
>>>> +     sys_check_timeouts();
>>>> +     /* Other tasks and actions */
>>>> +     schedule(
>>>
>>> Some scheduled tasks have a long duration, specifically video_sync_all()
>>> called via video_idle(). This change will lead to lost packages.
>>>
>>>
>>> @Tom, Simon
>>>
>>> The EFI code was impacted by 29caf9305b6f ("cyclic: Use schedule()
>>> instead of WATCHDOG_RESET()"). This added schedule() calls to EFI
>>> networking. Package losses can expected if schedule() takes more than 1
>>> µs. This is much shorter than the time needed for synchronizing a 1 MiB
>>> frame buffer. The cyclic framework was explicitly meant for short
>>> duration function calls, not for long duration tasks like syncing the
>>> video frame buffer.
>>>
>>>
>>> I think we should remove video_idle() from the cyclic framework.
>>> video_sync_all() should only be triggered if there is video output.
>>>
>>
>> Hi Heinrich,
>>
>> It makes sense what you are saying but I don't have the hardware to
>> test your scenario. Can you submit a patch for discussion with a fixes
>> tag?
> 
> Gentle ping here.
> 
> Can we get some discussion on the point Heinrich brings up about
> video_idle in cyclic? Without my patch here which this discussion is
> blocking, large lwIP network transfers are quite broken on boards with
> watchdogs.

Another option might be to increase the watchdog timeout?

Regards,
-- 
Jerome

> 
> Best Regards,
> 
> Tim