[U-Boot,3/3] wget.c, wget.h and modified tftp.c.

From: Duncan Hare <DuncanCHare@yahoo.com>

Modified tftp.c can be used to verify integrity of wget download by
setting compile directive and performing a second file transfer with
tftp transfer.

1 waring from patman about maintainers. How should this be handled?

Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
---

 cmd/Kconfig |   6 +
 cmd/net.c   |  13 ++
 net/tftp.c  |  35 ++++-
 net/wget.c  | 436 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/wget.h  |  22 +++
 5 files changed, 506 insertions(+), 6 deletions(-)
 create mode 100644 net/wget.c
 create mode 100644 net/wget.h

On Sat, Feb 24, 2018 at 4:47 PM,  <DH@synoia.com> wrote:
> From: Duncan Hare <DuncanCHare@yahoo.com>
>
> Modified tftp.c can be used to verify integrity of wget download by
> setting compile directive and performing a second file transfer with
> tftp transfer.

This is a great log for a patch that only modifies the TFTP code, but
please remove the TFTP verification from the implementation of WGET.
Instead write details about the wget implementation. Maybe a usage
example, etc. Maybe some performance results.

>
> 1 waring from patman about maintainers. How should this be handled?

Don't worry about that one.

> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> ---
>
>  cmd/Kconfig |   6 +
>  cmd/net.c   |  13 ++
>  net/tftp.c  |  35 ++++-
>  net/wget.c  | 436 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/wget.h  |  22 +++

Please move this header to include/net/wget.h

