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

[Tim Harvey]

On Wed, Jun 18, 2025 at 8:32 AM Tim Harvey <tharvey at gateworks.com> wrote:
>
> On Tue, Jun 17, 2025 at 3:45 PM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Sun, Jun 01, 2025 at 11:07:16PM +0200, Heinrich Schuchardt 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.
> >
> > So for testing purposes, if CYCLIC is disabled but LWIP is enabled,
> > things are fine here? And so we need to at least partially revert commit
> > 5c8dddadf1c9 ("video: Use cyclic to handle video sync") if not fully
> > revert it?
>
> Hi Tom,
>
> I can't disable CYCLIC without also disabling WDT which then works
> fine as the watchdog isn't being used.
>
> You are likely pointing to the correct patch that is causing
> Heinrich's issue but as far as I know he has the
> hardware/configuration needed to test this and I don't know how to
> re-create his issue.
>
> So in my opinion there is nothing wrong with my patch here and it's
> more of an issue with the video using cyclic for video sync.
>

Incidentally, I've noticed that I only have this
watchdog-not-getting-reset issue when using the IMX FEC driver. The
IMX QOS (imx8mp) has calls to udelay in the packet send and udelay
calls schedule. I'm not sure how many network drivers out there end up
calling udelay/schedule on their own.

Best Regards,

Tim