All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v10 0/3] Why netboot:
@ 2018-04-14 23:43 DH at synoia.com
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot DH at synoia.com
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: DH at synoia.com @ 2018-04-14 23:43 UTC (permalink / raw)
  To: u-boot

From: Duncan Hare <DuncanCHare@yahoo.com>

Central management, including logs and change control,
coupled with with enhanced security and unauthorized
change detection and remediation by exposing a
small attack surface.

Why TCP:

Currently file transfer are done using tftp or NFS both
over udp. This requires a request to be sent from client
(u-boot) to the boot server.

For a 4 Mbyte kernel, with a 1k block size this requires
4,000 request for a block.

Using a large block size, one greater than the Ethernet
maximum frame size limitation, would require fragmentation,
which u-boot supports. However missing fragment recovery
requires timeout detection and re-transmission requests
for missing fragments.

UDP is ideally suited to fast single packet exchanges,
inquiry/response, for example dns, becuse of the lack of
connection overhead.

UDP as a file transport mechanism is slow, even in low
latency networks, because file transfer with udp requires
poll/response mechanism to provide transfer integrity.

In networks with large latency, for example: the internet,
UDP is even slower. What is a 30 second transfer on a local
boot server and LAN increase to over 3 minutes, because of
all the requests/response traffic.

This was anticipated in the evolution of the IP protocols
and TCP was developed and then enhanced for high latency high
bandwidth networks.

The current standard is TCP with selective acknowledgment.

In our testing we have reduce kernel transmission time to
around 0.4 seconds for a 4Mbyte kernel, with a 100 Mbps
downlink.

Why http and wget:

HTTP is the most efficient file retrieval protocol in common
use. The client send a single request, after TCP connection,
to receive a file of any length.

WGET is the application which implements http file transfer
outside browsers as a file transfer protocol. Versions of
wget exists on many operating systems.

Changes in v10:
Initial changes for adding TCP

Duncan Hare (3):
  Adding TCP and wget into u-boot
  Adding TCP
  Adding wget

 cmd/Kconfig        |   5 +
 cmd/net.c          |  13 +
 include/net.h      |  33 ++-
 include/net/tcp.h  | 218 ++++++++++++++++
 include/net/wget.h |  17 ++
 net/Kconfig        |  10 +
 net/Makefile       |   3 +-
 net/net.c          |  89 +++++--
 net/ping.c         |   9 +-
 net/tcp.c          | 749 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/wget.c         | 420 ++++++++++++++++++++++++++++++
 11 files changed, 1532 insertions(+), 34 deletions(-)
 create mode 100644 include/net/tcp.h
 create mode 100644 include/net/wget.h
 create mode 100644 net/tcp.c
 create mode 100644 net/wget.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot
  2018-04-14 23:43 [U-Boot] [PATCH v10 0/3] Why netboot: DH at synoia.com
@ 2018-04-14 23:43 ` DH at synoia.com
  2018-04-30 23:44   ` Joe Hershberger
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 2/3] Adding TCP DH at synoia.com
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 22+ messages in thread
From: DH at synoia.com @ 2018-04-14 23:43 UTC (permalink / raw)
  To: u-boot

From: Duncan Hare <DuncanCHare@yahoo.com>

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

Added a protocol parameter to ip packet sending in net.c
Added UDP protocol for current applications to minimize
code changes to existing net apps.

All the code is new, and not copied from any source.


Changes in v10:
Initial changes for adding TCP

 include/net.h | 25 +++++++++++++++++++------
 net/net.c     | 52 ++++++++++++++++++++++++++++++++++------------------
 net/ping.c    |  9 ++-------
 3 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/include/net.h b/include/net.h
index 455b48f6c7..7e5f5a6a5b 100644
--- a/include/net.h
+++ b/include/net.h
@@ -15,17 +15,26 @@
 #include <asm/cache.h>
 #include <asm/byteorder.h>	/* for nton* / ntoh* stuff */
 
-#define DEBUG_LL_STATE 0	/* Link local state machine changes */
-#define DEBUG_DEV_PKT 0		/* Packets or info directed to the device */
-#define DEBUG_NET_PKT 0		/* Packets on info on the network at large */
+#define DEBUG_LL_STATE  0	/* Link local state machine changes */
+#define DEBUG_DEV_PKT   0	/* Packets or info directed to the device */
+#define DEBUG_NET_PKT   0	/* Packets on info on the network at large */
 #define DEBUG_INT_STATE 0	/* Internal network state changes */
 
 /*
  *	The number of receive packet buffers, and the required packet buffer
  *	alignment in memory.
  *
+ *	The number of buffers for TCP is used to calculate a static TCP window
+ *	size, becuse TCP window size is a promise to the sending TCP to be able
+ *	to buffer up to the window size of data.
+ *	When the sending TCP has a window size of outstanding unacknowledged
+ *	data, the sending TCP will stop sending.
  */
 
+#if defined(CONFIG_TCP)
+#define CONFIG_SYS_RX_ETH_BUFFER 12	/* For TCP */
+#endif
+
 #ifdef CONFIG_SYS_RX_ETH_BUFFER
 # define PKTBUFSRX	CONFIG_SYS_RX_ETH_BUFFER
 #else
@@ -354,6 +363,7 @@ struct vlan_ethernet_hdr {
 
 #define IPPROTO_ICMP	 1	/* Internet Control Message Protocol	*/
 #define IPPROTO_UDP	17	/* User Datagram Protocol		*/
+#define IPPROTO_TCP	 6	/* Transmission Control Protocol        */
 
 /*
  *	Internet Protocol (IP) header.
@@ -596,10 +606,10 @@ int net_set_ether(uchar *xet, const uchar *dest_ethaddr, uint prot);
 int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot);
 
 /* Set IP header */
-void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source);
+void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
+		       u16  pkt_len, u8 prot);
 void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport,
-				int sport, int len);
-
+			int sport, int len);
 /**
  * compute_ip_checksum() - Compute IP checksum
  *
@@ -670,6 +680,9 @@ static inline void net_send_packet(uchar *pkt, int len)
  * @param sport Source UDP port
  * @param payload_len Length of data after the UDP header
  */
+int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
+		       int payload_len, int proto);
+
 int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
 			int sport, int payload_len);
 
diff --git a/net/net.c b/net/net.c
index 4259c9e321..df4e7317e7 100644
--- a/net/net.c
+++ b/net/net.c
@@ -181,6 +181,7 @@ int		net_ntp_time_offset;
 static uchar net_pkt_buf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
 /* Receive packets */
 uchar *net_rx_packets[PKTBUFSRX];
+
 /* Current UDP RX packet handler */
 static rxhand_f *udp_packet_handler;
 /* Current ARP RX packet handler */
@@ -777,11 +778,23 @@ void net_set_timeout_handler(ulong iv, thand_f *f)
 }
 
 int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
-		int payload_len)
+			int payload_len)
+{
+	return net_send_ip_packet(ether, dest, dport, sport, payload_len,
+				  IPPROTO_UDP);
+}
+
+int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
+		       int payload_len, int proto)
 {
 	uchar *pkt;
 	int eth_hdr_size;
 	int pkt_hdr_size;
+	if (proto == IPPROTO_UDP) {
+		debug_cond(DEBUG_DEV_PKT,
+			   "UDP Send  (to=%pI4, from=%pI4, len=%d)\n",
+			   &dest, &net_ip, payload_len);
+	}
 
 	/* make sure the net_tx_packet is initialized (net_init() was called) */
 	assert(net_tx_packet != NULL);
@@ -799,9 +812,15 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 	pkt = (uchar *)net_tx_packet;
 
 	eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
-	pkt += eth_hdr_size;
-	net_set_udp_header(pkt, dest, dport, sport, payload_len);
-	pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
+	switch (proto) {
+	case IPPROTO_UDP:
+		net_set_udp_header(pkt + eth_hdr_size, dest,
+				   dport, sport, payload_len);
+		pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
+	break;
+	default:
+		return -1;
+	}
 
 	/* if MAC address was not discovered yet, do an ARP request */
 	if (memcmp(ether, net_null_ethaddr, 6) == 0) {
@@ -1157,9 +1176,6 @@ void net_process_received_packet(uchar *in_packet, int len)
 		/* Can't deal with anything except IPv4 */
 		if ((ip->ip_hl_v & 0xf0) != 0x40)
 			return;
-		/* Can't deal with IP options (headers != 20 bytes) */
-		if ((ip->ip_hl_v & 0x0f) > 0x05)
-			return;
 		/* Check the Checksum of the header */
 		if (!ip_checksum_ok((uchar *)ip, IP_HDR_SIZE)) {
 			debug("checksum bad\n");
@@ -1205,6 +1221,7 @@ void net_process_received_packet(uchar *in_packet, int len)
 		 * we send a tftp packet to a dead connection, or when
 		 * there is no server at the other end.
 		 */
+
 		if (ip->ip_p == IPPROTO_ICMP) {
 			receive_icmp(ip, len, src_ip, et);
 			return;
@@ -1365,8 +1382,7 @@ common:
 }
 /**********************************************************************/
 
-int
-net_eth_hdr_size(void)
+int net_eth_hdr_size(void)
 {
 	ushort myvlanid;
 
@@ -1426,25 +1442,28 @@ int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot)
 	}
 }
 
-void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source)
+void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
+		       u16  pkt_len, u8 prot)
 {
 	struct ip_udp_hdr *ip = (struct ip_udp_hdr *)pkt;
 
 	/*
-	 *	Construct an IP header.
+	 *      Construct an IP header.
 	 */
 	/* IP_HDR_SIZE / 4 (not including UDP) */
 	ip->ip_hl_v  = 0x45;
 	ip->ip_tos   = 0;
-	ip->ip_len   = htons(IP_HDR_SIZE);
+	ip->ip_len   = htons(pkt_len);
 	ip->ip_id    = htons(net_ip_id++);
-	ip->ip_off   = htons(IP_FLAGS_DFRAG);	/* Don't fragment */
+	ip->ip_off   = htons(IP_FLAGS_DFRAG);   /* Don't fragment */
 	ip->ip_ttl   = 255;
+	ip->ip_p     = prot;
 	ip->ip_sum   = 0;
 	/* already in network byte order */
 	net_copy_ip((void *)&ip->ip_src, &source);
 	/* already in network byte order */
 	net_copy_ip((void *)&ip->ip_dst, &dest);
+	ip->ip_sum  = compute_ip_checksum(ip, IP_HDR_SIZE);
 }
 
 void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
@@ -1460,11 +1479,8 @@ void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
 	if (len & 1)
 		pkt[IP_UDP_HDR_SIZE + len] = 0;
 
-	net_set_ip_header(pkt, dest, net_ip);
-	ip->ip_len   = htons(IP_UDP_HDR_SIZE + len);
-	ip->ip_p     = IPPROTO_UDP;
-	ip->ip_sum   = compute_ip_checksum(ip, IP_HDR_SIZE);
-
+	net_set_ip_header(pkt, dest, net_ip, IP_UDP_HDR_SIZE + len,
+			  IPPROTO_UDP);
 	ip->udp_src  = htons(sport);
 	ip->udp_dst  = htons(dport);
 	ip->udp_len  = htons(UDP_HDR_SIZE + len);
