On Tue, Jun 3, 2025 at 5:04 PM Tim Harvey <thar...@gateworks.com> wrote:
>
> On Sun, Jun 1, 2025 at 2:07 PM Heinrich Schuchardt
> <heinrich.schucha...@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.foriss...@linaro.org>
> > > Signed-off-by: Tim Harvey <thar...@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
