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

[Tim Harvey]

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?

Best Regards,

Tim