diff --git a/net/ping.c b/net/ping.c
index 9508cf1160..254b646193 100644
--- a/net/ping.c
+++ b/net/ping.c
@@ -20,16 +20,11 @@ struct in_addr net_ping_ip;
 static void set_icmp_header(uchar *pkt, struct in_addr dest)
 {
 	/*
-	 *	Construct an IP and ICMP header.
+	 *	Construct an ICMP header.
 	 */
-	struct ip_hdr *ip = (struct ip_hdr *)pkt;
 	struct icmp_hdr *icmp = (struct icmp_hdr *)(pkt + IP_HDR_SIZE);
 
-	net_set_ip_header(pkt, dest, net_ip);
-
-	ip->ip_len   = htons(IP_ICMP_HDR_SIZE);
-	ip->ip_p     = IPPROTO_ICMP;
-	ip->ip_sum   = compute_ip_checksum(ip, IP_HDR_SIZE);
+	net_set_ip_header(pkt, dest, net_ip, IP_ICMP_HDR_SIZE, IPPROTO_ICMP);
 
 	icmp->type = ICMP_ECHO_REQUEST;
 	icmp->code = 0;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 2/3] Adding TCP
  2018-04-14 23:43 [U-Boot] [PATCH v10 0/3] Why netboot: DH at synoia.com
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot DH at synoia.com
@ 2018-04-14 23:43 ` DH at synoia.com
  2018-05-01  1:44   ` Joe Hershberger
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 3/3] Adding wget DH at synoia.com
  2018-05-01  1:57 ` [U-Boot] [PATCH v10 0/3] Why netboot: Joe Hershberger
  3 siblings, 1 reply; 22+ messages in thread
From: DH at synoia.com @ 2018-04-14 23:43 UTC (permalink / raw)
  To: u-boot

From: Duncan Hare <DuncanCHare@yahoo.com>

All the code is new, and not copied from any source.

Series-changes
The previous patch was using an old version of net/Kconfig,
which prevented requesting options for a bootp/dhcp request.

A similar issue fixed with a cmd/Kconfig.
Items in include/net.h fixed.

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

Why TCP:

Currently file transfer are done using tftp or NFS both
over udp. This requires a request to be sent from client
(u-boot) to the boot server.

For a 4 Mbyte kernel, with a 1k block size this requires
4,000 request for a block.

Using a large block size, one greater than the Ethernet
maximum frame size limitation, would require fragmentation,
which u-boot supports. However missing fragment recovery
requires timeout detection and re-transmission requests
for missing fragments.

In networks with large latency, for example: the internet,
UDP is vwey slow. What is a 30 second transfer on a local
boot server and LAN increase to over 3 minutes, because of
all the requests/response traffic.

This was anticipated in the evolution of the IP protocols
and TCP was developed and then enhanced for high latency high
bandwidth networks.

The current standard is TCP with selective acknowledgment.

Routine tcp_print_buffer() is used to print portions of
non zero terminated buffers. If there is an existing routine
please let me know. I'm from the world of length fields
not zero terminated strings (zOS).


Changes in v10: None

 include/net.h     |   8 +-
 include/net/tcp.h | 218 ++++++++++++++++
 net/Kconfig       |   5 +
 net/Makefile      |   2 +-
 net/net.c         |  51 +++-
 net/tcp.c         | 749 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 1023 insertions(+), 10 deletions(-)
 create mode 100644 include/net/tcp.h
 create mode 100644 net/tcp.c

diff --git a/include/net.h b/include/net.h
index 7e5f5a6a5b..e29d804a23 100644
--- a/include/net.h
+++ b/include/net.h
@@ -548,7 +548,7 @@ extern int		net_restart_wrap;	/* Tried all network devices */
 
 enum proto_t {
 	BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
-	TFTPSRV, TFTPPUT, LINKLOCAL
+	TFTPSRV, TFTPPUT, LINKLOCAL, WGET
 };
 
 extern char	net_boot_file_name[1024];/* Boot File name */
@@ -681,11 +681,15 @@ static inline void net_send_packet(uchar *pkt, int len)
  * @param payload_len Length of data after the UDP header
  */
 int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
-		       int payload_len, int proto);
+		       int payload_len, int proto, u8 action, u32 tcp_seq_num,
+		       u32 tcp_ack_num);
 
 int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
 			int sport, int payload_len);
 
+int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
+			u32 tcp_seq_num, u32 tcp_ack_num);
+
 /* Processes a received packet */
 void net_process_received_packet(uchar *in_packet, int len);
 
diff --git a/include/net/tcp.h b/include/net/tcp.h
new file mode 100644
index 0000000000..81f263351e
--- /dev/null
+++ b/include/net/tcp.h
@@ -0,0 +1,218 @@
+/*
+ *	TCP Support for file transfer.
+ *
+ *	Copyright 2017 Duncan Hare, All rights reserved.
+ *
+ *      SPDX-License-Identifier:        GPL-2.0
+ */
+
+#define TCP_ACTIVITY 127		/* Activity on downloading      */
+
+struct ip_tcp_hdr {
+	u8		ip_hl_v;	/* header length and version	*/
+	u8		ip_tos;		/* type of service		*/
+	u16		ip_len;		/* total length			*/
+	u16		ip_id;		/* identification		*/
+	u16		ip_off;		/* fragment offset field	*/
+	u8		ip_ttl;		/* time to live			*/
+	u8		ip_p;		/* protocol			*/
+	u16		ip_sum;		/* checksum			*/
+	struct in_addr	ip_src;		/* Source IP address		*/
+	struct in_addr	ip_dst;		/* Destination IP address	*/
+	u16		tcp_src;	/* TCP source port		*/
+	u16		tcp_dst;	/* TCP destination port		*/
+	u32		tcp_seq;	/* TCP sequence number		*/
+	u32		tcp_ack;	/* TCP Acknowledgment number	*/
+	u8		tcp_hlen;	/* 4 bits TCP header Length/4	*/
+					/* 4 bits Reserved		*/
+					/* 2 more bits reserved		*/
+	u8		tcp_flags;	/* see defines			*/
+	u16		tcp_win;	/* TCP windows size		*/
+	u16		tcp_xsum;	/* Checksum			*/
+	u16		tcp_ugr;	/* Pointer to urgent data	*/
+} __packed;
+
+#define IP_TCP_HDR_SIZE		(sizeof(struct ip_tcp_hdr))
+#define TCP_HDR_SIZE		(IP_TCP_HDR_SIZE  - IP_HDR_SIZE)
+
+#define TCP_DATA	0x00	/* Data Packet - internal use only	*/
+#define TCP_FIN		0x01	/* Finish flag				*/
+#define TCP_SYN		0x02	/* Synch (start) flag			*/
+#define TCP_RST		0x04	/* reset flag				*/
+#define TCP_PUSH	0x08	/* Push - Notify app			*/
+#define TCP_ACK		0x10	/* Acknowledgment of data received	*/
+#define TCP_URG		0x20	/* Urgent				*/
+#define TCP_ECE		0x40	/* Congestion control			*/
+#define TCP_CWR		0x80	/* Congestion Control			*/
+
+/*
+ * TCP header options, Seq, MSS, and SACK
+ */
+
+#define TCP_SACK 32			/* Number of packets analyzed   */
+					/* on leading edge of stream    */
+
+#define TCP_O_END	0x00		/* End of option list		*/
+#define TCP_1_NOP	0x01		/* Single padding NOP		*/
+#define TCP_O_NOP	0x01010101	/* NOPs pad to 32 bit boundary	*/
+#define TCP_O_MSS	0x02		/* MSS Size option		*/
+#define TCP_O_SCL	0x03		/* Window Scale option		*/
+#define TCP_P_SACK	0x04		/* SACK permitted		*/
+#define TCP_V_SACK	0x05		/* SACK values			*/
+#define TCP_O_TS	0x08		/* Timestanp option		*/
+#define TCP_OPT_LEN_2	0x02
+#define TCP_OPT_LEN_3	0x03
+#define TCP_OPT_LEN_4	0x04
+#define TCP_OPT_LEN_6	0x06
+#define TCP_OPT_LEN_8	0x08
+#define TCP_OPT_LEN_A	0x0a		/* Timestamp Length		*/
+
+/*
+ * Please review the warning in net.c about these two parameters.
+ * They are part of a promise of RX buffer size to the sending TCP
+ */
+
+#define TCP_MSS		1460		/* Max segment size - 1460	*/
+#define TCP_SCALE	0x01		/* Scale 1			*/
+
+struct tcp_mss {			/* TCP Mex Segment size		*/
+	u8	kind;			/* 0x02				*/
+	u8	len;			/* 0x04				*/
+	u16	mss;			/* 1460 - Max segment size	*/
+} __packed;
+
+struct tcp_scale {			/* TCP Windows Scale		*/
+	u8	kind;			/* 0x03				*/
+	u8	len;			/* 0x03				*/
+	u8	scale;			/* win shift fat fast networks	*/
+} __packed;
+
+struct tcp_sack_p {			/* SACK permitted		*/
+	u8	kind;			/* 0x04				*/
+	u8	len;			/* Length			*/
+} __packed;
+
+struct sack_edges {
+	u32	l;
+	u32	r;
+} __packed;
+
+#define TCP_SACK_SIZE (sizeof(struct sack_edges))
+
+#define TCP_SACK_HILLS	4
+
+struct tcp_sack_v {
+	u8	kind;			/* 0x05			        */
+	u8	len;				 /* Length		*/
+	struct	sack_edges hill[TCP_SACK_HILLS]; /* L & R widow edges	*/
+} __packed;
+
+struct tcp_t_opt {			/* TCP time stamps option	*/
+	u8	kind;			/* 0x08				*/
+	u8	len;			/* 0x0a				*/
+	u32	t_snd;			/* Sender timestamp		*/
+	u32	t_rcv;			/* Receiver timestamp		*/
+} __packed;
+
+#define TCP_TSOPT_SIZE	(sizeof(struct tcp_t_opt))
+
+/*
+ * ip tcp  structure with options
+ */
+
+struct ip_tcp_hdr_o {
+	struct	ip_tcp_hdr	hdr;
+	struct	tcp_mss		mss;
+	struct	tcp_scale	scale;
+	struct	tcp_sack_p	sack_p;
+	struct	tcp_t_opt	t_opt;
+	u8	end;
+} __packed;
+
+#define IP_TCP_O_SIZE		(sizeof(struct ip_tcp_hdr_o))
+
+struct ip_tcp_hdr_s {
+	struct	ip_tcp_hdr	hdr;
+	struct	tcp_t_opt	t_opt;
+	struct	tcp_sack_v	sack_v;
+	u8	end;
+} __packed;
+
+#define IP_TCP_SACK_SIZE	(sizeof(struct ip_tcp_hdr_s))
+
+/*
+ * TCP pseudo header definitions
+ */
+#define PSEUDO_PAD_SIZE	8
+
+struct pseudo_hdr {
+	u8 padding[PSEUDO_PAD_SIZE];	/* pseudo hdr size = ip_tcp hdr size */
+	struct in_addr p_src;
+	struct in_addr p_dst;
+	u8      rsvd;
+	u8      p;
+	u16     len;
+} __packed;
+
+#define PSEUDO_HDR_SIZE	(sizeof(struct pseudo_hdr)) - PSEUDO_PAD_SIZE
+
+/*
+ * union for building IP/TCP packet.
+ * build Pseudo header in packed buffer first, calculate TCP checksum
+ * then build IP header in packed  buffer.
+ */
+
+union tcp_build_pkt {
+	struct pseudo_hdr ph;
+	struct ip_tcp_hdr_o ip;
+	struct ip_tcp_hdr_s sack;
+	uchar  raw[1600];
+} __packed;
+
+/*
+ * TCP STATE MACHINE STATES FOR SOCKET
+ */
+
+enum TCP_STATE {
+	TCP_CLOSED,		/* Need to send SYN to connect		  */
+	TCP_SYN_SENT,		/* Trying to connect, waiting for SYN ACK */
+	TCP_ESTABLISHED,	/* both server & client have a connection */
+	TCP_CLOSE_WAIT,		/* Rec FIN, passed to app for FIN, ACK rsp*/
+	TCP_CLOSING,		/* Rec FIN, sent FIN, ACK waiting for ACK */
+	TCP_FIN_WAIT_1,		/* Sent FIN waiting for response	  */
+	TCP_FIN_WAIT_2		/* Rec ACK from FIN sent, waiting for FIN */
+};
+
+int tcp_find_in_buffer(uchar raw[], int payload_len, uchar field[],
+		       int field_len);
+void tcp_print_buffer(uchar raw[], int pkt_len, int payload_len,
+		      int hdr_len, bool hide);
+enum TCP_STATE tcp_get_tcp_state(void);
+void tcp_set_tcp_state(enum TCP_STATE new_state);
+int tcp_set_tcp_header(uchar *pkt, int dport, int sport, int payload_len,
+		       u8 action, u32 tcp_seq_num, u32 tcp_ack_num);
+
+/*
+ * An incoming packet handler.
+ * @param pkt    pointer to the application packet
+ * @param dport  destination UDP port
+ * @param sip    source IP address
+ * @param sport  source UDP port
+ * @param len    packet length
+ */
+typedef void rxhand_tcp(uchar *pkt, unsigned int dport,
+			struct in_addr sip, unsigned int sport,
+			unsigned int len);
+void tcp_set_tcp_handler(rxhand_tcp *f);
+
+void rxhand_tcp_f(union tcp_build_pkt *b, unsigned int len);
+
+/*
+ * An incoming TCP packet handler for the TCP protocol.
+ * There is also a dynamic function pointer for TCP based commands to
+ * receive incoming traffic after the TCP protocol code has done its work.
+ */
+
+void rxhand_action(u8 tcp_action, int payload_len, u32 tcp_seq_num,
+		   u32 tcp_ack_num, unsigned int pkt_len,
+		   union tcp_build_pkt *b);
diff --git a/net/Kconfig b/net/Kconfig
index 143c4416cd..cc6dd83fc4 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -33,6 +33,11 @@ config NET_TFTP_VARS
 	  If unset, timeout and maximum are hard-defined as 1 second
 	  and 10 timouts per TFTP transfer.
 
+config TCP
+	bool "Include Subset TCP stack for wget"
+	help
+	  TCP protocol support for wget.
+
 config BOOTP_BOOTPATH
 	bool "Enable BOOTP BOOTPATH"
 	depends on CMD_NET
diff --git a/net/Makefile b/net/Makefile
index ae54eee5af..25729ebeeb 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -25,7 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
 obj-$(CONFIG_CMD_RARP) += rarp.o
 obj-$(CONFIG_CMD_SNTP) += sntp.o
 obj-$(CONFIG_CMD_NET)  += tftp.o
-
+obj-$(CONFIG_TCP)      += tcp.o
 # Disable this warning as it is triggered by:
 # sprintf(buf, index ? "foo%d" : "foo", index)
 # and this is intentional usage.
diff --git a/net/net.c b/net/net.c
index df4e7317e7..7adc4b472c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -107,6 +107,9 @@
 #if defined(CONFIG_CMD_SNTP)
 #include "sntp.h"
 #endif
+#if defined(CONFIG_TCP)
+#include <net/tcp.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -382,6 +385,9 @@ void net_init(void)
 
 		/* Only need to setup buffer pointers once. */
 		first_call = 0;
+#if defined(CONFIG_TCP)
+		tcp_set_tcp_state(TCP_CLOSED);
+#endif
 	}
 
 	net_init_loop();
@@ -781,11 +787,22 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 			int payload_len)
 {
 	return net_send_ip_packet(ether, dest, dport, sport, payload_len,
-				  IPPROTO_UDP);
+				  IPPROTO_UDP, 0, 0, 0);
+}
+
+#if defined(CONFIG_TCP)
+int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
+			u32 tcp_seq_num, u32 tcp_ack_num)
+{
+	return net_send_ip_packet(net_server_ethaddr, net_server_ip, dport,
+				  sport, payload_len, IPPROTO_TCP, action,
+				  tcp_seq_num, tcp_ack_num);
 }
+#endif
 
 int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
-		       int payload_len, int proto)
+		       int payload_len, int proto, u8 action, u32 tcp_seq_num,
+		       u32 tcp_ack_num)
 {
 	uchar *pkt;
 	int eth_hdr_size;
@@ -794,6 +811,13 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 		debug_cond(DEBUG_DEV_PKT,
 			   "UDP Send  (to=%pI4, from=%pI4, len=%d)\n",
 			   &dest, &net_ip, payload_len);
+#if defined(CONFIG_TCP)
+
+	} else {
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Send  (%pI4, %pI4, len=%d, A=%x)\n",
+			   &dest, &net_ip, payload_len, action);
+#endif
 	}
 
 	/* make sure the net_tx_packet is initialized (net_init() was called) */
@@ -812,15 +836,19 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 	pkt = (uchar *)net_tx_packet;
 
 	eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
-	switch (proto) {
-	case IPPROTO_UDP:
+	if (proto == IPPROTO_UDP) {
 		net_set_udp_header(pkt + eth_hdr_size, dest,
 				   dport, sport, payload_len);
 		pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
-	break;
-	default:
-		return -1;
+
+#if defined(CONFIG_TCP)
+	} else {
+		pkt_hdr_size = eth_hdr_size +
+		tcp_set_tcp_header(pkt + eth_hdr_size, dport, sport,
+				   payload_len, action,
+				   tcp_seq_num, tcp_ack_num);
 	}
+#endif
 
 	/* if MAC address was not discovered yet, do an ARP request */
 	if (memcmp(ether, net_null_ethaddr, 6) == 0) {
@@ -1225,6 +1253,15 @@ void net_process_received_packet(uchar *in_packet, int len)
 		if (ip->ip_p == IPPROTO_ICMP) {
 			receive_icmp(ip, len, src_ip, et);
 			return;
+#if defined(CONFIG_TCP)
+		} else if (ip->ip_p == IPPROTO_TCP) {
+			debug_cond(DEBUG_DEV_PKT,
+				   "TCP PH (to=%pI4, from=%pI4, len=%d)\n",
+				   &dst_ip, &src_ip, len);
+
+				rxhand_tcp_f((union tcp_build_pkt *)ip, len);
+			return;
+#endif
 		} else if (ip->ip_p != IPPROTO_UDP) {	/* Only UDP packets */
 			return;
 		}
diff --git a/net/tcp.c b/net/tcp.c
new file mode 100644
index 0000000000..9f4fc9fc20
--- /dev/null
+++ b/net/tcp.c
@@ -0,0 +1,749 @@
+/*
+ *	Copyright 2017 Duncan Hare, all rights reserved.
+ *	SPDX-License-Identifier:        GPL-2.0
+ */
+
+/*
+ * General Desription:
+ *
+ * TCP support for the wget command, for fast file downloading.
+ *
+ * HTTP/TCP Receiver:
+ *
+ *      Prequeisites:   - own ethernet address
+ *                      - own IP address
+ *                      - Server IP address
+ *                      - Server with TCP
+ *                      - TCP application (eg wget)
+ *      Next Step       HTTPS?
+ */
+#include <common.h>
+#include <command.h>
+#include <console.h>
+#include <environment.h>
+#include <errno.h>
+#include <net.h>
+#include <net/tcp.h>
+#if defined(CONFIG_CMD_WGET)
+#include <net/wget.h>
+#endif
+
+/*
+ * TCP sliding window  control
+ * used by us to request re-TX
+ */
+
+static struct tcp_sack_v tcp_lost;
+
+/* TCP option timestamp */
+static u32 loc_timestamp;
+static u32 rmt_timestamp;
+
+u32 tcp_seq_init;
+u32 tcp_ack_edge;
+u32 tcp_seq_max;
+
+int tcp_activity_count;
+
+/*
+ * Search for TCP_SACK and review the
+ * comments before the code section
+ * TCP_SACK is the number of packets
+ * at the front of the stream
+ */
+
+enum pkt_state {PKT, NOPKT};
+struct sack_r {
+	struct sack_edges se;
+	enum   pkt_state st;
+};
+
+struct sack_r edge_a[TCP_SACK];
+unsigned int sack_idx;
+unsigned int prev_len;
+
+/* TCP connection state */
+static enum TCP_STATE tcp_state;
+
+/*
+ * An incoming TCP packet handler for the TCP protocol.
+ * There is also a dymanic function pointer for TCP based commads to
+ * receive incoming traffic after the TCP protocol code has done its work.
+ */
+
+/* Current TCP RX packet handler */
+static rxhand_tcp *tcp_packet_handler;
+
+enum TCP_STATE tcp_get_tcp_state(void)
+{
+	return tcp_state;
+}
+
+void tcp_set_tcp_state(enum TCP_STATE new_state)
+{
+	tcp_state = new_state;
+}
+
+void tcp_print_buffer(uchar raw[], int pkt_len, int payload_len,
+		      int hdr_len, bool hide)
+{
+	int i;
+
+	for (i = pkt_len - payload_len; i < pkt_len; i++) {
+		if (i <= hdr_len)
+			printf("%02X", raw[i]);
+		else if ((raw[i] > 0x19) && (raw[i] < 0x7f))
+			putc(raw[i]);
+		else if (hide == 0)
+			putc(raw[i]);
+		else
+			printf("%02X", raw[i]);
+	}
+	printf("%s", "\n");
+}
+
+int tcp_find_in_buffer(uchar raw[], int payload_len, uchar field[],
+		       int field_len)
+{
+	int i, j;
+
+	for (i = 0; i < payload_len; i++) {
+		if (raw[i] == field[0]) {
+			for (j = 1; j < field_len; j++) {
+				if (raw[i + j] != field[j])
+					break;
+			if (j == field_len)
+				return i;
+			}
+		}
+	}
+	return 0;
+}
+
+static void dummy_handler(uchar *pkt, unsigned int dport,
+			  struct in_addr sip, unsigned int sport,
+			  unsigned int len)
+{
+}
+
+void tcp_set_tcp_handler(rxhand_tcp *f)
+{
+	debug_cond(DEBUG_INT_STATE, "--- net_loop TCP handler set (%p)\n", f);
+	if (!f)
+		tcp_packet_handler = dummy_handler;
+	else
+		tcp_packet_handler = f;
+}
+
+u16 tcp_set_pseudo_header(uchar *pkt, struct in_addr src, struct in_addr dest,
+			  int tcp_len, int pkt_len)
+{
+	union tcp_build_pkt *b = (union tcp_build_pkt *)pkt;
+	int checksum_len;
+
+	/*
+	 * Pseudo header
+	 *
+	 * Zero the byte after the last byte so that the header checksum
+	 * will always work.
+	 */
+
+	pkt[pkt_len] = 0x00;
+
+	net_copy_ip((void *)&b->ph.p_src, &src);
+	net_copy_ip((void *)&b->ph.p_dst, &dest);
+	b->ph.rsvd	= 0x00;
+	b->ph.p		= IPPROTO_TCP;
+	b->ph.len	= htons(tcp_len);
+	checksum_len	= tcp_len + PSEUDO_HDR_SIZE;
+
+	debug_cond(DEBUG_DEV_PKT,
+		   "TCP Pesudo  Header  (to=%pI4, from=%pI4, Len=%d)\n",
+		   &b->ph.p_dst, &b->ph.p_src, checksum_len);
+
+	return compute_ip_checksum(pkt + PSEUDO_PAD_SIZE, checksum_len);
+}
+
+int net_set_ack_options(union tcp_build_pkt *b)
+{
+	b->sack.hdr.tcp_hlen  = (TCP_HDR_SIZE >> 2) << 4;
+
+	b->sack.t_opt.kind		= TCP_O_TS;
+	b->sack.t_opt.len		= TCP_OPT_LEN_A;
+	b->sack.t_opt.t_snd		= htons(loc_timestamp);
+	b->sack.t_opt.t_rcv		= rmt_timestamp;
+	b->sack.sack_v.kind             = 0x01;
+	b->sack.sack_v.len		= 0x00;
+
+	if (tcp_lost.len > TCP_OPT_LEN_2) {
+		debug_cond(DEBUG_DEV_PKT, "TCP ack opt lost.len %x\n",
+			   tcp_lost.len);
+		b->sack.sack_v.len              = tcp_lost.len;
+		b->sack.sack_v.kind		= TCP_V_SACK;
+		b->sack.sack_v.hill[0].l = htonl(tcp_lost.hill[0].l);
+		b->sack.sack_v.hill[0].r = htonl(tcp_lost.hill[0].r);
+
+		/*
+		 * These SACK structures are initialized with NOPs to
+		 * provide TCP header alignment padding. There are 4
+		 * SACK structures used for both header padding and
+		 * internally.
+		 */
+
+		b->sack.sack_v.hill[1].l = htonl(tcp_lost.hill[1].l);
+		b->sack.sack_v.hill[1].r = htonl(tcp_lost.hill[1].r);
+		b->sack.sack_v.hill[2].l = htonl(tcp_lost.hill[2].l);
+		b->sack.sack_v.hill[2].r = htonl(tcp_lost.hill[2].r);
+		b->sack.sack_v.hill[3].l = TCP_O_NOP;
+		b->sack.sack_v.hill[3].r = TCP_O_NOP;
+	}
+
+	/*
+	 * TCP lengths are stored as a rounded up number of 32 bit words
+	 * Add 3 to length round up, rounded, then divided into the length
+	 * in 32 bit words.
+	 */
+
+	b->sack.hdr.tcp_hlen = (((TCP_HDR_SIZE + TCP_TSOPT_SIZE
+				+ tcp_lost.len + 3)  >> 2) << 4);
+
+	/*
+	 * This returns the actual rounded up length of the
+	 * TCP header to add to the total packet length
+	 */
+
+	return b->sack.hdr.tcp_hlen >> 2;
+}
+
+void net_set_syn_options(union tcp_build_pkt *b)
+{
+	tcp_lost.len		= 0;
+	b->ip.hdr.tcp_hlen      = 0xa0;
+
+	b->ip.mss.kind          = TCP_O_MSS;
+	b->ip.mss.len           = TCP_OPT_LEN_4;
+	b->ip.mss.mss           = htons(TCP_MSS);
+	b->ip.scale.kind        = TCP_O_SCL;
+	b->ip.scale.scale       = TCP_SCALE;
+	b->ip.scale.len         = TCP_OPT_LEN_3;
+	b->ip.sack_p.kind       = TCP_P_SACK;
+	b->ip.sack_p.len        = TCP_OPT_LEN_2;
+	b->ip.t_opt.kind        = TCP_O_TS;
+	b->ip.t_opt.len         = TCP_OPT_LEN_A;
+	loc_timestamp           = get_ticks() % 3072;
+	rmt_timestamp           = 0x00000000;
+	b->ip.t_opt.t_snd       = 0;
+	b->ip.t_opt.t_rcv       = 0x00000000;
+	b->ip.end               = TCP_O_END;
+}
+
+int tcp_set_tcp_header(uchar *pkt, int dport, int sport, int payload_len,
+		       u8 action, u32 tcp_seq_num, u32 tcp_ack_num)
+{
+	union tcp_build_pkt *b = (union tcp_build_pkt *)pkt;
+	int	pkt_hdr_len;
+	int	pkt_len;
+	int	tcp_len;
+
+	b->ip.hdr.tcp_flags	= action;
+	pkt_hdr_len		= IP_TCP_HDR_SIZE;
+	b->ip.hdr.tcp_hlen      = 0x50;		/* Header is 5 32 bit words  */
+						/* 4 bits TCP header Length/4*/
+						/* 4 bits Reserved           */
+						/* For options		     */
+	switch (action) {
+	case TCP_SYN:
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Hdr:SYN (%pI4, %pI4, sq=%d, ak=%d)\n",
+			   &net_server_ip, &net_ip,
+			   tcp_seq_num, tcp_ack_num);
+		tcp_activity_count = 0;
+		net_set_syn_options(b);
+		tcp_seq_num = 0;
+		tcp_ack_num = 0;
+		pkt_hdr_len = IP_TCP_O_SIZE;
+		if (tcp_state == TCP_SYN_SENT) {  /* Too many sins */
+			action    = TCP_FIN;
+			tcp_state = TCP_FIN_WAIT_1;
+		} else {
+			tcp_state = TCP_SYN_SENT;
+		}
+	break;
+	case TCP_ACK:
+		pkt_hdr_len         = IP_HDR_SIZE +
+				      net_set_ack_options(b);
+		b->ip.hdr.tcp_flags = action;
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Hdr:ACK (%pI4, %pI4, s=%d, a=%d, A=%x)\n",
+			   &net_server_ip, &net_ip, tcp_seq_num, tcp_ack_num,
+			   action);
+	break;
+	case TCP_FIN:
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Hdr:FIN  (%pI4, %pI4, s=%d, a=%d)\n",
+			   &net_server_ip, &net_ip, tcp_seq_num, tcp_ack_num);
+		payload_len = 0;
+		pkt_hdr_len = IP_TCP_HDR_SIZE;
+		tcp_state   = TCP_FIN_WAIT_1;
+
+	break;
+
+	/* Notify connection closing */
+
+	case (TCP_FIN | TCP_ACK):
+	case ((TCP_FIN | TCP_ACK) | TCP_PUSH):
+		if (tcp_state == TCP_CLOSE_WAIT)
+			tcp_state = TCP_CLOSING;
+		tcp_ack_edge++;
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Hdr:FIN ACK PSH(%pI4, %pI4, s=%d, a=%d, A=%x)\n",
+			   &net_server_ip, &net_ip,
+			   tcp_seq_num, tcp_ack_edge, action);
+					/* FALLTHRU */
+	default:
+		pkt_hdr_len         = IP_HDR_SIZE +
+				      net_set_ack_options(b);
+		b->ip.hdr.tcp_flags = action | TCP_PUSH | TCP_ACK;
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Hdr:dft  (%pI4, %pI4, s=%d, a=%d, A=%x)\n",
+			   &net_server_ip, &net_ip,
+			   tcp_seq_num, tcp_ack_num, action);
+	}
+
+	pkt_len	= pkt_hdr_len + payload_len;
+	tcp_len	= pkt_len - IP_HDR_SIZE;
+
+	/* TCP Header */
+	b->ip.hdr.tcp_ack       = htonl(tcp_ack_edge);
+	b->ip.hdr.tcp_src	= htons(sport);
+	b->ip.hdr.tcp_dst	= htons(dport);
+	b->ip.hdr.tcp_seq	= htonl(tcp_seq_num);
+	tcp_seq_num		= tcp_seq_num + payload_len;
+
+	/*
+	 * TCP window size - TCP header variable tcp_win.
+	 * Change tcp_win only if you have an understanding of network
+	 * overrun, congestion, TCP segment sizes, TCP windows, TCP scale,
+	 * queuing theory  and packet buffering. If there are too few buffers,
+	 * there will be data loss, recovery may work or the sending TCP,
+	 * the server, could abort the stream transmission.
+	 * MSS is governed by maximum Ethernet frame length.
+	 * The number of buffers is governed by the desire to have a queue of
+	 * full buffers to be processed at the destination to maximize
+	 * throughput. Temporary memory use for the boot phase on modern
+	 * SOCs is may not be considered a constraint to buffer space, if
+	 * it is, then the u-boot tftp or nfs kernel netboot should be
+	 * considered.
+	 */
+
+	b->ip.hdr.tcp_win	= htons(PKTBUFSRX * TCP_MSS >>  TCP_SCALE);
+
+	b->ip.hdr.tcp_xsum	= 0x0000;
+	b->ip.hdr.tcp_ugr	= 0x0000;
+
+	b->ip.hdr.tcp_xsum = tcp_set_pseudo_header(pkt, net_ip, net_server_ip,
+						   tcp_len, pkt_len);
+
+	/*
+	 * IP Header
+	 */
+
+	net_set_ip_header((uchar *)&b->ip, net_server_ip, net_ip,
+			  pkt_len, IPPROTO_TCP);
+
+	return pkt_hdr_len;
+}
+
+/*
+ * Selective Acknowledgment (Essential for fast stream transfer)
+ *
+ * Before modifying this section of code, which the author found difficult
+ * to write, please be familiar with the SACK (Selective Acknowledgment) RFCs.
+ */
+
+void tcp_hole(u32 tcp_seq_num, u32 len, u32 tcp_seq_max)
+{
+	unsigned int idx_sack;
+	unsigned int sack_end = TCP_SACK - 1;
+	unsigned int sack_in;
+	unsigned int hill = 0;
+	enum pkt_state expect = PKT;
+
+	u32 seq   = tcp_seq_num - tcp_seq_init;
+	u32 hol_l = tcp_ack_edge - tcp_seq_init;
+	u32 hol_r = 0;
+
+	/* Place seq number in correct place */
+
+	if (prev_len == 0)
+		prev_len = len;
+	idx_sack = sack_idx + ((tcp_seq_num - tcp_ack_edge) / prev_len);
+	if (idx_sack < TCP_SACK) {
+		edge_a[idx_sack].se.l = tcp_seq_num;
+		edge_a[idx_sack].se.r = tcp_seq_num + len;
+		edge_a[idx_sack].st   = PKT;
+
+		/* The fin (last) packet is not the same length as data packets,
+		 * and if it's length is recorded and used for array index
+		 * the array index calculations break.
+		 */
+		if (prev_len < len)
+			prev_len = len;
+	}
+
+	debug_cond(DEBUG_DEV_PKT,
+		   "TCP 1 seq %d, edg %d, len %d, sack_idx %d, sack_end %d\n",
+		    seq, hol_l, len, sack_idx, sack_end);
+
+	/* Right edge of contiguous stream, is the left edge of first hill */
+
+	hol_l = tcp_seq_num - tcp_seq_init;
+	hol_r = hol_l + len;
+
+	tcp_lost.len = TCP_OPT_LEN_2;
+
+	debug_cond(DEBUG_DEV_PKT,
+		   "TCP 1 in %d, seq %d, pkt_l %d, pkt_r %d, sack_idx %d, sack_end %d\n",
+		   idx_sack, seq, hol_l, hol_r, sack_idx, sack_end);
+
+	for (sack_in = sack_idx; sack_in < sack_end && hill < TCP_SACK_HILLS;
+		 sack_in++)  {
+		switch (expect) {
+		case NOPKT:
+			switch (edge_a[sack_in].st) {
+			case NOPKT:
+				debug_cond(DEBUG_INT_STATE, "N");
+			break;
+			case PKT:
+				debug_cond(DEBUG_INT_STATE, "n");
+					tcp_lost.hill[hill].l =
+						edge_a[sack_in].se.l;
+					tcp_lost.hill[hill].r =
+						edge_a[sack_in].se.r;
+				expect = PKT;
+			break;
+			}
+		break;
+		case PKT:
+			switch (edge_a[sack_in].st) {
+			case NOPKT:
+				debug_cond(DEBUG_INT_STATE, "p");
+				if ((sack_in > sack_idx) &&
+				    (hill < TCP_SACK_HILLS)) {
+					hill++;
+					tcp_lost.len += TCP_OPT_LEN_8;
+				}
+				expect = NOPKT;
+			break;
+			case PKT:
+				debug_cond(DEBUG_INT_STATE, "P");
+
+				if (tcp_ack_edge == edge_a[sack_in].se.l) {
+					tcp_ack_edge = edge_a[sack_in].se.r;
+					edge_a[sack_in].st = NOPKT;
+					sack_idx++;
+				} else {
+					if (hill < TCP_SACK_HILLS)
+						tcp_lost.hill[hill].r =
+							edge_a[sack_in].se.r;
+				if (sack_in == sack_end - 1)
+					tcp_lost.hill[hill].r =
+						edge_a[sack_in].se.r;
+				}
+			break;
+			}
+		break;
+		}
+	}
+	debug_cond(DEBUG_INT_STATE, "\n");
+	if (tcp_lost.len <= TCP_OPT_LEN_2)
+		sack_idx = 0;
+}
+
+void tcp_parse_options(uchar *o, int o_len)
+{
+	struct tcp_t_opt  *tsopt;
+	uchar *p = o;
+
+	for (p = o; p < (o + o_len); p = p + p[1]) {
+		if (p[1] != 0) {
+			switch (p[0]) {
+			case TCP_O_END:
+				return;
+			case TCP_O_MSS:
+			break;
+			case TCP_O_SCL:
+			break;
+			case TCP_P_SACK:
+			break;
+			case TCP_V_SACK:
+			break;
+			case TCP_O_TS:
+				tsopt = (struct tcp_t_opt *)p;
+				rmt_timestamp = tsopt->t_snd;
+				return;
+			break;
+			}
+			if (p[0] == TCP_O_NOP)
+				p++;
+		} else {
+			return;
+		}
+	}
+}
+
+u8 tcp_state_machine(u8 tcp_flags, u32 *tcp_seq_num, int payload_len)
+{
+	u8  tcp_fin  = tcp_flags & TCP_FIN;
+	u8  tcp_syn  = tcp_flags & TCP_SYN;
+	u8  tcp_rst  = tcp_flags & TCP_RST;
+	u8  tcp_push = tcp_flags & TCP_PUSH;
+	u8  tcp_ack  = tcp_flags & TCP_ACK;
+	u8  action   = TCP_DATA;
+	int i;
+
+	/*
+	 * tcp_flags are examined to determine TX action in a given state
+	 * tcp_push is interpreted to mean "inform the app"
+	 * urg, ece, cer and nonce flags are not supported.
+	 *
+	 * exe and crw are use to signal and confirm knowledge of congestion.
+	 * This TCP only sends a file request and acks. If it generates
+	 * congestion, the network is broken.
+	 */
+
+	debug_cond(DEBUG_INT_STATE, "TCP STATE ENTRY %x\n", action);
+	if (tcp_rst) {
+		action    = TCP_DATA;
+		tcp_state = TCP_CLOSED;
+		net_set_state(NETLOOP_FAIL);
+		debug_cond(DEBUG_INT_STATE, "TCP Reset %x\n", tcp_flags);
+		return TCP_RST;
+	}
+
+	switch  (tcp_state) {
+	case TCP_CLOSED:
+	debug_cond(DEBUG_INT_STATE, "TCP CLOSED %x\n", tcp_flags);
+		if (tcp_fin)
+			action = TCP_DATA;
+		if (tcp_syn)
+			action = TCP_RST;
+		if (tcp_ack)
+			action = TCP_DATA;
+	break;
+	case TCP_SYN_SENT:
+		debug_cond(DEBUG_INT_STATE, "TCP_SYN_SENT %x, %d\n",
+			   tcp_flags, *tcp_seq_num);
+		if (tcp_fin) {
+			action = action | TCP_PUSH;
+			tcp_state = TCP_CLOSE_WAIT;
+		}
+		if (tcp_syn) {
+			action = action |  TCP_ACK | TCP_PUSH;
+			if (tcp_ack) {
+				tcp_seq_init          = *tcp_seq_num;
+				*tcp_seq_num          = *tcp_seq_num + 1;
+				tcp_seq_max           = *tcp_seq_num;
+				tcp_ack_edge          = *tcp_seq_num;
+				sack_idx              = 0;
+				edge_a[sack_idx].se.l = *tcp_seq_num;
+				edge_a[sack_idx].se.r = *tcp_seq_num;
+				prev_len              = 0;
+				tcp_state             = TCP_ESTABLISHED;
+				for (i = 0; i < TCP_SACK; i++)
+					edge_a[i].st   = NOPKT;
+			}
+		} else {
+			if (tcp_ack)
+				action = TCP_DATA;
+		}
+	break;
+	case TCP_ESTABLISHED:
+		debug_cond(DEBUG_INT_STATE,
+			   "TCP_ESTABLISHED %x\n", tcp_flags);
+		if (*tcp_seq_num > tcp_seq_max)
+			tcp_seq_max = *tcp_seq_num;
+		if (payload_len > 0) {
+			tcp_hole(*tcp_seq_num, payload_len, tcp_seq_max);
+			tcp_fin = TCP_DATA;  /* cause standalone FIN */
+		}
+
+		if ((tcp_fin) && (tcp_lost.len <= TCP_OPT_LEN_2)) {
+			action    = action | TCP_FIN | TCP_PUSH | TCP_ACK;
+			tcp_state =  TCP_CLOSE_WAIT;
+		} else {
+			if (tcp_ack)
+				action = TCP_DATA;
+		}
+		if (tcp_push)
+			action = action | TCP_PUSH;
+		if (tcp_syn)
+			action = TCP_ACK + TCP_RST;
+	break;
+	case TCP_CLOSE_WAIT:
+		debug_cond(DEBUG_INT_STATE, "TCP_CLOSE_WAIT (%x)\n", tcp_flags);
+		action = TCP_DATA;			/* Wait for app	*/
+	break;
+	case TCP_FIN_WAIT_2:
+		debug_cond(DEBUG_INT_STATE, "TCP_FIN_WAIT_2 (%x)\n", tcp_flags);
+		if (tcp_fin)
+			action =  TCP_DATA;
+		if (tcp_syn)
+			action =  TCP_DATA;
+		if (tcp_ack) {
+			action =  TCP_PUSH | TCP_ACK;
+			tcp_state = TCP_CLOSED;
+			puts("\n");
+		}
+	break;
+	case TCP_FIN_WAIT_1:
+		debug_cond(DEBUG_INT_STATE, "TCP_FIN_WAIT_1 (%x)\n", tcp_flags);
+		if (tcp_fin) {
+			action = TCP_ACK | TCP_FIN;
+			 tcp_state = TCP_FIN_WAIT_2;
+		}
+		if (tcp_syn)
+			action =  TCP_RST;
+		if (tcp_ack) {
+			tcp_state = TCP_CLOSED;
+			tcp_seq_num = tcp_seq_num + 1;
+		}
+	break;
+	case TCP_CLOSING:
+		debug_cond(DEBUG_INT_STATE, "TCP_CLOSING (%x)\n", tcp_flags);
+		if (tcp_fin)
+			action = TCP_DATA;
+		if (tcp_syn)
+			action = TCP_RST;
+		if (tcp_ack) {
+			action = TCP_PUSH;
+			tcp_state = TCP_CLOSED;
+			puts("\n");
+		}
+	break;
+	}
+	return action;
+}
+
+void rxhand_tcp_f(union tcp_build_pkt *b, unsigned int pkt_len)
+{
+	int tcp_len     = pkt_len - IP_HDR_SIZE;
+	u16 tcp_rx_xsum = b->ip.hdr.ip_sum;
+	u8  tcp_action  = TCP_DATA;
+	u32 tcp_seq_num;
+	u32 tcp_ack_num;
+	struct in_addr action_and_state;
+
+	int tcp_hdr_len;
+	int payload_len;
+
+	/*
+	 * Verify IP header
+	 */
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP RX in RX Sum (to=%pI4, from=%pI4, len=%d)\n",
+			   &b->ip.hdr.ip_src, &b->ip.hdr.ip_dst, pkt_len);
+
+		debug_cond(DEBUG_DEV_PKT,
+			   "In__________________________________________\n");
+
+	b->ip.hdr.ip_src	= net_server_ip;
+	b->ip.hdr.ip_dst	= net_ip;
+	b->ip.hdr.ip_sum	= 0x0000;
+	if (tcp_rx_xsum != compute_ip_checksum(b, IP_HDR_SIZE)) {
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP RX IP xSum Error (%pI4, =%pI4, len=%d)\n",
+			   &net_ip, &net_server_ip, pkt_len);
+		return;
+	}
+
+	/*
+	 * Build Pseudo header and Verify TCP header
+	 */
+	tcp_rx_xsum = b->ip.hdr.tcp_xsum;
+	b->ip.hdr.tcp_xsum = 0x0000;
+	if (tcp_rx_xsum != tcp_set_pseudo_header((uchar *)b, b->ip.hdr.ip_src,
+						 b->ip.hdr.ip_dst, tcp_len,
+						 pkt_len)) {
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP RX TCP xSum Error (%pI4, %pI4, len=%d)\n",
+			   &net_ip, &net_server_ip, tcp_len);
+		return;
+	}
+
+	tcp_hdr_len = (b->ip.hdr.tcp_hlen >> 2);
+	payload_len = tcp_len - tcp_hdr_len;
+
+	if (tcp_hdr_len > TCP_HDR_SIZE)
+		tcp_parse_options((uchar *)b + IP_TCP_HDR_SIZE,
+				  tcp_hdr_len - TCP_HDR_SIZE);
+	/*
+	 * Incoming sequence and ack numbers are server's view of the numbers.
+	 * The app must swap the numbers when responding.
+	 */
+
+	tcp_seq_num = ntohl(b->ip.hdr.tcp_seq);
+	tcp_ack_num = ntohl(b->ip.hdr.tcp_ack);
+
+	/*
+	 * Sent first packet send from server as first to app
+	 * Other packets could be ordered, but are currently not ordered.
+	 */
+
+	tcp_action  = tcp_state_machine(b->ip.hdr.tcp_flags,
+					&tcp_seq_num, payload_len);
+
+	/*
+	 * State altering command to be sent.
+	 * The packet sequence and ack numbers are in the tcp_seq_num
+	 * and tcp_ack_num variables. The current packet, its position
+	 * in the data stream, is the in the range of those variables.
+	 *
+	 * In the "application push" invocation, the TCP header with all
+	 * its information is pointed to by the packet pointer.
+	 *
+	 * in the typedef
+	 *      void rxhand_tcp(uchar *pkt, unsigned int dport,
+	 *                      struct in_addr sip, unsigned int sport,
+	 *                      unsigned int len);
+	 * *pkt is the pointer to the payload
+	 * dport is used for tcp_seg_num
+	 * action_and_state.s_addr is used for TCP state
+	 * sport is used for tcp_ack_num (which is unused by the app)
+	 * pkt_ length is the payload length.
+	 *
+	 * TCP_PUSH from the state machine with a payload length of 0 is a
+	 * connect or disconnect event
+	 */
+
+	tcp_activity_count++;
+	if (tcp_activity_count > TCP_ACTIVITY) {
+		puts("| ");
+		tcp_activity_count = 0;
+	}
+
+	if ((tcp_action & TCP_PUSH) || (payload_len > 0)) {
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Notify (action=%x, Seq=%d,Ack=%d,Pay%d)\n",
+			   tcp_action, tcp_seq_num, tcp_ack_num, payload_len);
+
+		action_and_state.s_addr = tcp_action;
+		(*tcp_packet_handler) ((uchar *)b + pkt_len - payload_len,
+				       tcp_seq_num, action_and_state,
+				       tcp_ack_num, payload_len);
+
+	} else if (tcp_action != TCP_DATA) {
+		debug_cond(DEBUG_DEV_PKT,
+			   "TCP Action (action=%x,Seq=%d,Ack=%d,Pay=%d)\n",
+			   tcp_action, tcp_seq_num, tcp_ack_num, payload_len);
+
+	/*
+	 * Warning: Incoming Seq & Ack  sequence numbers are transposed here
+	 * to outgoing Seq & Ack sequence numbers
+	 */
+		net_send_tcp_packet(0, ntohs(b->ip.hdr.tcp_src),
+				    ntohs(b->ip.hdr.tcp_dst),
+				    (tcp_action & (~TCP_PUSH)),
+				    tcp_seq_num, tcp_ack_num);
+	}
+}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-14 23:43 [U-Boot] [PATCH v10 0/3] Why netboot: DH at synoia.com
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot DH at synoia.com
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 2/3] Adding TCP DH at synoia.com
@ 2018-04-14 23:43 ` DH at synoia.com
  2018-04-17 15:10   ` Simon Glass
  2018-05-01  2:18   ` Joe Hershberger
  2018-05-01  1:57 ` [U-Boot] [PATCH v10 0/3] Why netboot: Joe Hershberger
  3 siblings, 2 replies; 22+ messages in thread
From: DH at synoia.com @ 2018-04-14 23:43 UTC (permalink / raw)
  To: u-boot

From: Duncan Hare <DuncanCHare@yahoo.com>

Why http and wget:

HTTP is the most efficient file retrieval protocol in common
use. The client send a single request, after TCP connection,
to receive a file of any length.

WGET is the application which implements http file transfer
outside browsers as a file transfer protocol. Versions of
wget exists on many operating systems.
END

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

Changes in v10: None

 cmd/Kconfig        |   5 +
 cmd/net.c          |  13 ++
 include/net.h      |  16 +-
 include/net/wget.h |  17 +++
 net/Kconfig        |   7 +-
 net/Makefile       |   1 +
 net/wget.c         | 420 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 470 insertions(+), 9 deletions(-)
 create mode 100644 include/net/wget.h
 create mode 100644 net/wget.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 136836d146..1113d69950 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1094,6 +1094,11 @@ 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.
 endif
 
 endmenu
diff --git a/cmd/net.c b/cmd/net.c
index d7c776aacf..6a7a51f357 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:]path and image name]"
+);
+#endif
+
 static void netboot_update_env(void)
 {
 	char tmp[22];
diff --git a/include/net.h b/include/net.h
index e29d804a23..ec44bf2bec 100644
--- a/include/net.h
+++ b/include/net.h
@@ -15,10 +15,14 @@
 #include <asm/cache.h>
 #include <asm/byteorder.h>	/* for nton* / ntoh* stuff */
 
-#define DEBUG_LL_STATE  0	/* Link local state machine changes */
-#define DEBUG_DEV_PKT   0	/* Packets or info directed to the device */
+#define DEBUG_LL_STATE  0	/* Link local state machine changes        */
+#define DEBUG_DEV_PKT   0	/* Packets or info directed to the device  */
 #define DEBUG_NET_PKT   0	/* Packets on info on the network at large */
-#define DEBUG_INT_STATE 0	/* Internal network state changes */
+#define DEBUG_INT_STATE 0	/* Internal network state change           */
+
+#if defined(CONFIG_TCP)
+#define CONFIG_SYS_RX_ETH_BUFFER 12     /* For TCP */
+#endif
 
 /*
  *	The number of receive packet buffers, and the required packet buffer
@@ -31,10 +35,6 @@
  *	data, the sending TCP will stop sending.
  */
 
-#if defined(CONFIG_TCP)
-#define CONFIG_SYS_RX_ETH_BUFFER 12	/* For TCP */
-#endif
-
 #ifdef CONFIG_SYS_RX_ETH_BUFFER
 # define PKTBUFSRX	CONFIG_SYS_RX_ETH_BUFFER
 #else
@@ -362,8 +362,8 @@ struct vlan_ethernet_hdr {
 #define PROT_PPP_SES	0x8864		/* PPPoE session messages	*/
 
 #define IPPROTO_ICMP	 1	/* Internet Control Message Protocol	*/
+#define IPPROTO_TCP      6      /* Transmission Control Protocol        */
 #define IPPROTO_UDP	17	/* User Datagram Protocol		*/
-#define IPPROTO_TCP	 6	/* Transmission Control Protocol        */
 
 /*
  *	Internet Protocol (IP) header.
diff --git a/include/net/wget.h b/include/net/wget.h
new file mode 100644
index 0000000000..dce16c573d
--- /dev/null
+++ b/include/net/wget.h
@@ -0,0 +1,17 @@
+/*
+ * Duncan Hare Copyright 2017
+ */
+
+void wget_start(void);			/* Begin wget */
+
+enum WGET_STATE {
+	WGET_CLOSED,
+	WGET_CONNECTING,
+	WGET_CONNECTED,
+	WGET_TRANSFERRING,
+	WGET_TRANSFERRED};
+
+#define	DEBUG_WGET		0	/* Set to 1 for debug messges */
+#define	SERVER_PORT		80
+#define	WGET_RETRY_COUNT	30
+#define	WGET_TIMEOUT		2000UL
diff --git a/net/Kconfig b/net/Kconfig
index cc6dd83fc4..c973def3b8 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -36,7 +36,12 @@ config NET_TFTP_VARS
 config TCP
 	bool "Include Subset TCP stack for wget"
 	help
-	  TCP protocol support for wget.
+	  Selecting TCP protocol support adds TCP for receiving
+	  streams of data. The TCP packets are presented
+	  as received (possibly out of order). The application
+	  receiving the TCP stream places the date by sequence number
+	  as an offset from the kernel address. TCP transmission is
+	  not supported.
 
 config BOOTP_BOOTPATH
 	bool "Enable BOOTP BOOTPATH"
diff --git a/net/Makefile b/net/Makefile
index 25729ebeeb..f83df5b728 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CMD_RARP) += rarp.o
 obj-$(CONFIG_CMD_SNTP) += sntp.o
 obj-$(CONFIG_CMD_NET)  += tftp.o
 obj-$(CONFIG_TCP)      += tcp.o
+obj-$(CONFIG_CMD_WGET) += wget.o
 # Disable this warning as it is triggered by:
 # sprintf(buf, index ? "foo%d" : "foo", index)
 # and this is intentional usage.
diff --git a/net/wget.c b/net/wget.c
new file mode 100644
index 0000000000..5429a1f7b1
--- /dev/null
+++ b/net/wget.c
@@ -0,0 +1,420 @@
+/*
+ * WGET/HTTP support driver based on U-BOOT's nfs.c
+ * Copyright Duncan Hare <dh@synoia.com> 2017
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <mapmem.h>
+#include <net.h>
+#include <net/wget.h>
+#include <net/tcp.h>
+
+const char bootfile1[]   = "GET ";
+const  char bootfile3[]   = " HTTP/1.0\r\n\r\n";
+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 web_server_ip;
+static int our_port;
+static int wget_timeout_count;
+
+struct pkt_qd {
+	uchar *pkt;
+	unsigned int tcp_seq_num;
+	unsigned int len;
+};
+
+/*
+ * This is a control structure for out of order packets received.
+ * The actual packet bufers are in the kernel space, and are
+ * expected to be overwritten by the downloaded image.
+ */
+
+static struct pkt_qd pkt_q[PKTBUFSRX / 4];
+static int pkt_q_idx;
+static unsigned long content_length;
+static unsigned int packets;
+
+static unsigned int initial_data_seq_num;
+
+static enum  WGET_STATE wget_state;
+
+static char *image_url;
+static unsigned int wget_timeout = WGET_TIMEOUT;
+
+static void  wget_timeout_handler(void);
+
+static enum net_loop_state wget_loop_state;
+
+/* Timeout retry parameters */
+static u8 r_action;
+static unsigned int r_tcp_ack_num;
+static unsigned int r_tcp_seq_num;
+static int r_len;
+
+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_WGET */
+
+		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;
+}
+
+/*
+ * wget response dispatcher
+ * WARNING, This, and only this, is the place in wget,c where
+ * SEQUENCE NUMBERS are swapped between incoming (RX)
+ * and outgoing (TX).
+ * Procedure wget_handler() is correct for RX traffic.
+ */
+static void wget_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 (wget_state) {
+	case WGET_CLOSED:
+		debug_cond(DEBUG_WGET, "wget: send SYN\n");
+		wget_state = WGET_CONNECTING;
+		net_send_tcp_packet(0, SERVER_PORT, our_port, action,
+				    tcp_seq_num, tcp_ack_num);
+		packets = 0;
+	break;
+	case WGET_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, image_url, strlen(image_url));
+		offset = offset + strlen(image_url);
+
+		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);
+		wget_state = WGET_CONNECTED;
+	break;
+	case WGET_CONNECTED:
+	case WGET_TRANSFERRING:
+	case WGET_TRANSFERRED:
+		net_send_tcp_packet(0, SERVER_PORT, our_port, action,
+				    tcp_seq_num, tcp_ack_num);
+	break;
+	}
+}
+
+static void wget_send(u8 action, unsigned int tcp_ack_num,
+		      unsigned int tcp_seq_num, int len)
+{
+	r_action        = action;
+	r_tcp_ack_num   = tcp_ack_num;
+	r_tcp_seq_num   = tcp_seq_num;
+	r_len           = len;
+	wget_send_stored();
+}
+
+void wget_fail(char *error_message, unsigned int tcp_seq_num,
+	       unsigned int tcp_ack_num, u8 action)
+{
+	printf("%s", error_message);
+	printf("%s", "wget: Transfer Fail\n");
+	net_set_timeout_handler(0, NULL);
+	wget_send(action, tcp_seq_num, tcp_ack_num, 0);
+}
+
+void wget_success(u8 action, unsigned int tcp_seq_num,
+		  unsigned int tcp_ack_num, int len, int packets)
+{
+	printf("Packets received %d, Transfer Successful\n", packets);
+	wget_send(action, tcp_seq_num, tcp_ack_num, len);
+}
+
+/*
+ * Interfaces of U-BOOT
+ */
+static void wget_timeout_handler(void)
+{
+	if (++wget_timeout_count > WGET_RETRY_COUNT) {
+		puts("\nRetry count exceeded; starting again\n");
+		wget_send(TCP_RST, 0, 0, 0);
+		net_start_again();
+	} else {
+		puts("T ");
+		net_set_timeout_handler(wget_timeout +
+					WGET_TIMEOUT * wget_timeout_count,
+					wget_timeout_handler);
+		wget_send_stored();
+	}
+}
+
+static void wget_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;
+	int  i;
+
+	pkt[len] = '\0';
+	pos = strstr((char *)pkt, http_eom);
+
+	if (pos == 0) {
+		debug_cond(DEBUG_WGET,
+			   "wget: 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(DEBUG_WGET, "wget: 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);
+
+		wget_state = WGET_TRANSFERRING;
+
+		if (strstr((char *)pkt, http_ok) == 0) {
+			debug_cond(DEBUG_WGET,
+				   "wget: Connected Bad Xfer\n");
+			wget_loop_state = NETLOOP_FAIL;
+			wget_send(action, tcp_seq_num, tcp_ack_num, len);
+		} else {
+			debug_cond(DEBUG_WGET,
+				   "wget: 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) + 2;
+				strict_strtoul(pos, 10, &content_length);
+				debug_cond(DEBUG_WGET,
+					   "wget: Connected Len %lu\n",
+					   content_length);
+			}
+
+			net_boot_file_size = 0;
+
+			if (len > hlen)
+				store_block(pkt + hlen, 0, len - hlen);
+			debug_cond(DEBUG_WGET,
+				   "wget: 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(DEBUG_WGET,
+				   "wget: Connctd pkt Q %p len %x\n",
+				   pkt_q[i].pkt, pkt_q[i].len);
+			}
+		}
+	}
+	wget_send(action, tcp_seq_num, tcp_ack_num, len);
+}
+
+	/*
+	 * In the "application push" invocation, the TCP header with all
+	 * its information is pointed to by the packet pointer.
+	 *
+	 * in the typedef
+	 *      void rxhand_tcp(uchar *pkt, unsigned int dport,
+	 *                      struct in_addr sip, unsigned int sport,
+	 *                      unsigned int len);
+	 * *pkt is the pointer to the payload
+	 * dport is used for tcp_seg_num
+	 * action_and_state.s_addr is used for TCP state
+	 * sport is used for tcp_ack_num (which is unused by the app)
+	 * pkt_ length is the payload length.
+	 */
+static void wget_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 wget_tcp_state = tcp_get_tcp_state();
+	u8 action = action_and_state.s_addr;
+
+	net_set_timeout_handler(wget_timeout, wget_timeout_handler);
+	packets++;
+
+	switch (wget_state) {
+	case WGET_CLOSED:
+		debug_cond(DEBUG_WGET, "wget: Handler: Error!, State wrong\n");
+	break;
+	case WGET_CONNECTING:
+		debug_cond(DEBUG_WGET,
+			   "wget: Connecting In len=%x, Seq=%x, Ack=%x\n",
+			   len, tcp_seq_num, tcp_ack_num);
+		if (len == 0) {
+			if (wget_tcp_state == TCP_ESTABLISHED) {
+				debug_cond(DEBUG_WGET,
+					   "wget: Cting, send, len=%x\n", len);
+			wget_send(action, tcp_seq_num, tcp_ack_num, len);
+			} else {
+				tcp_print_buffer(pkt, len, len, -1, 0);
+				wget_fail("wget: Handler Connected Fail\n",
+					  tcp_seq_num, tcp_ack_num, action);
+			}
+		}
+	break;
+	case WGET_CONNECTED:
+		debug_cond(DEBUG_WGET, "wget: Connected seq=%x, len=%x\n",
+			   tcp_seq_num, len);
+		if (len == 0) {
+			wget_fail("Image not found, no data returned\n",
+				  tcp_seq_num, tcp_ack_num, action);
+		} else {
+			wget_connected(pkt, tcp_seq_num, action_and_state,
+				       tcp_ack_num, len);
+		}
+	break;
+	case WGET_TRANSFERRING:
+		debug_cond(DEBUG_WGET,
+			   "wget: 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) {
+			wget_fail("wget: store error\n",
+				  tcp_seq_num, tcp_ack_num, action);
+		} else {
+			switch (wget_tcp_state) {
+			case TCP_FIN_WAIT_2:
+				wget_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:
+				wget_send(TCP_ACK, tcp_seq_num, tcp_ack_num,
+					  len);
+				wget_loop_state = NETLOOP_SUCCESS;
+			break;
+			case TCP_CLOSE_WAIT:     /* End of transfer */
+				wget_state = WGET_TRANSFERRED;
+				wget_send(action | TCP_ACK | TCP_FIN,
+					  tcp_seq_num, tcp_ack_num, len);
+			break;
+			}
+		}
+	break;
+	case WGET_TRANSFERRED:
+		printf("Packets received %d, Transfer Successful\n", packets);
+		net_set_state(wget_loop_state);
+	break;
+	}
+}
+
+void wget_start(void)
+{
+	debug_cond(DEBUG_WGET, "%s\n", __func__);
+
+	image_url = strchr(net_boot_file_name, ':');
+	if (!image_url) {
+		web_server_ip = string_to_ip(net_boot_file_name);
+		++image_url;
+	} else {
+		web_server_ip = net_server_ip;
+		image_url = net_boot_file_name;
+	}
+
+	debug_cond(DEBUG_WGET,
+		   "wget: Transfer HTTP Server %pI4; our IP %pI4\n",
+		   &web_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_cond(DEBUG_WGET,
+				   "wget: sending through gateway %pI4",
+				   &net_gateway);
+	}
+	debug_cond(DEBUG_WGET, "URL '%s\n", image_url);
+
+	if (net_boot_file_expected_size_in_blocks) {
+		debug_cond(DEBUG_WGET, "wget: Size is 0x%x Bytes = ",
+			   net_boot_file_expected_size_in_blocks << 9);
+		print_size(net_boot_file_expected_size_in_blocks << 9, "");
+	}
+	debug_cond(DEBUG_WGET,
+		   "\nwget:Load address: 0x%lx\nLoading: *\b", load_addr);
+
+	net_set_timeout_handler(wget_timeout, wget_timeout_handler);
+	tcp_set_tcp_handler(wget_handler);
+
+	wget_timeout_count = 0;
+	wget_state = WGET_CLOSED;
+
+	our_port = random_port();
+
+	/* zero out server ether in case the server ip has changed */
+	memset(net_server_ethaddr, 0, 6);
+
+	wget_send(TCP_SYN, 0, 0, 0);
+}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 3/3] Adding wget DH at synoia.com
@ 2018-04-17 15:10   ` Simon Glass
  2018-04-18 15:50     ` Duncan Hare
                       ` (2 more replies)
  2018-05-01  2:18   ` Joe Hershberger
  1 sibling, 3 replies; 22+ messages in thread
From: Simon Glass @ 2018-04-17 15:10 UTC (permalink / raw)
  To: u-boot

Hi Duncan,

On 14 April 2018 at 17:43,  <DH@synoia.com> wrote:
> From: Duncan Hare <DuncanCHare@yahoo.com>
>
> Why http and wget:
>
> HTTP is the most efficient file retrieval protocol in common
> use. The client send a single request, after TCP connection,
> to receive a file of any length.
>
> WGET is the application which implements http file transfer
> outside browsers as a file transfer protocol. Versions of
> wget exists on many operating systems.
> END

Why END?

>
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> ---
>
> Changes in v10: None

There is no change log here. Has nothing changed since v1?

This code looks OK to me, but please can you run it through patman? I
see what look like some style errors.

How can we create a test for this? Can we add something to test_net.py ?

Regards,
Simon

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-17 15:10   ` Simon Glass
@ 2018-04-18 15:50     ` Duncan Hare
       [not found]     ` <217820715.1487025.1524002336830@mail.yahoo.com>
  2018-05-01  1:50     ` [U-Boot] " Joe Hershberger
  2 siblings, 0 replies; 22+ messages in thread
From: Duncan Hare @ 2018-04-18 15:50 UTC (permalink / raw)
  To: u-boot




Hi Duncan,

On 14 April 2018 at 17:43,  <DH@synoia.com> wrote:
>> From: Duncan Hare <DuncanCHare@yahoo.com>
>>
> Why http and wget:
>> END

Why END?
Don't know.  will delete.

>
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> ---
>
> Changes in v10: None

There is no change log here. Has nothing changed since v1?
changed c to C for cover meno tag.

This code looks OK to me, but please can you run it through patman? I
see what look like some style errors.
Has passed patman. Only error messages are for packed structures.

How can we create a test for this? Can we add something to test_net.py ?
Do not see why not. It's only an app and has no hardware dependencies. 
One needs a web server and test file, and code the verify the integrity of the test file in local memory.I used a file with line numbers and a pattern repeated on every "line". Make it easy to findinvalid offsets mis ordered packets.
We use nginx as the web server. We used wbox for a while, but then mvbed to nginx. We could not get Apacheconfigured.
For internet test we use a Ubuntu image & nginx on the Amazon cloud. 
The internet path to Amazon does a nice job of scrambling the order of packets.

Regards,
Duncan


   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
       [not found]       ` <CAPnjgZ3yNbMVJCbNQVDxxQ9RaX1pujxKSopk=s1MNUyb=oAyiQ@mail.gmail.com>
