[PATCH v2 4/6] net: eth_bootdev_hunt() should not run DHCP

[Simon Glass]

Hi Heinrich,

On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> Currently when booting dhcp_run() may be executed multiple times:
> once in eth_bootdev_hunt() and once in the network booting bootmeth.
>
> 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.
>
> We can reduce the bootime by not executing dhcp_run() in
> eth_bootdev_hunt().
>
> Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy
> network statck leads to downloading a file via TFTP and to booting the

spelling

> downloaded file.

I have now found the feature you're referring to - the calls to
env_get_autostart(). Perhaps we can remove this feature? It seems
pretty old and I don't see boards using it.

That feature stops bootstd working properly.

But if we don't remove it, we need to disable autostart in dhcp_run(),
since in that case it does not behave correctly. I can do a patch for
that if needed.

>
> Instead of running dchp_run() just check that there is a network device
> in eth_bootdev_hunt().
>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> v2:
>         reword the commit message
> ---
>  net/eth_bootdev.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c
> index 6ee54e3c790..b0fca6e8313 100644
> --- a/net/eth_bootdev.c
> +++ b/net/eth_bootdev.c
> @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev)
>         return 0;
>  }
>
> +/**
> + * eth_bootdev_hunt() - probe all network devices
> + *
> + * Network devices can also come from USB, but that is a higher
> + * priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been
> + * enumerated already. If something like 'bootflow scan dhcp' is used,
> + * then the user will need to run 'usb start' first.
> + *
> + * @info:      info structure describing this hunter
> + * @show:      true to show information from the hunter
> + *
> + * Return:     0 if device found, -EINVAL otherwise
> + */
>  static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show)
>  {
>         int ret;
> +       struct udevice *dev = NULL;
>
>         if (!test_eth_enabled())
>                 return 0;
> @@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show)
>                         log_warning("Failed to init PCI (%dE)\n", ret);
>         }
>
> -       /*
> -        * Ethernet devices can also come from USB, but that is a higher
> -        * priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been
> -        * enumerated already. If something like 'bootflow scan dhcp' is used
> -        * then the user will need to run 'usb start' first.
> -        */

Please keep this comment

> -       if (IS_ENABLED(CONFIG_CMD_DHCP)) {
> -               ret = dhcp_run(0, NULL, false);
> -               if (ret)
> -                       return -EINVAL;
> -       }
> +       ret = -EINVAL;
> +       uclass_foreach_dev_probe(UCLASS_ETH, dev)
> +               ret = 0;

There is a uclass_probe_all() function.

My suggestion from the original series was to just do the dhcp once.

How does probing the ethernet devices actually help EFI? It is not
needed for bootstd, since it probes any devices it uses.

>
> -       return 0;
> +       return ret;
>  }
>
>  struct bootdev_ops eth_bootdev_ops = {
> --
> 2.45.2
>

Regards,
Simon