[PATCH] Revert "net: eth_bootdev_hunt() should not run DHCP"

[Simon Glass]

Hi Heiko,

On Thu, 3 Apr 2025 at 10:36, Heiko Stübner <heiko at sntech.de> wrote:
>
> Am Dienstag, 1. April 2025, 18:13:35 MESZ schrieb Heinrich Schuchardt:
> > On 01.04.25 17:51, Simon Glass wrote:
> > > On Tue, 1 Apr 2025 at 21:38, Heiko Stuebner <heiko at sntech.de> wrote:
> > >>
> > >> This reverts commit 1f68057e03206e6597ca8b2be8bb1c49d4bd47d0.
> > >>
> > >> Commit 1f68057e0320 ("net: eth_bootdev_hunt() should not run DHCP")
> > >> aims to reduce EFI boot times by disabling the dhcp_run when
> > >> checking ethernet bootdevices, by preventing it from running double,
> > >> with the reasoning
> > >>
> > >>      We need to call eth_bootdev_hunt() when setting up the EFI sub-system to
> > >>      supply the simple network protocol. We don't need an IP address set up.
> > >>
> > >> That might by true for EFI, but not for everything else, because when
> > >> running distro-boot and for example the PXE method in it, nothing will
> > >> set up an IP address now.
> >
> > The removed call was dhcp_run(addr, NULL, true);
> >
> > We have:
> >
> > distro_efi_read_bootflow_net():
> > boot/bootmeth_efi.c:205:        ret = dhcp_run(addr, NULL, true);
> >
> > script_read_bootflow_net():
> > boot/bootmeth_script.c:132:     ret = dhcp_run(addr, fname, true);
> >
> > extlinux_pxe_read_bootflow() seems to be lacking the call.
> >
> > So instead of reverting
> > 1f68057e0320 ("net: eth_bootdev_hunt() should not run DHCP")
> > we should add the missing call in extlinux_pxe_read_bootflow().
>
> doing
>
> ----- 8< -----
> diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
> index b91e61bcbc4..6e5e0f99ea4 100644
> --- a/boot/bootmeth_pxe.c
> +++ b/boot/bootmeth_pxe.c
> @@ -73,6 +73,10 @@ static int extlinux_pxe_read_bootflow(struct udevice *dev,
>                 return log_msg_ret("pxeb", -EPERM);
>         addr = simple_strtoul(addr_str, NULL, 16);
>
> +       ret = dhcp_run(addr, NULL, false);
> +       if (ret)
> +               return log_msg_ret("dhc", ret);
> +
>         log_debug("calling pxe_get()\n");
>         ret = pxe_get(addr, &bootdir, &size, false);
>         log_debug("pxe_get() returned %d\n", ret);
> ----- 8< -----
>
> does seem to work in _my_ usecase and gets me loading stuff over pxe
> again.
>
> I guess this whole thing now becomes a strategy decision ;-) :
>
> - it's very late in the release, do we revert to the known working state
>   or hope the above fixes all issues
> - Simon's wish of not sprinkling dhcp_run calls in multiple places
>
> I guess I'm fine with both way, as either fixes my use-case ;-)

For the reasons you patch seems fine to me. As you say, it is very
late in the cycle.

I already gave this feedback to Heinrich, so once the revert is in he
may have time to take a look.

Regards,
Simon