[PATCH v6 10/12] net/netcat: add netcat over tcp support

Simon Glass sjg at chromium.org
Thu Sep 19 16:10:15 CEST 2024


Hi Mikhail,

On Sun, 15 Sept 2024 at 09:06, Mikhail Kshevetskiy
<mikhail.kshevetskiy at genexis.eu> wrote:
>
>
> On 12.09.2024 03:59, Simon Glass wrote:
> > Hi Mikhail,
> >
> > On Mon, 9 Sept 2024 at 16:27, Mikhail Kshevetskiy
> > <mikhail.kshevetskiy at iopsys.eu> wrote:
> >> This patch adds downloading/uploading of data with netcat.
> >> Client/server mode both supported.
> >>
> >> How to test:
> >> ============
> >>   netcat-openbsd=1.219-1 from debian were used for a tests
> >>
> >>   a) Load data from remote host.
> >>        * U-Boot listen on tcp port 3456
> >>        * PC connects
> >>
> >>      u-boot: netcat load ${loadaddr} 3456
> >>      PC:     netcat -q1 ${UBOOT_IP} 3456 < image.itb
> >>
> >>   b) Load data from remote host.
> >>        * PC listen on tcp port 3456
> >>        * U-Boot connects
> >>
> >>      PC:     netcat -q1 -l -p 3456 < image.itb
> >>      u-boot: netcat load ${loadaddr} ${PC_IP}:3456
> >>
> >>   c) Save data to remote host
> >>        * U-Boot listen on tcp port 3456
> >>        * PC connects
> >>
> >>      u-boot: netcat save ${loadaddr} ${data_size_in_hex} 3456
> >>      PC:     netcat -w1 ${UBOOT_IP} 3456 >image.itb
> >>
> >>   d) Save data to remote host
> >>        * PC listen on tcp port 3456
> >>        * U-Boot connects
> >>
> >>      PC:     netcat -w1 -l -p 3456 >image.itb
> >>      u-boot: netcat save ${loadaddr} ${data_size_in_hex} ${PC_IP}:3456
> >>
> >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
> >> ---
> >>  cmd/Kconfig          |   7 ++
> >>  cmd/net.c            |  34 ++++++--
> >>  include/net.h        |   2 +-
> >>  include/net/netcat.h |  20 +++++
> >>  net/Makefile         |   1 +
> >>  net/net.c            |   9 +++
> >>  net/netcat.c         | 181 +++++++++++++++++++++++++++++++++++++++++++
> >>  7 files changed, 248 insertions(+), 6 deletions(-)
> >>  create mode 100644 include/net/netcat.h
> >>  create mode 100644 net/netcat.c
> > Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Can/should I add this "Reviewed-by" to the next patch version?

Yes

