Add an API for binding the client UDP PCB before a transfer. This lets callers select a source port while retaining lwIP's automatic ephemeral-port selection when they do not bind one.
The lwIP TFTP command uses a fixed first-response watchdog and ignores the standard source-port, destination-port, timeout and retry-count environment variables. The watchdog also duplicates retransmission handling in the TFTP client. Configure the client from tftpsrcp, tftpdstp, tftptimeout and tftptimeoutcountmax. Match the legacy CONFIG_NET_RETRY_COUNT * 2 default. Make NET_TFTP_VARS available to either network stack and reject timeouts above lwIP's wrap-safe timer limit. Keep only the receive server's initial listen timer, using the legacy five-second interval and CONFIG_NET_RETRY_COUNT * 2 retries for both listening and accepted transfers. Do not apply client timeout environment overrides to the server. Its separate inactivity watchdog otherwise aborts before the TFTP engine has exhausted its retries. Exercise a receive command that completes after retransmitting ACK 0. Recognize a server prefix only when it is an IP address or lwIP's empty placeholder. Preserve other colons in filenames while retaining the numeric destination-port extension. Retain the allocation base used by the parser so it can be freed. Signed-off-by: James Hilliard <[email protected]> --- cmd/Kconfig | 21 +- lib/lwip/lwip/src/apps/tftp/tftp.c | 15 ++ lib/lwip/lwip/src/include/lwip/apps/tftp_client.h | 1 + net/lwip/tftp.c | 233 ++++++++++++++-------- test/dm/lwip_tftp.c | 62 +++++- 5 files changed, 236 insertions(+), 96 deletions(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 0c184df8288..06764fa76ca 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2155,17 +2155,6 @@ config CMD_TFTPPUT help TFTP put command, for uploading files to a server -config NET_TFTP_VARS - bool "Control TFTP timeout and count through environment" - depends on CMD_TFTPBOOT - default y - help - If set, allows controlling the TFTP timeout through the - environment variable tftptimeout, and the TFTP maximum - timeout count through the variable tftptimeoutcountmax. - If unset, timeout and maximum are hard-defined as 1 second - and 10 timouts per TFTP transfer. - config CMD_RARP bool "rarpboot" help @@ -2226,6 +2215,16 @@ config CMD_WOL endif # if NET_LEGACY +config NET_TFTP_VARS + bool "Control TFTP transfer options through environment" + depends on CMD_TFTPBOOT + default y + help + Allow tftpblocksize, tftpwindowsize, tftptimeout and + tftptimeoutcountmax to override the TFTP client's compiled-in + block size, receive window, timeout and retry-count defaults. + The receive server uses its compiled-in timeout and retry count. + config CMD_DHCP bool "dhcp" select PROT_DHCP_LWIP if NET_LWIP diff --git a/lib/lwip/lwip/src/apps/tftp/tftp.c b/lib/lwip/lwip/src/apps/tftp/tftp.c index 6a8c846cc8a..e3453fa9bfa 100644 --- a/lib/lwip/lwip/src/apps/tftp/tftp.c +++ b/lib/lwip/lwip/src/apps/tftp/tftp.c @@ -933,6 +933,21 @@ tftp_set_timeout(u32_t timeout_msecs, u32_t max_retries) return ERR_OK; } +/** @ingroup tftp + * Bind the TFTP client to a local UDP port. + * @param port Local UDP port + * @return lwIP error code returned by udp_bind() + */ +err_t +tftp_client_bind(u16_t port) +{ + LWIP_ERROR("TFTP client is not enabled (tftp_init)", + (tftp_state.tftp_mode & LWIP_TFTP_MODE_CLIENT) != 0, + return ERR_VAL); + + return udp_bind(tftp_state.upcb, IP_ANY_TYPE, port); +} + /** @ingroup tftp * Deinitialize ("turn off") TFTP client/server. */ diff --git a/lib/lwip/lwip/src/include/lwip/apps/tftp_client.h b/lib/lwip/lwip/src/include/lwip/apps/tftp_client.h index 17fdbb47e4f..6a5019c079b 100644 --- a/lib/lwip/lwip/src/include/lwip/apps/tftp_client.h +++ b/lib/lwip/lwip/src/include/lwip/apps/tftp_client.h @@ -44,6 +44,7 @@ enum tftp_transfer_mode { }; err_t tftp_init_client(const struct tftp_context* ctx); +err_t tftp_client_bind(u16_t port); void tftp_client_set_blksize(u16_t blksize); void tftp_client_set_tsize(u32_t tsize); u32_t tftp_client_get_tsize(void); diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c index a88073865c1..61a3521c576 100644 --- a/net/lwip/tftp.c +++ b/net/lwip/tftp.c @@ -18,10 +18,8 @@ #include <time.h> #define PROGRESS_PRINT_STEP_BYTES (10 * 1024) -/* Max time to wait for first data packet from server */ -#define NO_RSP_TIMEOUT_MS 10000 -/* Max time to wait for an incoming TFTP write request */ -#define TFTPSRV_LISTEN_TIMEOUT_MS 50000 +#define UBOOT_TFTP_TIMEOUT_MS 5000 +#define UBOOT_TFTP_MAX_RETRIES (CONFIG_NET_RETRY_COUNT * 2) enum done_state { NOT_DONE = 0, @@ -39,6 +37,7 @@ struct tftp_ctx { enum done_state done; bool is_server; bool wrq_accepted; + u32 listen_retries; char fname[TFTP_MAX_FILENAME_LEN + 1]; }; @@ -48,20 +47,6 @@ struct tftp_ctx { */ static struct tftp_ctx *tftpsrv_active_ctx; -static void transfer_timeout(void *arg) -{ - struct tftp_ctx *ctx = (struct tftp_ctx *)arg; - - printf("Timeout!\n"); - ctx->done = FAILURE; -} - -static void restart_transfer_timeout(struct tftp_ctx *ctx) -{ - sys_untimeout(transfer_timeout, ctx); - sys_timeout(TFTP_TIMEOUT_MSECS, transfer_timeout, ctx); -} - /** * store_block() - copy received data * @@ -129,8 +114,6 @@ static void *tftp_open(const char *fname, const char *mode, u8_t is_write) ctx->wrq_accepted = true; ctx->start_time = get_timer(0); snprintf(ctx->fname, sizeof(ctx->fname), "%s", fname); - restart_transfer_timeout(ctx); - printf("\nReceiving '%s' mode '%s'\n", fname, mode); puts("Loading: "); @@ -143,8 +126,6 @@ static void tftp_close(void *handle) ulong tftp_tsize; ulong elapsed; - sys_untimeout(transfer_timeout, ctx); - if (ctx->done == FAILURE || ctx->done == ABORTED) { /* Closing after an error or Ctrl-C */ return; @@ -193,8 +174,6 @@ static int tftp_write(void *handle, struct pbuf *p) } } - restart_transfer_timeout(ctx); - return 0; } @@ -218,23 +197,15 @@ static const struct tftp_context tftp_context = { tftp_error }; -static void no_response(void *arg) -{ - struct tftp_ctx *ctx = (struct tftp_ctx *)arg; - - if (ctx->size) - return; - - printf("Timeout!\n"); - ctx->done = FAILURE; -} - static int tftp_loop(struct net_lwip_ctx *net, ulong addr, char *fname, - ip_addr_t srvip, uint16_t srvport) + ip_addr_t srvip, u16 srvport, u16 srcport, + u32 timeout_msecs, u32 max_retries) { int blksize = CONFIG_TFTP_BLOCKSIZE; struct tftp_ctx ctx; const char *ep; + bool initialized = false; + int ret = -1; err_t err; if (!fname || addr == 0) @@ -243,14 +214,9 @@ static int tftp_loop(struct net_lwip_ctx *net, ulong addr, char *fname, if (!srvport) srvport = TFTP_PORT; + memset(&ctx, 0, sizeof(ctx)); ctx.done = NOT_DONE; - ctx.size = 0; - ctx.block_count = 0; - ctx.hash_count = 0; ctx.daddr = addr; - ctx.is_server = false; - ctx.wrq_accepted = false; - ctx.fname[0] = '\0'; printf("Using %s device\n", net->dev->name); printf("TFTP from server %s; our IP address is %s\n", @@ -264,8 +230,23 @@ static int tftp_loop(struct net_lwip_ctx *net, ulong addr, char *fname, log_err("tftp_init_client err: %d\n", err); return -1; } + initialized = true; - ep = env_get("tftpblocksize"); + if (srcport) { + err = tftp_client_bind(srcport); + if (err != ERR_OK) { + log_err("tftp_client_bind err: %d\n", err); + goto out_cleanup; + } + } + + err = tftp_set_timeout(timeout_msecs, max_retries); + if (err != ERR_OK) { + log_err("tftp_set_timeout err: %d\n", err); + goto out_cleanup; + } + + ep = IS_ENABLED(CONFIG_NET_TFTP_VARS) ? env_get("tftpblocksize") : NULL; if (ep) blksize = simple_strtol(ep, NULL, 10); tftp_client_set_blksize(blksize); @@ -275,11 +256,9 @@ static int tftp_loop(struct net_lwip_ctx *net, ulong addr, char *fname, /* might return different errors, like routing problems */ if (err != ERR_OK) { printf("tftp_get() error %d\n", err); - tftp_cleanup(); - return -1; + goto out_cleanup; } - sys_timeout(NO_RSP_TIMEOUT_MS, no_response, &ctx); while (!ctx.done) { net_lwip_poll(); if (ctrlc()) { @@ -288,9 +267,10 @@ static int tftp_loop(struct net_lwip_ctx *net, ulong addr, char *fname, break; } } - sys_untimeout(no_response, (void *)&ctx); - tftp_cleanup(); +out_cleanup: + if (initialized) + tftp_cleanup(); if (ctx.done == SUCCESS) { if (env_set_hex("fileaddr", addr)) { @@ -299,10 +279,10 @@ static int tftp_loop(struct net_lwip_ctx *net, ulong addr, char *fname, } efi_set_bootdev("Net", "", fname, map_sysmem(addr, 0), ctx.size); - return 0; + ret = 0; } - return -1; + return ret; } static void no_request(void *arg) @@ -312,6 +292,12 @@ static void no_request(void *arg) if (ctx->wrq_accepted) return; + if (ctx->listen_retries < UBOOT_TFTP_MAX_RETRIES) { + ctx->listen_retries++; + sys_timeout(UBOOT_TFTP_TIMEOUT_MS, no_request, ctx); + return; + } + printf("Timeout!\n"); ctx->done = FAILURE; } @@ -348,8 +334,13 @@ static int tftpsrv_loop(struct net_lwip_ctx *net, ulong addr) goto out; } + /* Legacy tftpsrv uses compiled defaults, not client env overrides. */ + err = tftp_set_timeout(UBOOT_TFTP_TIMEOUT_MS, UBOOT_TFTP_MAX_RETRIES); + if (err != ERR_OK) + goto out_cleanup; + ctx.start_time = get_timer(0); - sys_timeout(TFTPSRV_LISTEN_TIMEOUT_MS, no_request, &ctx); + sys_timeout(UBOOT_TFTP_TIMEOUT_MS, no_request, &ctx); while (!ctx.done) { net_lwip_poll(); if (ctrlc()) { @@ -358,8 +349,8 @@ static int tftpsrv_loop(struct net_lwip_ctx *net, ulong addr) break; } } +out_cleanup: sys_untimeout(no_request, &ctx); - sys_untimeout(transfer_timeout, &ctx); tftp_cleanup(); @@ -432,18 +423,22 @@ out: int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct net_lwip_ctx net = {}; + u32 timeout_msecs = UBOOT_TFTP_TIMEOUT_MS; + u32 max_retries = UBOOT_TFTP_MAX_RETRIES; int ret = CMD_RET_SUCCESS; char *arg = NULL; - char *words[3] = { }; + char *arg_copy = NULL; char *fname = NULL; char *server_ip = NULL; char *server_port = NULL; char *end; ip_addr_t srvip; + u16 srcport = 0; u16 port = TFTP_PORT; ulong laddr; ulong addr; - int i; + const char *ep; + long value; laddr = env_get_ulong("loadaddr", 16, image_load_addr); @@ -463,12 +458,22 @@ int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) laddr = addr; fname = env_get("bootfile"); } else { - arg = strdup(argv[1]); + arg_copy = strdup(argv[1]); + arg = arg_copy; + if (!arg) { + ret = CMD_RET_FAILURE; + goto out; + } } break; case 3: laddr = hextoul(argv[1], NULL); - arg = strdup(argv[2]); + arg_copy = strdup(argv[2]); + arg = arg_copy; + if (!arg) { + ret = CMD_RET_FAILURE; + goto out; + } break; default: ret = CMD_RET_USAGE; @@ -476,29 +481,39 @@ int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } if (!arg) - arg = net_boot_file_name; - - if (*arg) { - /* Parse [ip:[port:]]fname */ - i = 0; - while ((*(words + i) = strsep(&arg, ":"))) - i++; - - switch (i) { - case 3: - server_ip = words[0]; - server_port = words[1]; - fname = words[2]; - break; - case 2: - server_ip = words[0]; - fname = words[1]; - break; - case 1: - fname = words[0]; - break; - default: - break; + arg = fname; + if (arg && *arg) { + if (!arg_copy) { + arg_copy = strdup(arg); + if (!arg_copy) { + ret = CMD_RET_FAILURE; + goto out; + } + } + fname = arg_copy; + end = strchr(fname, ':'); + if (end) { + *end = '\0'; + /* Like legacy, only an IP address introduces a server. + * Retain the lwIP empty-server and numeric-port extensions, + * but leave other colons in the filename untouched. + */ + if (!*fname || ipaddr_aton(fname, &srvip)) { + server_ip = fname; + fname = end + 1; + end = strchr(fname, ':'); + if (end) { + *end = '\0'; + if (strspn(fname, "0123456789") == strlen(fname)) { + server_port = fname; + fname = end + 1; + } else { + *end = ':'; + } + } + } else { + *end = ':'; + } } } @@ -512,8 +527,58 @@ int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) goto out; } - if (server_port) - port = dectoul(server_port, NULL); + if (!server_port) + server_port = env_get("tftpdstp"); + if (server_port) { + value = simple_strtol(server_port, NULL, 10); + if (value < 0 || value > 0xffff) { + log_err("error: invalid TFTP destination port\n"); + ret = CMD_RET_FAILURE; + goto out; + } + port = value; + } + + ep = env_get("tftpsrcp"); + if (ep) { + value = simple_strtol(ep, NULL, 10); + if (value < 0 || value > 0xffff) { + log_err("error: invalid TFTP source port\n"); + ret = CMD_RET_FAILURE; + goto out; + } + srcport = value; + } + + if (IS_ENABLED(CONFIG_NET_TFTP_VARS)) { + ep = env_get("tftptimeout"); + if (ep && *ep) { + ulong timeout; + + if (strict_strtoul(ep, 10, &timeout) || timeout > U32_MAX / 4) { + log_err("error: invalid TFTP timeout\n"); + ret = CMD_RET_FAILURE; + goto out; + } + timeout_msecs = timeout; + } + if (timeout_msecs < 1000) { + printf("TFTP timeout (%u ms) too low, set min = 1000 ms\n", + timeout_msecs); + timeout_msecs = 1000; + } + + ep = env_get("tftptimeoutcountmax"); + if (ep) { + value = simple_strtol(ep, NULL, 10); + if (value < 0) { + printf("TFTP timeout count max (%ld) negative, set to 0\n", + value); + value = 0; + } + max_retries = value; + } + } if (!ipaddr_aton(server_ip, &srvip)) { log_err("error: ipaddr_aton\n"); @@ -538,13 +603,13 @@ int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) goto out; } - if (tftp_loop(&net, laddr, fname, srvip, port) < 0) + if (tftp_loop(&net, laddr, fname, srvip, port, srcport, + timeout_msecs, max_retries) < 0) ret = CMD_RET_FAILURE; else image_load_addr = laddr; out: net_lwip_stop(&net); - if (arg != net_boot_file_name) - free(arg); + free(arg_copy); return ret; } diff --git a/test/dm/lwip_tftp.c b/test/dm/lwip_tftp.c index 7abe4a47d87..4f303276a9b 100644 --- a/test/dm/lwip_tftp.c +++ b/test/dm/lwip_tftp.c @@ -1,9 +1,12 @@ // SPDX-License-Identifier: GPL-2.0+ /* Copyright 2026 James Hilliard <[email protected]> */ +#include <command.h> #include <dm.h> #include <env.h> +#include <image.h> #include <malloc.h> +#include <mapmem.h> #include <net.h> #include <time.h> #include <asm/unaligned.h> @@ -24,6 +27,7 @@ struct tftp_protocol_test { unsigned int closed; unsigned int written; unsigned int reads; + unsigned int server_phase; int error; int read_error; bool initialized; @@ -602,11 +606,61 @@ static int tftp_test_server_retries(struct unit_test_state *uts, return 0; } +#if IS_ENABLED(CONFIG_CMD_TFTPSRV) +static void tftp_test_server_input(void *arg) +{ + static const char wrq[] = "\000\002file\000octet"; + static const u8 data[] = { 0, 3, 0, 1, 'a', 'b', 'c' }; + struct tftp_protocol_test *test = arg; + + if (!test->server_phase++) { + test->port = 69; + test->error = tftp_test_input(test, wrq, sizeof(wrq), 0); + /* Lose DATA 1 long enough for the server to retransmit ACK 0. */ + timer_test_add_offset(5001); + sys_timeout(1, tftp_test_server_input, test); + } else { + test->error = tftp_test_input(test, data, sizeof(data), 0); + } +} + +static int tftp_test_server_command(struct unit_test_state *uts, + struct tftp_protocol_test *test) +{ + int ret; + + if (test->initialized) + tftp_cleanup(); + memset(test, 0, sizeof(*test)); + ipaddr_aton("1.1.2.2", &test->server); + ut_assertok(env_set("autostart", "yes")); + sys_timeout(1, tftp_test_server_input, test); + ret = run_command("tftpsrv 2000000", 0); + sys_untimeout(tftp_test_server_input, test); + ut_assertok(ret); + ut_assertok(test->error); + ut_asserteq(2, test->server_phase); + ut_asserteq(3, test->sent); + ut_asserteq(4, test->opcode); + ut_asserteq(1, test->block); + ut_asserteq(0x2000000, image_load_addr); + ut_asserteq_str("3", env_get("filesize")); + ut_asserteq_str("2000000", env_get("fileaddr")); + ut_asserteq_str("yes", env_get("autostart")); + ut_asserteq_mem("abc", map_sysmem(0x2000000, 3), 3); + return 0; +} +#endif + static int dm_test_lwip_tftp_protocol(struct unit_test_state *uts) { - static const char * const vars[] = { "ethact", "ipaddr", "netmask" }; + static const char * const vars[] = { + "ethact", "ipaddr", "netmask", "fileaddr", "filesize", "autostart", + }; struct tftp_protocol_test test = {}; struct net_lwip_ctx net = {}; + ulong load_addr = image_load_addr; + u32 size = net_boot_file_size; char *saved[ARRAY_SIZE(vars)] = {}; netif_output_fn output; int i, ret = -ENOMEM; @@ -644,12 +698,18 @@ static int dm_test_lwip_tftp_protocol(struct unit_test_state *uts) ret = tftp_test_timeout_option(uts, &test); if (!ret) ret = tftp_test_server_retries(uts, &test); +#if IS_ENABLED(CONFIG_CMD_TFTPSRV) + if (!ret) + ret = tftp_test_server_command(uts, &test); +#endif if (test.initialized) tftp_cleanup(); net.netif->output = output; tftp_test_active = NULL; net_lwip_stop(&net); restore_env: + image_load_addr = load_addr; + net_boot_file_size = size; for (i = 0; i < ARRAY_SIZE(vars); i++) if (env_set(vars[i], saved[i])) ret = -EINVAL; -- 2.53.0
