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

[Heinrich Schuchardt]

On 26.11.24 16:38, Simon Glass wrote:
> Hi Heinrich,
> 
> On Tue, 26 Nov 2024 at 00:42, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>> 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.
> 
> Here is the code:
> 
> int dhcp_run(ulong addr, const char *fname, bool autoload)
> {
> char *dhcp_argv[] = {"dhcp", NULL, (char *)fname, NULL};
> struct cmd_tbl cmdtp = {}; /* dummy */
> char file_addr[17];
> int old_autoload;
> int ret, result;
> 
> log_debug("addr=%lx, fname=%s, autoload=%d\n", addr, fname, autoload);
> old_autoload = env_get_yesno("autoload");
> ret = env_set("autoload", autoload ? "y" : "n");
> if (ret)
> return log_msg_ret("en1", -EINVAL);
> 
> It respects the autoload parameter. If there is a bug in that, we
> should fix it, but it seems to work OK to me. Please fix your commit
> message.

The cited code only sets "autoload". It does not adjust "autostart".

As I have shown below this leads to trying to boot via DHCP.

This is exactly what the commit message says: "load a file and boot it 
if autostart=yes".

> 
>>
>> In all cases dhcp_run() wastes boot time.
> 
> Yes, it uses time. With bootstd the hunters are only called as needed.
> In your case you are calling all of them at once.
> 
> Is there a way to just call hunters which relate to block devices, perhaps?

We need to probe network to provide the simple network protocol 
(lib/efi_loader/efi_net.c) but we don't need to execute DHCP.

We also need the correct device-tree provided by the extension hunter.

Best regards

Heinrich

> 
>>
>> 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
>> );
> 
> See above
> 
> [..]
> 
> Regards,
> Simon