Re: [PATCH 09/15] net-lwip: add support for EFI_HTTP_BOOT

On 5/22/24 20:19, Ilias Apalodimas wrote:

Hi Jerome,

On Wed, 22 May 2024 at 19:04, Jerome Forissier jerome.forissier@linaro.org wrote:

...

Implement the wget_with_dns() function which is needed by CONFIG_EFI_HTTP_BOOT=y. Note that there is no dependency added on CONFIG_CMD_DNS_LWIP because CONFIG_CMD_WGET_LWIP natively supports hostname resolution.

Signed-off-by: Jerome Forissier jerome.forissier@linaro.org

lib/efi_loader/Kconfig | 5 +++-- net-lwip/wget.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 430bb7f0f7..9d8dc8c756 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -505,8 +505,9 @@ config EFI_RISCV_BOOT_PROTOCOL

config EFI_HTTP_BOOT bool "EFI HTTP Boot support"

•   select CMD_DNS
  

•   select CMD_WGET
  

•   select CMD_DNS if NET
  

•   select CMD_WGET if NET
  

•   select CMD_WGET_LWIP if NET_LWIP
    select BLKMAP
    help
      Enabling this option adds EFI HTTP Boot support. It allows to
  

diff --git a/net-lwip/wget.c b/net-lwip/wget.c index 0ed8b77c47..a3e65dc80e 100644 --- a/net-lwip/wget.c +++ b/net-lwip/wget.c @@ -166,3 +166,19 @@ int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])

    return CMD_RET_FAILURE;

}

+#if defined(CONFIG_EFI_HTTP_BOOT) +int wget_with_dns(ulong dst_addr, char *uri) +{

•   char addr_str[11];
  

•   struct cmd_tbl cmdtp = {};
  

•   char *argv[] = { "wget", addr_str, uri };
  

•   snprintf(addr_str, sizeof(addr_str), "0x%lx", dst_addr);
  

•   if (do_wget(&cmdtp, 0, ARRAY_SIZE(argv), argv) == CMD_RET_SUCCESS)
  

•           return 0;
  

Calling a command directly feels a bit weird. Can we do this instead:

• Split do_wget in patch #7 and carve out 2 functions. The first will

prepare the arguments and the second will call httpc_get_file_dns() and run the loop

• Once that's split you can use the newly split function here and

avoid calling a command

Addressed in next version. I have implemented wget_with_dns() in patch #7 to that #9 doesn't need to add any code. And do_wget() simply calls wget_with_dns().

Thanks.