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

[Tim Harvey]

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.

Best Regards,

Tim