Re: [PATCH v2 4/6] net: eth_bootdev_hunt() must not try to boot

[Heinrich Schuchardt]

On 11/26/24 01:32, Simon Glass wrote:
Hi Heinrich,
On Sat, 23 Nov 2024 at 14:46, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:

eth_bootdev_hunt() should not execute dhcp_run() as this itself would load
a file and boot it if autostart=yes.

Instead just check that there is a network device.

If you look at dhcp_run() you will see an 'autoload' parameter. This
is 'false' in this code, so it never loads a file.

This is not the complete truth.

If $autostart=true and CONFIG_NET=y, dhcp_run(false) still tries to load 
a file.

In all cases dhcp_run() wastes boot time.

CONFIG_NET=y
=> dhcp_run
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 10.0.2.15 (1004 ms)

=> setenv autostart true
=> dhcp run
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 10.0.2.15 (1002 ms)
Using e1000#0 device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename 'run'.
Load address: 0x40200000
Loading: *
TFTP error: 'File not found' (1)
Not retrying...

CONFIG_NET_LWIP=y
=> dhcp_run
DHCP client bound to address 10.0.2.15 (1007 ms)

Where command dhcp_run is provided by:

#include <command.h>
#include <env.h>
#include <net-common.h>
#include <vsprintf.h>

static int do_dhcp_run(struct cmd_tbl *cmdtp, int flag, int argc,
                  char *const argv[])
{
        ulong addr = hextoul(env_get("loadaddr"), NULL);

        dhcp_run(addr, NULL, false);

        return 0;
}

U_BOOT_CMD(
        dhcp_run,       CONFIG_SYS_MAXARGS,     1,      do_dhcp_run,
        "dhcp_run",
        NULL
);

Best regards

Heinrich


Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
v2:
         new patch
---
  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.
-        */
-       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;

-       return 0;
+       return ret;
  }

  struct bootdev_ops eth_bootdev_ops = {
--
2.45.2