@ 2018-04-23  3:22         ` Duncan Hare
  2018-04-25  5:01           ` Simon Glass
  0 siblings, 1 reply; 22+ messages in thread
From: Duncan Hare @ 2018-04-23  3:22 UTC (permalink / raw)
  To: u-boot



>From: Simon Glass <sjg@chromium.org>
 >To: Duncan Hare <dh@synoia.com> 
 >Sent: Sunday, April 22, 2018 1:10 PM
 >Subject: Re: [PATCH v10 3/3] Adding wget
   >
>Hi Duncan,

>On 17 April 2018 at 15:58, Duncan Hare <dh@synoia.com> wrote:
>> From: Simon Glass <sjg@chromium.org>
>> It has been through patman, and the only errors flagged a packed structures,
>> necessary for packed headers.

>It should be possible in the Python test to enable an http server on localhost with a few lines of code, and then connect to it in the test?
Yes server at port 80. The server can be tested with the wget command which can be installed on linux.I doubt that loop-back like this will produce the scrambling of packet order which is a feature of push down stacks for packet queuesin the internet.
The pi is easy to overrun when testing on a low latency network, because the TCP send window size is defined by the number ofnet_rx_buffers, which is controlled the CONFIG_SYS_RX_ETH_BUFFER in include.net.h,  which sets PKTBUFSRX at the beginning of include/net.h, 
which in-turn defines a buffer pool in net/net.c, array named  net_pkt_buf.
Hence my comment in a different thread about buffering on the pi. Few of the socs appear to use net_pkt_buf  buffers for net traffic.

