[PATCH v2 08/14] net-lwip: add wget command

Jerome Forissier jerome.forissier at linaro.org
Thu Jun 6 11:56:43 CEST 2024



On 5/28/24 15:39, Maxim Uvarov wrote:
> пт, 24 мая 2024 г. в 19:22, Jerome Forissier <jerome.forissier at linaro.org>:
>>
>> Add support for the wget command with NET_LWIP.
>>
>> About the small change in cmd/efidebug.c: when the wget command based
>> on the lwIP stack is used the wget command has a built-in URL
>> validation function since it needs to parse it anyways (in parse_url()).
>> Therefore wget_validate_uri() doesn't exist. So, guard the call in
>> efidebug.c with CONFIG_CMD_WGET.
>>
>> Based on code initially developed by Maxim U.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>> Co-developed-by: Maxim Uvarov <muvarov at gmail.com>
>> Cc: Maxim Uvarov <muvarov at gmail.com>
>> ---
>>  cmd/Kconfig        |   7 ++
>>  cmd/Makefile       |   5 +-
>>  cmd/efidebug.c     |   8 +-
>>  cmd/net-common.c   | 112 ++++++++++++++++++++++++++++
>>  cmd/net-lwip.c     |  12 +++
>>  cmd/net.c          | 115 -----------------------------
>>  include/net-lwip.h |  51 +++++++++++++
>>  net-lwip/Makefile  |   1 +
>>  net-lwip/wget.c    | 180 +++++++++++++++++++++++++++++++++++++++++++++
>>  9 files changed, 372 insertions(+), 119 deletions(-)
>>  create mode 100644 cmd/net-common.c
>>  create mode 100644 net-lwip/wget.c
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 6ef0b52cd34..d9a86540be6 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -2117,6 +2117,13 @@ config CMD_TFTPBOOT
>>         help
>>           tftpboot - load file via network using TFTP protocol
>>
>> +config CMD_WGET
>> +       bool "wget"
>> +       select PROT_TCP_LWIP
>> +       help
>> +         wget is a simple command to download kernel, or other files,
>> +         from a http server over TCP.
>> +
>>  endif
>>
>>  endif
>> diff --git a/cmd/Makefile b/cmd/Makefile
>> index 535b6838ca5..e90f2f68211 100644
>> --- a/cmd/Makefile
>> +++ b/cmd/Makefile
>> @@ -129,8 +129,11 @@ obj-$(CONFIG_CMD_MUX) += mux.o
>>  obj-$(CONFIG_CMD_NAND) += nand.o
>>  obj-$(CONFIG_CMD_NET) += net.o
>>  obj-$(CONFIG_CMD_NET_LWIP) += net-lwip.o
>> +obj-$(filter y,$(CONFIG_CMD_NET) $(CONFIG_CMD_NET_LWIP)) += net-common.o
>>  ifdef CONFIG_CMD_NET_LWIP
>> -CFLAGS_net-lwip.o := -I$(srctree)/lib/lwip/lwip/src/include -I$(srctree)/lib/lwip/u-boot
>> +lwip-includes := -I$(srctree)/lib/lwip/lwip/src/include -I$(srctree)/lib/lwip/u-boot
>> +CFLAGS_net-lwip.o := $(lwip-includes)
>> +CFLAGS_net-common.o := $(lwip-includes)
>>  endif
>>  obj-$(CONFIG_ENV_SUPPORT) += nvedit.o
>>  obj-$(CONFIG_CMD_NVEDIT_EFI) += nvedit_efi.o
>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>> index c2c525f2351..d80e91ecadd 100644
>> --- a/cmd/efidebug.c
>> +++ b/cmd/efidebug.c
>> @@ -741,9 +741,11 @@ static int efi_boot_add_uri(int argc, char *const argv[], u16 *var_name16,
>>         if (!label)
>>                 return CMD_RET_FAILURE;
>>
>> -       if (!wget_validate_uri(argv[3])) {
>> -               printf("ERROR: invalid URI\n");
>> -               return CMD_RET_FAILURE;
>> +       if (IS_ENABLED(CONFIG_CMD_WGET)) {
>> +               if (!wget_validate_uri(argv[3])) {
>> +                       printf("ERROR: invalid URI\n");
>> +                       return CMD_RET_FAILURE;
>> +               }
>>         }
>>
>>         efi_create_indexed_name(var_name16, var_name16_size, "Boot", id);
>> diff --git a/cmd/net-common.c b/cmd/net-common.c
>> new file mode 100644
>> index 00000000000..b5dfd2c8866
>> --- /dev/null
>> +++ b/cmd/net-common.c
>> @@ -0,0 +1,112 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2000
>> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>> + */
>> +
>> +#include <command.h>
>> +#include <dm/device.h>
>> +#include <dm/uclass.h>
>> +#ifdef CONFIG_NET
>> +#include <net.h>
>> +#elif defined CONFIG_NET_LWIP
>> +#include <net-lwip.h>
>> +#else
>> +#error Either NET or NET_LWIP must be enabled
>> +#endif
>> +#include <linux/compat.h>
>> +#include <linux/ethtool.h>
>> +
>> +static int do_net_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>> +{
>> +       const struct udevice *current = eth_get_dev();
>> +       unsigned char env_enetaddr[ARP_HLEN];
>> +       const struct udevice *dev;
>> +       struct uclass *uc;
>> +
>> +       uclass_id_foreach_dev(UCLASS_ETH, dev, uc) {
>> +               eth_env_get_enetaddr_by_index("eth", dev_seq(dev), env_enetaddr);
>> +               printf("eth%d : %s %pM %s\n", dev_seq(dev), dev->name, env_enetaddr,
>> +                      current == dev ? "active" : "");
>> +       }
>> +       return CMD_RET_SUCCESS;
>> +}
>> +
>> +static int do_net_stats(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>> +{
>> +       int nstats, err, i, off;
>> +       struct udevice *dev;
>> +       u64 *values;
>> +       u8 *strings;
>> +
>> +       if (argc < 2)
>> +               return CMD_RET_USAGE;
>> +
>> +       err = uclass_get_device_by_name(UCLASS_ETH, argv[1], &dev);
>> +       if (err) {
>> +               printf("Could not find device %s\n", argv[1]);
>> +               return CMD_RET_FAILURE;
>> +       }
>> +
>> +       if (!eth_get_ops(dev)->get_sset_count ||
>> +           !eth_get_ops(dev)->get_strings ||
>> +           !eth_get_ops(dev)->get_stats) {
>> +               printf("Driver does not implement stats dump!\n");
>> +               return CMD_RET_FAILURE;
>> +       }
>> +
>> +       nstats = eth_get_ops(dev)->get_sset_count(dev);
>> +       strings = kcalloc(nstats, ETH_GSTRING_LEN, GFP_KERNEL);
>> +       if (!strings)
>> +               return CMD_RET_FAILURE;
>> +
>> +       values = kcalloc(nstats, sizeof(u64), GFP_KERNEL);
>> +       if (!values)
>> +               goto err_free_strings;
>> +
>> +       eth_get_ops(dev)->get_strings(dev, strings);
>> +       eth_get_ops(dev)->get_stats(dev, values);
>> +
>> +       off = 0;
>> +       for (i = 0; i < nstats; i++) {
>> +               printf("  %s: %llu\n", &strings[off], values[i]);
>> +               off += ETH_GSTRING_LEN;
>> +       };
>> +
> 
> It looks like here you missed kfree().

Good catch! This is already missing in master (cmd/net.c). I will fix
in v3 and also send a patch for master.

>> +       return CMD_RET_SUCCESS;
>> +
>> +err_free_strings:
>> +       kfree(strings);
>> +
>> +       return CMD_RET_FAILURE;
>> +}
>> +

[...]

>> diff --git a/net-lwip/wget.c b/net-lwip/wget.c
>> new file mode 100644
>> index 00000000000..25b75040806
>> --- /dev/null
>> +++ b/net-lwip/wget.c
>> @@ -0,0 +1,180 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/* Copyright (C) 2024 Linaro Ltd. */
>> +
>> +#include <command.h>
>> +#include <console.h>
>> +#include <display_options.h>
>> +#include <image.h>
>> +#include <lwip/apps/http_client.h>
>> +#include <lwip/timeouts.h>
>> +#include <net-lwip.h>
>> +#include <time.h>
>> +

[...]
>> +
>> +static void httpc_result_cb(void *arg, httpc_result_t httpc_result,
>> +                           u32_t rx_content_len, u32_t srv_res, err_t err)
>> +{
>> +       ulong elapsed;
>> +
>> +       if (httpc_result != HTTPC_RESULT_OK) {
>> +               log_err("\nHTTP client error %d\n", httpc_result);
>> +               done = FAILURE;
>> +               return;
>> +       }
>> +
>> +       elapsed = get_timer(start_time);
>> +        log_info("\n%u bytes transferred in %lu ms (", rx_content_len,
> 
> This print I commented for another patch to match current python tests.

What do you mean? Is this text incorrect because some tests expect something
else? Which ones?
 
>> +                 get_timer(start_time));
>> +        print_size(rx_content_len / elapsed * 1000, "/s)\n");
>> +
>> +       if (env_set_hex("filesize", rx_content_len) ||
>> +           env_set_hex("fileaddr", saved_daddr)) {
>> +               log_err("Could not set filesize or fileaddr\n");
>> +               done = FAILURE;
>> +               return;
>> +       }
>> +
>> +       done = SUCCESS;
>> +}
>> +
>> +int wget_with_dns(ulong dst_addr, char *uri)
> 
> static?

No, this function is called from lib/efi_loader/efi_bootmgr.c.

>> +{
>> +       char server_name[SERVER_NAME_SIZE];
>> +       httpc_connection_t conn;
>> +       httpc_state_t *state;
>> +       char *path;
>> +       u16 port;
>> +
>> +       daddr = dst_addr;
>> +       saved_daddr = dst_addr;
>> +       done = NOT_DONE;
>> +       size = 0;
>> +       prevsize = 0;
>> +
>> +       if (parse_url(uri, server_name, &port, &path))
>> +               return CMD_RET_USAGE;
>> +
>> +       memset(&conn, 0, sizeof(conn));
>> +       conn.result_fn = httpc_result_cb;
>> +       start_time = get_timer(0);
>> +       if (httpc_get_file_dns(server_name, port, path, &conn, httpc_recv_cb,
>> +                              NULL, &state))
>> +               return CMD_RET_FAILURE;
>> +
>> +       while (!done) {
>> +               eth_rx();
>> +               sys_check_timeouts();
>> +               if (ctrlc())
>> +                       break;
>> +       }
>> +
>> +       if (done == SUCCESS)
>> +               return 0;
>> +
>> +       return -1;
>> +}
>> +
>> +int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> +       char *url;
>> +       ulong dst_addr;
>> +
>> +       if (argc < 2 || argc > 3)
>> +               return CMD_RET_USAGE;
>> +
>> +       dst_addr = image_load_addr;
>> +
>> +       if (!strncmp(argv[1], "0x", 2)) {
> 
> As I remember u-boot can eat hex and decimal numbers for arguments for
> most of the commands.

Right. I'll use hextoul() properly in v3.

>> +               if (argc < 3)
>> +                       return CMD_RET_USAGE;
>> +               dst_addr = hextoul(argv[1], NULL);
>> +               url = argv[2];
>> +       } else {
>> +               url = argv[1];
>> +       }
>> +
>> +       if (wget_with_dns(dst_addr, url))
>> +               return CMD_RET_FAILURE;
>> +
>> +       return CMD_RET_SUCCESS;
>> +}
>> --
>> 2.40.1

Thanks,
-- 
Jerome


More information about the U-Boot mailing list