Hi Sean,
On Tue, 3 Oct 2023 at 11:58, Sean Edmond <seanedm...@linux.microsoft.com> wrote:
>
>
> On 2023-09-26 2:41 a.m., Maxim Uvarov wrote:
> > Replace original commands: ping, tftp, dhcp and wget.
> >
> > Signed-off-by: Maxim Uvarov<maxim.uva...@linaro.org>
> > ---
> > boot/bootmeth_efi.c | 18 +++++++---
> > boot/bootmeth_pxe.c | 21 ++++++-----
> > cmd/net.c | 86 +++++----------------------------------------
> > cmd/pxe.c | 19 +++++-----
> > include/net.h | 8 +++--
> > include/net/ulwip.h | 64 +++++++++++++++++++++++++++++++++
> > 6 files changed, 113 insertions(+), 103 deletions(-)
> > create mode 100644 include/net/ulwip.h
> >
> > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > index ae936c8daa..52399d627c 100644
> > --- a/boot/bootmeth_efi.c
> > +++ b/boot/bootmeth_efi.c
> > @@ -20,6 +20,8 @@
> > #include <mapmem.h>
> > #include <mmc.h>
> > #include <net.h>
> > +#include <net/lwip.h>
> > +#include <net/ulwip.h>
> > #include <pxe_utils.h>
> > #include <linux/sizes.h>
> >
> > @@ -319,9 +321,7 @@ static int distro_efi_try_bootflow_files(struct udevice
> > *dev,
> >
> > static int distro_efi_read_bootflow_net(struct bootflow *bflow)
> > {
> > - char file_addr[17], fname[256];
> > - char *tftp_argv[] = {"tftp", file_addr, fname, NULL};
> > - struct cmd_tbl cmdtp = {}; /* dummy */
> > + char fname[256];
> > const char *addr_str, *fdt_addr_str;
> > int ret, arch, size;
> > ulong addr, fdt_addr;
> > @@ -368,7 +368,6 @@ static int distro_efi_read_bootflow_net(struct bootflow
> > *bflow)
> > if (!fdt_addr_str)
> > return log_msg_ret("fdt", -EINVAL);
> > fdt_addr = hextoul(fdt_addr_str, NULL);
> > - sprintf(file_addr, "%lx", fdt_addr);
> >
> > /* We only allow the first prefix with PXE */
> > ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
> > @@ -379,7 +378,16 @@ static int distro_efi_read_bootflow_net(struct
> > bootflow *bflow)
> > if (!bflow->fdt_fname)
> > return log_msg_ret("fil", -ENOMEM);
> >
> > - if (!do_tftpb(&cmdtp, 0, 3, tftp_argv)) {
> > + ret = ulwip_init();
> > + if (ret)
> > + return log_msg_ret("ulwip_init", ret);
> > +
> > + ret = ulwip_tftp(fdt_addr, fname);
> > + if (ret)
> > + return log_msg_ret("ulwip_tftp", ret);
> > +
> > + ret = ulwip_loop();
> > + if (!ret) {
> > bflow->fdt_size = env_get_hex("filesize", 0);
> > bflow->fdt_addr = fdt_addr;
> > } else {
> > diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
> > index 8d489a11aa..fc6aabaa18 100644
> > --- a/boot/bootmeth_pxe.c
> > +++ b/boot/bootmeth_pxe.c
> > @@ -21,6 +21,8 @@
> > #include <mapmem.h>
> > #include <mmc.h>
> > #include <net.h>
> > +#include <net/ulwip.h>
> > +#include <net/lwip.h>
> > #include <pxe_utils.h>
> >
> > static int extlinux_pxe_getfile(struct pxe_context *ctx, const char
> > *file_path,
> > @@ -116,18 +118,21 @@ static int extlinux_pxe_read_file(struct udevice
> > *dev, struct bootflow *bflow,
> > const char *file_path, ulong addr,
> > ulong *sizep)
> > {
> > - char *tftp_argv[] = {"tftp", NULL, NULL, NULL};
> > - struct pxe_context *ctx = dev_get_priv(dev);
> > - char file_addr[17];
> > ulong size;
> > int ret;
> >
> > - sprintf(file_addr, "%lx", addr);
> > - tftp_argv[1] = file_addr;
> > - tftp_argv[2] = (void *)file_path;
> > + ret = ulwip_init();
> > + if (ret)
> > + return log_msg_ret("ulwip_init", ret);
> > +
> > + ret = ulwip_tftp(addr, file_path);
> > + if (ret)
> > + return log_msg_ret("ulwip_tftp", ret);
> > +
> > + ret = ulwip_loop();
> > + if (ret)
> > + return log_msg_ret("ulwip_loop", ret);
> >
> > - if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv))
> > - return -ENOENT;
> > ret = pxe_get_file_size(&size);
> > if (ret)
> > return log_msg_ret("tftp", ret);
> > diff --git a/cmd/net.c b/cmd/net.c
> > index d407d8320a..dc5a114309 100644
> > --- a/cmd/net.c
> > +++ b/cmd/net.c
> > @@ -22,6 +22,7 @@
> > #include <net/udp.h>
> > #include <net/sntp.h>
> > #include <net/ncsi.h>
> > +#include <net/lwip.h>
> >
> > static int netboot_common(enum proto_t, struct cmd_tbl *, int, char *
> > const []);
> >
> > @@ -40,19 +41,9 @@ U_BOOT_CMD(
> > #endif
> >
> > #ifdef CONFIG_CMD_TFTPBOOT
> > -int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > -{
> > - int ret;
> > -
> > - bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
> > - ret = netboot_common(TFTPGET, cmdtp, argc, argv);
> > - bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
> > - return ret;
> > -}
> > -
> > #if IS_ENABLED(CONFIG_IPV6)
> > U_BOOT_CMD(
> > - tftpboot, 4, 1, do_tftpb,
> > + tftpboot, 4, 1, do_lwip_tftp,
>
> It looks like LWIP doesn't support TFTP with IPv6 addressing. Perhaps
> we need to fall back onto the existing TFTP implementation until LWIP
> supports it?
>
> Note, that currently, IPv6 TFTP is enabled using the "-ipv6" argument.
> The intention is that netboot_common() sees the argument and sets the
> "use_ip6" variable. It looks like the new implementation in
> do_lwip_tftp() doesn't re-use the argument parsing in netboot_common()
> and that it doesn't handle the addition of the "-ipv6" flag.
>
> I support the addition of LWIP, but I'm concerned about how abrupt
> changes like this one will be for existing users. The underlying stack
> will change, with no easy way for the user to revert to the previous
> stack. Has there been any discussion about preserving the existing
> functionality, with an option to enable/disable LWIP stack?This would
> give the community time to transition/validate LWIP before deprecating
> the old u-boot networking stack.
IPv6 is news to me...and surprising.
Regards,
Simon