If there are too many transmission errors the sending tcp drops the connection. My solution to this is to halve the size of 
CONFIG_SYS_RX_ETH_BUFFER until transmission works.
If I was thinking about a buffer pool for in the drivers, I'd implement it in net/net.c. Interface "getbuffer," returns a pointer, 
and increments an index to the next net_rx_buffer with no protection for overrun. Overrun is managed by ack numbers in TCPand timeout in UDP.

Possibly CONFIG_SYS_RX_ETH_BUFFER could come under Kconfig.

>Regards,
>Simon
RegardsDuncan


   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-23  3:22         ` Duncan Hare
@ 2018-04-25  5:01           ` Simon Glass
  2018-04-25 14:33             ` Duncan Hare
  0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2018-04-25  5:01 UTC (permalink / raw)
  To: u-boot

Hi Duncan,

On 22 April 2018 at 21:22, Duncan Hare <dh@synoia.com> wrote:
>
>
> ________________________________
>>From: Simon Glass <sjg@chromium.org>
>>To: Duncan Hare <dh@synoia.com>
>>Sent: Sunday, April 22, 2018 1:10 PM
>>Subject: Re: [PATCH v10 3/3] Adding wget
>>
>>Hi Duncan,
>
>>On 17 April 2018 at 15:58, Duncan Hare <dh@synoia.com> wrote:
>>> From: Simon Glass <sjg@chromium.org>
>
>>> It has been through patman, and the only errors flagged a packed
>>> structures,
>>> necessary for packed headers.
>
>>It should be possible in the Python test to enable an http server on
>> localhost with a few lines of code, and then connect to it in the test?
>
> Yes server at port 80. The server can be tested with the wget command
which
> can be installed on linux.
> I doubt that loop-back like this will produce the scrambling of packet
order
> which is a feature of push down stacks for packet queues
> in the internet.
>
> The pi is easy to overrun when testing on a low latency network, because
the
> TCP send window size is defined by the number of
> net_rx_buffers, which is controlled the CONFIG_SYS_RX_ETH_BUFFER in
> include.net.h,  which sets PKTBUFSRX at the beginning of include/net.h,
> which in-turn defines a buffer pool in net/net.c, array named
net_pkt_buf.
>
> Hence my comment in a different thread about buffering on the pi. Few of
the
> socs appear to use net_pkt_buf  buffers for net traffic.
>
> If there are too many transmission errors the sending tcp drops the
> connection. My solution to this is to halve the size of
> CONFIG_SYS_RX_ETH_BUFFER until transmission works.
>
> If I was thinking about a buffer pool for in the drivers, I'd implement it
> in net/net.c. Interface "getbuffer," returns a pointer,
> and increments an index to the next net_rx_buffer with no protection for
> overrun. Overrun is managed by ack numbers in TCP
> and timeout in UDP.
>
> Possibly CONFIG_SYS_RX_ETH_BUFFER could come under Kconfig.

Just to be clear, I was wondering about having an automated test. Manual
tests are not very useful since people won't do them. See 'make tests' for
all the test that we currently run. I'm pretty sure you could standard up a
little server, run your wget, then shut it down, all within a pytest test.

Regards,
Simon

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-25  5:01           ` Simon Glass
@ 2018-04-25 14:33             ` Duncan Hare
  2018-04-25 23:44               ` Simon Glass
  0 siblings, 1 reply; 22+ messages in thread
From: Duncan Hare @ 2018-04-25 14:33 UTC (permalink / raw)
  To: u-boot



 From: Simon Glass <sjg@chromium.org>
 To: Duncan Hare <dh@synoia.com> 
Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Joe Hershberger <joe.hershberger@ni.com>
 Sent: Tuesday, April 24, 2018 10:01 PM
 Subject: Re: [PATCH v10 3/3] Adding wget
   
Hi Duncan,

On 22 April 2018 at 21:22, Duncan Hare <dh@synoia.com> wrote:
>
>>The server can be tested with the wget command which
>> can be installed on linux.
>> I doubt that loop-back like this will produce the scrambling of packet order
>> which is a feature of push down stacks for packet queues
>> in the internet.
>>
>> Hence my comment in a different thread about buffering on the pi. Few of the
>> socs appear to use net_pkt_buf  buffers for net traffic.
>>
>> If there are too many transmission errors the sending tcp drops the
>> connection. My solution to this is to halve the size of
>> CONFIG_SYS_RX_ETH_BUFFER until transmission works.
>>
 > 
>> Possibly CONFIG_SYS_RX_ETH_BUFFER could come under Kconfig.
>
>>Just to be clear, I was wondering about having an automated test. Manual tests are not very useful since people won't do them. See 'make tests' for all the test that we >currently >run. I'm pretty sure you could standard up a little server, run your wget, then shut it down, all within a pytest test.

>>Regards,
>>Simon

Hi Wolfgang. Simon
Can we put a test 4 Mbyte kernel on the u-boot website for an automated test for other users of TCP & Wget in u-boot?
Then I can produce a standard u-boot script for testing.
RegardsDuncan Hare
   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-25 14:33             ` Duncan Hare
@ 2018-04-25 23:44               ` Simon Glass
  2018-04-25 23:52                 ` Duncan Hare
  0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2018-04-25 23:44 UTC (permalink / raw)
  To: u-boot

Hi Duncan,

On 25 April 2018 at 08:33, Duncan Hare <dh@synoia.com> wrote:
>
>
> ________________________________
> From: Simon Glass <sjg@chromium.org>
> To: Duncan Hare <dh@synoia.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Joe Hershberger
> <joe.hershberger@ni.com>
> Sent: Tuesday, April 24, 2018 10:01 PM
> Subject: Re: [PATCH v10 3/3] Adding wget
>
> Hi Duncan,
>
> On 22 April 2018 at 21:22, Duncan Hare <dh@synoia.com> wrote:
>>
>>>The server can be tested with the wget command which
>>> can be installed on linux.
>>> I doubt that loop-back like this will produce the scrambling of packet
>>> order
>>> which is a feature of push down stacks for packet queues
>>> in the internet.
>>>
>>> Hence my comment in a different thread about buffering on the pi. Few of
>>> the
>>> socs appear to use net_pkt_buf  buffers for net traffic.
>>>
>>> If there are too many transmission errors the sending tcp drops the
>>> connection. My solution to this is to halve the size of
>>> CONFIG_SYS_RX_ETH_BUFFER until transmission works.
>>>
>  >
>>> Possibly CONFIG_SYS_RX_ETH_BUFFER could come under Kconfig.
>>
>>>Just to be clear, I was wondering about having an automated test. Manual
>>> tests are not very useful since people won't do them. See 'make tests' for
>>> all the test that we >currently >run. I'm pretty sure you could standard up
>>> a little server, run your wget, then shut it down, all within a pytest test.
>
>
>>>Regards,
>>>Simon
>
> Hi Wolfgang. Simon
>
> Can we put a test 4 Mbyte kernel on the u-boot website for an automated test
> for other users of TCP & Wget in u-boot?
>
> Then I can produce a standard u-boot script for testing.

How about the test just creates a little (4KB) file. We don't want the
tests to access a real network, if possible, just use localhost.

Regards,
Simon

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-25 23:44               ` Simon Glass
@ 2018-04-25 23:52                 ` Duncan Hare
  2018-05-01  1:54                   ` Joe Hershberger
  0 siblings, 1 reply; 22+ messages in thread
From: Duncan Hare @ 2018-04-25 23:52 UTC (permalink / raw)
  To: u-boot



   From: Simon Glass <sjg@chromium.org>
 To: Duncan Hare <dh@synoia.com> 
Cc: Wolfgang Denk <wd@denx.de>; U-Boot Mailing List <u-boot@lists.denx.de>; Joe Hershberger <joe.hershberger@ni.com>
 Sent: Wednesday, April 25, 2018 4:44 PM
 Subject: Re: [PATCH v10 3/3] Adding wget
   
Hi Duncan,

On 25 April 2018 at 08:33, Duncan Hare <dh@synoia.com> wrote:
____________________
>> From: Simon Glass <sjg@chromium.org>
>> To: Duncan Hare <dh@synoia.com>
>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Joe Hershberger
>> <joe.hershberger@ni.com>
>> Sent: Tuesday, April 24, 2018 10:01 PM
>> Subject: Re: [PATCH v10 3/3] Adding wget
>>
>> Hi Duncan,
>>
>>> On 22 April 2018 at 21:22, Duncan Hare <dh@synoia.com> wrote:
>>>
>>>>The server can be tested with the wget command which
>>>> can be installed on linux.
>>>> I doubt that loop-back like this will produce the scrambling of packet
>>>> order
>>>> which is a feature of push down stacks for packet queues
>>>> in the internet.
>>>>
>>>> Hence my comment in a different thread about buffering on the pi. Few of
>>>> the
>>>> socs appear to use net_pkt_buf  buffers for net traffic.
>>>>
>>>> If there are too many transmission errors the sending tcp drops the
>>>> connection. My solution to this is to halve the size of
>>>> CONFIG_SYS_RX_ETH_BUFFER until transmission works.
>>>>
>>> >
>>>> Possibly CONFIG_SYS_RX_ETH_BUFFER could come under Kconfig.
>>>
>>>Just to be clear, I was wondering about having an automated test. Manual
>>>> tests are not very useful since people won't do them. See 'make tests' for
>>>> all the test that we >currently >run. I'm pretty sure you could standard up
>>>> a little server, run your wget, then shut it down, all within a pytest test.
>>
>>
>>>>Regards,
>>>>Simon
>>
>> Hi Wolfgang. Simon
>>
>> Can we put a test 4 Mbyte kernel on the u-boot website for an automated test
>> for other users of TCP & Wget in u-boot?
>>
>> Then I can produce a standard u-boot script for testing.

>How about the test just creates a little (4KB) file. We don't want the
>tests to access a real network, if possible, just use localhost.

>Regards,
>Simon
4k is 4 packets. I believe most kernels are larger. 
I was think of a static server set up with a known dns name.
Thta's what I've got. 

Do the test setup once.



   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot DH at synoia.com
@ 2018-04-30 23:44   ` Joe Hershberger
  0 siblings, 0 replies; 22+ messages in thread
From: Joe Hershberger @ 2018-04-30 23:44 UTC (permalink / raw)
  To: u-boot

Hi Duncan,

Please change the subject of this patch to: "Prepare to add TCP and wget"

On Sat, Apr 14, 2018 at 6:43 PM,  <DH@synoia.com> wrote:
> From: Duncan Hare <DuncanCHare@yahoo.com>

Include in the body of the log a high-level description, such as:
"Consolidating UDP header functions to make it easier to add TCP
versions of the same, while reusing the IP portions. This patch should
not change any behaviors."

>
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> ---
>
> Added a protocol parameter to ip packet sending in net.c
> Added UDP protocol for current applications to minimize
> code changes to existing net apps.
>
> All the code is new, and not copied from any source.
>
>
> Changes in v10:
> Initial changes for adding TCP
>
>  include/net.h | 25 +++++++++++++++++++------
>  net/net.c     | 52 ++++++++++++++++++++++++++++++++++------------------
>  net/ping.c    |  9 ++-------
>  3 files changed, 55 insertions(+), 31 deletions(-)
>
> diff --git a/include/net.h b/include/net.h
> index 455b48f6c7..7e5f5a6a5b 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -15,17 +15,26 @@
>  #include <asm/cache.h>
>  #include <asm/byteorder.h>     /* for nton* / ntoh* stuff */
>
> -#define DEBUG_LL_STATE 0       /* Link local state machine changes */
> -#define DEBUG_DEV_PKT 0                /* Packets or info directed to the device */
> -#define DEBUG_NET_PKT 0                /* Packets on info on the network at large */
> +#define DEBUG_LL_STATE  0      /* Link local state machine changes */
> +#define DEBUG_DEV_PKT   0      /* Packets or info directed to the device */
> +#define DEBUG_NET_PKT   0      /* Packets on info on the network at large */

If you want to do general formatting cleanup, please put it in a
separate patch (as patch 1).

>  #define DEBUG_INT_STATE 0      /* Internal network state changes */
>
>  /*
>   *     The number of receive packet buffers, and the required packet buffer
>   *     alignment in memory.
>   *
> + *     The number of buffers for TCP is used to calculate a static TCP window
> + *     size, becuse TCP window size is a promise to the sending TCP to be able
> + *     to buffer up to the window size of data.
> + *     When the sending TCP has a window size of outstanding unacknowledged
> + *     data, the sending TCP will stop sending.

Please add this in the TCP patch.

>   */
>
> +#if defined(CONFIG_TCP)
> +#define CONFIG_SYS_RX_ETH_BUFFER 12    /* For TCP */

That comment is unnecessary... it is obvious from the ifdef
surrounding it. Remove the comment.

> +#endif
> +
>  #ifdef CONFIG_SYS_RX_ETH_BUFFER
>  # define PKTBUFSRX     CONFIG_SYS_RX_ETH_BUFFER
>  #else
> @@ -354,6 +363,7 @@ struct vlan_ethernet_hdr {
>
>  #define IPPROTO_ICMP    1      /* Internet Control Message Protocol    */
>  #define IPPROTO_UDP    17      /* User Datagram Protocol               */
> +#define IPPROTO_TCP     6      /* Transmission Control Protocol        */

Don't add this define in this patch. Save it for the TCP patch. When
you do add it, do so in numerical order.

>
>  /*
>   *     Internet Protocol (IP) header.
> @@ -596,10 +606,10 @@ int net_set_ether(uchar *xet, const uchar *dest_ethaddr, uint prot);
>  int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot);
>
>  /* Set IP header */
> -void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source);
> +void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
> +                      u16  pkt_len, u8 prot);
>  void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport,
> -                               int sport, int len);
> -
> +                       int sport, int len);

This is just a formatting change.

>  /**
>   * compute_ip_checksum() - Compute IP checksum
>   *
> @@ -670,6 +680,9 @@ static inline void net_send_packet(uchar *pkt, int len)
>   * @param sport Source UDP port
>   * @param payload_len Length of data after the UDP header
>   */
> +int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> +                      int payload_len, int proto);
> +

Separating with a blank line here and removing it above. Why the inconsistency?

>  int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
>                         int sport, int payload_len);
>
> diff --git a/net/net.c b/net/net.c
> index 4259c9e321..df4e7317e7 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -181,6 +181,7 @@ int         net_ntp_time_offset;
>  static uchar net_pkt_buf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
>  /* Receive packets */
>  uchar *net_rx_packets[PKTBUFSRX];
> +

Random formatting change.

>  /* Current UDP RX packet handler */
>  static rxhand_f *udp_packet_handler;
>  /* Current ARP RX packet handler */
> @@ -777,11 +778,23 @@ void net_set_timeout_handler(ulong iv, thand_f *f)
>  }
>
>  int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> -               int payload_len)
> +                       int payload_len)

Formatting change.

> +{
> +       return net_send_ip_packet(ether, dest, dport, sport, payload_len,
> +                                 IPPROTO_UDP);
> +}
> +
> +int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> +                      int payload_len, int proto)

Put the declarations and the definitions in the same order (this
should be before net_send_udp_packet() ).

>  {
>         uchar *pkt;
>         int eth_hdr_size;
>         int pkt_hdr_size;
> +       if (proto == IPPROTO_UDP) {
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "UDP Send  (to=%pI4, from=%pI4, len=%d)\n",
> +                          &dest, &net_ip, payload_len);
> +       }
>
>         /* make sure the net_tx_packet is initialized (net_init() was called) */
>         assert(net_tx_packet != NULL);
> @@ -799,9 +812,15 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>         pkt = (uchar *)net_tx_packet;
>
>         eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
> -       pkt += eth_hdr_size;
> -       net_set_udp_header(pkt, dest, dport, sport, payload_len);
> -       pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
> +       switch (proto) {
> +       case IPPROTO_UDP:
> +               net_set_udp_header(pkt + eth_hdr_size, dest,
> +                                  dport, sport, payload_len);
> +               pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
> +       break;
> +       default:
> +               return -1;

You should return a real -errno... -EINVAL.

> +       }
>
>         /* if MAC address was not discovered yet, do an ARP request */
>         if (memcmp(ether, net_null_ethaddr, 6) == 0) {
> @@ -1157,9 +1176,6 @@ void net_process_received_packet(uchar *in_packet, int len)
>                 /* Can't deal with anything except IPv4 */
>                 if ((ip->ip_hl_v & 0xf0) != 0x40)
>                         return;
> -               /* Can't deal with IP options (headers != 20 bytes) */
> -               if ((ip->ip_hl_v & 0x0f) > 0x05)
> -                       return;

This seems like it is more relevant to the TCP patch. What have you
added in this patch that changes what headers we can support. This
patch is supposed to refactor and not change behavior.

>                 /* Check the Checksum of the header */
>                 if (!ip_checksum_ok((uchar *)ip, IP_HDR_SIZE)) {
>                         debug("checksum bad\n");
> @@ -1205,6 +1221,7 @@ void net_process_received_packet(uchar *in_packet, int len)
>                  * we send a tftp packet to a dead connection, or when
>                  * there is no server at the other end.
>                  */
> +

Random formatting change.

>                 if (ip->ip_p == IPPROTO_ICMP) {
>                         receive_icmp(ip, len, src_ip, et);
>                         return;
> @@ -1365,8 +1382,7 @@ common:
>  }
>  /**********************************************************************/
>
> -int
> -net_eth_hdr_size(void)
> +int net_eth_hdr_size(void)

Random formatting change.

>  {
>         ushort myvlanid;
>
> @@ -1426,25 +1442,28 @@ int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot)
>         }
>  }
>
> -void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source)
> +void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
> +                      u16  pkt_len, u8 prot)
>  {
>         struct ip_udp_hdr *ip = (struct ip_udp_hdr *)pkt;
>
>         /*
> -        *      Construct an IP header.
> +        *      Construct an IP header.

Don't change formatting in a functional patch.

>          */
>         /* IP_HDR_SIZE / 4 (not including UDP) */
>         ip->ip_hl_v  = 0x45;
>         ip->ip_tos   = 0;
> -       ip->ip_len   = htons(IP_HDR_SIZE);
> +       ip->ip_len   = htons(pkt_len);

It used to be pretty misleading that this function would set a value
of this to include only the IP header, only to be overwritten later.
This is way better.

>         ip->ip_id    = htons(net_ip_id++);
> -       ip->ip_off   = htons(IP_FLAGS_DFRAG);   /* Don't fragment */
> +       ip->ip_off   = htons(IP_FLAGS_DFRAG);   /* Don't fragment */

Don't change formatting in a functional patch.

>         ip->ip_ttl   = 255;
> +       ip->ip_p     = prot;
>         ip->ip_sum   = 0;
>         /* already in network byte order */
>         net_copy_ip((void *)&ip->ip_src, &source);
>         /* already in network byte order */
>         net_copy_ip((void *)&ip->ip_dst, &dest);
> +       ip->ip_sum  = compute_ip_checksum(ip, IP_HDR_SIZE);
>  }
>
>  void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
> @@ -1460,11 +1479,8 @@ void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
>         if (len & 1)
>                 pkt[IP_UDP_HDR_SIZE + len] = 0;
>
> -       net_set_ip_header(pkt, dest, net_ip);
> -       ip->ip_len   = htons(IP_UDP_HDR_SIZE + len);
> -       ip->ip_p     = IPPROTO_UDP;
> -       ip->ip_sum   = compute_ip_checksum(ip, IP_HDR_SIZE);
> -
> +       net_set_ip_header(pkt, dest, net_ip, IP_UDP_HDR_SIZE + len,
> +                         IPPROTO_UDP);
>         ip->udp_src  = htons(sport);
>         ip->udp_dst  = htons(dport);
>         ip->udp_len  = htons(UDP_HDR_SIZE + len);
> diff --git a/net/ping.c b/net/ping.c
> index 9508cf1160..254b646193 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -20,16 +20,11 @@ struct in_addr net_ping_ip;
>  static void set_icmp_header(uchar *pkt, struct in_addr dest)
>  {
>         /*
> -        *      Construct an IP and ICMP header.
> +        *      Construct an ICMP header.
>          */
> -       struct ip_hdr *ip = (struct ip_hdr *)pkt;
>         struct icmp_hdr *icmp = (struct icmp_hdr *)(pkt + IP_HDR_SIZE);
>
> -       net_set_ip_header(pkt, dest, net_ip);
> -
> -       ip->ip_len   = htons(IP_ICMP_HDR_SIZE);
> -       ip->ip_p     = IPPROTO_ICMP;
> -       ip->ip_sum   = compute_ip_checksum(ip, IP_HDR_SIZE);
> +       net_set_ip_header(pkt, dest, net_ip, IP_ICMP_HDR_SIZE, IPPROTO_ICMP);
>
>         icmp->type = ICMP_ECHO_REQUEST;
>         icmp->code = 0;
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 2/3] Adding TCP
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 2/3] Adding TCP DH at synoia.com
@ 2018-05-01  1:44   ` Joe Hershberger
  0 siblings, 0 replies; 22+ messages in thread