>  5 files changed, 506 insertions(+), 6 deletions(-)
>  create mode 100644 net/wget.c
>  create mode 100644 net/wget.h
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 5a6afab99b..46b489a966 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1035,6 +1035,12 @@ config CMD_ETHSW
>           operations such as enabling / disabling a port and
>           viewing/maintaining the filtering database (FDB)
>
> +config CMD_WGET
> +       bool "wget"
> +       select TCP
> +       help
> +         Download a kernel, or other files, from a web server over TCP.
> +
>  endmenu
>
>  menu "Misc commands"
> diff --git a/cmd/net.c b/cmd/net.c
> index d7c776aacf..f5c2d0f8ed 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -110,6 +110,19 @@ U_BOOT_CMD(
>  );
>  #endif
>
> +#if defined(CONFIG_CMD_WGET)
> +static int do_wget(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       return netboot_common(WGET, cmdtp, argc, argv);
> +}
> +
> +U_BOOT_CMD(
> +       wget,   3,      1,      do_wget,
> +       "boot image via network using HTTP protocol",
> +       "[loadAddress] [[hostIPaddr:]bootfilename]"

Looks good!

> +);
> +#endif
> +
>  static void netboot_update_env(void)
>  {
>         char tmp[22];
> diff --git a/net/tftp.c b/net/tftp.c
> index 6671b1f7ca..c3f8fd3053 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -6,6 +6,9 @@
>   *                Luca Ceresoli <luca.ceresoli@comelit.it>
>   */
>
> +/* define if kernel load or verify kernal from a previous load */
> +#define K_LOAD 1

Please remove this symbol throughout.

> +
>  #include <common.h>
>  #include <command.h>
>  #include <efi_loader.h>
> @@ -16,6 +19,9 @@
>  #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP
>  #include <flash.h>
>  #endif
> +#ifndef K_LOAD
> +#include "tcp.h"
> +#endif
>
>  /* Well known TFTP port # */
>  #define WELL_KNOWN_PORT        69
> @@ -29,7 +35,6 @@
>  #endif
>  /* Number of "loading" hashes per line (for checking the image size) */
>  #define HASHES_PER_LINE        65
> -
>  /*
>   *     TFTP operations.
>   */
> @@ -166,7 +171,7 @@ static void mcast_cleanup(void)
>
>  static inline void store_block(int block, uchar *src, unsigned len)
>  {
> -       ulong offset = block * tftp_block_size + tftp_block_wrap_offset;
> +       uint offset = block * tftp_block_size + tftp_block_wrap_offset;
>         ulong newsize = offset + len;
>  #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP
>         int i, rc = 0;
> @@ -188,14 +193,29 @@ static inline void store_block(int block, uchar *src, unsigned len)
>                         net_set_state(NETLOOP_FAIL);
>                         return;
>                 }
> -       } else
> +       } else {
>  #endif /* CONFIG_SYS_DIRECT_FLASH_TFTP */
> -       {
> -               void *ptr = map_sysmem(load_addr + offset, len);
>

Why the blank line here? Please remove.

> +               void *ptr = map_sysmem(load_addr + offset, len);
> +#ifdef K_LOAD
>                 memcpy(ptr, src, len);

Always do this.

> +#endif
> +#ifndef K_LOAD
> +               if (memcmp(ptr, src, len) != 0) {
> +                       debug_cond(1,
> +                                  "File Xfer Mismatch, offset=%d, error=%d\n",
> +                                  offset, memcmp(ptr, src, len));
> +                       printf("TFTP Download Verification - Memory\n");
> +                       tcp_print_buffer(ptr, len, len, -1, 0);
> +                       printf("TFTP Download Verification - Packet\n");
> +                       tcp_print_buffer(src, len, len, -1, 0);
> +                       net_set_state(NETLOOP_FAIL);

Please drop this stuff. Instead, add an example in the documentation
that does the transfer two ways and compares the binary transferred
using "cmp.b". Or maybe figure a way to add it to the
test/py/tests/test_net.py and the .travis.yml.

> +               }
> +#endif
>                 unmap_sysmem(ptr);
> +#ifdef CONFIG_SYS_DIRECT_FLASH_TFTP
>         }
> +#endif
>  #ifdef CONFIG_MCAST_TFTP
>         if (tftp_mcast_active)
>                 ext2_set_bit(block, tftp_mcast_bitmap);
> @@ -761,6 +781,7 @@ void tftp_start(enum proto_t protocol)
>         }
>
>         printf("Using %s device\n", eth_get_name());
> +
>         printf("TFTP %s server %pI4; our IP address is %pI4",
>  #ifdef CONFIG_CMD_TFTPPUT
>                protocol == TFTPPUT ? "to" : "from",
> @@ -780,7 +801,9 @@ void tftp_start(enum proto_t protocol)
>                         printf("; sending through gateway %pI4", &net_gateway);
>         }
>         putc('\n');
> -
> +#ifndef K_LOAD
> +       printf("TFTP Download Verification\n");
> +#endif
>         printf("Filename '%s'.", tftp_filename);
>
>         if (net_boot_file_expected_size_in_blocks) {
> diff --git a/net/wget.c b/net/wget.c
> new file mode 100644
> index 0000000000..1a06f944f5
> --- /dev/null
> +++ b/net/wget.c
> @@ -0,0 +1,436 @@
> +/*
> + * FILE  support driver - based on etherboot and U-BOOT's tftp.c

Please correct this.

> + *
> + * Copyright Duncan Hare <dh@synoia.com> 2017
> + *
> + * SPDX-License-Identifier:<TAB>GPL-2.0+

Why "<TAB>"?

> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <mapmem.h>
> +#include <net.h>
> +#include "wget.h"
> +#include "tcp.h"
> +
> +char bootfile1[]          = "GET ";

const

> +char bootfile3[]          = " HTTP/1.0\r\n\r\n";

const

> +const char http_eom[]     = "\r\n\r\n";
> +const char http_ok[]      = "200";
> +const char content_len[]  = "Content-Length";
> +const char linefeed[]     = "\r\n";
> +static struct in_addr file_server_ip;
> +static int our_port;
> +static int file_timeout_count;
> +
> +struct pkt_qd {
> +       uchar *pkt;
> +       unsigned int tcp_seq_num;
> +       unsigned int len;
> +};
> +
> +struct pkt_qd pkt_q[PKTBUFSRX / 4];
> +int pkt_q_idx;
> +unsigned int content_length;
> +unsigned int packets;

These should either be static or they should be prefixed with wget_.

> +
> +static unsigned int initial_data_seq_num;
> +
> +static enum  FILE_STATE file_state;
> +
> +static char *file_filename;
> +static char *file_path;
> +static char  file_path_buff[2048];

It would be great if this could reuse the same buffer as other
protocols, but I guess it doesn't have to happen now.

> +static unsigned int file_timeout = FILE_TIMEOUT;
> +
> +static void  file_timeout_handler(void);
> +
> +enum net_loop_state loop_state;

Don't redefine this here. Instead use a different name
(wget_loop_state?) and make this variable static.

> +
> +/* Timeout retry parameters */
> +u8 r_action;
> +unsigned int r_tcp_ack_num;
> +unsigned int r_tcp_seq_num;
> +int r_len;

These should either be static or they should be prefixed with wget_.

> +
> +static inline int store_block(uchar *src, unsigned int offset, unsigned int len)
> +{
> +       ulong newsize = offset + len;
> +       uchar *ptr;
> +
> +#ifdef CONFIG_SYS_DIRECT_FLASH_WGET
> +       int i, rc = 0;
> +
> +       for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
> +               /* start address in flash? */
> +               if (load_addr + offset >= flash_info[i].start[0]) {
> +                       rc = 1;
> +                       break;
> +               }
> +       }
> +
> +       if (rc) { /* Flash is destination for this packet */
> +               rc = flash_write((uchar *)src,
> +                                (ulong)(load_addr + offset), len);
> +               if (rc) {
> +                       flash_perror(rc);
> +                       return -1;
> +               }
> +       } else {
> +#endif /* CONFIG_SYS_DIRECT_FLASH_NFS */

Incorrect comment about the endif.

> +
> +               ptr = map_sysmem(load_addr + offset, len);
> +               memcpy(ptr, src, len);
> +               unmap_sysmem(ptr);
> +
> +#ifdef CONFIG_SYS_DIRECT_FLASH_WGET
> +       }
> +#endif
> +       if (net_boot_file_size < (offset + len))
> +               net_boot_file_size = newsize;
> +       return 0;
> +}
> +
> +/*
> + * File request dispatcher

Blank line here would help (with ' *').

> + * WARNING, This, and only this, is the place in wget,c where
> + * SEQUENCE NUMBERS are swapped between incoming (RX)
> + * and outgoing (TX).
> + * Procedure file_handler() is correct for RX traffic.
> + */
> +static void file_send_stored(void)
> +{
> +       u8 action                  = r_action;
> +       unsigned int tcp_ack_num   = r_tcp_ack_num;
> +       unsigned int tcp_seq_num   = r_tcp_seq_num;
> +       int len                    = r_len;
> +       uchar *ptr;
> +       uchar *offset;
> +
> +       tcp_ack_num = tcp_ack_num + len;
> +
> +       switch (file_state) {
> +       case FILE_CLOSED:
> +               debug_cond(FILE_TEST, "File send: send SYN\n");
> +               file_state = FILE_CONNECTING;
> +               net_send_tcp_packet(0, SERVER_PORT, our_port, action,
> +                                   tcp_seq_num, tcp_ack_num);
> +               packets = 0;
> +       break;
> +       case FILE_CONNECTING:
> +               pkt_q_idx = 0;
> +               net_send_tcp_packet(0, SERVER_PORT, our_port, action,
> +                                   tcp_seq_num, tcp_ack_num);
> +
> +               ptr = net_tx_packet + net_eth_hdr_size()
> +                       + IP_TCP_HDR_SIZE + TCP_TSOPT_SIZE + 2;
> +               offset = ptr;
> +
> +               memcpy(offset, &bootfile1, strlen(bootfile1));
> +               offset = offset + strlen(bootfile1);
> +
> +               memcpy(offset, file_path, strlen(file_path));
> +               offset = offset + strlen(file_path);
> +
> +               memcpy(offset, &bootfile3, strlen(bootfile3));
> +               offset = offset + strlen(bootfile3);
> +               net_send_tcp_packet((offset - ptr), SERVER_PORT, our_port,
> +                                   TCP_PUSH, tcp_seq_num, tcp_ack_num);
> +               file_state = FILE_CONNECTED;
> +       break;
> +       case FILE_CONNECTED:
> +       case FILE_TRANSFERRING:
> +       case FILE_TRANSFERRED:
> +               net_send_tcp_packet(0, SERVER_PORT, our_port, action,
> +                                   tcp_seq_num, tcp_ack_num);
> +       break;
> +       }
> +}
> +
> +static void file_send(u8 action, unsigned int tcp_ack_num,
> +                     unsigned int tcp_seq_num, int len)

