Retire DHCP/BOOTP before TFTP borrows its runtime for one download attempt. Allow the address-configuration owner to refresh the interface from the environment after publishing the acquired configuration.
A retryable download failure must repeat address acquisition and obtain fresh boot configuration on the selected interface. Share one network retry budget between acquisition and download, and preserve BOOTP_MAY_FAIL's scope as an address-acquisition policy. Perform autostart only after the parent's protocol callbacks, runtime attachment and option data have been released. Keep internal helpers download-only and refuse to restart another client's shared runtime. Exercise refreshed leases, servers and filenames, interface rotation, shared retry budgets and autostart cleanup in sandbox. Signed-off-by: James Hilliard <[email protected]> --- include/net-lwip.h | 1 + net/lwip/dhcp.c | 52 ++++++++++++++++++++---------- net/lwip/net-lwip.c | 17 +++++----- net/lwip/tftp.c | 48 +++++++++++++++++++++------- test/dm/lwip.c | 4 +++ test/dm/lwip_bootp.c | 24 ++++++++++++++ test/dm/lwip_dhcp.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 199 insertions(+), 37 deletions(-) diff --git a/include/net-lwip.h b/include/net-lwip.h index e67497bddec..8fd1c094dff 100644 --- a/include/net-lwip.h +++ b/include/net-lwip.h @@ -81,6 +81,7 @@ int net_lwip_restart(struct net_lwip_ctx *ctx); int net_lwip_refresh(struct net_lwip_ctx *ctx); int net_lwip_poll(void); int net_lwip_dns_resolve(char *name_or_ip, ip_addr_t *ip); +int net_lwip_tftp_download(struct net_lwip_ctx *net, int argc, char *const argv[]); /** * wget_validate_uri() - varidate the uri diff --git a/net/lwip/dhcp.c b/net/lwip/dhcp.c index 2e9cf08c60a..51664d77dfc 100644 --- a/net/lwip/dhcp.c +++ b/net/lwip/dhcp.c @@ -6,6 +6,7 @@ #include <efi_loader.h> #include <env.h> #include <hexdump.h> +#include <image.h> #include <log.h> #include <lwip-dhcp.h> #include <malloc.h> @@ -960,6 +961,8 @@ static int bootp_run(struct cmd_tbl *cmdtp, int flag, int argc, struct dhcp_options *options; const char *autoload; const char *filename = NULL; + bool downloaded = false; + bool acquisition_timeout; ulong addr; char *end; const char *hostname, *vendor; @@ -1029,25 +1032,46 @@ static int bootp_run(struct cmd_tbl *cmdtp, int flag, int argc, ret = bootp_loop(&net, !!filename, options); if (IS_ENABLED(CONFIG_CMD_DHCP) && !bootp) ret = dhcp_loop(&net, !!filename, options); - if (ret != -ETIMEDOUT) - break; + /* Retire the address protocol before handing its runtime to TFTP. */ if (IS_ENABLED(CONFIG_CMD_DHCP) && !bootp) dhcp_retire(&net); - if (IS_ENABLED(CONFIG_BOOTP_MAY_FAIL) && + acquisition_timeout = ret == -ETIMEDOUT; + if (!ret) { + autoload = env_get("autoload"); + if ((autoload && !strcmp(autoload, "NFS")) || + env_get_yesno("autoload") == 0) + break; + if (!IS_ENABLED(CONFIG_CMD_TFTPBOOT)) { + log_err("Cannot autoload with TFTP: command is disabled\n"); + ret = CMD_RET_FAILURE; + break; + } + ret = net_lwip_refresh(&net); + if (ret) + break; + if (IS_ENABLED(CONFIG_CMD_TFTPBOOT)) + ret = net_lwip_tftp_download(&net, argc, argv); + if (ret != -EAGAIN) { + downloaded = !ret; + break; + } + puts("Starting again\n\n"); + } else if (!acquisition_timeout) { + break; + } + /* BOOTP_MAY_FAIL limits address acquisition, not TFTP recovery. */ + if (acquisition_timeout && IS_ENABLED(CONFIG_BOOTP_MAY_FAIL) && (env_get_yesno("ethrotate") == 0 || net_restart_wrap)) break; /* Other runtime clients must not lose their interface on retry. */ ret = net_lwip_restart(&net); - if (ret || (IS_ENABLED(CONFIG_BOOTP_MAY_FAIL) && net_restart_wrap)) { + if (ret || (acquisition_timeout && IS_ENABLED(CONFIG_BOOTP_MAY_FAIL) && + net_restart_wrap)) { ret = CMD_RET_FAILURE; break; } } - /* - * A download needs strict addressing, incompatible with the - * address-less runtime attachment. Retire DHCP before handing off; - * the BOOTP loop has already removed its receive callback. - */ + /* No protocol callbacks may survive the shared runtime or option data. */ if (IS_ENABLED(CONFIG_CMD_DHCP) && !bootp) dhcp_retire(&net); active_options = NULL; @@ -1056,6 +1080,8 @@ static int bootp_run(struct cmd_tbl *cmdtp, int flag, int argc, if (ret) return CMD_RET_FAILURE; + if (downloaded) + return net_boot_file_size ? bootm_maybe_autostart(cmdtp, argv[0]) : 0; autoload = env_get("autoload"); if (autoload && !strcmp(autoload, "NFS")) { if (IS_ENABLED(CONFIG_CMD_NFS)) @@ -1063,13 +1089,7 @@ static int bootp_run(struct cmd_tbl *cmdtp, int flag, int argc, log_err("Cannot autoload with NFS: command is disabled\n"); return CMD_RET_FAILURE; } - if (env_get_yesno("autoload") == 0) - return CMD_RET_SUCCESS; - if (IS_ENABLED(CONFIG_CMD_TFTPBOOT)) - return do_tftpb(cmdtp, flag, argc, argv); - log_err("Cannot autoload with TFTP: command is disabled\n"); - - return CMD_RET_FAILURE; + return CMD_RET_SUCCESS; } int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) diff --git a/net/lwip/net-lwip.c b/net/lwip/net-lwip.c index df11322ec3b..60fcf9b3225 100644 --- a/net/lwip/net-lwip.c +++ b/net/lwip/net-lwip.c @@ -553,25 +553,26 @@ int net_lwip_restart(struct net_lwip_ctx *ctx) /** * net_lwip_refresh - Refresh environment addressing for the shared interface - * @ctx: Active environment-addressed client attachment + * @ctx: Active client attachment * - * Address-less clients take priority, so flexible-client refreshes are - * deferred until the last such client detaches. A refresh which would change - * the address while a strict client is attached fails with -EBUSY. + * The address configuration owner may refresh after publishing its acquired + * configuration and retiring DHCP/BOOTP, before starting an autoload transfer. + * Flexible-client refreshes are deferred while that owner remains attached. + * A refresh which would change the address while a strict client is attached + * fails with -EBUSY. * * Return: 0 on success, or a negative error code. */ int net_lwip_refresh(struct net_lwip_ctx *ctx) { if (!ctx || ctx->netif != net_lwip_runtime.netif || - ctx->dev != net_lwip_runtime.dev || - ctx->addr_mode == NET_LWIP_ADDR_NONE) + ctx->dev != net_lwip_runtime.dev || !net_lwip_runtime.users) return -EINVAL; - if (net_lwip_runtime.no_addr_users) + if (net_lwip_runtime.no_addr_users && ctx->addr_mode != NET_LWIP_ADDR_NONE) return 0; - return net_lwip_configure(ctx->addr_mode); + return net_lwip_configure(NET_LWIP_ADDR_ENV_FLEXIBLE); } /* diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c index 9e19857d19b..21c5e885a92 100644 --- a/net/lwip/tftp.c +++ b/net/lwip/tftp.c @@ -612,10 +612,11 @@ static void tftp_default_filename(struct net_lwip_ctx *net, char *fname, printf("*** Warning: no boot file name; using '%s'\n", fname); } -static int do_tftp(int argc, char *const argv[], enum tftp_operation operation) +static int do_tftp(int argc, char *const argv[], enum tftp_operation operation, + struct net_lwip_ctx *parent) { struct net_lwip_ctx local = {}; - struct net_lwip_ctx *net = &local; + struct net_lwip_ctx *net = parent ?: &local; u32 timeout_msecs = UBOOT_TFTP_TIMEOUT_MS; u32 max_retries = UBOOT_TFTP_MAX_RETRIES; int ret = CMD_RET_SUCCESS; @@ -785,12 +786,14 @@ static int do_tftp(int argc, char *const argv[], enum tftp_operation operation) goto out; } - if (net_lwip_start(net, NET_LWIP_ADDR_ENV_STRICT)) { - ret = CMD_RET_FAILURE; - goto out; + if (!parent) { + if (net_lwip_start(net, NET_LWIP_ADDR_ENV_STRICT)) { + ret = CMD_RET_FAILURE; + goto out; + } + net_try_count = 1; + net_restart_wrap = 0; } - net_try_count = 1; - net_restart_wrap = 0; for (;;) { ret = tftp_check_ip(net); @@ -802,7 +805,8 @@ static int do_tftp(int argc, char *const argv[], enum tftp_operation operation) ret = tftp_loop(net, operation, laddr, size, fname && *fname ? fname : default_fname, srvip, port, srcport, timeout_msecs, max_retries); - if (ret != -EAGAIN) + /* Autoload retries must reacquire the parent's boot configuration. */ + if (ret != -EAGAIN || parent) break; /* The failed transfer's PCB and timers are already gone. Never * change interfaces underneath another shared-runtime client. @@ -814,18 +818,38 @@ static int do_tftp(int argc, char *const argv[], enum tftp_operation operation) } if (!ret && operation == TFTP_DOWNLOAD) image_load_addr = laddr; - ret = ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS; + if (!parent) + ret = ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS; out: - net_lwip_stop(net); + if (!parent) + net_lwip_stop(net); free(arg_copy); return ret; } +/** + * net_lwip_tftp_download() - Attempt autoload using the caller's runtime + * @net: Active attachment with configured addressing and DHCP already retired + * @argc: Number of command arguments + * @argv: BOOTP/DHCP command arguments, using the tftpboot argument syntax + * + * Keep runtime and retry ownership with BOOTP/DHCP. All TFTP callbacks and + * timers are removed before returning. The caller handles autostart after + * releasing its own resources. + * + * Return: 0 on success, -EAGAIN for a retryable transfer failure, or another + * nonzero value for a terminal error. + */ +int net_lwip_tftp_download(struct net_lwip_ctx *net, int argc, char *const argv[]) +{ + return do_tftp(argc, argv, TFTP_DOWNLOAD, net); +} + int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { int ret; - ret = do_tftp(argc, argv, TFTP_DOWNLOAD); + ret = do_tftp(argc, argv, TFTP_DOWNLOAD, NULL); if (ret || !net_boot_file_size) return ret; /* The transfer's callbacks and runtime attachment are already gone. */ @@ -834,5 +858,5 @@ int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) int do_tftpput(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - return do_tftp(argc, argv, TFTP_UPLOAD); + return do_tftp(argc, argv, TFTP_UPLOAD, NULL); } diff --git a/test/dm/lwip.c b/test/dm/lwip.c index 8bb7e2bad9a..4dd4a562434 100644 --- a/test/dm/lwip.c +++ b/test/dm/lwip.c @@ -304,6 +304,10 @@ static int _dm_test_lwip_runtime(struct unit_test_state *uts, #if CONFIG_IS_ENABLED(PROT_DHCP_LWIP) ut_assertok(lwip_test_dhcp_stop(uts, no_addr_client->netif)); #endif + /* The configuration owner can publish its lease before autoload. */ + ut_assertok(net_lwip_refresh(no_addr_client)); + ip4addr_aton("1.1.2.4", &expected); + ut_assert(ip4_addr_cmp(netif_ip4_addr(client_b->netif), &expected)); net_lwip_stop(no_addr_client); ip4addr_aton("1.1.2.4", &expected); ut_assert(ip4_addr_cmp(netif_ip4_addr(client_b->netif), &expected)); diff --git a/test/dm/lwip_bootp.c b/test/dm/lwip_bootp.c index 3f29e2691d2..c6f5876c085 100644 --- a/test/dm/lwip_bootp.c +++ b/test/dm/lwip_bootp.c @@ -44,6 +44,7 @@ struct bootp_test { unsigned int requests; unsigned int bind_after; unsigned int tftp; + unsigned int transfer_failures; u32 tftp_server; u32 xid; u16 secs; @@ -268,6 +269,11 @@ static int bootp_tx(struct udevice *dev, void *packet, unsigned int len) data = (void *)(reply_ip + 1); put_unaligned_be16(test->transfer_fail ? 5 : 3, data); put_unaligned_be16(1, data + 2); + if (test->transfer_failures) { + put_unaligned_be16(5, data); + put_unaligned_be16(3, data + 2); + test->transfer_failures--; + } data[4] = 'x'; data[5] = 0; priv->recv_packet_length[priv->recv_packets++] = @@ -471,6 +477,24 @@ static int bootp_check(struct unit_test_state *uts, struct bootp_test *test) ut_asserteq(CMD_RET_FAILURE, do_bootp(NULL, 0, 3, argv)); ut_assert(!test->port_busy); ut_assertnull(netif_default); + if (IS_ENABLED(CONFIG_CMD_TFTPBOOT)) { + ut_assertok(env_set("netretry", "once")); + for (i = 0; i < 2; i++) { + *test = (struct bootp_test){ .transfer_failures = 1 }; + ut_assertok(env_set("ethact", "eth@10002000")); + ut_assertok(env_set("ethrotate", i ? NULL : "no")); + ut_assertok(env_set("ipaddr5", NULL)); + ut_assertok(do_bootp(NULL, 0, 3, argv)); + ut_asserteq(2, test->requests); + ut_asserteq(2, test->tftp); + ut_asserteq_str("explicit.bin", test->filename); + ut_asserteq_str(i ? "eth@10003000" : "eth@10002000", env_get("ethact")); + ut_assert(!test->port_busy); + ut_assertnull(netif_default); + } + ut_assertok(env_set("ethact", "eth@10002000")); + ut_assertok(env_set("netretry", "no")); + } /* * Plain and RFC 1048 replies without siaddr retain the configured server, * but must clear stale TFTP overrides before the download handoff. diff --git a/test/dm/lwip_dhcp.c b/test/dm/lwip_dhcp.c index 57e1ffdba10..a780c077a4f 100644 --- a/test/dm/lwip_dhcp.c +++ b/test/dm/lwip_dhcp.c @@ -23,12 +23,18 @@ struct dhcp_handoff_test { bool dhcp_alive; bool rx_burst; bool duplicate_offer; + bool refresh_offer; + unsigned int discovers; + unsigned int drop_acquisitions; + unsigned int tftp_failures; unsigned int arp_replies; unsigned int dhcp_replies; unsigned int tftp; unsigned int nfs; u32 boot_server; u32 nfs_server; + u32 tftp_server; + u32 tftp_local; char filename[128]; }; @@ -45,6 +51,12 @@ static int dhcp_handoff_reply(struct udevice *dev, struct dhcp_msg *request) if (type != DHCP_DISCOVER && type != DHCP_REQUEST) return 0; + if (type == DHCP_DISCOVER) + test->discovers++; + if (net_try_count <= test->drop_acquisitions) { + timer_test_add_offset(1000001); + return 0; + } if (test->timeout) { timer_test_add_offset(10001); return 0; @@ -69,6 +81,11 @@ static int dhcp_handoff_reply(struct udevice *dev, struct dhcp_msg *request) put_unaligned_be32(test->boot_server, &reply->siaddr); reply->cookie = htonl(DHCP_MAGIC_COOKIE); strlcpy((char *)reply->file, test->offer, sizeof(reply->file)); + if (test->refresh_offer && test->discovers > 1) { + put_unaligned_be32(0x01010205, &reply->yiaddr); + put_unaligned_be32(0x01010204, &reply->siaddr); + strcpy((char *)reply->file, "second.bin"); + } pos = reply->options; *pos++ = DHCP_OPTION_MESSAGE_TYPE; *pos++ = 1; @@ -166,6 +183,8 @@ static int dhcp_handoff_tx(struct udevice *dev, void *packet, unsigned int len) get_unaligned_be16(ip + 1) != 1) return 0; test->tftp++; + test->tftp_server = ntohl(ip->ip_dst.s_addr); + test->tftp_local = ntohl(ip->ip_src.s_addr); test->dhcp_alive |= !!netif_dhcp_data(netif_default); strlcpy(test->filename, (char *)(ip + 1) + 2, sizeof(test->filename)); if (priv->recv_packets >= PKTBUFSRX) @@ -189,6 +208,11 @@ static int dhcp_handoff_tx(struct udevice *dev, void *packet, unsigned int len) data = (void *)(reply_ip + 1); put_unaligned_be16(test->fail ? 5 : 3, data); put_unaligned_be16(1, data + 2); + if (test->tftp_failures) { + put_unaligned_be16(5, data); + put_unaligned_be16(3, data + 2); + test->tftp_failures--; + } data[4] = 'x'; data[5] = 0; priv->recv_packet_length[priv->recv_packets++] = @@ -197,6 +221,64 @@ static int dhcp_handoff_tx(struct udevice *dev, void *packet, unsigned int len) return 0; } +static int dhcp_autoload_retries(struct unit_test_state *uts, + struct dhcp_handoff_test *test) +{ + char * const argv[] = { "dhcp", "2000000", "explicit.bin" }; + struct net_lwip_ctx other = {}; + int i, ret; + + if (!IS_ENABLED(CONFIG_CMD_TFTPBOOT)) + return 0; + ut_assertok(env_set("autoload", "yes")); + ut_assertok(env_set("autostart", "no")); + ut_assertok(env_set("netretry", "once")); + for (i = 0; i < 3; i++) { + *test = (struct dhcp_handoff_test){ + .offer = "first.bin", .refresh_offer = true, .tftp_failures = 1, + }; + ut_assertok(env_set("ethact", "eth@10002000")); + ut_assertok(env_set("ethrotate", i == 2 ? NULL : "no")); + ut_assertok(env_set("ipaddr5", NULL)); + ut_assertok(env_set("serverip", "1.1.2.2")); + ut_assertok(do_dhcp(NULL, 0, i == 1 ? 3 : 1, argv)); + ut_asserteq(2, test->discovers); + ut_asserteq(2, test->tftp); + ut_asserteq(0x01010205, test->tftp_local); + ut_asserteq(IS_ENABLED(CONFIG_BOOTP_SERVERIP) || + IS_ENABLED(CONFIG_BOOTP_PREFER_SERVERIP) ? + 0x01010202 : 0x01010204, test->tftp_server); + ut_asserteq_str(i == 1 ? "explicit.bin" : "second.bin", test->filename); + ut_asserteq_str(i == 2 ? "eth@10003000" : "eth@10002000", env_get("ethact")); + ut_assert(!test->dhcp_alive); + ut_assertnull(netif_default); + } + /* An acquisition retry and a transfer retry share the same budget. */ + *test = (struct dhcp_handoff_test){ + .offer = "first.bin", .drop_acquisitions = 1, .tftp_failures = 1, + }; + ut_assertok(env_set("ethact", "eth@10002000")); + ut_asserteq(CMD_RET_FAILURE, do_dhcp(NULL, 0, 1, argv)); + ut_asserteq(1, test->tftp); + ut_asserteq(0, test->tftp_failures); + ut_assertnull(netif_default); + + /* An autoload failure must not restart another client's runtime. */ + *test = (struct dhcp_handoff_test){ .offer = "first.bin", .tftp_failures = 1 }; + ut_assertok(env_set("ethact", "eth@10002000")); + ut_assertok(net_lwip_start(&other, NET_LWIP_ADDR_ENV_FLEXIBLE)); + ret = do_dhcp(NULL, 0, 1, argv); + i = other.netif == netif_default && !netif_dhcp_data(other.netif); + net_lwip_stop(&other); + ut_asserteq(CMD_RET_FAILURE, ret); + ut_assert(i); + ut_asserteq(1, test->discovers); + ut_asserteq(1, test->tftp); + ut_assertnull(netif_default); + ut_assertok(env_set("netretry", "no")); + return 0; +} + static int dhcp_handoff_check(struct unit_test_state *uts, struct dhcp_handoff_test *test) { @@ -211,6 +293,7 @@ static int dhcp_handoff_check(struct unit_test_state *uts, ut_assertok(env_set("nfsserverip", NULL)); ut_assertok(env_set("tftpdstp", NULL)); ut_assertok(env_set("tftpsrcp", NULL)); + ut_assertok(dhcp_autoload_retries(uts, test)); for (i = 0; i < 4; i++) { *test = (struct dhcp_handoff_test){ .offer = "offered.bin" }; ut_assertok(env_set("autoload", i == 3 ? "no" : "yes")); @@ -334,7 +417,8 @@ static int dm_test_lwip_dhcp_handoff(struct unit_test_state *uts) "ethact", "ipaddr", "netmask", "gatewayip", "serverip", "tftpserverip", "nfsserverip", "autoload", "autostart", "bootfile", "loadaddr", "dnsip", "dnsip2", "ntpserverip", "filesize", "fileaddr", - "tftpdstp", "tftpsrcp", + "tftpdstp", "tftpsrcp", "netretry", "ethrotate", + "ipaddr5", "netmask5", "gatewayip5", }; struct dhcp_handoff_test test = {}; char bootfile[sizeof(net_boot_file_name)]; @@ -354,9 +438,13 @@ static int dm_test_lwip_dhcp_handoff(struct unit_test_state *uts) memcpy(bootfile, net_boot_file_name, sizeof(bootfile)); sandbox_eth_set_tx_handler(0, dhcp_handoff_tx); sandbox_eth_set_priv(0, &test); + sandbox_eth_set_tx_handler(1, dhcp_handoff_tx); + sandbox_eth_set_priv(1, &test); ret = dhcp_handoff_check(uts, &test); sandbox_eth_set_tx_handler(0, NULL); sandbox_eth_set_priv(0, NULL); + sandbox_eth_set_tx_handler(1, NULL); + sandbox_eth_set_priv(1, NULL); memcpy(net_boot_file_name, bootfile, sizeof(bootfile)); image_load_addr = load_addr; for (i = 0; i < ARRAY_SIZE(vars); i++) -- 2.53.0
