[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.schuchardt at 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.schuchardt at 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
>>