From: Joe Hershberger @ 2018-05-01  1:44 UTC (permalink / raw)
  To: u-boot

Hi Duncan,

On Sat, Apr 14, 2018 at 6:43 PM,  <DH@synoia.com> wrote:
> From: Duncan Hare <DuncanCHare@yahoo.com>
>
> All the code is new, and not copied from any source.
>
> Series-changes
> The previous patch was using an old version of net/Kconfig,
> which prevented requesting options for a bootp/dhcp request.
>
> A similar issue fixed with a cmd/Kconfig.
> Items in include/net.h fixed.

Please follow the format as listed in the patman README:
"""
Series-changes: n
- Guinea pig moved into its cage
- Other changes ending with a blank line
<blank line>
        This can appear in any commit. It lists the changes for a
        particular version n of that commit. The change list is
        created based on this information. Each commit gets its own
        change list and also the whole thing is repeated in the cover
        letter (where duplicate change lines are merged).

        By adding your change lists into your commits it is easier to
        keep track of what happened. When you amend a commit, remember
        to update the log there and then, knowing that the script will
        do the rest.
"""

When you make changes to the patches based on my feedback, add to the
list of changes for version 11 as you make the changes so you won't
forget anything.

> Signed-off-by: Duncan Hare <DH@Synoia.com>
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> ---
>
> Why TCP:
>
> Currently file transfer are done using tftp or NFS both
> over udp. This requires a request to be sent from client
> (u-boot) to the boot server.
>
> For a 4 Mbyte kernel, with a 1k block size this requires
> 4,000 request for a block.
>
> Using a large block size, one greater than the Ethernet
> maximum frame size limitation, would require fragmentation,
> which u-boot supports. However missing fragment recovery
> requires timeout detection and re-transmission requests
> for missing fragments.
>
> In networks with large latency, for example: the internet,
> UDP is vwey slow. What is a 30 second transfer on a local
> boot server and LAN increase to over 3 minutes, because of
> all the requests/response traffic.
>
> This was anticipated in the evolution of the IP protocols
> and TCP was developed and then enhanced for high latency high
> bandwidth networks.
>
> The current standard is TCP with selective acknowledgment.

You make no mention in the commit log of this patch supporting
selective ack or not. Honestly, all of this description (above this
line) seems like info that should actually stay in the patch's log,
not in a note.

> Routine tcp_print_buffer() is used to print portions of
> non zero terminated buffers. If there is an existing routine
> please let me know. I'm from the world of length fields
> not zero terminated strings (zOS).
>
>
> Changes in v10: None
>
>  include/net.h     |   8 +-
>  include/net/tcp.h | 218 ++++++++++++++++
>  net/Kconfig       |   5 +
>  net/Makefile      |   2 +-
>  net/net.c         |  51 +++-
>  net/tcp.c         | 749 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 1023 insertions(+), 10 deletions(-)
>  create mode 100644 include/net/tcp.h
>  create mode 100644 net/tcp.c
>
> diff --git a/include/net.h b/include/net.h
> index 7e5f5a6a5b..e29d804a23 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -548,7 +548,7 @@ extern int          net_restart_wrap;       /* Tried all network devices */
>
>  enum proto_t {
>         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
> -       TFTPSRV, TFTPPUT, LINKLOCAL
> +       TFTPSRV, TFTPPUT, LINKLOCAL, WGET

Seems like this should be in the WGET patch, right?

>  };
>
>  extern char    net_boot_file_name[1024];/* Boot File name */
> @@ -681,11 +681,15 @@ static inline void net_send_packet(uchar *pkt, int len)
>   * @param payload_len Length of data after the UDP header
>   */
>  int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> -                      int payload_len, int proto);
> +                      int payload_len, int proto, u8 action, u32 tcp_seq_num,
> +                      u32 tcp_ack_num);
>
>  int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
>                         int sport, int payload_len);
>
> +int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
> +                       u32 tcp_seq_num, u32 tcp_ack_num);
> +
>  /* Processes a received packet */
>  void net_process_received_packet(uchar *in_packet, int len);
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> new file mode 100644
> index 0000000000..81f263351e
> --- /dev/null
> +++ b/include/net/tcp.h
> @@ -0,0 +1,218 @@
> +/*
> + *     TCP Support for file transfer.
> + *
> + *     Copyright 2017 Duncan Hare, All rights reserved.
> + *
> + *      SPDX-License-Identifier:        GPL-2.0
> + */
> +
> +#define TCP_ACTIVITY 127               /* Activity on downloading      */

What is the meaning of this number? Number of packets before the
console adds a progress mark? Please add a more verbose comment.

> +
> +struct ip_tcp_hdr {
> +       u8              ip_hl_v;        /* header length and version    */
> +       u8              ip_tos;         /* type of service              */
> +       u16             ip_len;         /* total length                 */
> +       u16             ip_id;          /* identification               */
> +       u16             ip_off;         /* fragment offset field        */
> +       u8              ip_ttl;         /* time to live                 */
> +       u8              ip_p;           /* protocol                     */
> +       u16             ip_sum;         /* checksum                     */
> +       struct in_addr  ip_src;         /* Source IP address            */
> +       struct in_addr  ip_dst;         /* Destination IP address       */
> +       u16             tcp_src;        /* TCP source port              */
> +       u16             tcp_dst;        /* TCP destination port         */
> +       u32             tcp_seq;        /* TCP sequence number          */
> +       u32             tcp_ack;        /* TCP Acknowledgment number    */
> +       u8              tcp_hlen;       /* 4 bits TCP header Length/4   */
> +                                       /* 4 bits Reserved              */
> +                                       /* 2 more bits reserved         */
> +       u8              tcp_flags;      /* see defines                  */
> +       u16             tcp_win;        /* TCP windows size             */
> +       u16             tcp_xsum;       /* Checksum                     */
> +       u16             tcp_ugr;        /* Pointer to urgent data       */
> +} __packed;
> +
> +#define IP_TCP_HDR_SIZE                (sizeof(struct ip_tcp_hdr))
> +#define TCP_HDR_SIZE           (IP_TCP_HDR_SIZE  - IP_HDR_SIZE)
> +
> +#define TCP_DATA       0x00    /* Data Packet - internal use only      */
> +#define TCP_FIN                0x01    /* Finish flag                          */
> +#define TCP_SYN                0x02    /* Synch (start) flag                   */
> +#define TCP_RST                0x04    /* reset flag                           */
> +#define TCP_PUSH       0x08    /* Push - Notify app                    */
> +#define TCP_ACK                0x10    /* Acknowledgment of data received      */
> +#define TCP_URG                0x20    /* Urgent                               */
> +#define TCP_ECE                0x40    /* Congestion control                   */
> +#define TCP_CWR                0x80    /* Congestion Control                   */

Why do these have the same comment? Mistake? If not, elaborate in the comment.

> +
> +/*
> + * TCP header options, Seq, MSS, and SACK
> + */
> +
> +#define TCP_SACK 32                    /* Number of packets analyzed   */
> +                                       /* on leading edge of stream    */
> +
> +#define TCP_O_END      0x00            /* End of option list           */
> +#define TCP_1_NOP      0x01            /* Single padding NOP           */
> +#define TCP_O_NOP      0x01010101      /* NOPs pad to 32 bit boundary  */
> +#define TCP_O_MSS      0x02            /* MSS Size option              */
> +#define TCP_O_SCL      0x03            /* Window Scale option          */
> +#define TCP_P_SACK     0x04            /* SACK permitted               */
> +#define TCP_V_SACK     0x05            /* SACK values                  */
> +#define TCP_O_TS       0x08            /* Timestanp option             */

Timestanp  -> Timestamp

> +#define TCP_OPT_LEN_2  0x02
> +#define TCP_OPT_LEN_3  0x03
> +#define TCP_OPT_LEN_4  0x04
> +#define TCP_OPT_LEN_6  0x06
> +#define TCP_OPT_LEN_8  0x08
> +#define TCP_OPT_LEN_A  0x0a            /* Timestamp Length             */
> +
> +/*
> + * Please review the warning in net.c about these two parameters.
> + * They are part of a promise of RX buffer size to the sending TCP
> + */
> +
> +#define TCP_MSS                1460            /* Max segment size - 1460      */

Not sure why you are adding the value to the comment as well... just 2
places to touch if they need to change. Please remove the numbers from
these comments.

> +#define TCP_SCALE      0x01            /* Scale 1                      */
> +
> +struct tcp_mss {                       /* TCP Mex Segment size         */

Mex -> Max

> +       u8      kind;                   /* 0x02                         */

Why are you putting numbers here? Seems like they should instead be
"#define TCP_MSS_KIND_x 0x02" that specify valid numbers for "kind".
Same applies for other struct fields here.

> +       u8      len;                    /* 0x04                         */
> +       u16     mss;                    /* 1460 - Max segment size      */
> +} __packed;
> +
> +struct tcp_scale {                     /* TCP Windows Scale            */
> +       u8      kind;                   /* 0x03                         */
> +       u8      len;                    /* 0x03                         */
> +       u8      scale;                  /* win shift fat fast networks  */

"fat"? Also, what units?

> +} __packed;
> +
> +struct tcp_sack_p {                    /* SACK permitted               */
> +       u8      kind;                   /* 0x04                         */
> +       u8      len;                    /* Length                       */
> +} __packed;
> +
> +struct sack_edges {
> +       u32     l;
> +       u32     r;

More verbose names. "left" and "right"?

> +} __packed;
> +
> +#define TCP_SACK_SIZE (sizeof(struct sack_edges))
> +
> +#define TCP_SACK_HILLS 4

Please add a comment - what a "hill" is.

> +
> +struct tcp_sack_v {
> +       u8      kind;                   /* 0x05                         */
> +       u8      len;                             /* Length              */
> +       struct  sack_edges hill[TCP_SACK_HILLS]; /* L & R widow edges   */
> +} __packed;
> +
> +struct tcp_t_opt {                     /* TCP time stamps option       */
> +       u8      kind;                   /* 0x08                         */
> +       u8      len;                    /* 0x0a                         */
> +       u32     t_snd;                  /* Sender timestamp             */
> +       u32     t_rcv;                  /* Receiver timestamp           */
> +} __packed;
> +
> +#define TCP_TSOPT_SIZE (sizeof(struct tcp_t_opt))
> +
> +/*
> + * ip tcp  structure with options
> + */
> +
> +struct ip_tcp_hdr_o {
> +       struct  ip_tcp_hdr      hdr;
> +       struct  tcp_mss         mss;
> +       struct  tcp_scale       scale;
> +       struct  tcp_sack_p      sack_p;
> +       struct  tcp_t_opt       t_opt;
> +       u8      end;
> +} __packed;
> +
> +#define IP_TCP_O_SIZE          (sizeof(struct ip_tcp_hdr_o))

In general, when you are not trying to align a number of defines (and
in my opinion, even then) only use a single space between the name and
the implementation.

> +
> +struct ip_tcp_hdr_s {
> +       struct  ip_tcp_hdr      hdr;
> +       struct  tcp_t_opt       t_opt;
> +       struct  tcp_sack_v      sack_v;
> +       u8      end;
> +} __packed;
> +
> +#define IP_TCP_SACK_SIZE       (sizeof(struct ip_tcp_hdr_s))
> +
> +/*
> + * TCP pseudo header definitions
> + */
> +#define PSEUDO_PAD_SIZE        8
> +
> +struct pseudo_hdr {
> +       u8 padding[PSEUDO_PAD_SIZE];    /* pseudo hdr size = ip_tcp hdr size */
> +       struct in_addr p_src;
> +       struct in_addr p_dst;
> +       u8      rsvd;
> +       u8      p;
> +       u16     len;
> +} __packed;
> +
> +#define PSEUDO_HDR_SIZE        (sizeof(struct pseudo_hdr)) - PSEUDO_PAD_SIZE
> +
> +/*
> + * union for building IP/TCP packet.
> + * build Pseudo header in packed buffer first, calculate TCP checksum
> + * then build IP header in packed  buffer.

Clean up formatting. Two spaces in third line.Missing comma at end of
second line, Capitalize Build, lowercase pseudo.

Generally written as TCP/IP. Is there some reason you are reversing that here?

> + */
> +
> +union tcp_build_pkt {
> +       struct pseudo_hdr ph;
> +       struct ip_tcp_hdr_o ip;
> +       struct ip_tcp_hdr_s sack;
> +       uchar  raw[1600];
> +} __packed;
> +
> +/*
> + * TCP STATE MACHINE STATES FOR SOCKET

It is cool, but there's no need to shout. Use normal English capitalization.

> + */
> +
> +enum TCP_STATE {
> +       TCP_CLOSED,             /* Need to send SYN to connect            */
> +       TCP_SYN_SENT,           /* Trying to connect, waiting for SYN ACK */
> +       TCP_ESTABLISHED,        /* both server & client have a connection */
> +       TCP_CLOSE_WAIT,         /* Rec FIN, passed to app for FIN, ACK rsp*/
> +       TCP_CLOSING,            /* Rec FIN, sent FIN, ACK waiting for ACK */
> +       TCP_FIN_WAIT_1,         /* Sent FIN waiting for response          */
> +       TCP_FIN_WAIT_2          /* Rec ACK from FIN sent, waiting for FIN */
> +};
> +

Please add function documentation here for the functions below.

> +int tcp_find_in_buffer(uchar raw[], int payload_len, uchar field[],
> +                      int field_len);
> +void tcp_print_buffer(uchar raw[], int pkt_len, int payload_len,
> +                     int hdr_len, bool hide);
> +enum TCP_STATE tcp_get_tcp_state(void);
> +void tcp_set_tcp_state(enum TCP_STATE new_state);
> +int tcp_set_tcp_header(uchar *pkt, int dport, int sport, int payload_len,
> +                      u8 action, u32 tcp_seq_num, u32 tcp_ack_num);
> +
> +/*
> + * An incoming packet handler.
> + * @param pkt    pointer to the application packet
> + * @param dport  destination UDP port
> + * @param sip    source IP address
> + * @param sport  source UDP port
> + * @param len    packet length
> + */
> +typedef void rxhand_tcp(uchar *pkt, unsigned int dport,
> +                       struct in_addr sip, unsigned int sport,
> +                       unsigned int len);
> +void tcp_set_tcp_handler(rxhand_tcp *f);
> +
> +void rxhand_tcp_f(union tcp_build_pkt *b, unsigned int len);
> +
> +/*
> + * An incoming TCP packet handler for the TCP protocol.
> + * There is also a dynamic function pointer for TCP based commands to
> + * receive incoming traffic after the TCP protocol code has done its work.
> + */
> +
> +void rxhand_action(u8 tcp_action, int payload_len, u32 tcp_seq_num,
> +                  u32 tcp_ack_num, unsigned int pkt_len,
> +                  union tcp_build_pkt *b);
> diff --git a/net/Kconfig b/net/Kconfig
> index 143c4416cd..cc6dd83fc4 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -33,6 +33,11 @@ config NET_TFTP_VARS
>           If unset, timeout and maximum are hard-defined as 1 second
>           and 10 timouts per TFTP transfer.
>
> +config TCP
> +       bool "Include Subset TCP stack for wget"

Replace with "TCP stack"

> +       help
> +         TCP protocol support for wget.
> +
>  config BOOTP_BOOTPATH
>         bool "Enable BOOTP BOOTPATH"
>         depends on CMD_NET
> diff --git a/net/Makefile b/net/Makefile
> index ae54eee5af..25729ebeeb 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -25,7 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
>  obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_NET)  += tftp.o
> -
> +obj-$(CONFIG_TCP)      += tcp.o
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
>  # and this is intentional usage.
> diff --git a/net/net.c b/net/net.c
> index df4e7317e7..7adc4b472c 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -107,6 +107,9 @@
>  #if defined(CONFIG_CMD_SNTP)
>  #include "sntp.h"
>  #endif
> +#if defined(CONFIG_TCP)

There is nothing in the tcp header that we can't include when TCP is
not enabled, right? Remove the include guard and always include it.

