[PATCHv10 14/15] net/lwip: replace original net commands with lwip

[Simon Glass]

Hi Sean,

On Tue, 3 Oct 2023 at 11:58, Sean Edmond <seanedmond at 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.uvarov at 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