This is an awkward name for this function. You aren't sending a file.
Maybe if you replace file with wget in all these functions it will
help.

> +{
> +       r_action        = action;
> +       r_tcp_ack_num   = tcp_ack_num;
> +       r_tcp_seq_num   = tcp_seq_num;
> +       r_len           = len;
> +       file_send_stored();
> +}
> +
> +void file_fail(char *error_message, unsigned int tcp_seq_num,
> +              unsigned int tcp_ack_num, u8 action)
> +{
> +       printf("%s", error_message);
> +       printf("%s", "File Transfer Fail\n");
> +       net_set_timeout_handler(0, NULL);
> +       file_send(action, tcp_seq_num, tcp_ack_num, 0);
> +}
> +
> +void file_success(u8 action, unsigned int tcp_seq_num,
> +                 unsigned int tcp_ack_num, int len, int packets)
> +{
> +       printf("Packets received %d, File Send Success\n", packets);
> +       file_send(action, tcp_seq_num, tcp_ack_num, len);
> +}
> +
> +/*
> + * Interfaces of U-BOOT
> + */
> +static void file_timeout_handler(void)
> +{
> +       if (++file_timeout_count > FILE_RETRY_COUNT) {
> +               puts("\nRetry count exceeded; starting again\n");
> +               file_send(TCP_RST, 0, 0, 0);
> +               net_start_again();
> +       } else {
> +               puts("T ");
> +               net_set_timeout_handler(file_timeout +
> +                                       FILE_TIMEOUT * file_timeout_count,
> +                                       file_timeout_handler);
> +               file_send_stored();
> +       }
> +}
> +
> +static void file_connected(uchar *pkt, unsigned int tcp_seq_num,
> +                          struct in_addr action_and_state,
> +                          unsigned int tcp_ack_num, unsigned int len)
> +{
> +       u8 action = action_and_state.s_addr;
> +       uchar *pkt_in_q;
> +       char *pos;
> +       int  hlen;
> +       char *clen;
> +       int  i, j;
> +
> +       pkt[len] = '\0';
> +       pos = strstr((char *)pkt, http_eom);
> +
> +       if (pos == 0) {
> +               debug_cond(FILE_TEST,
> +                          "File Connected data before Header %p\n", pkt);
> +               pkt_in_q = (void *)load_addr + 0x20000 + (pkt_q_idx * 0x800);
> +               memcpy(pkt_in_q, pkt, len);
> +               pkt_q[pkt_q_idx].pkt              = pkt_in_q;
> +               pkt_q[pkt_q_idx].tcp_seq_num      = tcp_seq_num;
> +               pkt_q[pkt_q_idx].len              = len;
> +               pkt_q_idx++;
> +       } else {
> +               debug_cond(FILE_TEST, "File Connected HTTP Header %p\n", pkt);
> +               hlen = pos - (char *)pkt + sizeof(http_eom) - 1;
> +               pos  = strstr((char *)pkt, linefeed);
> +               if (pos > 0)
> +                       i = pos - (char *)pkt;
> +               else
> +                       i = hlen;
> +               tcp_print_buffer(pkt, i, i, -1, 0);
> +
> +               file_state = FILE_TRANSFERRING;
> +
> +               if (strstr((char *)pkt, http_ok) == 0) {
> +                       debug_cond(FILE_TEST,
> +                                  "File Connected Bad Xfer\n");
> +                       loop_state = NETLOOP_FAIL;
> +                       file_send(action, tcp_seq_num, tcp_ack_num, len);
> +               } else {
> +                       debug_cond(FILE_TEST, "File Connctd pkt %p  hlen %x\n",
> +                                  pkt, hlen);
> +                       initial_data_seq_num = tcp_seq_num + hlen;
> +
> +                       pos  = strstr((char *)pkt, content_len);
> +                       if (!pos) {
> +                               content_length = -1;
> +                       } else {
> +                               pos = pos + sizeof(content_len);
> +                               clen = strstr(pos + 3, linefeed) - 1;
> +                               j = clen - pos;
> +                               content_length = 0x0f & pos[j];
> +                               i = 10;
> +                               for (j = (clen - pos - 1); j > 0; j--) {
> +                                       content_length += (0x0f & pos[j]) * i;
> +                                       i = i * 10;
> +                               }

Please use strict_strtoul() here instead.

> +                               debug_cond(FILE_TEST, "File Connected Len %d\n",
> +                                          content_length);
> +                       }
> +
> +                       net_boot_file_size = 0;
> +
> +                       if (len > hlen)
> +                               store_block(pkt + hlen, 0, len - hlen);
> +                       debug_cond(FILE_TEST, "File Connected Pkt %p hlen %x\n",
> +                                  pkt, hlen);
> +
> +                       for (i = 0; i < pkt_q_idx; i++) {
> +                               store_block(pkt_q[i].pkt,
> +                                           pkt_q[i].tcp_seq_num -
> +                                               initial_data_seq_num,
> +                                           pkt_q[i].len);
> +                       debug_cond(FILE_TEST, "File Connctd pkt Q %p len %x\n",

Connctd -> Connected

> +                                  pkt_q[i].pkt, pkt_q[i].len);
> +                       }
> +               }
> +       }
> +       file_send(action, tcp_seq_num, tcp_ack_num, len);
> +}
> +
> +       /*
> +        * In the "application push" invocation the TCP header with all its
> +        * information is the packet pointer, and the other variable
> +        * "repurposed" (or misused) to carry sequence numbers and  TCP state.

Like before, what other variable. Be explicit.

> +        *

Remove trailing blank line in comment

> +        */
> +
Remove blank line