>
> >
> > nits below
> >
> >> diff --git a/cmd/Kconfig b/cmd/Kconfig
> >> index 978f44eda42..abcd003f7f1 100644
> >> --- a/cmd/Kconfig
> >> +++ b/cmd/Kconfig
> >> @@ -2007,6 +2007,13 @@ config CMD_WGET
> >>           wget is a simple command to download kernel, or other files,
> >>           from a http server over TCP.
> >>
> >> +config CMD_NETCAT
> >> +       bool "netcat"
> >> +       select PROT_TCP
> >> +       help
> >> +         netcat is a simple command to load/store kernel, or other files,
> >> +         using netcat like manner over TCP.
> > netcat-like
> >
> > Please add a reference to the protocol or the 'netcat' tool or both.
> > Help must be at least 3 lines.
>
> Netcat (https://en.wikipedia.org/wiki/Netcat)  uses plain TCP/UDP for
> data delivery. In the TCP case, TCP by itself provides reliable,
> ordered, and error-checked data delivery, so no additional protocols are
> required.
>
> Is it enough to write the following description?
>
> netcat is a simple command to load/store kernel, or other files,
> using well-known netcat (nc) utility. Unlike classic netcat utility
> this command supports TCP-based data transfer only, thus no data
> will be lost or reordered. Any netcat implementation should work,
> but openbsd one was tested only.

Yes, thanks

>
>
> >> +
> >>  config CMD_MII
> >>         bool "mii"
> >>         imply CMD_MDIO
> >> diff --git a/cmd/net.c b/cmd/net.c
> >> index 53ce2bc5d0c..364139ec5b9 100644
> >> --- a/cmd/net.c
> >> +++ b/cmd/net.c
> >> @@ -206,6 +206,28 @@ U_BOOT_CMD(
> >>  );
> >>  #endif
> >>
> >> +#if defined(CONFIG_CMD_NETCAT)
> >> +static int do_netcat(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> >> +{
> >> +       if (argc < 2)
> >> +               return 1;
> >> +
> >> +       if (!strcmp(argv[1], "load"))
> >> +               return netboot_common(NETCAT_LOAD, cmdtp, argc - 1, argv + 1);
> >> +       else if (!strcmp(argv[1], "save"))
> >> +               return netboot_common(NETCAT_SAVE, cmdtp, argc - 1, argv + 1);
> >> +       else
> >> +               return 1;
> >> +}
> >> +
> > Needs longhelp
> Could you explain? Usage examples?

I just mean something like this:

U_BOOT_LONGHELP(bootmeth,
        "list [-a]     - list available bootmeths (-a all)\n"
        "bootmeth order [<bd> ...]  - select bootmeth order / subset to use");

That example is from bootmeth.c

> >
> >> +U_BOOT_CMD(
> >> +       netcat,   5,      1,      do_netcat,
> >> +       "load/store data via tcp netcat utility",
> >> +       "load [loadAddress] [[hostIPaddr:]port]\n"
> >> +       "save Address Size [[hostIPaddr:]port]\n"
> >> +);
> >> +#endif
> >> +
> >>  static void netboot_update_env(void)
> >>  {
> >>         char tmp[46];
> >> @@ -323,16 +345,17 @@ static int parse_args(enum proto_t proto, int argc, char *const argv[])
> >>
> >>         switch (argc) {
> >>         case 1:
> >> -               if (IS_ENABLED(CONFIG_CMD_TFTPPUT) && proto == TFTPPUT)
> >> +               if ((IS_ENABLED(CONFIG_CMD_TFTPPUT) && proto == TFTPPUT) ||
> >> +                   (IS_ENABLED(CONFIG_CMD_NETCAT) && proto == NETCAT_SAVE))
> >>                         return 1;
> >> -
> >>                 /* refresh bootfile name from env */
> >>                 copy_filename(net_boot_file_name, env_get("bootfile"),
> >>                               sizeof(net_boot_file_name));
> >>                 break;
> >>
> >>         case 2:
> >> -               if (IS_ENABLED(CONFIG_CMD_TFTPPUT) && proto == TFTPPUT)
> >> +               if ((IS_ENABLED(CONFIG_CMD_TFTPPUT) && proto == TFTPPUT) ||
> >> +                   (IS_ENABLED(CONFIG_CMD_NETCAT) && proto == NETCAT_SAVE))
> >>                         return 1;
> >>                 /*
> >>                  * Only one arg - accept two forms:
> >> @@ -354,7 +377,8 @@ static int parse_args(enum proto_t proto, int argc, char *const argv[])
> >>                 break;
> >>
> >>         case 3:
> >> -               if (IS_ENABLED(CONFIG_CMD_TFTPPUT) && proto == TFTPPUT) {
> >> +               if ((IS_ENABLED(CONFIG_CMD_TFTPPUT) && proto == TFTPPUT) ||
> >> +                   (IS_ENABLED(CONFIG_CMD_NETCAT) && proto == NETCAT_SAVE)) {
> >>                         if (parse_addr_size(argv))
> >>                                 return 1;
> >>                 } else {
> >> @@ -365,7 +389,7 @@ static int parse_args(enum proto_t proto, int argc, char *const argv[])
> >>                 }
> >>                 break;
> >>
> >> -#ifdef CONFIG_CMD_TFTPPUT
> >> +#if defined(CONFIG_CMD_TFTPPUT) || defined(CONFIG_CMD_NETCAT)
> >>         case 4:
> >>                 if (parse_addr_size(argv))
> >>                         return 1;
> >> diff --git a/include/net.h b/include/net.h
> >> index b0ce13e0a9d..0af6493788a 100644
> >> --- a/include/net.h
> >> +++ b/include/net.h
> >> @@ -515,7 +515,7 @@ extern int          net_restart_wrap;       /* Tried all network devices */
> >>  enum proto_t {
> >>         BOOTP, RARP, ARP, TFTPGET, DHCP, DHCP6, PING, PING6, DNS, NFS, CDP,
> >>         NETCONS, SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT_UDP, FASTBOOT_TCP,
> >> -       WOL, UDP, NCSI, WGET, RS
> >> +       WOL, UDP, NCSI, WGET, NETCAT_LOAD, NETCAT_SAVE, RS
> >>  };
> >>
> >>  extern char    net_boot_file_name[1024];/* Boot File name */
> >> diff --git a/include/net/netcat.h b/include/net/netcat.h
> >> new file mode 100644
> >> index 00000000000..d8e09aaaa55
> >> --- /dev/null
> >> +++ b/include/net/netcat.h
> >> @@ -0,0 +1,20 @@
> >> +/* SPDX-License-Identifier: BSD-2-Clause
> >> + *
> >> + * netcat include file
> >> + * Copyright (C) 2024 IOPSYS Software Solutions AB
> >> + * Author: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
> >> + */
> >> +#ifndef __NET_NETCAT_TCP_H__
> >> +#define __NET_NETCAT_TCP_H__
> >> +
> >> +/**
> >> + * netcat_load_start() - begin netcat in loading mode
> >> + */
> >> +void netcat_load_start(void);
> >> +
> >> +/**
> >> + * netcat_save_start() - begin netcat in data saving mode
> >> + */
> >> +void netcat_save_start(void);
> >> +
> >> +#endif /* __NET_NETCAT_TCP_H__ */
> >> diff --git a/net/Makefile b/net/Makefile
> >> index 64ab7ec740a..dac7b4859fb 100644
> >> --- a/net/Makefile
> >> +++ b/net/Makefile
> >> @@ -33,6 +33,7 @@ obj-$(CONFIG_CMD_WOL)  += wol.o
> >>  obj-$(CONFIG_PROT_UDP) += udp.o
> >>  obj-$(CONFIG_PROT_TCP) += tcp.o
> >>  obj-$(CONFIG_CMD_WGET) += wget.o
> >> +obj-$(CONFIG_CMD_NETCAT) += netcat.o
> >>
> >>  # Disable this warning as it is triggered by:
> >>  # sprintf(buf, index ? "foo%d" : "foo", index)
> >> diff --git a/net/net.c b/net/net.c
> >> index 86182cc504e..d81604a57ce 100644
> >> --- a/net/net.c
> >> +++ b/net/net.c
> >> @@ -108,6 +108,7 @@
> >>  #include <test/test.h>
> >>  #include <net/tcp.h>
> >>  #include <net/wget.h>
> >> +#include <net/netcat.h>
> > That should be above net/tcp
>
> Is there any reasons?
>
> Should I move <net/wget.h> as well?

Oh, yes, net/xxx should be before test/xxx

See here: https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files

[..]

Regards,
Simon


More information about the U-Boot mailing list