On 5/28/24 15:39, Maxim Uvarov wrote:
> пт, 24 мая 2024 г. в 19:22, Jerome Forissier <jerome.foriss...@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.foriss...@linaro.org>
>> Co-developed-by: Maxim Uvarov <muva...@gmail.com>
>> Cc: Maxim Uvarov <muva...@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, w...@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
