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?
Best Regards,
Tim
