Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
Simon Goldschmidt
goldsimon at gmx.de
Wed Oct 4 22:14:40 CEST 2023
Am 4. Oktober 2023 10:29:54 MESZ schrieb Maxim Uvarov <maxim.uvarov at linaro.org>:
>On Wed, 4 Oct 2023 at 08:12, Simon Glass <sjg at google.com> wrote:
>
>> 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
>>
>
>For this time I enabled only ipv4 in lwip U-boot implementation and planned
>to keep the original ping6 dhcp6 tftp6 on first submission. After the first
>ipv4 lwip patches can be merged I planned to work on ipv6 support. I think
>it will require more time for the test environment to be set up, then
>actually enabling ipv6 in the configuration header. lwIP can work in dual
>stack mode. But I'm not sure that example applications support ipv6 at this
>time.
All lwIP example apps building on UDP or TCP should support IPv6 just fine. Ping is of course a special case in that it uses a different protocol for IPv6 and IPv4 and no one has bothered yet to port it to IPv6. That's the reason it remained in contrib/apps instead of src/apps.
Aside from DNS, we do not have built in a fallback mechanism from IPv6 to IPv4 or the other way around, so that would have to be coded by the application (U-Boot command in this case).
Regards,
Simon
More information about the U-Boot
mailing list