> +#include <net/tcp.h>
> +#endif
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -382,6 +385,9 @@ void net_init(void)
>
>                 /* Only need to setup buffer pointers once. */
>                 first_call = 0;
> +#if defined(CONFIG_TCP)
> +               tcp_set_tcp_state(TCP_CLOSED);
> +#endif
>         }
>
>         net_init_loop();
> @@ -781,11 +787,22 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>                         int payload_len)
>  {
>         return net_send_ip_packet(ether, dest, dport, sport, payload_len,
> -                                 IPPROTO_UDP);
> +                                 IPPROTO_UDP, 0, 0, 0);
> +}
> +
> +#if defined(CONFIG_TCP)
> +int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
> +                       u32 tcp_seq_num, u32 tcp_ack_num)
> +{
> +       return net_send_ip_packet(net_server_ethaddr, net_server_ip, dport,
> +                                 sport, payload_len, IPPROTO_TCP, action,
> +                                 tcp_seq_num, tcp_ack_num);
>  }
> +#endif
>
>  int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> -                      int payload_len, int proto)
> +                      int payload_len, int proto, u8 action, u32 tcp_seq_num,
> +                      u32 tcp_ack_num)
>  {
>         uchar *pkt;
>         int eth_hdr_size;
> @@ -794,6 +811,13 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>                 debug_cond(DEBUG_DEV_PKT,
>                            "UDP Send  (to=%pI4, from=%pI4, len=%d)\n",
>                            &dest, &net_ip, payload_len);
> +#if defined(CONFIG_TCP)
> +
> +       } else {
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Send  (%pI4, %pI4, len=%d, A=%x)\n",
> +                          &dest, &net_ip, payload_len, action);
> +#endif
>         }
>
>         /* make sure the net_tx_packet is initialized (net_init() was called) */
> @@ -812,15 +836,19 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>         pkt = (uchar *)net_tx_packet;
>
>         eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
> -       switch (proto) {
> -       case IPPROTO_UDP:
> +       if (proto == IPPROTO_UDP) {

I don't like the switch away from the case structure and the
defaulting to sending TCP packets if the proto isn't UDP. Please keep
the case structure and the error on unimplemented protocols.

>                 net_set_udp_header(pkt + eth_hdr_size, dest,
>                                    dport, sport, payload_len);
>                 pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
> -       break;
> -       default:
> -               return -1;
> +
> +#if defined(CONFIG_TCP)
> +       } else {
> +               pkt_hdr_size = eth_hdr_size +
> +               tcp_set_tcp_header(pkt + eth_hdr_size, dport, sport,
> +                                  payload_len, action,
> +                                  tcp_seq_num, tcp_ack_num);
>         }
> +#endif

This looks like it wouldn't compile. Shouldn't this #endif be before
the close brace?

>
>         /* if MAC address was not discovered yet, do an ARP request */
>         if (memcmp(ether, net_null_ethaddr, 6) == 0) {
> @@ -1225,6 +1253,15 @@ void net_process_received_packet(uchar *in_packet, int len)
>                 if (ip->ip_p == IPPROTO_ICMP) {

Looks like this would be better implemented with a case structure at
this point. Such a change should go in the preparation patch.

>                         receive_icmp(ip, len, src_ip, et);
>                         return;
> +#if defined(CONFIG_TCP)
> +               } else if (ip->ip_p == IPPROTO_TCP) {
> +                       debug_cond(DEBUG_DEV_PKT,
> +                                  "TCP PH (to=%pI4, from=%pI4, len=%d)\n",
> +                                  &dst_ip, &src_ip, len);
> +
> +                               rxhand_tcp_f((union tcp_build_pkt *)ip, len);
> +                       return;
> +#endif
>                 } else if (ip->ip_p != IPPROTO_UDP) {   /* Only UDP packets */
>                         return;
>                 }
> diff --git a/net/tcp.c b/net/tcp.c
> new file mode 100644
> index 0000000000..9f4fc9fc20
> --- /dev/null
> +++ b/net/tcp.c
> @@ -0,0 +1,749 @@
> +/*
> + *     Copyright 2017 Duncan Hare, all rights reserved.

You can change or range this year, but include 2018.

> + *     SPDX-License-Identifier:        GPL-2.0
> + */
> +
> +/*
> + * General Desription:
> + *
> + * TCP support for the wget command, for fast file downloading.
> + *
> + * HTTP/TCP Receiver:
> + *
> + *      Prequeisites:   - own ethernet address
> + *                      - own IP address
> + *                      - Server IP address
> + *                      - Server with TCP
> + *                      - TCP application (eg wget)
> + *      Next Step       HTTPS?
> + */
> +#include <common.h>
> +#include <command.h>
> +#include <console.h>
> +#include <environment.h>
> +#include <errno.h>
> +#include <net.h>
> +#include <net/tcp.h>
> +#if defined(CONFIG_CMD_WGET)

Don't put ifdefs around includes. Also, there is no concept of wget at
this point, so move this to the wget patch.

> +#include <net/wget.h>
> +#endif
> +
> +/*
> + * TCP sliding window  control
> + * used by us to request re-TX
> + */

Why is this multi-line and stumpy? Make it a single line and remove
the blank line between the comment and what it comments on.

> +
> +static struct tcp_sack_v tcp_lost;
> +
> +/* TCP option timestamp */
> +static u32 loc_timestamp;
> +static u32 rmt_timestamp;
> +
> +u32 tcp_seq_init;
> +u32 tcp_ack_edge;
> +u32 tcp_seq_max;
> +
> +int tcp_activity_count;
> +
> +/*
> + * Search for TCP_SACK and review the
> + * comments before the code section
> + * TCP_SACK is the number of packets
> + * at the front of the stream
> + */

Make comments use as much of 80 chars as possible for wrapping.

> +
> +enum pkt_state {PKT, NOPKT};
> +struct sack_r {
> +       struct sack_edges se;
> +       enum   pkt_state st;
> +};
> +
> +struct sack_r edge_a[TCP_SACK];
> +unsigned int sack_idx;
> +unsigned int prev_len;
> +
> +/* TCP connection state */
> +static enum TCP_STATE tcp_state;
> +
> +/*
> + * An incoming TCP packet handler for the TCP protocol.
> + * There is also a dymanic function pointer for TCP based commads to
dymanic  -> dynamic
commads  -> commands
> + * receive incoming traffic after the TCP protocol code has done its work.
> + */
> +
> +/* Current TCP RX packet handler */
> +static rxhand_tcp *tcp_packet_handler;
> +
> +enum TCP_STATE tcp_get_tcp_state(void)
> +{
> +       return tcp_state;
> +}
> +
> +void tcp_set_tcp_state(enum TCP_STATE new_state)
> +{
> +       tcp_state = new_state;
> +}
> +
> +void tcp_print_buffer(uchar raw[], int pkt_len, int payload_len,
> +                     int hdr_len, bool hide)

Should this be a debug-only function? Presumably we don't want to
print in general.

I also think you could just use the print_buffer() function in
lib/display_options.c. If you really think you need to add the hiding
option, it makes sense to add it in print_buffer().

> +{
> +       int i;
> +
> +       for (i = pkt_len - payload_len; i < pkt_len; i++) {
> +               if (i <= hdr_len)
> +                       printf("%02X", raw[i]);
> +               else if ((raw[i] > 0x19) && (raw[i] < 0x7f))
> +                       putc(raw[i]);
> +               else if (hide == 0)
> +                       putc(raw[i]);
> +               else
> +                       printf("%02X", raw[i]);
> +       }
> +       printf("%s", "\n");
> +}
> +
> +int tcp_find_in_buffer(uchar raw[], int payload_len, uchar field[],
> +                      int field_len)

This doesn't seem to be used anywhere in your series. Maybe it should
be removed?

> +{
> +       int i, j;
> +
> +       for (i = 0; i < payload_len; i++) {
> +               if (raw[i] == field[0]) {
> +                       for (j = 1; j < field_len; j++) {
> +                               if (raw[i + j] != field[j])
> +                                       break;
> +                       if (j == field_len)
> +                               return i;
> +                       }
> +               }
> +       }
> +       return 0;
> +}
> +
> +static void dummy_handler(uchar *pkt, unsigned int dport,
> +                         struct in_addr sip, unsigned int sport,
> +                         unsigned int len)
> +{
> +}
> +
> +void tcp_set_tcp_handler(rxhand_tcp *f)
> +{
> +       debug_cond(DEBUG_INT_STATE, "--- net_loop TCP handler set (%p)\n", f);
> +       if (!f)
> +               tcp_packet_handler = dummy_handler;
> +       else
> +               tcp_packet_handler = f;
> +}
> +
> +u16 tcp_set_pseudo_header(uchar *pkt, struct in_addr src, struct in_addr dest,
> +                         int tcp_len, int pkt_len)
> +{
> +       union tcp_build_pkt *b = (union tcp_build_pkt *)pkt;
> +       int checksum_len;
> +
> +       /*
> +        * Pseudo header
> +        *
> +        * Zero the byte after the last byte so that the header checksum
> +        * will always work.
> +        */
> +
> +       pkt[pkt_len] = 0x00;
> +
> +       net_copy_ip((void *)&b->ph.p_src, &src);
> +       net_copy_ip((void *)&b->ph.p_dst, &dest);
> +       b->ph.rsvd      = 0x00;
> +       b->ph.p         = IPPROTO_TCP;
> +       b->ph.len       = htons(tcp_len);
> +       checksum_len    = tcp_len + PSEUDO_HDR_SIZE;
> +
> +       debug_cond(DEBUG_DEV_PKT,
> +                  "TCP Pesudo  Header  (to=%pI4, from=%pI4, Len=%d)\n",
> +                  &b->ph.p_dst, &b->ph.p_src, checksum_len);
> +
> +       return compute_ip_checksum(pkt + PSEUDO_PAD_SIZE, checksum_len);
> +}
> +
> +int net_set_ack_options(union tcp_build_pkt *b)
> +{
> +       b->sack.hdr.tcp_hlen  = (TCP_HDR_SIZE >> 2) << 4;
> +
> +       b->sack.t_opt.kind              = TCP_O_TS;
> +       b->sack.t_opt.len               = TCP_OPT_LEN_A;
> +       b->sack.t_opt.t_snd             = htons(loc_timestamp);
> +       b->sack.t_opt.t_rcv             = rmt_timestamp;
> +       b->sack.sack_v.kind             = 0x01;

Please don't use magic numbers. Make a #define for the value in tcp.h
and give it a meaningful name.

> +       b->sack.sack_v.len              = 0x00;
> +
> +       if (tcp_lost.len > TCP_OPT_LEN_2) {
> +               debug_cond(DEBUG_DEV_PKT, "TCP ack opt lost.len %x\n",
> +                          tcp_lost.len);
> +               b->sack.sack_v.len              = tcp_lost.len;
> +               b->sack.sack_v.kind             = TCP_V_SACK;
> +               b->sack.sack_v.hill[0].l = htonl(tcp_lost.hill[0].l);
> +               b->sack.sack_v.hill[0].r = htonl(tcp_lost.hill[0].r);
> +
> +               /*
> +                * These SACK structures are initialized with NOPs to
> +                * provide TCP header alignment padding. There are 4
> +                * SACK structures used for both header padding and
> +                * internally.
> +                */
> +
> +               b->sack.sack_v.hill[1].l = htonl(tcp_lost.hill[1].l);
> +               b->sack.sack_v.hill[1].r = htonl(tcp_lost.hill[1].r);
> +               b->sack.sack_v.hill[2].l = htonl(tcp_lost.hill[2].l);
> +               b->sack.sack_v.hill[2].r = htonl(tcp_lost.hill[2].r);
> +               b->sack.sack_v.hill[3].l = TCP_O_NOP;
> +               b->sack.sack_v.hill[3].r = TCP_O_NOP;
> +       }
> +
> +       /*
> +        * TCP lengths are stored as a rounded up number of 32 bit words
> +        * Add 3 to length round up, rounded, then divided into the length
> +        * in 32 bit words.
> +        */
> +
> +       b->sack.hdr.tcp_hlen = (((TCP_HDR_SIZE + TCP_TSOPT_SIZE
> +                               + tcp_lost.len + 3)  >> 2) << 4);
> +
> +       /*
> +        * This returns the actual rounded up length of the
> +        * TCP header to add to the total packet length
> +        */
> +
> +       return b->sack.hdr.tcp_hlen >> 2;
> +}
> +
> +void net_set_syn_options(union tcp_build_pkt *b)
> +{
> +       tcp_lost.len            = 0;
> +       b->ip.hdr.tcp_hlen      = 0xa0;
> +
> +       b->ip.mss.kind          = TCP_O_MSS;
> +       b->ip.mss.len           = TCP_OPT_LEN_4;
> +       b->ip.mss.mss           = htons(TCP_MSS);
> +       b->ip.scale.kind        = TCP_O_SCL;
> +       b->ip.scale.scale       = TCP_SCALE;
> +       b->ip.scale.len         = TCP_OPT_LEN_3;
> +       b->ip.sack_p.kind       = TCP_P_SACK;
> +       b->ip.sack_p.len        = TCP_OPT_LEN_2;
> +       b->ip.t_opt.kind        = TCP_O_TS;
> +       b->ip.t_opt.len         = TCP_OPT_LEN_A;
> +       loc_timestamp           = get_ticks() % 3072;

Please don't use magic numbers. What is this?

> +       rmt_timestamp           = 0x00000000;
> +       b->ip.t_opt.t_snd       = 0;
> +       b->ip.t_opt.t_rcv       = 0x00000000;
> +       b->ip.end               = TCP_O_END;
> +}
> +
> +int tcp_set_tcp_header(uchar *pkt, int dport, int sport, int payload_len,
> +                      u8 action, u32 tcp_seq_num, u32 tcp_ack_num)
> +{
> +       union tcp_build_pkt *b = (union tcp_build_pkt *)pkt;
> +       int     pkt_hdr_len;
> +       int     pkt_len;
> +       int     tcp_len;
> +
> +       b->ip.hdr.tcp_flags     = action;
> +       pkt_hdr_len             = IP_TCP_HDR_SIZE;
> +       b->ip.hdr.tcp_hlen      = 0x50;         /* Header is 5 32 bit words  */
> +                                               /* 4 bits TCP header Length/4*/
> +                                               /* 4 bits Reserved           */
> +                                               /* For options               */

Reserved for options? If so, then: /* 4 bits Reserved for options */
If that makes the line too long, then remove the extra tab after the
semicolon on the first comment, or move all to a multi-line above the
code.

> +       switch (action) {
> +       case TCP_SYN:
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Hdr:SYN (%pI4, %pI4, sq=%d, ak=%d)\n",
> +                          &net_server_ip, &net_ip,
> +                          tcp_seq_num, tcp_ack_num);
> +               tcp_activity_count = 0;
> +               net_set_syn_options(b);
> +               tcp_seq_num = 0;
> +               tcp_ack_num = 0;
> +               pkt_hdr_len = IP_TCP_O_SIZE;
> +               if (tcp_state == TCP_SYN_SENT) {  /* Too many sins */

Guess we had better buy some indulgences.

Or maybe you meant "Too many SYNs".

> +                       action    = TCP_FIN;
> +                       tcp_state = TCP_FIN_WAIT_1;
> +               } else {
> +                       tcp_state = TCP_SYN_SENT;
> +               }
> +       break;
> +       case TCP_ACK:
> +               pkt_hdr_len         = IP_HDR_SIZE +
> +                                     net_set_ack_options(b);
> +               b->ip.hdr.tcp_flags = action;
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Hdr:ACK (%pI4, %pI4, s=%d, a=%d, A=%x)\n",
> +                          &net_server_ip, &net_ip, tcp_seq_num, tcp_ack_num,
> +                          action);
> +       break;
> +       case TCP_FIN:
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Hdr:FIN  (%pI4, %pI4, s=%d, a=%d)\n",
> +                          &net_server_ip, &net_ip, tcp_seq_num, tcp_ack_num);
> +               payload_len = 0;
> +               pkt_hdr_len = IP_TCP_HDR_SIZE;
> +               tcp_state   = TCP_FIN_WAIT_1;
> +
> +       break;
> +
> +       /* Notify connection closing */
> +
> +       case (TCP_FIN | TCP_ACK):
> +       case ((TCP_FIN | TCP_ACK) | TCP_PUSH):
> +               if (tcp_state == TCP_CLOSE_WAIT)
> +                       tcp_state = TCP_CLOSING;
> +               tcp_ack_edge++;
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Hdr:FIN ACK PSH(%pI4, %pI4, s=%d, a=%d, A=%x)\n",
> +                          &net_server_ip, &net_ip,
> +                          tcp_seq_num, tcp_ack_edge, action);
> +                                       /* FALLTHRU */
> +       default:
> +               pkt_hdr_len         = IP_HDR_SIZE +
> +                                     net_set_ack_options(b);
> +               b->ip.hdr.tcp_flags = action | TCP_PUSH | TCP_ACK;
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Hdr:dft  (%pI4, %pI4, s=%d, a=%d, A=%x)\n",
> +                          &net_server_ip, &net_ip,
> +                          tcp_seq_num, tcp_ack_num, action);
> +       }
> +
> +       pkt_len = pkt_hdr_len + payload_len;
> +       tcp_len = pkt_len - IP_HDR_SIZE;
> +
> +       /* TCP Header */
> +       b->ip.hdr.tcp_ack       = htonl(tcp_ack_edge);
> +       b->ip.hdr.tcp_src       = htons(sport);
> +       b->ip.hdr.tcp_dst       = htons(dport);
> +       b->ip.hdr.tcp_seq       = htonl(tcp_seq_num);
> +       tcp_seq_num             = tcp_seq_num + payload_len;
> +
> +       /*
> +        * TCP window size - TCP header variable tcp_win.
> +        * Change tcp_win only if you have an understanding of network
> +        * overrun, congestion, TCP segment sizes, TCP windows, TCP scale,
> +        * queuing theory  and packet buffering. If there are too few buffers,
> +        * there will be data loss, recovery may work or the sending TCP,
> +        * the server, could abort the stream transmission.
> +        * MSS is governed by maximum Ethernet frame length.
> +        * The number of buffers is governed by the desire to have a queue of
> +        * full buffers to be processed at the destination to maximize
> +        * throughput. Temporary memory use for the boot phase on modern
> +        * SOCs is may not be considered a constraint to buffer space, if
> +        * it is, then the u-boot tftp or nfs kernel netboot should be
> +        * considered.
> +        */
> +
> +       b->ip.hdr.tcp_win       = htons(PKTBUFSRX * TCP_MSS >>  TCP_SCALE);
> +
> +       b->ip.hdr.tcp_xsum      = 0x0000;
> +       b->ip.hdr.tcp_ugr       = 0x0000;
> +
> +       b->ip.hdr.tcp_xsum = tcp_set_pseudo_header(pkt, net_ip, net_server_ip,
> +                                                  tcp_len, pkt_len);
> +
> +       /*
> +        * IP Header
> +        */

I don't think this comment provides any value. Please remove it.

> +
> +       net_set_ip_header((uchar *)&b->ip, net_server_ip, net_ip,
> +                         pkt_len, IPPROTO_TCP);
> +
> +       return pkt_hdr_len;
> +}
> +
> +/*
> + * Selective Acknowledgment (Essential for fast stream transfer)
> + *
> + * Before modifying this section of code, which the author found difficult
> + * to write, please be familiar with the SACK (Selective Acknowledgment) RFCs.

This commentary seems unnecessary, but if you feel strongly you can leave it in.

> + */
> +
> +void tcp_hole(u32 tcp_seq_num, u32 len, u32 tcp_seq_max)
> +{
> +       unsigned int idx_sack;
> +       unsigned int sack_end = TCP_SACK - 1;
> +       unsigned int sack_in;
> +       unsigned int hill = 0;
> +       enum pkt_state expect = PKT;
> +
> +       u32 seq   = tcp_seq_num - tcp_seq_init;
> +       u32 hol_l = tcp_ack_edge - tcp_seq_init;
> +       u32 hol_r = 0;
> +
> +       /* Place seq number in correct place */

This doesn't provide information.

> +
> +       if (prev_len == 0)
> +               prev_len = len;
> +       idx_sack = sack_idx + ((tcp_seq_num - tcp_ack_edge) / prev_len);
> +       if (idx_sack < TCP_SACK) {
> +               edge_a[idx_sack].se.l = tcp_seq_num;
> +               edge_a[idx_sack].se.r = tcp_seq_num + len;
> +               edge_a[idx_sack].st   = PKT;
> +
> +               /* The fin (last) packet is not the same length as data packets,

This is an invalid multi-line comment format. Use standalone "/*"

> +                * and if it's length is recorded and used for array index
> +                * the array index calculations break.
> +                */
> +               if (prev_len < len)
> +                       prev_len = len;
> +       }
> +
> +       debug_cond(DEBUG_DEV_PKT,
> +                  "TCP 1 seq %d, edg %d, len %d, sack_idx %d, sack_end %d\n",
> +                   seq, hol_l, len, sack_idx, sack_end);
> +
> +       /* Right edge of contiguous stream, is the left edge of first hill */
> +
> +       hol_l = tcp_seq_num - tcp_seq_init;
> +       hol_r = hol_l + len;
> +
> +       tcp_lost.len = TCP_OPT_LEN_2;
> +
> +       debug_cond(DEBUG_DEV_PKT,
> +                  "TCP 1 in %d, seq %d, pkt_l %d, pkt_r %d, sack_idx %d, sack_end %d\n",
> +                  idx_sack, seq, hol_l, hol_r, sack_idx, sack_end);
> +
> +       for (sack_in = sack_idx; sack_in < sack_end && hill < TCP_SACK_HILLS;
> +                sack_in++)  {
> +               switch (expect) {
> +               case NOPKT:
> +                       switch (edge_a[sack_in].st) {
> +                       case NOPKT:
> +                               debug_cond(DEBUG_INT_STATE, "N");
> +                       break;
> +                       case PKT:
> +                               debug_cond(DEBUG_INT_STATE, "n");
> +                                       tcp_lost.hill[hill].l =
> +                                               edge_a[sack_in].se.l;
> +                                       tcp_lost.hill[hill].r =
> +                                               edge_a[sack_in].se.r;
> +                               expect = PKT;
> +                       break;
> +                       }
> +               break;
> +               case PKT:
> +                       switch (edge_a[sack_in].st) {
> +                       case NOPKT:
> +                               debug_cond(DEBUG_INT_STATE, "p");
> +                               if ((sack_in > sack_idx) &&
> +                                   (hill < TCP_SACK_HILLS)) {
> +                                       hill++;
> +                                       tcp_lost.len += TCP_OPT_LEN_8;
> +                               }
> +                               expect = NOPKT;
> +                       break;
> +                       case PKT:
> +                               debug_cond(DEBUG_INT_STATE, "P");
> +
> +                               if (tcp_ack_edge == edge_a[sack_in].se.l) {
> +                                       tcp_ack_edge = edge_a[sack_in].se.r;
> +                                       edge_a[sack_in].st = NOPKT;
> +                                       sack_idx++;
> +                               } else {
> +                                       if (hill < TCP_SACK_HILLS)
> +                                               tcp_lost.hill[hill].r =
> +                                                       edge_a[sack_in].se.r;
> +                               if (sack_in == sack_end - 1)
> +                                       tcp_lost.hill[hill].r =
> +                                               edge_a[sack_in].se.r;
> +                               }
> +                       break;
> +                       }
> +               break;
> +               }
> +       }
> +       debug_cond(DEBUG_INT_STATE, "\n");
> +       if (tcp_lost.len <= TCP_OPT_LEN_2)
> +               sack_idx = 0;
> +}
> +
> +void tcp_parse_options(uchar *o, int o_len)
> +{
> +       struct tcp_t_opt  *tsopt;
> +       uchar *p = o;
> +
> +       for (p = o; p < (o + o_len); p = p + p[1]) {
> +               if (p[1] != 0) {
> +                       switch (p[0]) {
> +                       case TCP_O_END:
> +                               return;

Isn't this the same as break;?

> +                       case TCP_O_MSS:
> +                       break;
> +                       case TCP_O_SCL:
> +                       break;
> +                       case TCP_P_SACK:
> +                       break;
> +                       case TCP_V_SACK:
> +                       break;

Combine all of these "break" cases. Also, indent "break;".

> +                       case TCP_O_TS:
> +                               tsopt = (struct tcp_t_opt *)p;
> +                               rmt_timestamp = tsopt->t_snd;
> +                               return;
> +                       break;
> +                       }
> +                       if (p[0] == TCP_O_NOP)
> +                               p++;
> +               } else {
> +                       return;

Isn't this the same as break;?

> +               }
> +       }
> +}
> +
> +u8 tcp_state_machine(u8 tcp_flags, u32 *tcp_seq_num, int payload_len)
> +{
> +       u8  tcp_fin  = tcp_flags & TCP_FIN;
> +       u8  tcp_syn  = tcp_flags & TCP_SYN;
> +       u8  tcp_rst  = tcp_flags & TCP_RST;
> +       u8  tcp_push = tcp_flags & TCP_PUSH;
> +       u8  tcp_ack  = tcp_flags & TCP_ACK;
> +       u8  action   = TCP_DATA;
> +       int i;
> +
> +       /*
> +        * tcp_flags are examined to determine TX action in a given state
> +        * tcp_push is interpreted to mean "inform the app"
> +        * urg, ece, cer and nonce flags are not supported.
> +        *
> +        * exe and crw are use to signal and confirm knowledge of congestion.
> +        * This TCP only sends a file request and acks. If it generates
> +        * congestion, the network is broken.
> +        */
> +
> +       debug_cond(DEBUG_INT_STATE, "TCP STATE ENTRY %x\n", action);
> +       if (tcp_rst) {
> +               action    = TCP_DATA;
> +               tcp_state = TCP_CLOSED;
> +               net_set_state(NETLOOP_FAIL);
> +               debug_cond(DEBUG_INT_STATE, "TCP Reset %x\n", tcp_flags);
> +               return TCP_RST;
> +       }
> +
> +       switch  (tcp_state) {
> +       case TCP_CLOSED:
> +       debug_cond(DEBUG_INT_STATE, "TCP CLOSED %x\n", tcp_flags);

Fix indent.

> +               if (tcp_fin)
> +                       action = TCP_DATA;
> +               if (tcp_syn)
> +                       action = TCP_RST;
> +               if (tcp_ack)
> +                       action = TCP_DATA;
> +       break;

Fix indent.

> +       case TCP_SYN_SENT:
> +               debug_cond(DEBUG_INT_STATE, "TCP_SYN_SENT %x, %d\n",
> +                          tcp_flags, *tcp_seq_num);
> +               if (tcp_fin) {
> +                       action = action | TCP_PUSH;
> +                       tcp_state = TCP_CLOSE_WAIT;
> +               }
> +               if (tcp_syn) {
> +                       action = action |  TCP_ACK | TCP_PUSH;
> +                       if (tcp_ack) {
> +                               tcp_seq_init          = *tcp_seq_num;
> +                               *tcp_seq_num          = *tcp_seq_num + 1;
> +                               tcp_seq_max           = *tcp_seq_num;
> +                               tcp_ack_edge          = *tcp_seq_num;
> +                               sack_idx              = 0;
> +                               edge_a[sack_idx].se.l = *tcp_seq_num;
> +                               edge_a[sack_idx].se.r = *tcp_seq_num;
> +                               prev_len              = 0;
> +                               tcp_state             = TCP_ESTABLISHED;
> +                               for (i = 0; i < TCP_SACK; i++)
> +                                       edge_a[i].st   = NOPKT;
> +                       }
> +               } else {
> +                       if (tcp_ack)
> +                               action = TCP_DATA;
> +               }
> +       break;

Fix indent

> +       case TCP_ESTABLISHED:
> +               debug_cond(DEBUG_INT_STATE,
> +                          "TCP_ESTABLISHED %x\n", tcp_flags);
> +               if (*tcp_seq_num > tcp_seq_max)
> +                       tcp_seq_max = *tcp_seq_num;
> +               if (payload_len > 0) {
> +                       tcp_hole(*tcp_seq_num, payload_len, tcp_seq_max);
> +                       tcp_fin = TCP_DATA;  /* cause standalone FIN */
> +               }
> +
> +               if ((tcp_fin) && (tcp_lost.len <= TCP_OPT_LEN_2)) {
> +                       action    = action | TCP_FIN | TCP_PUSH | TCP_ACK;
> +                       tcp_state =  TCP_CLOSE_WAIT;
> +               } else {
> +                       if (tcp_ack)
> +                               action = TCP_DATA;
> +               }
> +               if (tcp_push)
> +                       action = action | TCP_PUSH;
> +               if (tcp_syn)
> +                       action = TCP_ACK + TCP_RST;
> +       break;

Fix indent.

> +       case TCP_CLOSE_WAIT:
> +               debug_cond(DEBUG_INT_STATE, "TCP_CLOSE_WAIT (%x)\n", tcp_flags);
> +               action = TCP_DATA;                      /* Wait for app */
> +       break;

Fix indent.

> +       case TCP_FIN_WAIT_2:
> +               debug_cond(DEBUG_INT_STATE, "TCP_FIN_WAIT_2 (%x)\n", tcp_flags);
> +               if (tcp_fin)
> +                       action =  TCP_DATA;
> +               if (tcp_syn)
> +                       action =  TCP_DATA;
> +               if (tcp_ack) {
> +                       action =  TCP_PUSH | TCP_ACK;
> +                       tcp_state = TCP_CLOSED;
> +                       puts("\n");
> +               }
> +       break;

Fix indent.

> +       case TCP_FIN_WAIT_1:
> +               debug_cond(DEBUG_INT_STATE, "TCP_FIN_WAIT_1 (%x)\n", tcp_flags);
> +               if (tcp_fin) {
> +                       action = TCP_ACK | TCP_FIN;
> +                        tcp_state = TCP_FIN_WAIT_2;
> +               }
> +               if (tcp_syn)
> +                       action =  TCP_RST;
> +               if (tcp_ack) {
> +                       tcp_state = TCP_CLOSED;
> +                       tcp_seq_num = tcp_seq_num + 1;
> +               }
> +       break;

Fix indent.

> +       case TCP_CLOSING:
> +               debug_cond(DEBUG_INT_STATE, "TCP_CLOSING (%x)\n", tcp_flags);
> +               if (tcp_fin)
> +                       action = TCP_DATA;
> +               if (tcp_syn)
> +                       action = TCP_RST;
> +               if (tcp_ack) {
> +                       action = TCP_PUSH;
> +                       tcp_state = TCP_CLOSED;
> +                       puts("\n");
> +               }
> +       break;

Fix indent.

> +       }
> +       return action;
> +}
> +
> +void rxhand_tcp_f(union tcp_build_pkt *b, unsigned int pkt_len)
> +{
> +       int tcp_len     = pkt_len - IP_HDR_SIZE;

Single space before '='.

> +       u16 tcp_rx_xsum = b->ip.hdr.ip_sum;
> +       u8  tcp_action  = TCP_DATA;

Single space before '='.

> +       u32 tcp_seq_num;
> +       u32 tcp_ack_num;
> +       struct in_addr action_and_state;
> +
> +       int tcp_hdr_len;
> +       int payload_len;
> +
> +       /*
> +        * Verify IP header
> +        */
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP RX in RX Sum (to=%pI4, from=%pI4, len=%d)\n",
> +                          &b->ip.hdr.ip_src, &b->ip.hdr.ip_dst, pkt_len);
> +
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "In__________________________________________\n");