> +static void file_handler(uchar *pkt, unsigned int tcp_seq_num,
> +                        struct in_addr action_and_state,
> +                        unsigned int tcp_ack_num, unsigned int len)
> +{
> +       enum TCP_STATE file_tcp_state = tcp_get_tcp_state();
> +       u8 action = action_and_state.s_addr;
> +
> +       net_set_timeout_handler(file_timeout, file_timeout_handler);
> +       packets++;
> +
> +       switch (file_state) {
> +       case FILE_CLOSED:
> +               debug_cond(FILE_TEST, "File Handler: Error!, State wrong\n");

Odd punctuation.

> +       break;
> +       case FILE_CONNECTING:
> +               debug_cond(FILE_TEST,
> +                          "File Connecting In len=%x, Seq=%x, Ack=%x\n",
> +                          len, tcp_seq_num, tcp_ack_num);
> +               if (len == 0) {
> +                       if (file_tcp_state == TCP_ESTABLISHED) {
> +                               debug_cond(FILE_TEST,
> +                                          "File Cting, send, len=%x\n", len);
> +                       file_send(action, tcp_seq_num, tcp_ack_num, len);
> +                       } else {
> +                               tcp_print_buffer(pkt, len, len, -1, 0);
> +                               file_fail("File Handler Connected Fail\n",

Why are you using "File Handler" all over instead of wget? Please switch.

> +                                         tcp_seq_num, tcp_ack_num, action);
> +                       }
> +               }
> +       break;
> +       case FILE_CONNECTED:
> +               debug_cond(FILE_TEST, "File Connected seq=%x, len=%x\n",
> +                          tcp_seq_num, len);
> +               if (len == 0) {
> +                       file_fail("File not found, no data returned\n",
> +                                 tcp_seq_num, tcp_ack_num, action);
> +               } else {
> +                       file_connected(pkt, tcp_seq_num, action_and_state,
> +                                      tcp_ack_num, len);
> +               }
> +       break;
> +       case FILE_TRANSFERRING:
> +               debug_cond(FILE_TEST,
> +                          "File Transferring, seq=%x, ack=%x,len=%x\n",
> +                          tcp_seq_num, tcp_ack_num, len);
> +
> +               if (store_block(pkt,
> +                               tcp_seq_num - initial_data_seq_num, len) != 0) {
> +                       file_fail("File store error\n",
> +                                 tcp_seq_num, tcp_ack_num, action);
> +               } else {
> +                       switch (file_tcp_state) {
> +                       case TCP_FIN_WAIT_2:
> +                               file_send(TCP_ACK, tcp_seq_num, tcp_ack_num,
> +                                         len);
> +                       case TCP_SYN_SENT:
> +                       case TCP_CLOSING:
> +                       case TCP_FIN_WAIT_1:
> +                       case TCP_CLOSED:
> +                               net_set_state(NETLOOP_FAIL);
> +                       break;
> +                       case TCP_ESTABLISHED:
> +                               file_send(TCP_ACK, tcp_seq_num, tcp_ack_num,
> +                                         len);
> +                               loop_state = NETLOOP_SUCCESS;
> +                       break;
> +                       case TCP_CLOSE_WAIT:     /* End of file */
> +                               file_state = FILE_TRANSFERRED;
> +                               file_send(action | TCP_ACK | TCP_FIN,
> +                                         tcp_seq_num, tcp_ack_num, len);
> +                       break;
> +                       }
> +               }
> +       break;
> +       case FILE_TRANSFERRED:
> +               printf("Packets received %d, File Send Success\n", packets);
> +               net_set_state(loop_state);
> +       break;
> +       }
> +}
> +
> +void wget_start(void)
> +{
> +       debug("%s\n", __func__);
> +
> +       file_server_ip = net_server_ip;
> +       file_path = (char *)file_path_buff;
> +
> +       if (!file_path) {
> +               net_set_state(NETLOOP_FAIL);
> +               debug("*** ERROR: Fail allocate memory\n");

That doesn't make any sense. This is a static variable. It's
"allocated" by the linker, so it either worked or you don't have a
binary. I guess you could assert(), but even that seems needless.

> +               return;
> +       }
> +
> +       if (net_boot_file_name[0] == '\0') {
> +               sprintf(file_path, "/fileroot/%02X%02X%02X%02X.img",
> +                       net_ip.s_addr & 0xFF,
> +                       (net_ip.s_addr >>  8) & 0xFF,
> +                       (net_ip.s_addr >> 16) & 0xFF,
> +                       (net_ip.s_addr >> 24) & 0xFF);

Doesn't it make more sense to use the ethaddr for a default instead?
Also, I would drop the "fileroot" path. Just look in the actual root
of the webserver if it's default.

> +
> +               debug("*** Warning: no boot file name; using '%s'\n",
> +                     file_path);
> +       } else {
> +               char *p = net_boot_file_name;
> +
> +               p = strchr(p, ':');
> +
> +               if (p) {
> +                       file_server_ip = string_to_ip(net_boot_file_name);
> +                       ++p;
> +                       strcpy(file_path, p);
> +               } else {
> +                       strcpy(file_path, net_boot_file_name);
> +               }
> +       }
> +
> +       debug_cond(FILE_TEST, "Using %s device\n", eth_get_name());
> +       debug_cond(FILE_TEST, "File transfer HTTP Server %pI4; our IP %pI4\n",
> +                  &file_server_ip, &net_ip);
> +
> +       /* Check if we need to send across this subnet */
> +       if (net_gateway.s_addr && net_netmask.s_addr) {
> +               struct in_addr our_net;
> +               struct in_addr server_net;
> +
> +               our_net.s_addr = net_ip.s_addr & net_netmask.s_addr;
> +               server_net.s_addr = net_server_ip.s_addr & net_netmask.s_addr;
> +               if (our_net.s_addr != server_net.s_addr)
> +                       debug("; sending through gateway %pI4",
> +                             &net_gateway);

Use a debug_cond here.

> +       }
> +       debug_cond(FILE_TEST, "Filename '%s, %s'.\n", file_path, file_filename);
> +
> +       if (net_boot_file_expected_size_in_blocks) {
> +               debug(" Size is 0x%x Bytes = ",
> +                     net_boot_file_expected_size_in_blocks << 9);
> +               print_size(net_boot_file_expected_size_in_blocks << 9, "");
> +       }
> +       debug("\nLoad address: 0x%lx\nLoading: *\b", load_addr);
> +
> +       net_set_timeout_handler(file_timeout, file_timeout_handler);
> +       tcp_set_tcp_handler(file_handler);
> +
> +       file_timeout_count = 0;
> +       file_state = FILE_CLOSED;
> +
> +       our_port = 4096 + (get_ticks() % 3072);

Please use random_port() instead.

> +
> +       /* zero out server ether in case the server ip has changed */
> +       memset(net_server_ethaddr, 0, 6);
> +
> +       file_send(TCP_SYN, 0, 0, 0);
> +}
> diff --git a/net/wget.h b/net/wget.h
> new file mode 100644
> index 0000000000..504943d9c8
> --- /dev/null
> +++ b/net/wget.h
> @@ -0,0 +1,22 @@
> +/*
> + * Duncan Hare Copyright 2017

Include SPDX-License-Identifier. Also include email address.

> + */
> +
> +void wget_start(void);                 /* Begin wget */
> +
> +enum FILE_STATE {
> +       FILE_CLOSED,
> +       FILE_CONNECTING,
> +       FILE_CONNECTED,
> +       FILE_TRANSFERRING,
> +       FILE_TRANSFERRED};
> +
> +#define K_LOAD                 1       /* Load and run kernel */

Remove.

> +
> +#define        FILE_TEST               0       /* Set to 1 for debug messges */

messges -> messages
Change the define to "DEBUG_WGET" instead.

> +#define        SERVER_PORT             80
> +#define        FILE_RETRY_COUNT        30
> +#define        FILE_TIMEOUT            2000UL
> +
> +void file_fail(char *error_message, unsigned int tcp_seq_num,
> +              unsigned int tcp_ack_num, u8 action);

Seems like this can be static and not be exposed in this header.

> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot