On 12.09.2024 03:59, Simon Glass wrote:

Hi Mikhail,

On Mon, 9 Sept 2024 at 16:27, Mikhail Kshevetskiy mikhail.kshevetskiy@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@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@chromium.org

Can/should I add this "Reviewed-by" to the next patch version?

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)%C2%A0 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.

...

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?

...

+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@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?

...

#include "arp.h" #include "bootp.h" #include "cdp.h" @@ -564,6 +565,14 @@ restart: wget_start(); break; #endif +#if defined(CONFIG_CMD_NETCAT)

•           case NETCAT_LOAD:
  

•                   netcat_load_start();
  

•                   break;
  

•           case NETCAT_SAVE:
  

•                   netcat_save_start();
  

•                   break;
  

+#endif #if defined(CONFIG_CMD_CDP) case CDP: cdp_start(); diff --git a/net/netcat.c b/net/netcat.c new file mode 100644 index 00000000000..633afe5c60b --- /dev/null +++ b/net/netcat.c @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • netcat support driver
• • Copyright (C) 2024 IOPSYS Software Solutions AB
• • Author: Mikhail Kshevetskiy mikhail.kshevetskiy@iopsys.eu
• */

+#include <command.h> +#include <display_options.h> +#include <env.h> +#include <image.h> +#include <mapmem.h> +#include <net.h> +#include <net/tcp.h> +#include <net/netcat.h>

+#define HASHES_PER_LINE 65

+static struct in_addr server_ip; +static u16 server_port; +static u16 local_port; +static int listen; +static int reading; +static unsigned int packets; +static enum net_loop_state netcat_loop_state;

+static void show_block_marker(void) +{

•   if ((packets % 10) == 0)
  

•           putc('#');
  

•   else if (((packets + 1) % (10 * HASHES_PER_LINE)) == 0)
  

else if (!((packets + 1) % (10 * HASHS_PER_LINE)))

but it would be nice if you could use 0x10 instead of 10 and masks here to avoid division. Or perhaps it doesn't matter.

...

•           puts("\n");
  

+}

+static void tcp_stream_on_closed(struct tcp_stream *tcp) +{

•   if (tcp->status != TCP_ERR_OK)
  

•           netcat_loop_state = NETLOOP_FAIL;
  

•   /*
  

•    * The status line will be shown after the download/upload
  

•    * progress (see show_block_marker() function). This progress
  

•    * generally have no final "\n", so jump to new line before
  

•    * output the status
  

•    */
  

•   if (netcat_loop_state != NETLOOP_SUCCESS)
  

•           printf("\nnetcat: Transfer Fail, TCP status - %d\n", tcp->status);
  

•   else
  

•           printf("\nPackets %s %d, Transfer Successful\n",
  

•                  reading ? "received" : "transmitted", packets);
  

•   net_set_state(netcat_loop_state);
  

+}

+static void tcp_stream_on_rcv_nxt_update(struct tcp_stream *tcp, u32 rx_bytes) +{

•   net_boot_file_size = rx_bytes;
  

•   show_block_marker();
  

+}

+static void tcp_stream_on_snd_una_update(struct tcp_stream *tcp, u32 tx_bytes) +{

•   show_block_marker();
  

•   if (tx_bytes == image_save_size)
  

•           tcp_stream_close(tcp);
  

+}

+static void tcp_stream_on_established(struct tcp_stream *tcp) +{

•   netcat_loop_state = NETLOOP_SUCCESS;
  

+}

+static u32 tcp_stream_rx(struct tcp_stream *tcp, u32 rx_offs, void *buf, u32 len) +{

•   void *ptr;
  

•   packets++;
  

•   ptr = map_sysmem(image_load_addr + rx_offs, len);
  

•   memcpy(ptr, buf, len);
  

•   unmap_sysmem(ptr);
  

blank line before final return. Please fix globally

...

•   return len;
  

+}

+static u32 tcp_stream_tx(struct tcp_stream *tcp, u32 tx_offs, void *buf, u32 maxlen) +{

•   void *ptr;
  

•   if (tx_offs + maxlen > image_save_size)
  

•           maxlen = image_save_size - tx_offs;
  

•   if (maxlen == 0)
  

•           return 0;
  

•   packets++;
  

•   ptr = map_sysmem(image_save_addr + tx_offs, maxlen);
  

•   memcpy(buf, ptr, maxlen);
  

•   unmap_sysmem(ptr);
  

•   return maxlen;
  

+}

+/*

• • This function will be called on new connections (incoming or outgoing)
• • It MUST:
• • -- setup required tcp_stream callbacks (if connection will be accepted)
• • -- return a connection verdict:

• •    0: discard connection
    

• •    1: accept connection
    

• */

+static int tcp_stream_on_create(struct tcp_stream *tcp) +{

•   if (listen) {
  

•           /*
  

•            * We are listening for incoming connections.
  

•            * Accept connections to netcat listen port only.
  

•            */
  

•           if (tcp->lport != local_port)
  

•                   return 0;
  

•   } else {
  

•           /*
  

•            * We are connecting to remote host.
  

•            * Check remote IP:port to make sure that this is our connection
  

•            */
  

•           if ((tcp->rhost.s_addr != server_ip.s_addr) ||
  

•               (tcp->rport != server_port))
  

•                   return 0;
  

•   }
  

•   netcat_loop_state = NETLOOP_FAIL;
  

•   net_boot_file_size = 0;
  

•   packets = 0;
  

•   tcp->on_closed = tcp_stream_on_closed;
  

•   tcp->on_established = tcp_stream_on_established;
  

•   if (reading) {
  

•           tcp->on_rcv_nxt_update = tcp_stream_on_rcv_nxt_update;
  

•           tcp->rx = tcp_stream_rx;
  

•   } else {
  

•           tcp->on_snd_una_update = tcp_stream_on_snd_una_update;
  

•           tcp->tx = tcp_stream_tx;
  

•   }
  

•   return 1;
  

+}

+static void netcat_start(void) +{

•   struct tcp_stream *tcp;
  

•   memset(net_server_ethaddr, 0, 6);
  

•   tcp_stream_set_on_create_handler(tcp_stream_on_create);
  

•   if (strchr(net_boot_file_name, ':') == NULL) {
  

!strchr

Please don't compare against 0 or NULL

...

•           listen = 1;
  

•           printf("Listening on port %s...\n", net_boot_file_name);
  

•           local_port = dectoul(net_boot_file_name, NULL);
  

•   } else {
  

•           listen = 0;
  

•           printf("Connecting to %s...\n", net_boot_file_name);
  

•           server_ip = string_to_ip(net_boot_file_name);
  

•           server_port = dectoul(strchr(net_boot_file_name, ':') + 1, NULL);
  

•           tcp = tcp_stream_connect(server_ip, server_port);
  

•           if (tcp == NULL) {
  

!tcp

...

•                   printf("No free tcp streams\n");
  

•                   net_set_state(NETLOOP_FAIL);
  

•                   return;
  

•           }
  

•           tcp_stream_put(tcp);
  

•   }
  

+}

+void netcat_load_start(void) +{

•   reading = 1;
  

•   return netcat_start();
  

+}

+void netcat_save_start(void) +{

•   reading = 0;
  

•   return netcat_start();
  

+}

2.45.2

Regards, Simon