Fix indent of these 2 functions.

> +
> +       b->ip.hdr.ip_src        = net_server_ip;
> +       b->ip.hdr.ip_dst        = net_ip;
> +       b->ip.hdr.ip_sum        = 0x0000;

Why all the spaces. Use a single space.

> +       if (tcp_rx_xsum != compute_ip_checksum(b, IP_HDR_SIZE)) {
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP RX IP xSum Error (%pI4, =%pI4, len=%d)\n",
> +                          &net_ip, &net_server_ip, pkt_len);
> +               return;
> +       }
> +
> +       /*
> +        * Build Pseudo header and Verify TCP header

lower-case "pseudo" and "verify".

> +        */
> +       tcp_rx_xsum = b->ip.hdr.tcp_xsum;
> +       b->ip.hdr.tcp_xsum = 0x0000;
> +       if (tcp_rx_xsum != tcp_set_pseudo_header((uchar *)b, b->ip.hdr.ip_src,
> +                                                b->ip.hdr.ip_dst, tcp_len,
> +                                                pkt_len)) {
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP RX TCP xSum Error (%pI4, %pI4, len=%d)\n",
> +                          &net_ip, &net_server_ip, tcp_len);
> +               return;
> +       }
> +
> +       tcp_hdr_len = (b->ip.hdr.tcp_hlen >> 2);
> +       payload_len = tcp_len - tcp_hdr_len;
> +
> +       if (tcp_hdr_len > TCP_HDR_SIZE)
> +               tcp_parse_options((uchar *)b + IP_TCP_HDR_SIZE,
> +                                 tcp_hdr_len - TCP_HDR_SIZE);
> +       /*
> +        * Incoming sequence and ack numbers are server's view of the numbers.
> +        * The app must swap the numbers when responding.
> +        */
> +
> +       tcp_seq_num = ntohl(b->ip.hdr.tcp_seq);
> +       tcp_ack_num = ntohl(b->ip.hdr.tcp_ack);
> +
> +       /*
> +        * Sent first packet send from server as first to app

Swap "send" and "sent". Also, end with '.'

> +        * Other packets could be ordered, but are currently not ordered.
> +        */
> +
> +       tcp_action  = tcp_state_machine(b->ip.hdr.tcp_flags,
> +                                       &tcp_seq_num, payload_len);
> +
> +       /*
> +        * State altering command to be sent.

"State altering" -> "State-altering"

> +        * The packet sequence and ack numbers are in the tcp_seq_num
> +        * and tcp_ack_num variables. The current packet, its position
> +        * in the data stream, is the in the range of those variables.
> +        *
> +        * In the "application push" invocation, the TCP header with all
> +        * its information is pointed to by the packet pointer.
> +        *
> +        * in the typedef

"in" -> "In"

> +        *      void rxhand_tcp(uchar *pkt, unsigned int dport,
> +        *                      struct in_addr sip, unsigned int sport,
> +        *                      unsigned int len);
> +        * *pkt is the pointer to the payload
> +        * dport is used for tcp_seg_num
> +        * action_and_state.s_addr is used for TCP state
> +        * sport is used for tcp_ack_num (which is unused by the app)
> +        * pkt_ length is the payload length.
> +        *
> +        * TCP_PUSH from the state machine with a payload length of 0 is a
> +        * connect or disconnect event
> +        */
> +
> +       tcp_activity_count++;
> +       if (tcp_activity_count > TCP_ACTIVITY) {
> +               puts("| ");
> +               tcp_activity_count = 0;
> +       }
> +
> +       if ((tcp_action & TCP_PUSH) || (payload_len > 0)) {
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Notify (action=%x, Seq=%d,Ack=%d,Pay%d)\n",
> +                          tcp_action, tcp_seq_num, tcp_ack_num, payload_len);
> +
> +               action_and_state.s_addr = tcp_action;
> +               (*tcp_packet_handler) ((uchar *)b + pkt_len - payload_len,
> +                                      tcp_seq_num, action_and_state,
> +                                      tcp_ack_num, payload_len);
> +
> +       } else if (tcp_action != TCP_DATA) {
> +               debug_cond(DEBUG_DEV_PKT,
> +                          "TCP Action (action=%x,Seq=%d,Ack=%d,Pay=%d)\n",
> +                          tcp_action, tcp_seq_num, tcp_ack_num, payload_len);
> +
> +       /*
> +        * Warning: Incoming Seq & Ack  sequence numbers are transposed here
> +        * to outgoing Seq & Ack sequence numbers
> +        */

Fix indent.

> +               net_send_tcp_packet(0, ntohs(b->ip.hdr.tcp_src),
> +                                   ntohs(b->ip.hdr.tcp_dst),
> +                                   (tcp_action & (~TCP_PUSH)),
> +                                   tcp_seq_num, tcp_ack_num);
> +       }
> +}
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-17 15:10   ` Simon Glass
  2018-04-18 15:50     ` Duncan Hare
       [not found]     ` <217820715.1487025.1524002336830@mail.yahoo.com>
@ 2018-05-01  1:50     ` Joe Hershberger
  2 siblings, 0 replies; 22+ messages in thread
From: Joe Hershberger @ 2018-05-01  1:50 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 17, 2018 at 10:10 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Duncan,
>
> On 14 April 2018 at 17:43,  <DH@synoia.com> wrote:
>> From: Duncan Hare <DuncanCHare@yahoo.com>
>>
>> Why http and wget:
>>
>> HTTP is the most efficient file retrieval protocol in common
>> use. The client send a single request, after TCP connection,
>> to receive a file of any length.
>>
>> WGET is the application which implements http file transfer
>> outside browsers as a file transfer protocol. Versions of
>> wget exists on many operating systems.
>> END
>
> Why END?

I would assume that (from reading this blurb) this is really a series
cover letter and the END would terminate that... so the Cover-letter:
tag is probably malformed.

Which begs the question, where the actual log is for the wget patch?

>>
>> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
>> ---
>>
>> Changes in v10: None
>
> There is no change log here. Has nothing changed since v1?

Duncan has been struggling to use patman effectively.

>
> This code looks OK to me, but please can you run it through patman? I
> see what look like some style errors.

This is after using patman since v2 or so.

>
> How can we create a test for this? Can we add something to test_net.py ?
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-25 23:52                 ` Duncan Hare
@ 2018-05-01  1:54                   ` Joe Hershberger
  2018-05-13 21:05                     ` [U-Boot] net: " Duncan Hare
  0 siblings, 1 reply; 22+ messages in thread
From: Joe Hershberger @ 2018-05-01  1:54 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 25, 2018 at 6:52 PM, Duncan Hare <dh@synoia.com> wrote:
>
>
>    From: Simon Glass <sjg@chromium.org>
>  To: Duncan Hare <dh@synoia.com>
> Cc: Wolfgang Denk <wd@denx.de>; U-Boot Mailing List <u-boot@lists.denx.de>; Joe Hershberger <joe.hershberger@ni.com>
>  Sent: Wednesday, April 25, 2018 4:44 PM
>  Subject: Re: [PATCH v10 3/3] Adding wget
>
> Hi Duncan,
>
> On 25 April 2018 at 08:33, Duncan Hare <dh@synoia.com> wrote:
> ____________________
>>> From: Simon Glass <sjg@chromium.org>
>>> To: Duncan Hare <dh@synoia.com>
>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Joe Hershberger
>>> <joe.hershberger@ni.com>
>>> Sent: Tuesday, April 24, 2018 10:01 PM
>>> Subject: Re: [PATCH v10 3/3] Adding wget
>>>
>>> Hi Duncan,
>>>
>>>> On 22 April 2018 at 21:22, Duncan Hare <dh@synoia.com> wrote:
>>>>
>>>>>The server can be tested with the wget command which
>>>>> can be installed on linux.
>>>>> I doubt that loop-back like this will produce the scrambling of packet
>>>>> order
>>>>> which is a feature of push down stacks for packet queues
>>>>> in the internet.
>>>>>
>>>>> Hence my comment in a different thread about buffering on the pi. Few of
>>>>> the
>>>>> socs appear to use net_pkt_buf  buffers for net traffic.
>>>>>
>>>>> If there are too many transmission errors the sending tcp drops the
>>>>> connection. My solution to this is to halve the size of
>>>>> CONFIG_SYS_RX_ETH_BUFFER until transmission works.
>>>>>
>>>> >
>>>>> Possibly CONFIG_SYS_RX_ETH_BUFFER could come under Kconfig.
>>>>
>>>>Just to be clear, I was wondering about having an automated test. Manual
>>>>> tests are not very useful since people won't do them. See 'make tests' for
>>>>> all the test that we >currently >run. I'm pretty sure you could standard up
>>>>> a little server, run your wget, then shut it down, all within a pytest test.
>>>
>>>
>>>>>Regards,
>>>>>Simon
>>>
>>> Hi Wolfgang. Simon
>>>
>>> Can we put a test 4 Mbyte kernel on the u-boot website for an automated test
>>> for other users of TCP & Wget in u-boot?
>>>
>>> Then I can produce a standard u-boot script for testing.
>
>>How about the test just creates a little (4KB) file. We don't want the
>>tests to access a real network, if possible, just use localhost.

This makes it portable to where ever the test is run.

>
>>Regards,
>>Simon
> 4k is 4 packets. I believe most kernels are larger.
> I was think of a static server set up with a known dns name.
> Thta's what I've got.
>
> Do the test setup once.

This assume accessibility to this Internet server and that this server
is up /still configured when the test runs.

Please setup a test that can run in an environment without the
Internet. That is critical for unit tests.

Hand tests for Internet usage and the environmental effects are great,
but that can't be what we include in the auto tests for repeat-ability
reasons. Simon is asking for a separate type of test.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 0/3] Why netboot:
  2018-04-14 23:43 [U-Boot] [PATCH v10 0/3] Why netboot: DH at synoia.com
                   ` (2 preceding siblings ...)
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 3/3] Adding wget DH at synoia.com
@ 2018-05-01  1:57 ` Joe Hershberger
  2018-05-01 21:29   ` Joe Hershberger
  3 siblings, 1 reply; 22+ messages in thread
From: Joe Hershberger @ 2018-05-01  1:57 UTC (permalink / raw)
  To: u-boot

On Sat, Apr 14, 2018 at 6:43 PM,  <DH@synoia.com> wrote:
> From: Duncan Hare <DuncanCHare@yahoo.com>
>
> Central management, including logs and change control,
> coupled with with enhanced security and unauthorized
> change detection and remediation by exposing a
> small attack surface.
>
> Why TCP:
>
> Currently file transfer are done using tftp or NFS both
> over udp. This requires a request to be sent from client
> (u-boot) to the boot server.
>
> For a 4 Mbyte kernel, with a 1k block size this requires
> 4,000 request for a block.
>
> Using a large block size, one greater than the Ethernet
> maximum frame size limitation, would require fragmentation,
> which u-boot supports. However missing fragment recovery
> requires timeout detection and re-transmission requests
> for missing fragments.
>
> UDP is ideally suited to fast single packet exchanges,
> inquiry/response, for example dns, becuse of the lack of
> connection overhead.
>
> UDP as a file transport mechanism is slow, even in low
> latency networks, because file transfer with udp requires
> poll/response mechanism to provide transfer integrity.
>
> In networks with large latency, for example: the internet,
> UDP is even slower. What is a 30 second transfer on a local
> boot server and LAN increase to over 3 minutes, because of
> all the requests/response traffic.
>
> This was anticipated in the evolution of the IP protocols
> and TCP was developed and then enhanced for high latency high
> bandwidth networks.
>
> The current standard is TCP with selective acknowledgment.
>
> In our testing we have reduce kernel transmission time to
> around 0.4 seconds for a 4Mbyte kernel, with a 100 Mbps
> downlink.
>
> Why http and wget:
>
> HTTP is the most efficient file retrieval protocol in common
> use. The client send a single request, after TCP connection,
> to receive a file of any length.
>
> WGET is the application which implements http file transfer
> outside browsers as a file transfer protocol. Versions of
> wget exists on many operating systems.
>
> Changes in v10:
> Initial changes for adding TCP
>
> Duncan Hare (3):
>   Adding TCP and wget into u-boot
>   Adding TCP
>   Adding wget

From https://www.denx.de/wiki/U-Boot/Patches
"""
Use the imperative tense in your summary line (e.g., "Add support for
X" rather than "Adds support for X"). In general, you can think of the
summary line as "this commit is meant to 'Add support for X'"
"""

These patch subjects don't comply.

>
>  cmd/Kconfig        |   5 +
>  cmd/net.c          |  13 +
>  include/net.h      |  33 ++-
>  include/net/tcp.h  | 218 ++++++++++++++++
>  include/net/wget.h |  17 ++
>  net/Kconfig        |  10 +
>  net/Makefile       |   3 +-
>  net/net.c          |  89 +++++--
>  net/ping.c         |   9 +-
>  net/tcp.c          | 749 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/wget.c         | 420 ++++++++++++++++++++++++++++++
>  11 files changed, 1532 insertions(+), 34 deletions(-)
>  create mode 100644 include/net/tcp.h
>  create mode 100644 include/net/wget.h
>  create mode 100644 net/tcp.c
>  create mode 100644 net/wget.c
>
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 3/3] Adding wget
  2018-04-14 23:43 ` [U-Boot] [PATCH v10 3/3] Adding wget DH at synoia.com
  2018-04-17 15:10   ` Simon Glass
