Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
Am 4. Oktober 2023 10:29:54 MESZ schrieb Maxim Uvarov :
>On Wed, 4 Oct 2023 at 08:12, Simon Glass wrote:
>
>> Hi Sean,
>>
>> On Tue, 3 Oct 2023 at 11:58, Sean Edmond
>> 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
>> > > ---
>> > > 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
>> > > #include
>> > > #include
>> > > +#include
>> > > +#include
>> > > #include
>> > > #include
>> > >
>> > > @@ -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
>> > > #include
>> > > #include
>> > > +#include
>> > > +#include
>> > > #include
>> > >
>> > > 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
>> > > #include
>> > > #include
>> > > +#include
>> > >
>> > > 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");
>>
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On Wed, 4 Oct 2023 at 08:12, Simon Glass wrote:
> Hi Sean,
>
> On Tue, 3 Oct 2023 at 11:58, Sean Edmond
> 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
> > > ---
> > > 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
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > #include
> > > #include
> > >
> > > @@ -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
> > > #include
> > > #include
> > > +#include
> > > +#include
> > > #include
> > >
> > > 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
> > > #include
> > > #include
> > > +#include
> > >
> > > 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_
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
Hi Sean,
On Tue, 3 Oct 2023 at 11:58, Sean Edmond 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
> > ---
> > 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
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> >
> > @@ -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
> > #include
> > #include
> > +#include
> > +#include
> > #include
> >
> > 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
> > #include
> > #include
> > +#include
> >
> > 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,
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On 2023-10-03 2:58 p.m., Peter Robinson wrote:
On Tue, Oct 3, 2023 at 6:58 PM Sean Edmond
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
---
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
#include
#include
+#include
+#include
#include
#include
@@ -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
#include
#include
+#include
+#include
#include
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
#include
#include
+#include
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?
Is it that LWIP upstream doesn't support IPv6 with TFTP or it just
hasn't been dealt with in this patch set? If the former might be
useful to reference details.
Apologies for the misleading comment, LWIP does support IPv6 addressing,
it just hasn't been dealt with in this patch.
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.
Is there a reason why there's a need of an explicit argument for IPv6?
I would have thought if there was a local IPv6 address assigned that
you would automatically try IPv6 and if it fails for back to v4, or
even better do a DNS lookup and use what ever gets returned. Having to
know what to use by m
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On Tue, Oct 3, 2023 at 6:58 PM Sean Edmond
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
> ---
> 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
> #include
> #include
> +#include
> +#include
> #include
> #include
>
> @@ -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
> #include
> #include
> +#include
> +#include
> #include
>
> 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
> #include
> #include
> +#include
>
> 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?
Is it that LWIP upstream doesn't support IPv6 with TFTP or it just
hasn't been dealt with in this patch set? If the former might be
useful to reference details.
> 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.
Is there a reason why there's a need of an explicit argument for IPv6?
I would have thought if there was a local IPv6 address assigned that
you would automatically try IPv6 and if it fails for back to v4, or
even better do a DNS l
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On 2023-09-26 2:41 a.m., Maxim Uvarov wrote:
Replace original commands: ping, tftp, dhcp and wget.
Signed-off-by: Maxim Uvarov
---
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
#include
#include
+#include
+#include
#include
#include
@@ -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
#include
#include
+#include
+#include
#include
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
#include
#include
+#include
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
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On Tue, 26 Sept 2023 at 03:46, Maxim Uvarov wrote: > > Replace original commands: ping, tftp, dhcp and wget. > > Signed-off-by: Maxim Uvarov > --- > 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 Reviewed-by: Simon Glass I don't really understand the error handling. I'm not sure why we have void functions everywhere?
