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

["Heiko Stübner" <heiko@sntech.de>]

Am Mittwoch, 2. April 2025, 23:36:46 MESZ schrieb Heiko Stübner:
> 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@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.

For reference, I implemented that now in
https://lore.kernel.org/u-boot/20250402215025.2655384-1-heiko@sntech.de/

So people can decide which way to go :-)


Heiko


> 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 ;-)
> 
> Heiko