@ 2018-05-01  2:18   ` Joe Hershberger
  1 sibling, 0 replies; 22+ messages in thread
From: Joe Hershberger @ 2018-05-01  2:18 UTC (permalink / raw)
  To: u-boot

On Sat, Apr 14, 2018 at 6:43 PM,  <DH@synoia.com> wrote:
> From: Duncan Hare <DuncanCHare@yahoo.com>
>
> Why http and wget:
>
> HTTP is the most efficient file retrieval protocol in common
> use. The client send a single request, after TCP connection,
> to receive a file of any length.
>
> WGET is the application which implements http file transfer
> outside browsers as a file transfer protocol. Versions of
> wget exists on many operating systems.
> END
>
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> ---
>
> Changes in v10: None
>
>  cmd/Kconfig        |   5 +
>  cmd/net.c          |  13 ++
>  include/net.h      |  16 +-
>  include/net/wget.h |  17 +++
>  net/Kconfig        |   7 +-
>  net/Makefile       |   1 +
>  net/wget.c         | 420 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 470 insertions(+), 9 deletions(-)
>  create mode 100644 include/net/wget.h
>  create mode 100644 net/wget.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 136836d146..1113d69950 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1094,6 +1094,11 @@ 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.
>  endif
>
>  endmenu
> diff --git a/cmd/net.c b/cmd/net.c
> index d7c776aacf..6a7a51f357 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:]path and image name]"
> +);
> +#endif
> +
>  static void netboot_update_env(void)
>  {
>         char tmp[22];
> diff --git a/include/net.h b/include/net.h
> index e29d804a23..ec44bf2bec 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -15,10 +15,14 @@
>  #include <asm/cache.h>
>  #include <asm/byteorder.h>     /* for nton* / ntoh* stuff */
>
> -#define DEBUG_LL_STATE  0      /* Link local state machine changes */
> -#define DEBUG_DEV_PKT   0      /* Packets or info directed to the device */
> +#define DEBUG_LL_STATE  0      /* Link local state machine changes        */
> +#define DEBUG_DEV_PKT   0      /* Packets or info directed to the device  */
>  #define DEBUG_NET_PKT   0      /* Packets on info on the network at large */
> -#define DEBUG_INT_STATE 0      /* Internal network state changes */
> +#define DEBUG_INT_STATE 0      /* Internal network state change           */

Random formatting change... also changed differently in a former patch
if I recall.

> +
> +#if defined(CONFIG_TCP)

Why would a #define need to be protected?

> +#define CONFIG_SYS_RX_ETH_BUFFER 12     /* For TCP */
> +#endif
>
>  /*
>   *     The number of receive packet buffers, and the required packet buffer
> @@ -31,10 +35,6 @@
>   *     data, the sending TCP will stop sending.
>   */
>
> -#if defined(CONFIG_TCP)
> -#define CONFIG_SYS_RX_ETH_BUFFER 12    /* For TCP */
> -#endif

Why not just put it in the proper place in the TCP patch instead of
moving it later?

> -
>  #ifdef CONFIG_SYS_RX_ETH_BUFFER
>  # define PKTBUFSRX     CONFIG_SYS_RX_ETH_BUFFER
>  #else
> @@ -362,8 +362,8 @@ struct vlan_ethernet_hdr {
>  #define PROT_PPP_SES   0x8864          /* PPPoE session messages       */
>
>  #define IPPROTO_ICMP    1      /* Internet Control Message Protocol    */
> +#define IPPROTO_TCP      6      /* Transmission Control Protocol        */
>  #define IPPROTO_UDP    17      /* User Datagram Protocol               */
> -#define IPPROTO_TCP     6      /* Transmission Control Protocol        */

Yeah... same thing... Looks like you didn't ignore me when I made this
in an earlier version, you just applied the change to the wrong patch.

>
>  /*
>   *     Internet Protocol (IP) header.
> diff --git a/include/net/wget.h b/include/net/wget.h
> new file mode 100644
> index 0000000000..dce16c573d
> --- /dev/null
> +++ b/include/net/wget.h
> @@ -0,0 +1,17 @@
> +/*
> + * Duncan Hare Copyright 2017
> + */
> +
> +void wget_start(void);                 /* Begin wget */
> +
> +enum WGET_STATE {
> +       WGET_CLOSED,
> +       WGET_CONNECTING,
> +       WGET_CONNECTED,
> +       WGET_TRANSFERRING,
> +       WGET_TRANSFERRED};

Closing brace on its own line.

> +
> +#define        DEBUG_WGET              0       /* Set to 1 for debug messges */
> +#define        SERVER_PORT             80
> +#define        WGET_RETRY_COUNT        30
> +#define        WGET_TIMEOUT            2000UL
> diff --git a/net/Kconfig b/net/Kconfig
> index cc6dd83fc4..c973def3b8 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -36,7 +36,12 @@ config NET_TFTP_VARS
>  config TCP
>         bool "Include Subset TCP stack for wget"
>         help
> -         TCP protocol support for wget.
> +         Selecting TCP protocol support adds TCP for receiving
> +         streams of data. The TCP packets are presented
> +         as received (possibly out of order). The application
> +         receiving the TCP stream places the date by sequence number
> +         as an offset from the kernel address. TCP transmission is
> +         not supported.

This should be in the TCP patch, not modified after-the-fact in the wget patch.

>
>  config BOOTP_BOOTPATH
>         bool "Enable BOOTP BOOTPATH"
> diff --git a/net/Makefile b/net/Makefile
> index 25729ebeeb..f83df5b728 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_NET)  += tftp.o
>  obj-$(CONFIG_TCP)      += tcp.o
> +obj-$(CONFIG_CMD_WGET) += wget.o
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
>  # and this is intentional usage.
> diff --git a/net/wget.c b/net/wget.c
> new file mode 100644
> index 0000000000..5429a1f7b1
> --- /dev/null
> +++ b/net/wget.c
> @@ -0,0 +1,420 @@
> +/*
> + * WGET/HTTP support driver based on U-BOOT's nfs.c
> + * Copyright Duncan Hare <dh@synoia.com> 2017
> + * SPDX-License-Identifier:GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <mapmem.h>
> +#include <net.h>
> +#include <net/wget.h>
> +#include <net/tcp.h>
> +
> +const char bootfile1[]   = "GET ";
> +const  char bootfile3[]   = " HTTP/1.0\r\n\r\n";
> +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 web_server_ip;
> +static int our_port;
> +static int wget_timeout_count;
> +
> +struct pkt_qd {
> +       uchar *pkt;
> +       unsigned int tcp_seq_num;
> +       unsigned int len;
> +};
> +
> +/*
> + * This is a control structure for out of order packets received.
> + * The actual packet bufers are in the kernel space, and are

bufers -> buffers

> + * expected to be overwritten by the downloaded image.
> + */
> +
> +static struct pkt_qd pkt_q[PKTBUFSRX / 4];
> +static int pkt_q_idx;
> +static unsigned long content_length;
> +static unsigned int packets;
> +
> +static unsigned int initial_data_seq_num;
> +
> +static enum  WGET_STATE wget_state;
> +
> +static char *image_url;
> +static unsigned int wget_timeout = WGET_TIMEOUT;
> +
> +static void  wget_timeout_handler(void);
> +
> +static enum net_loop_state wget_loop_state;
> +
> +/* Timeout retry parameters */
> +static u8 r_action;
> +static unsigned int r_tcp_ack_num;
> +static unsigned int r_tcp_seq_num;
> +static int r_len;

"r_" is not clear enough... please use "retry_"

> +
> +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_WGET */
> +
> +               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;
> +}
> +
> +/*
> + * wget response dispatcher
> + * WARNING, This, and only this, is the place in wget,c where

wget,c -> wget.c

> + * SEQUENCE NUMBERS are swapped between incoming (RX)
> + * and outgoing (TX).
> + * Procedure wget_handler() is correct for RX traffic.
> + */
> +static void wget_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 (wget_state) {
> +       case WGET_CLOSED:
> +               debug_cond(DEBUG_WGET, "wget: send SYN\n");
> +               wget_state = WGET_CONNECTING;
> +               net_send_tcp_packet(0, SERVER_PORT, our_port, action,
> +                                   tcp_seq_num, tcp_ack_num);
> +               packets = 0;
> +       break;

Fix indent.

> +       case WGET_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, image_url, strlen(image_url));
> +               offset = offset + strlen(image_url);
> +
> +               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);
> +               wget_state = WGET_CONNECTED;
> +       break;

Fix indent.

> +       case WGET_CONNECTED:
> +       case WGET_TRANSFERRING:
> +       case WGET_TRANSFERRED:
> +               net_send_tcp_packet(0, SERVER_PORT, our_port, action,
> +                                   tcp_seq_num, tcp_ack_num);
> +       break;

Fix indent... and everywhere else that you add a case structure.

> +       }
> +}
> +
> +static void wget_send(u8 action, unsigned int tcp_ack_num,
> +                     unsigned int tcp_seq_num, int len)
> +{
> +       r_action        = action;
> +       r_tcp_ack_num   = tcp_ack_num;
> +       r_tcp_seq_num   = tcp_seq_num;
> +       r_len           = len;
> +       wget_send_stored();
> +}
> +
> +void wget_fail(char *error_message, unsigned int tcp_seq_num,
> +              unsigned int tcp_ack_num, u8 action)
> +{
> +       printf("%s", error_message);
> +       printf("%s", "wget: Transfer Fail\n");
> +       net_set_timeout_handler(0, NULL);
> +       wget_send(action, tcp_seq_num, tcp_ack_num, 0);
> +}
> +
> +void wget_success(u8 action, unsigned int tcp_seq_num,
> +                 unsigned int tcp_ack_num, int len, int packets)
> +{
> +       printf("Packets received %d, Transfer Successful\n", packets);
> +       wget_send(action, tcp_seq_num, tcp_ack_num, len);
> +}
> +
> +/*
> + * Interfaces of U-BOOT
> + */
> +static void wget_timeout_handler(void)
> +{
> +       if (++wget_timeout_count > WGET_RETRY_COUNT) {
> +               puts("\nRetry count exceeded; starting again\n");
> +               wget_send(TCP_RST, 0, 0, 0);
> +               net_start_again();
> +       } else {
> +               puts("T ");
> +               net_set_timeout_handler(wget_timeout +
> +                                       WGET_TIMEOUT * wget_timeout_count,
> +                                       wget_timeout_handler);
> +               wget_send_stored();
> +       }
> +}
> +
> +static void wget_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;
> +       int  i;
> +
> +       pkt[len] = '\0';
> +       pos = strstr((char *)pkt, http_eom);
> +
> +       if (pos == 0) {
> +               debug_cond(DEBUG_WGET,
> +                          "wget: 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(DEBUG_WGET, "wget: 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);
> +
> +               wget_state = WGET_TRANSFERRING;
> +
> +               if (strstr((char *)pkt, http_ok) == 0) {
> +                       debug_cond(DEBUG_WGET,
> +                                  "wget: Connected Bad Xfer\n");
> +                       wget_loop_state = NETLOOP_FAIL;
> +                       wget_send(action, tcp_seq_num, tcp_ack_num, len);
> +               } else {
> +                       debug_cond(DEBUG_WGET,
> +                                  "wget: 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) + 2;
> +                               strict_strtoul(pos, 10, &content_length);
> +                               debug_cond(DEBUG_WGET,
> +                                          "wget: Connected Len %lu\n",
> +                                          content_length);
> +                       }
> +
> +                       net_boot_file_size = 0;
> +
> +                       if (len > hlen)
> +                               store_block(pkt + hlen, 0, len - hlen);
> +                       debug_cond(DEBUG_WGET,
> +                                  "wget: 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(DEBUG_WGET,
> +                                  "wget: Connctd pkt Q %p len %x\n",
> +                                  pkt_q[i].pkt, pkt_q[i].len);
> +                       }
> +               }
> +       }
> +       wget_send(action, tcp_seq_num, tcp_ack_num, len);
> +}
> +
> +       /*
> +        * In the "application push" invocation, the TCP header with all
> +        * its information is pointed to by the packet pointer.
> +        *
> +        * in the typedef
> +        *      void rxhand_tcp(uchar *pkt, unsigned int dport,
> +        *                      struct in_addr sip, unsigned int sport,
> +        *                      unsigned int len);
> +        * *pkt is the pointer to the payload
> +        * dport is used for tcp_seg_num
> +        * action_and_state.s_addr is used for TCP state
> +        * sport is used for tcp_ack_num (which is unused by the app)
> +        * pkt_ length is the payload length.
> +        */
> +static void wget_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 wget_tcp_state = tcp_get_tcp_state();
> +       u8 action = action_and_state.s_addr;
> +
> +       net_set_timeout_handler(wget_timeout, wget_timeout_handler);
> +       packets++;
> +
> +       switch (wget_state) {
> +       case WGET_CLOSED:
> +               debug_cond(DEBUG_WGET, "wget: Handler: Error!, State wrong\n");
> +       break;
> +       case WGET_CONNECTING:
> +               debug_cond(DEBUG_WGET,
> +                          "wget: Connecting In len=%x, Seq=%x, Ack=%x\n",
> +                          len, tcp_seq_num, tcp_ack_num);
> +               if (len == 0) {
> +                       if (wget_tcp_state == TCP_ESTABLISHED) {
> +                               debug_cond(DEBUG_WGET,
> +                                          "wget: Cting, send, len=%x\n", len);
> +                       wget_send(action, tcp_seq_num, tcp_ack_num, len);

Fix indent.

> +                       } else {
> +                               tcp_print_buffer(pkt, len, len, -1, 0);
> +                               wget_fail("wget: Handler Connected Fail\n",
> +                                         tcp_seq_num, tcp_ack_num, action);
> +                       }
> +               }
> +       break;
> +       case WGET_CONNECTED:
> +               debug_cond(DEBUG_WGET, "wget: Connected seq=%x, len=%x\n",
> +                          tcp_seq_num, len);
> +               if (len == 0) {
> +                       wget_fail("Image not found, no data returned\n",
> +                                 tcp_seq_num, tcp_ack_num, action);
> +               } else {
> +                       wget_connected(pkt, tcp_seq_num, action_and_state,
> +                                      tcp_ack_num, len);
> +               }
> +       break;
> +       case WGET_TRANSFERRING:
> +               debug_cond(DEBUG_WGET,
> +                          "wget: 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) {
> +                       wget_fail("wget: store error\n",
> +                                 tcp_seq_num, tcp_ack_num, action);

Just return here, then you don't need the next block to be in the else
case and you can reduce the indentation.

> +               } else {
> +                       switch (wget_tcp_state) {
> +                       case TCP_FIN_WAIT_2:
> +                               wget_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:
> +                               wget_send(TCP_ACK, tcp_seq_num, tcp_ack_num,
> +                                         len);
> +                               wget_loop_state = NETLOOP_SUCCESS;
> +                       break;
> +                       case TCP_CLOSE_WAIT:     /* End of transfer */
> +                               wget_state = WGET_TRANSFERRED;
> +                               wget_send(action | TCP_ACK | TCP_FIN,
> +                                         tcp_seq_num, tcp_ack_num, len);
> +                       break;
> +                       }
> +               }
> +       break;
> +       case WGET_TRANSFERRED:
> +               printf("Packets received %d, Transfer Successful\n", packets);
> +               net_set_state(wget_loop_state);
> +       break;
> +       }
> +}
> +
> +void wget_start(void)
> +{
> +       debug_cond(DEBUG_WGET, "%s\n", __func__);
> +
> +       image_url = strchr(net_boot_file_name, ':');
> +       if (!image_url) {
> +               web_server_ip = string_to_ip(net_boot_file_name);
> +               ++image_url;
> +       } else {
> +               web_server_ip = net_server_ip;
> +               image_url = net_boot_file_name;
> +       }
> +
> +       debug_cond(DEBUG_WGET,
> +                  "wget: Transfer HTTP Server %pI4; our IP %pI4\n",
> +                  &web_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_cond(DEBUG_WGET,
> +                                  "wget: sending through gateway %pI4",
> +                                  &net_gateway);
> +       }
> +       debug_cond(DEBUG_WGET, "URL '%s\n", image_url);
> +
> +       if (net_boot_file_expected_size_in_blocks) {
> +               debug_cond(DEBUG_WGET, "wget: Size is 0x%x Bytes = ",
> +                          net_boot_file_expected_size_in_blocks << 9);
> +               print_size(net_boot_file_expected_size_in_blocks << 9, "");
> +       }
> +       debug_cond(DEBUG_WGET,
> +                  "\nwget:Load address: 0x%lx\nLoading: *\b", load_addr);
> +
> +       net_set_timeout_handler(wget_timeout, wget_timeout_handler);
> +       tcp_set_tcp_handler(wget_handler);
> +
> +       wget_timeout_count = 0;
> +       wget_state = WGET_CLOSED;
> +
> +       our_port = random_port();
> +
> +       /* zero out server ether in case the server ip has changed */
> +       memset(net_server_ethaddr, 0, 6);

Why would we expect that to be the case?

> +
> +       wget_send(TCP_SYN, 0, 0, 0);
> +}
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH v10 0/3] Why netboot:
  2018-05-01  1:57 ` [U-Boot] [PATCH v10 0/3] Why netboot: Joe Hershberger
@ 2018-05-01 21:29   ` Joe Hershberger
  0 siblings, 0 replies; 22+ messages in thread
From: Joe Hershberger @ 2018-05-01 21:29 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 8:57 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
> On Sat, Apr 14, 2018 at 6:43 PM,  <DH@synoia.com> wrote:
>> From: Duncan Hare <DuncanCHare@yahoo.com>
>>
>> Central management, including logs and change control,
>> coupled with with enhanced security and unauthorized
>> change detection and remediation by exposing a
>> small attack surface.
>>
>> Why TCP:
>>
>> Currently file transfer are done using tftp or NFS both
>> over udp. This requires a request to be sent from client
>> (u-boot) to the boot server.
>>
>> For a 4 Mbyte kernel, with a 1k block size this requires
>> 4,000 request for a block.
>>
>> Using a large block size, one greater than the Ethernet
>> maximum frame size limitation, would require fragmentation,
>> which u-boot supports. However missing fragment recovery
>> requires timeout detection and re-transmission requests
>> for missing fragments.
>>
>> UDP is ideally suited to fast single packet exchanges,
>> inquiry/response, for example dns, becuse of the lack of
>> connection overhead.
>>
>> UDP as a file transport mechanism is slow, even in low
>> latency networks, because file transfer with udp requires
>> poll/response mechanism to provide transfer integrity.
>>
>> In networks with large latency, for example: the internet,
>> UDP is even slower. What is a 30 second transfer on a local
>> boot server and LAN increase to over 3 minutes, because of
>> all the requests/response traffic.
>>
>> This was anticipated in the evolution of the IP protocols
>> and TCP was developed and then enhanced for high latency high
>> bandwidth networks.
>>
>> The current standard is TCP with selective acknowledgment.
>>
>> In our testing we have reduce kernel transmission time to
>> around 0.4 seconds for a 4Mbyte kernel, with a 100 Mbps
>> downlink.
>>
>> Why http and wget:
>>
>> HTTP is the most efficient file retrieval protocol in common
>> use. The client send a single request, after TCP connection,
>> to receive a file of any length.
>>
>> WGET is the application which implements http file transfer
>> outside browsers as a file transfer protocol. Versions of
>> wget exists on many operating systems.
>>
>> Changes in v10:
>> Initial changes for adding TCP
>>
>> Duncan Hare (3):
>>   Adding TCP and wget into u-boot
>>   Adding TCP
>>   Adding wget
>
> From https://www.denx.de/wiki/U-Boot/Patches
> """
> Use the imperative tense in your summary line (e.g., "Add support for
> X" rather than "Adds support for X"). In general, you can think of the
> summary line as "this commit is meant to 'Add support for X'"
> """
>
> These patch subjects don't comply.

Additionally, please prefix each of these patches with "net: " like
all other patches to the net subsystem.

Thanks,
-Joe

>>
>>  cmd/Kconfig        |   5 +
>>  cmd/net.c          |  13 +
>>  include/net.h      |  33 ++-
>>  include/net/tcp.h  | 218 ++++++++++++++++
>>  include/net/wget.h |  17 ++
>>  net/Kconfig        |  10 +
>>  net/Makefile       |   3 +-
>>  net/net.c          |  89 +++++--
>>  net/ping.c         |   9 +-
>>  net/tcp.c          | 749 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  net/wget.c         | 420 ++++++++++++++++++++++++++++++
>>  11 files changed, 1532 insertions(+), 34 deletions(-)
>>  create mode 100644 include/net/tcp.h
>>  create mode 100644 include/net/wget.h
>>  create mode 100644 net/tcp.c
>>  create mode 100644 net/wget.c
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] net:   [PATCH v10 3/3] Adding wget
  2018-05-01  1:54                   ` Joe Hershberger
@ 2018-05-13 21:05                     ` Duncan Hare
  2018-05-13 22:00                       ` Simon Glass
  0 siblings, 1 reply; 22+ messages in thread
From: Duncan Hare @ 2018-05-13 21:05 UTC (permalink / raw)
  To: u-boot

 

>>
>>Please setup a test that can run in an environment without the
>>Internet. That is critical for unit tests.
>>
>>Hand tests for Internet usage and the environmental effects are great,
>>but that can't be what we include in the auto tests for repeat-ability
>>reasons. Simon is asking for a separate type of test.

Is the test environment another patch in the series or an addition to the wget patch?
Thanks Duncan


   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] net:  [PATCH v10 3/3] Adding wget
  2018-05-13 21:05                     ` [U-Boot] net: " Duncan Hare
@ 2018-05-13 22:00                       ` Simon Glass
  2018-05-14 15:26                         ` Duncan Hare
  0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2018-05-13 22:00 UTC (permalink / raw)
  To: u-boot

Hi,

On 14 May 2018 at 07:05, Duncan Hare <dh@synoia.com> wrote:
>
>
>
> ________________________________
>>>
>>>Please setup a test that can run in an environment without the
>>>Internet. That is critical for unit tests.
>>>
>>>Hand tests for Internet usage and the environmental effects are great,
>>>but that can't be what we include in the auto tests for repeat-ability
>>>reasons. Simon is asking for a separate type of test.
>
> Is the test environment another patch in the series or an addition to the
> wget patch?

I suggest a separate patch.

Regards,
Simon

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] net:  [PATCH v10 3/3] Adding wget
  2018-05-13 22:00                       ` Simon Glass
@ 2018-05-14 15:26                         ` Duncan Hare
  2018-05-14 19:51                           ` Simon Glass
  0 siblings, 1 reply; 22+ messages in thread
From: Duncan Hare @ 2018-05-14 15:26 UTC (permalink / raw)
  To: u-boot

From: Simon Glass <sjg@chromium.org>


 To: Duncan Hare <dh@synoia.com> 
Cc: "joe.hershberger at ni.com" <joe.hershberger@ni.com>; U-Boot Mailing List <u-boot@lists.denx.de>
 Sent: Sunday, May 13, 2018 3:00 PM
 Subject: Re: net: [U-Boot] [PATCH v10 3/3] Adding wget
   
>>>>
>>>>Please setup a test that can run in an environment without the
>>>>Internet. That is critical for unit tests.
>>>>
>>>>Hand tests for Internet usage and the environmental effects are great,
>>>>but that can't be what we include in the auto tests for repeat-ability
>>>>reasons. Simon is asking for a separate type of test.
>>>
>>> Is the test environment another patch in the series or an addition to the
>>> wget patch?

>I suggest a separate patch.
>>Regards,>>Simon
Separate patch, patch 4 of the series or a completely new patch?
Conceptual approach:

apt-get install nginx (in debian)
I'll provide a config file pointing to test kernel a a specified file location.

Test kernel will be numbered lines of printable characters. Printable, not binary, easy to expand in a spreadsheet.

Can the u-boot print command print sections of memory? That is
print $loadaddr $length? 

That will work for small test kernels, or 

I tested by using tftp to download a kernel, then used a modified wget to download a seconf time and verify thedownloads were identical.

compare $loadaddr1 $loadaddr2 $length

Is that a preferred approach?
 Duncan Hare

714 931 7952

   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] net:  [PATCH v10 3/3] Adding wget
  2018-05-14 15:26                         ` Duncan Hare
@ 2018-05-14 19:51                           ` Simon Glass
  0 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2018-05-14 19:51 UTC (permalink / raw)
  To: u-boot

Hi Duncan,

On 14 May 2018 at 09:26, Duncan Hare <dh@synoia.com> wrote:
> From: Simon Glass <sjg@chromium.org>
>
>
> To: Duncan Hare <dh@synoia.com>
> Cc: "joe.hershberger at ni.com" <joe.hershberger@ni.com>; U-Boot Mailing List
> <u-boot@lists.denx.de>
> Sent: Sunday, May 13, 2018 3:00 PM
> Subject: Re: net: [U-Boot] [PATCH v10 3/3] Adding wget
>
>>>>>
>>>>>Please setup a test that can run in an environment without the
>>>>>Internet. That is critical for unit tests.
>>>>>
>>>>>Hand tests for Internet usage and the environmental effects are great,
>>>>>but that can't be what we include in the auto tests for repeat-ability
>>>>>reasons. Simon is asking for a separate type of test.
>>>>
>>>> Is the test environment another patch in the series or an addition to
>>>> the
>>>> wget patch?
>
>>I suggest a separate patch.
>>
>>Regards,
>>>Simon
>
> Separate patch, patch 4 of the series or a completely new patch?

Patch 4 in the series.

>
> Conceptual approach:
>
> apt-get install nginx (in debian)
>
> I'll provide a config file pointing to test kernel a a specified file
> location.
>
> Test kernel will be numbered lines of printable characters. Printable, not
> binary, easy to expand in a spreadsheet.
>
> Can the u-boot print command print sections of memory? That is
>
> print $loadaddr $length?
>
> That will work for small test kernels, or
>
> I tested by using tftp to download a kernel, then used a modified wget to
> download a seconf time and verify the
> downloads were identical.
>
> compare $loadaddr1 $loadaddr2 $length
>
> Is that a preferred approach?

You don't need to download a kernel. Just download a small file (say
10KB) that you generate in your test. You might find test_vboot.py and
test_net.py helpful. Something like:

- start an http server on localhost (in Python)
- put a single file in the server that can be read (generate 10KB of
known data?)
- run U-Boot with the commands to wget from that server
- check that U-Boot gets the expected file

Regards,
Simon

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-05-14 19:51 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-14 23:43 [U-Boot] [PATCH v10 0/3] Why netboot: DH at synoia.com
2018-04-14 23:43 ` [U-Boot] [PATCH v10 1/3] Adding TCP and wget into u-boot DH at synoia.com
2018-04-30 23:44   ` Joe Hershberger
2018-04-14 23:43 ` [U-Boot] [PATCH v10 2/3] Adding TCP DH at synoia.com
2018-05-01  1:44   ` Joe Hershberger
2018-04-14 23:43 ` [U-Boot] [PATCH v10 3/3] Adding wget DH at synoia.com
2018-04-17 15:10   ` Simon Glass
2018-04-18 15:50     ` Duncan Hare
     [not found]     ` <217820715.1487025.1524002336830@mail.yahoo.com>
     [not found]       ` <CAPnjgZ3yNbMVJCbNQVDxxQ9RaX1pujxKSopk=s1MNUyb=oAyiQ@mail.gmail.com>
2018-04-23  3:22         ` Duncan Hare
2018-04-25  5:01           ` Simon Glass
2018-04-25 14:33             ` Duncan Hare
2018-04-25 23:44               ` Simon Glass
2018-04-25 23:52                 ` Duncan Hare
2018-05-01  1:54                   ` Joe Hershberger
2018-05-13 21:05                     ` [U-Boot] net: " Duncan Hare
2018-05-13 22:00                       ` Simon Glass
2018-05-14 15:26                         ` Duncan Hare
2018-05-14 19:51                           ` Simon Glass
2018-05-01  1:50     ` [U-Boot] " Joe Hershberger
2018-05-01  2:18   ` Joe Hershberger
2018-05-01  1:57 ` [U-Boot] [PATCH v10 0/3] Why netboot: Joe Hershberger
2018-05-01 21:29   ` Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.