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

[Tim Harvey]

On Mon, Jun 16, 2025 at 12:18 AM Jerome Forissier
<jerome.forissier at linaro.org> wrote:
>
>
>
> 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?
>
Hi Jerome,

I don't think that makes much sense. Long transfers for compressed
disk images can take minutes sometimes. It doesn't make sense to not
call any cyclic's in that timeframe. Additionally its possible some
older less capable watchdogs don't have that much flexibility in
timeouts.

What Heinrich points out here seems valid but I'm not familiar with
the video framework to work up a patch.

Best Regards,

Tim