All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v14 0/2] add TCP and HTTP for downloading images
@ 2022-04-21 16:54 Ying-Chun Liu
  2022-04-21 16:54 ` [PATCH v14 1/2] net: Add TCP protocol Ying-Chun Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ying-Chun Liu @ 2022-04-21 16:54 UTC (permalink / raw)
  To: u-boot; +Cc: Grant.Likely, Ying-Chun Liu (PaulLiu)

From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

This patch is a refresh from previous patches made by
Duncan Hare <DuncanCHare@yahoo.com>. I've contacted him and
continue to work on this patch.

This patch introduce a TCP stack with SACK. And a simple wget command
to download images from http server.

v13: Fix some issues which is reviewed by Christian
v14: Add options to enable/disable SACK.

Ying-Chun Liu (PaulLiu) (2):
  net: Add TCP protocol
  net: Add wget application

 cmd/Kconfig        |   7 +
 cmd/net.c          |  13 +
 include/net.h      |  10 +-
 include/net/tcp.h  | 222 ++++++++++++++
 include/net/wget.h |  19 ++
 net/Kconfig        |  12 +
 net/Makefile       |   2 +
 net/net.c          |  36 +++
 net/tcp.c          | 709 +++++++++++++++++++++++++++++++++++++++++++++
 net/wget.c         | 436 ++++++++++++++++++++++++++++
 10 files changed, 1464 insertions(+), 2 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.35.1


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

* [PATCH v14 1/2] net: Add TCP protocol
  2022-04-21 16:54 [PATCH v14 0/2] add TCP and HTTP for downloading images Ying-Chun Liu
@ 2022-04-21 16:54 ` Ying-Chun Liu
  2022-04-21 16:59   ` Ying-Chun Liu (PaulLiu)
  2022-05-18  9:05   ` Michal Simek
  2022-04-21 16:54 ` [PATCH v14 2/2] net: Add wget application Ying-Chun Liu
  2022-05-18  9:53 ` [PATCH v14 0/2] add TCP and HTTP for downloading images Michal Simek
  2 siblings, 2 replies; 8+ messages in thread
From: Ying-Chun Liu @ 2022-04-21 16:54 UTC (permalink / raw)
  To: u-boot
  Cc: Grant.Likely, Ying-Chun Liu (PaulLiu),
	Duncan Hare, Duncan Hare, Christian Gmeiner, Joe Hershberger,
	Ramon Fried

From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

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

The current standard is TCP with selective acknowledgment.

Signed-off-by: Duncan Hare <DH@Synoia.com>
Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
Cc: Joe Hershberger <joe.hershberger@ni.com>
Cc: Ramon Fried <rfried.dev@gmail.com>
---
v13: Fix some issues which is reviewed by Christian
v14: Add options to enable/disable SACK.
---
 include/net.h     |   8 +-
 include/net/tcp.h | 222 +++++++++++++++
 net/Kconfig       |  12 +
 net/Makefile      |   1 +
 net/net.c         |  30 ++
 net/tcp.c         | 709 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 981 insertions(+), 1 deletion(-)
 create mode 100644 include/net/tcp.h
 create mode 100644 net/tcp.c

diff --git a/include/net.h b/include/net.h
index 675bf4171b..220eba2e21 100644
--- a/include/net.h
+++ b/include/net.h
@@ -365,6 +365,7 @@ struct vlan_ethernet_hdr {
 #define PROT_NCSI	0x88f8		/* NC-SI control packets        */
 
 #define IPPROTO_ICMP	 1	/* Internet Control Message Protocol	*/
+#define IPPROTO_TCP	 6	/* Transmission Control Protocol	*/
 #define IPPROTO_UDP	17	/* User Datagram Protocol		*/
 
 /*
@@ -687,7 +688,7 @@ static inline void net_send_packet(uchar *pkt, int len)
 }
 
 /*
- * Transmit "net_tx_packet" as UDP packet, performing ARP request if needed
+ * Transmit "net_tx_packet" as UDP or TCP packet, send ARP request if needed
  *  (ether will be populated)
  *
  * @param ether Raw packet buffer
@@ -695,10 +696,15 @@ static inline void net_send_packet(uchar *pkt, int len)
  * @param dport Destination UDP port
  * @param sport Source UDP port
  * @param payload_len Length of data after the UDP header
+ * @param action TCP action to be performed
+ * @param tcp_seq_num TCP sequence number of this transmission
+ * @param tcp_ack_num TCP stream acknolegement number
  */
 int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 		       int payload_len, int proto, u8 action, u32 tcp_seq_num,
 		       u32 tcp_ack_num);
+int net_send_tcp_packet(int payload_len, int dport, int sport, 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);
 
diff --git a/include/net/tcp.h b/include/net/tcp.h
new file mode 100644
index 0000000000..417a6d0f4e
--- /dev/null
+++ b/include/net/tcp.h
@@ -0,0 +1,222 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * TCP Support with SACK for file transfer.
+ *
+ * Copyright 2017 Duncan Hare, All rights reserved.
+ */
+
+#define TCP_ACTIVITY 127		/* Number of packets received   */
+					/* before console progress mark */
+
+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		/* Timestamp 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		*/
+#define TCP_SCALE	0x01		/* Scale			*/
+
+struct tcp_mss {			/* TCP Max Segment size		*/
+	u8	kind;			/* Field ID			*/
+	u8	len;			/* Field length			*/
+	u16	mss;			/* Segment size value		*/
+} __packed;
+
+struct tcp_scale {			/* TCP Windows Scale		*/
+	u8	kind;			/* Field ID			*/
+	u8	len;			/* Filed length			*/
+	u8	scale;			/* windows shift value used for */
+					/* networks with many hops	*/
+					/* Typically 4 or more hops	*/
+} __packed;
+
+struct tcp_sack_p {			/* SACK permitted		*/
+	u8	kind;			/* Field Id			*/
+	u8	len;			/* Field length			*/
+} __packed;
+
+struct sack_edges {
+	u32	l;			/* Left edge of stream		*/
+	u32	r;			/* right edge of stream		*/
+} __packed;
+
+#define TCP_SACK_SIZE (sizeof(struct sack_edges))
+
+/*
+ * A TCP stream has holes when packets are missing or disordered.
+ * A hill is the inverse of a hole, and is data received.
+ * TCP received hills (a sequence of data), and inferrs Holes
+ * from the "hills" or packets received.
+ */
+
+#define TCP_SACK_HILLS	4
+
+struct tcp_sack_v {
+	u8	kind;			/* Field ID		        */
+	u8	len;			/* Field Length			*/
+	struct	sack_edges hill[TCP_SACK_HILLS]; /* L & R window edges	*/
+} __packed;
+
+struct tcp_t_opt {			/* TCP time stamps option	*/
+	u8	kind;			/* Field id			*/
+	u8	len;			/* Field length			*/
+	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 TCP/IP 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 connection
+ */
+
+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 */
+};
+
+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 ef0aa161f7..73e4bcb88e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -159,6 +159,18 @@ config BOOTP_SERVERIP
 	  variable, not the BOOTP server. This affects the operation of both
 	  bootp and tftp.
 
+config PROT_TCP
+	bool "TCP stack"
+	help
+	  Enable a generic tcp framework that allows defining a custom
+	  handler for tcp protocol.
+
+config PROT_TCP_SACK
+	bool "TCP SACK support"
+	help
+	  TCP protocol with SACK. Selecting this will provide the fastest
+	  file transfer possible.
+
 endif   # if NET
 
 config SYS_RX_ETH_BUFFER
diff --git a/net/Makefile b/net/Makefile
index fb3eba840f..f68ad94767 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
 obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
 obj-$(CONFIG_CMD_WOL)  += wol.o
 obj-$(CONFIG_PROT_UDP) += udp.o
+obj-$(CONFIG_PROT_TCP) += tcp.o
 
 # Disable this warning as it is triggered by:
 # sprintf(buf, index ? "foo%d" : "foo", index)
diff --git a/net/net.c b/net/net.c
index 072a82d8f9..28d9fbd227 100644
--- a/net/net.c
+++ b/net/net.c
@@ -116,6 +116,7 @@
 #if defined(CONFIG_CMD_WOL)
 #include "wol.h"
 #endif
+#include <net/tcp.h>
 
 /** BOOTP EXTENTIONS **/
 
@@ -386,6 +387,8 @@ int net_init(void)
 
 		/* Only need to setup buffer pointers once. */
 		first_call = 0;
+		if (IS_ENABLED(CONFIG_PROT_TCP))
+			tcp_set_tcp_state(TCP_CLOSED);
 	}
 
 	return net_init_loop();
@@ -816,6 +819,16 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 				  IPPROTO_UDP, 0, 0, 0);
 }
 
+#if defined(CONFIG_PROT_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, u8 action, u32 tcp_seq_num,
 		       u32 tcp_ack_num)
@@ -847,6 +860,14 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 				   payload_len);
 		pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
 		break;
+#if defined(CONFIG_PROT_TCP)
+	case IPPROTO_TCP:
+		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);
+	break;
+#endif
 	default:
 		return -EINVAL;
 	}
@@ -1253,6 +1274,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_PROT_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..ebd349c3e8
--- /dev/null
+++ b/net/tcp.c
@@ -0,0 +1,709 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2017 Duncan Hare, all rights reserved.
+ */
+
+/*
+ * General Desription:
+ *
+ * TCP support for the wget command, for fast file downloading.
+ *
+ * HTTP/TCP Receiver:
+ *
+ *      Prerequisites:  - 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 <env_internal.h>
+#include <errno.h>
+#include <net.h>
+#include <net/tcp.h>
+
+/*
+ * 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 dynamic function pointer for TCP based commands 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;
+}
+
+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 = TCP_1_NOP;
+	b->sack.sack_v.len  = 0x00;
+
+#if defined(CONFIG_PROT_TCP_SACK)
+	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);
+
+#else
+	b->sack.sack_v.kind = 0x00;
+	b->sack.hdr.tcp_hlen = (((TCP_HDR_SIZE + TCP_TSOPT_SIZE
+				+ 3)  >> 2) << 4);
+#endif
+	/*
+	 * 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;
+#if defined(CONFIG_PROT_TCP_SACK)
+	b->ip.sack_p.kind       = TCP_P_SACK;
+	b->ip.sack_p.len        = TCP_OPT_LEN_2;
+#else
+	b->ip.sack_p.kind       = TCP_1_NOP;
+	b->ip.sack_p.len        = TCP_1_NOP;
+#endif
+	b->ip.t_opt.kind        = TCP_O_TS;
+	b->ip.t_opt.len         = TCP_OPT_LEN_A;
+	loc_timestamp           = get_ticks();
+	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;
+
+/*
+ * Header: 5 32 bit words. 4 bits TCP header Length, 4 bits reserved options
+ */
+	b->ip.hdr.tcp_flags	= action;
+	pkt_hdr_len		= IP_TCP_HDR_SIZE;
+	b->ip.hdr.tcp_hlen      = 0x50;
+
+	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 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);
+
+	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)
+ */
+
+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 new seq number in correct place in receive array */
+
+	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 calculation, calculation breaks.
+ */
+		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;
+/*
+ *	NOPs are options with a zero length, and thus are special.
+ *	All other options have length fields.
+ */
+
+	for (p = o; p < (o + o_len); p = p + p[1]) {
+		if (p[1] != 0) {
+			switch (p[0]) {
+			case TCP_O_END:
+				return; /* Finished processing options */
+			case TCP_O_MSS:
+			case TCP_O_SCL:
+			case TCP_P_SACK:
+			case TCP_V_SACK:
+				break; /* Continue to process options */
+			case TCP_O_TS:
+				tsopt = (struct tcp_t_opt *)p;
+				rmt_timestamp = tsopt->t_snd;
+				return;
+			break;
+			} /* End switch, process optional NOPs */
+
+			if (p[0] == TCP_O_NOP)
+				p++;
+		} else {
+			return; /* Finished processing options */
+		}
+	}
+}
+
+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);
+
+	/* Packets are not ordered. Send to app as received. */
+
+	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 Ack & Seq 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.35.1


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

* [PATCH v14 2/2] net: Add wget application
  2022-04-21 16:54 [PATCH v14 0/2] add TCP and HTTP for downloading images Ying-Chun Liu
  2022-04-21 16:54 ` [PATCH v14 1/2] net: Add TCP protocol Ying-Chun Liu
@ 2022-04-21 16:54 ` Ying-Chun Liu
  2022-05-18  9:51   ` Michal Simek
  2022-05-18  9:53 ` [PATCH v14 0/2] add TCP and HTTP for downloading images Michal Simek
  2 siblings, 1 reply; 8+ messages in thread
From: Ying-Chun Liu @ 2022-04-21 16:54 UTC (permalink / raw)
  To: u-boot
  Cc: Grant.Likely, Ying-Chun Liu (PaulLiu),
	Duncan Hare, Christian Gmeiner, Joe Hershberger, Ramon Fried

From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

This commit adds a simple wget command that can download files
from http server.

Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
Cc: Joe Hershberger <joe.hershberger@ni.com>
Cc: Ramon Fried <rfried.dev@gmail.com>
---
v13: Fix some issues which is reviewed by Christian
---
 cmd/Kconfig        |   7 +
 cmd/net.c          |  13 ++
 include/net.h      |   2 +-
 include/net/wget.h |  19 ++
 net/Makefile       |   1 +
 net/net.c          |   6 +
 net/wget.c         | 436 +++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 483 insertions(+), 1 deletion(-)
 create mode 100644 include/net/wget.h
 create mode 100644 net/wget.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index d3abe3a06b..6eff068feb 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1667,6 +1667,13 @@ config NFS_TIMEOUT
 	  "ERROR: Cannot umount" in nfs command, try longer timeout such as
 	  10000.
 
+config CMD_WGET
+	bool "wget"
+	select TCP
+	help
+	  Download kernel, or other files, from a web server over TCP.
+	  Fast file transfer over networks with latenc
+
 config CMD_MII
 	bool "mii"
 	imply CMD_MDIO
diff --git a/cmd/net.c b/cmd/net.c
index 3619c843d8..60fd785061 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -124,6 +124,19 @@ U_BOOT_CMD(
 );
 #endif
 
+#if defined(CONFIG_CMD_WGET)
+static int do_wget(struct cmd_tbl *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 220eba2e21..3553f17a8e 100644
--- a/include/net.h
+++ b/include/net.h
@@ -557,7 +557,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, FASTBOOT, WOL, UDP
+	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, WGET
 };
 
 extern char	net_boot_file_name[1024];/* Boot File name */
diff --git a/include/net/wget.h b/include/net/wget.h
new file mode 100644
index 0000000000..61bdd851f9
--- /dev/null
+++ b/include/net/wget.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * 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/Makefile b/net/Makefile
index f68ad94767..54ef337e80 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
 obj-$(CONFIG_CMD_WOL)  += wol.o
 obj-$(CONFIG_PROT_UDP) += udp.o
 obj-$(CONFIG_PROT_TCP) += tcp.o
+obj-$(CONFIG_CMD_WGET) += wget.o
 
 # Disable this warning as it is triggered by:
 # sprintf(buf, index ? "foo%d" : "foo", index)
diff --git a/net/net.c b/net/net.c
index 28d9fbd227..5044c42add 100644
--- a/net/net.c
+++ b/net/net.c
@@ -117,6 +117,7 @@
 #include "wol.h"
 #endif
 #include <net/tcp.h>
+#include <net/wget.h>
 
 /** BOOTP EXTENTIONS **/
 
@@ -505,6 +506,11 @@ restart:
 			nfs_start();
 			break;
 #endif
+#if defined(CONFIG_CMD_WGET)
+		case WGET:
+			wget_start();
+			break;
+#endif
 #if defined(CONFIG_CMD_CDP)
 		case CDP:
 			cdp_start();
diff --git a/net/wget.c b/net/wget.c
new file mode 100644
index 0000000000..e3d1c8cc28
--- /dev/null
+++ b/net/wget.c
@@ -0,0 +1,436 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * WGET/HTTP support driver based on U-BOOT's nfs.c
+ * Copyright Duncan Hare <dh@synoia.com> 2017
+ */
+
+#include <common.h>
+#include <command.h>
+#include <mapmem.h>
+#include <env.h>
+#include <image.h>
+#include <net.h>
+#include <net/wget.h>
+#include <net/tcp.h>
+
+static const char bootfile1[]   = "GET ";
+static const char bootfile3[]   = " HTTP/1.0\r\n\r\n";
+static const char http_eom[]     = "\r\n\r\n";
+static const char http_ok[]      = "200";
+static const char content_len[]  = "Content-Length";
+static 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 retry_action;
+static unsigned int retry_tcp_ack_num;
+static unsigned int retry_tcp_seq_num;
+static int retry_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 (image_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)(image_load_addr + offset), len);
+		if (rc) {
+			flash_perror(rc);
+			return -1;
+		}
+	} else {
+#endif /* CONFIG_SYS_DIRECT_FLASH_WGET */
+
+		ptr = map_sysmem(image_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                  = retry_action;
+	unsigned int tcp_ack_num   = retry_tcp_ack_num;
+	unsigned int tcp_seq_num   = retry_tcp_seq_num;
+	int len                    = retry_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)
+{
+	retry_action      = action;
+	retry_tcp_ack_num = tcp_ack_num;
+	retry_tcp_seq_num = tcp_seq_num;
+	retry_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 *)image_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;
+		printf("%.*s", i,  pkt);
+
+		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 {
+				printf("%.*s", len,  pkt);
+				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);
+			return;
+		}
+
+		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;
+	}
+}
+
+/*
+ * make port a little random (1024-17407)
+ * This keeps the math somewhat trivial to compute, and seems to work with
+ * all supported protocols/clients/servers
+ */
+static unsigned int random_port(void)
+{
+	return 1024 + (get_timer(0) % 0x4000);
+}
+
+void wget_start(void)
+{
+	image_url = strchr(net_boot_file_name, ':');
+	if (image_url > 0) {
+		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", image_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 to forece apr resolution in case
+	 * the server ip for the previous u-boot commsnd, for exanple dns
+	 * is not the same as the web server ip.
+	 */
+
+	memset(net_server_ethaddr, 0, 6);
+
+	wget_send(TCP_SYN, 0, 0, 0);
+}
-- 
2.35.1


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

* Re: [PATCH v14 1/2] net: Add TCP protocol
  2022-04-21 16:54 ` [PATCH v14 1/2] net: Add TCP protocol Ying-Chun Liu
@ 2022-04-21 16:59   ` Ying-Chun Liu (PaulLiu)
  2022-05-18  9:05   ` Michal Simek
  1 sibling, 0 replies; 8+ messages in thread
From: Ying-Chun Liu (PaulLiu) @ 2022-04-21 16:59 UTC (permalink / raw)
  To: Ying-Chun Liu, u-boot
  Cc: Grant.Likely, Duncan Hare, Duncan Hare, Christian Gmeiner,
	Joe Hershberger, Ramon Fried

>Hi Ying,
>You woke up an old can, Can you elaborate on the changes you did to
>the original patchset ?
>I'm reviewing this instead of Joe, so I want to get some context.
>
>I would appreciate it if you could separate the SACK support to a
>separate patch, preferably option based.
>The TCP stack is complicated enough and will require a lot of
>tinkering / bug fixing in the future. It's best if we could turn
>on/off specific RFC which are not originally part of the TCP protocol.
>
>Thanks,
>Ramon

Hi Ramon,

Please review this patch again.

I've added an option to disable/enable SACK.
However it is harder to separate the SACK support as a separate commit.
I continue the old one which was together with SACK already.

Thanks a lot,
Paul


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

* Re: [PATCH v14 1/2] net: Add TCP protocol
  2022-04-21 16:54 ` [PATCH v14 1/2] net: Add TCP protocol Ying-Chun Liu
  2022-04-21 16:59   ` Ying-Chun Liu (PaulLiu)
@ 2022-05-18  9:05   ` Michal Simek
  1 sibling, 0 replies; 8+ messages in thread
From: Michal Simek @ 2022-05-18  9:05 UTC (permalink / raw)
  To: Ying-Chun Liu, u-boot
  Cc: Grant.Likely, Ying-Chun Liu (PaulLiu),
	Duncan Hare, Duncan Hare, Christian Gmeiner, Joe Hershberger,
	Ramon Fried



On 4/21/22 18:54, Ying-Chun Liu wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> 
> Currently file transfers are done using tftp or NFS both
> over udp. This requires a request to be sent from client
> (u-boot) to the boot server.
> 
> The current standard is TCP with selective acknowledgment.
> 
> Signed-off-by: Duncan Hare <DH@Synoia.com>
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> ---
> v13: Fix some issues which is reviewed by Christian
> v14: Add options to enable/disable SACK.
> ---
>   include/net.h     |   8 +-
>   include/net/tcp.h | 222 +++++++++++++++
>   net/Kconfig       |  12 +
>   net/Makefile      |   1 +
>   net/net.c         |  30 ++
>   net/tcp.c         | 709 ++++++++++++++++++++++++++++++++++++++++++++++
>   6 files changed, 981 insertions(+), 1 deletion(-)
>   create mode 100644 include/net/tcp.h
>   create mode 100644 net/tcp.c
> 
> diff --git a/include/net.h b/include/net.h
> index 675bf4171b..220eba2e21 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -365,6 +365,7 @@ struct vlan_ethernet_hdr {
>   #define PROT_NCSI	0x88f8		/* NC-SI control packets        */
>   
>   #define IPPROTO_ICMP	 1	/* Internet Control Message Protocol	*/
> +#define IPPROTO_TCP	 6	/* Transmission Control Protocol	*/

here is tab followed by space. I know it is on ICMP but no reason to c&p the 
same issue. Just use tab for indentation.


>   #define IPPROTO_UDP	17	/* User Datagram Protocol		*/
>   
>   /*
> @@ -687,7 +688,7 @@ static inline void net_send_packet(uchar *pkt, int len)
>   }
>   
>   /*

Why not to switch it to kernel-doc format and let it also check.

> - * Transmit "net_tx_packet" as UDP packet, performing ARP request if needed
> + * Transmit "net_tx_packet" as UDP or TCP packet, send ARP request if needed
>    *  (ether will be populated)
>    *
>    * @param ether Raw packet buffer
> @@ -695,10 +696,15 @@ static inline void net_send_packet(uchar *pkt, int len)
>    * @param dport Destination UDP port
>    * @param sport Source UDP port
>    * @param payload_len Length of data after the UDP header
> + * @param action TCP action to be performed
> + * @param tcp_seq_num TCP sequence number of this transmission
> + * @param tcp_ack_num TCP stream acknolegement number
>    */
>   int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>   		       int payload_len, int proto, u8 action, u32 tcp_seq_num,
>   		       u32 tcp_ack_num);
> +int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
> +			u32 tcp_seq_num, u32 tcp_ack_num);

new function should require also kernel-doc documentation.

>   int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
>   			int sport, int payload_len);
>   
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> new file mode 100644
> index 0000000000..417a6d0f4e
> --- /dev/null
> +++ b/include/net/tcp.h

[u-boot](debian-sent3)$ ./scripts/kernel-doc -v -man include/net/tcp.h > /dev/null
include/net/tcp.h:1: warning: no structured comments found

Do you really have nothing to document?


> @@ -0,0 +1,222 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * TCP Support with SACK for file transfer.
> + *
> + * Copyright 2017 Duncan Hare, All rights reserved.
> + */
> +
> +#define TCP_ACTIVITY 127		/* Number of packets received   */
> +					/* before console progress mark */
> +
> +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	*/

I would move all comments directly to kernel-doc documentation. The same for 
others below.

> +} __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		/* Timestamp 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
> + */

Is this still valid?

> +
> +#define TCP_MSS		1460		/* Max segment size		*/
> +#define TCP_SCALE	0x01		/* Scale			*/
> +
> +struct tcp_mss {			/* TCP Max Segment size		*/
> +	u8	kind;			/* Field ID			*/
> +	u8	len;			/* Field length			*/
> +	u16	mss;			/* Segment size value		*/
> +} __packed;
> +
> +struct tcp_scale {			/* TCP Windows Scale		*/
> +	u8	kind;			/* Field ID			*/
> +	u8	len;			/* Filed length			*/
> +	u8	scale;			/* windows shift value used for */
> +					/* networks with many hops	*/
> +					/* Typically 4 or more hops	*/
> +} __packed;
> +
> +struct tcp_sack_p {			/* SACK permitted		*/
> +	u8	kind;			/* Field Id			*/
> +	u8	len;			/* Field length			*/
> +} __packed;
> +
> +struct sack_edges {
> +	u32	l;			/* Left edge of stream		*/
> +	u32	r;			/* right edge of stream		*/
> +} __packed;
> +
> +#define TCP_SACK_SIZE (sizeof(struct sack_edges))
> +
> +/*
> + * A TCP stream has holes when packets are missing or disordered.
> + * A hill is the inverse of a hole, and is data received.
> + * TCP received hills (a sequence of data), and inferrs Holes
> + * from the "hills" or packets received.
> + */
> +
> +#define TCP_SACK_HILLS	4
> +
> +struct tcp_sack_v {
> +	u8	kind;			/* Field ID		        */
> +	u8	len;			/* Field Length			*/
> +	struct	sack_edges hill[TCP_SACK_HILLS]; /* L & R window edges	*/
> +} __packed;
> +
> +struct tcp_t_opt {			/* TCP time stamps option	*/
> +	u8	kind;			/* Field id			*/
> +	u8	len;			/* Field length			*/
> +	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 TCP/IP 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 connection
> + */
> +
> +enum TCP_STATE {

what's the reason for using uper case. When I look at other enums they are 
normally lower case.

> +	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 */
> +};
> +
> +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
> + */

kernel-doc please.

> +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 ef0aa161f7..73e4bcb88e 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -159,6 +159,18 @@ config BOOTP_SERVERIP
>   	  variable, not the BOOTP server. This affects the operation of both
>   	  bootp and tftp.
>   
> +config PROT_TCP
> +	bool "TCP stack"
> +	help
> +	  Enable a generic tcp framework that allows defining a custom
> +	  handler for tcp protocol.
> +
> +config PROT_TCP_SACK
> +	bool "TCP SACK support"

here should be at least dependency on PROT_TCP. If you don't have it and select 
PROT_TCP_SACK but not PROT_TCP you get build issue when 2/2 is applied.

> +	help
> +	  TCP protocol with SACK. Selecting this will provide the fastest

you have good opportunity to describe what SACK is. Based on this description 
you are telling to users to study what SACK is and if make sense to enable it.
Any guindance where this should be selected would be also useful.

> +	  file transfer possible.
> +
>   endif   # if NET
>   
>   config SYS_RX_ETH_BUFFER
> diff --git a/net/Makefile b/net/Makefile
> index fb3eba840f..f68ad94767 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>   obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>   obj-$(CONFIG_CMD_WOL)  += wol.o
>   obj-$(CONFIG_PROT_UDP) += udp.o
> +obj-$(CONFIG_PROT_TCP) += tcp.o
>   
>   # Disable this warning as it is triggered by:
>   # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/net.c b/net/net.c
> index 072a82d8f9..28d9fbd227 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -116,6 +116,7 @@
>   #if defined(CONFIG_CMD_WOL)
>   #include "wol.h"
>   #endif
> +#include <net/tcp.h>
>   
>   /** BOOTP EXTENTIONS **/
>   
> @@ -386,6 +387,8 @@ int net_init(void)
>   
>   		/* Only need to setup buffer pointers once. */
>   		first_call = 0;
> +		if (IS_ENABLED(CONFIG_PROT_TCP))
> +			tcp_set_tcp_state(TCP_CLOSED);
>   	}
>   
>   	return net_init_loop();
> @@ -816,6 +819,16 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>   				  IPPROTO_UDP, 0, 0, 0);
>   }
>   
> +#if defined(CONFIG_PROT_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, u8 action, u32 tcp_seq_num,
>   		       u32 tcp_ack_num)
> @@ -847,6 +860,14 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>   				   payload_len);
>   		pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
>   		break;
> +#if defined(CONFIG_PROT_TCP)
> +	case IPPROTO_TCP:
> +		pkt_hdr_size = eth_hdr_size +
> +		tcp_set_tcp_header(pkt + eth_hdr_size, dport, sport,

indentation is weird.

> +				   payload_len, action, tcp_seq_num,
> +				   tcp_ack_num);
> +	break;
> +#endif
>   	default:
>   		return -EINVAL;
>   	}
> @@ -1253,6 +1274,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_PROT_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..ebd349c3e8
> --- /dev/null
> +++ b/net/tcp.c
> @@ -0,0 +1,709 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2017 Duncan Hare, all rights reserved.
> + */
> +
> +/*
> + * General Desription:
> + *
> + * TCP support for the wget command, for fast file downloading.
> + *
> + * HTTP/TCP Receiver:
> + *
> + *      Prerequisites:  - 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 <env_internal.h>
> +#include <errno.h>
> +#include <net.h>
> +#include <net/tcp.h>
> +
> +/*
> + * TCP sliding window  control used by us to request re-TX
> + */
> +
> +static struct tcp_sack_v tcp_lost;

this is only valid when PROT_TCP_SACK is enabled. You should find a way how to 
handle this.

> +
> +/* 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;

All of these should be static.


> +
> +/*
> + * 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;

The same for these. They should be static and any description what they are used 
for would be also useful.

> +
> +/* TCP connection state */
> +static enum TCP_STATE tcp_state;
> +
> +/*
> + * 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.
> + */
> +
> +/* 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;
> +}
> +

documentation would be helpful.

> +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;

0x00 is just 0

> +
> +	net_copy_ip((void *)&b->ph.p_src, &src);
> +	net_copy_ip((void *)&b->ph.p_dst, &dest);
> +	b->ph.rsvd	= 0x00;

here too. And remove this indentation in front of =.


> +	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;

here is also attempt for indentation which is not even aligned with the code 
below. Just remove that indentation completely and you don't need to worrry 
about it.

> +
> +	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 = TCP_1_NOP;
> +	b->sack.sack_v.len  = 0x00;
> +
> +#if defined(CONFIG_PROT_TCP_SACK)

Reported by checkpatch. No reason to use if defined here.

> +	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;

another indentation.

> +		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.
> +	 */

this comment is also valid for your

> +
> +	b->sack.hdr.tcp_hlen = (((TCP_HDR_SIZE + TCP_TSOPT_SIZE
> +				+ tcp_lost.len + 3)  >> 2) << 4);

This is at least second time I see >> 2) << 4 and one more below. Why not to 
create any macro for it or macro for that values.

> +
> +#else
> +	b->sack.sack_v.kind = 0x00;
> +	b->sack.hdr.tcp_hlen = (((TCP_HDR_SIZE + TCP_TSOPT_SIZE
> +				+ 3)  >> 2) << 4);

Does it make sense to create macro for this?
CP_HDR_SIZE + TCP_TSOPT_SIZE + 3


> +#endif
> +	/*
> +	 * 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;

And another shift. Is it the same 2 as above? Use macros.

> +}
> +
> +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;
> +#if defined(CONFIG_PROT_TCP_SACK)

another checkpatch warning here.


> +	b->ip.sack_p.kind       = TCP_P_SACK;
> +	b->ip.sack_p.len        = TCP_OPT_LEN_2;
> +#else
> +	b->ip.sack_p.kind       = TCP_1_NOP;
> +	b->ip.sack_p.len        = TCP_1_NOP;
> +#endif
> +	b->ip.t_opt.kind        = TCP_O_TS;
> +	b->ip.t_opt.len         = TCP_OPT_LEN_A;
> +	loc_timestamp           = get_ticks();
> +	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;

single line and no indenatation please.

> +
> +/*
> + * Header: 5 32 bit words. 4 bits TCP header Length, 4 bits reserved options
> + */

this should be aligned with the code.

> +	b->ip.hdr.tcp_flags	= action;
> +	pkt_hdr_len		= IP_TCP_HDR_SIZE;
> +	b->ip.hdr.tcp_hlen      = 0x50;

no indenatation please. and 0x50 looks like magic value - macro for it please.

> +
> +	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 SYNs */
> +			action    = TCP_FIN;

remove indenatation

> +			tcp_state = TCP_FIN_WAIT_1;
> +		} else {
> +			tcp_state = TCP_SYN_SENT;
> +		}
> +	break;

indent with the code. The same for breaks below.

> +	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):

this looks quite weird. Please fix it.

> +		if (tcp_state == TCP_CLOSE_WAIT)
> +			tcp_state = TCP_CLOSING;

newline

> +		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 */

Checkpatch reports this as an error.

CHECK: Prefer 'fallthrough;' over fallthrough comment
#271: FILE: net/tcp.c:271:
+					/* 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;

indentation.

> +
> +	/*
> +	 * 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);

 >>double space - please remove.

> +
> +	b->ip.hdr.tcp_xsum	= 0x0000;
> +	b->ip.hdr.tcp_ugr	= 0x0000;

indentation.

> +
> +	b->ip.hdr.tcp_xsum = tcp_set_pseudo_header(pkt, net_ip, net_server_ip,
> +						   tcp_len, pkt_len);
> +
> +	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)
> + */
> +
> +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;

please squashed some of them together. So many lines for nothing.

> +	enum pkt_state expect = PKT;
> +

no reason for newline here.

> +	u32 seq   = tcp_seq_num - tcp_seq_init;
> +	u32 hol_l = tcp_ack_edge - tcp_seq_init;
> +	u32 hol_r = 0;

any reason to mix unsigned int with u32. Just use the same typ.

> +
> +	/* Place new seq number in correct place in receive array */
> +
remove this newline.

> +	if (prev_len == 0)
> +		prev_len = len;

newline

> +	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 calculation, calculation breaks.
> + */

not aligned comment with the code.

> +		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++)  {

alignment should be different.

> +		switch (expect) {
> +		case NOPKT:
> +			switch (edge_a[sack_in].st) {
> +			case NOPKT:
> +				debug_cond(DEBUG_INT_STATE, "N");
> +			break;

indent it with the code.

> +			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;

ditto.

> +			}
> +		break;

ditto and the same below.

> +		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;
> +/*
> + *	NOPs are options with a zero length, and thus are special.
> + *	All other options have length fields.
> + */

indent with the code.

> +
> +	for (p = o; p < (o + o_len); p = p + p[1]) {
> +		if (p[1] != 0) {
> +			switch (p[0]) {
> +			case TCP_O_END:
> +				return; /* Finished processing options */

useless comment

> +			case TCP_O_MSS:
> +			case TCP_O_SCL:
> +			case TCP_P_SACK:
> +			case TCP_V_SACK:
> +				break; /* Continue to process options */
> +			case TCP_O_TS:
> +				tsopt = (struct tcp_t_opt *)p;
> +				rmt_timestamp = tsopt->t_snd;
> +				return;
> +			break;

indentation.

> +			} /* End switch, process optional NOPs */

we see that this is end of switch. Just remove it and put it to next line.

> +
> +			if (p[0] == TCP_O_NOP)
> +				p++;
> +		} else {
> +			return; /* Finished processing options */

It will be much better if you remove one level of indenation and simply do

if (!p[1])
	return;

switch (p[0]) {
....




> +		}
> +	}
> +}
> +
> +u8 tcp_state_machine(u8 tcp_flags, u32 *tcp_seq_num, int payload_len)

static?

> +{
> +	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;

indentation - fix the whole file. I see it below too. Fix it everywhere.

> +	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;

does order matter here? Or is it
if
else if
else if?

> +		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;

another double space here.

> +			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)

else if (tcp_ack) {
...

> +				action = TCP_DATA;
> +		}
> +		break;
> +	case TCP_ESTABLISHED:
> +		debug_cond(DEBUG_INT_STATE,
> +			   "TCP_ESTABLISHED %x\n", tcp_flags);

this doesn't look longer than 80 chars.

> +		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)

else if {

> +				action = TCP_DATA;
> +		}
> +		if (tcp_push)
> +			action = action | TCP_PUSH;
> +		if (tcp_syn)
> +			action = TCP_ACK + TCP_RST;

here is another pattern as above and below it is the same. Does order matter 
here because you are rewriting action. Is it correct?


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

is comment valid?


> +		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;

pack it a little bit.

> +
> +	/*
> +	 * Verify IP header
> +	 */

single line comment.

> +	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");

hmm - interesting description.

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

for me 0x0000 is just 0.

> +	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
> +	 */

single line.

> +	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);

another >> 2 - macro please.

> +	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);
> +
> +	/* Packets are not ordered. Send to app as received. */
> +
> +	tcp_action  = tcp_state_machine(b->ip.hdr.tcp_flags,

double space - check the whole file.

> +					&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 Ack & Seq 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);
> +	}
> +}

M

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

* Re: [PATCH v14 2/2] net: Add wget application
  2022-04-21 16:54 ` [PATCH v14 2/2] net: Add wget application Ying-Chun Liu
@ 2022-05-18  9:51   ` Michal Simek
  0 siblings, 0 replies; 8+ messages in thread
From: Michal Simek @ 2022-05-18  9:51 UTC (permalink / raw)
  To: Ying-Chun Liu, u-boot
  Cc: Grant.Likely, Ying-Chun Liu (PaulLiu),
	Duncan Hare, Christian Gmeiner, Joe Hershberger, Ramon Fried



On 4/21/22 18:54, Ying-Chun Liu wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> 
> This commit adds a simple wget command that can download files
> from http server.

I think description can be much bigger. I was running just wget and it did 
something.

> 
> Signed-off-by: Duncan Hare <DuncanCHare@yahoo.com>
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> ---
> v13: Fix some issues which is reviewed by Christian
> ---
>   cmd/Kconfig        |   7 +
>   cmd/net.c          |  13 ++
>   include/net.h      |   2 +-
>   include/net/wget.h |  19 ++
>   net/Makefile       |   1 +
>   net/net.c          |   6 +
>   net/wget.c         | 436 +++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 483 insertions(+), 1 deletion(-)
>   create mode 100644 include/net/wget.h
>   create mode 100644 net/wget.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index d3abe3a06b..6eff068feb 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1667,6 +1667,13 @@ config NFS_TIMEOUT
>   	  "ERROR: Cannot umount" in nfs command, try longer timeout such as
>   	  10000.
>   
> +config CMD_WGET
> +	bool "wget"
> +	select TCP
> +	help
> +	  Download kernel, or other files, from a web server over TCP.
> +	  Fast file transfer over networks with latenc

latenc?


> +
>   config CMD_MII
>   	bool "mii"
>   	imply CMD_MDIO
> diff --git a/cmd/net.c b/cmd/net.c
> index 3619c843d8..60fd785061 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -124,6 +124,19 @@ U_BOOT_CMD(
>   );
>   #endif
>   
> +#if defined(CONFIG_CMD_WGET)
> +static int do_wget(struct cmd_tbl *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 220eba2e21..3553f17a8e 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -557,7 +557,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, FASTBOOT, WOL, UDP
> +	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, WGET
>   };
>   
>   extern char	net_boot_file_name[1024];/* Boot File name */
> diff --git a/include/net/wget.h b/include/net/wget.h
> new file mode 100644
> index 0000000000..61bdd851f9
> --- /dev/null
> +++ b/include/net/wget.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Duncan Hare Copyright 2017
> + */
> +
> +void wget_start(void);			/* Begin wget */

comment on previous line.

> +
> +enum WGET_STATE {
> +	WGET_CLOSED,
> +	WGET_CONNECTING,
> +	WGET_CONNECTED,
> +	WGET_TRANSFERRING,
> +	WGET_TRANSFERRED
> +};

Please initialize them.


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

comment has typo there.


> +#define	SERVER_PORT		80
> +#define	WGET_RETRY_COUNT	30
> +#define	WGET_TIMEOUT		2000UL

fix that indenatation.

> diff --git a/net/Makefile b/net/Makefile
> index f68ad94767..54ef337e80 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>   obj-$(CONFIG_CMD_WOL)  += wol.o
>   obj-$(CONFIG_PROT_UDP) += udp.o
>   obj-$(CONFIG_PROT_TCP) += tcp.o
> +obj-$(CONFIG_CMD_WGET) += wget.o
>   
>   # Disable this warning as it is triggered by:
>   # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/net.c b/net/net.c
> index 28d9fbd227..5044c42add 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -117,6 +117,7 @@
>   #include "wol.h"
>   #endif
>   #include <net/tcp.h>
> +#include <net/wget.h>
>   
>   /** BOOTP EXTENTIONS **/
>   
> @@ -505,6 +506,11 @@ restart:
>   			nfs_start();
>   			break;
>   #endif
> +#if defined(CONFIG_CMD_WGET)
> +		case WGET:
> +			wget_start();
> +			break;
> +#endif
>   #if defined(CONFIG_CMD_CDP)
>   		case CDP:
>   			cdp_start();
> diff --git a/net/wget.c b/net/wget.c
> new file mode 100644
> index 0000000000..e3d1c8cc28
> --- /dev/null
> +++ b/net/wget.c
> @@ -0,0 +1,436 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * WGET/HTTP support driver based on U-BOOT's nfs.c
> + * Copyright Duncan Hare <dh@synoia.com> 2017
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <mapmem.h>
> +#include <env.h>

please sort it.


> +#include <image.h>
> +#include <net.h>
> +#include <net/wget.h>
> +#include <net/tcp.h>
> +
> +static const char bootfile1[]   = "GET ";
> +static const char bootfile3[]   = " HTTP/1.0\r\n\r\n";
> +static const char http_eom[]     = "\r\n\r\n";
> +static const char http_ok[]      = "200";
> +static const char content_len[]  = "Content-Length";
> +static const char linefeed[]     = "\r\n";

fix indentation

> +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);

Can't you resort that functions to remove this line?

> +
> +static enum net_loop_state wget_loop_state;
> +
> +/* Timeout retry parameters */
> +static u8 retry_action;
> +static unsigned int retry_tcp_ack_num;
> +static unsigned int retry_tcp_seq_num;
> +static int retry_len;

So many variables without any description. Please add it.

> +
> +static inline int store_block(uchar *src, unsigned int offset, unsigned int len)

kernel-doc for all these functions would help with reviewing.

> +{
> +	ulong newsize = offset + len;
> +	uchar *ptr;
> +
> +#ifdef CONFIG_SYS_DIRECT_FLASH_WGET

add it to the code. checkpatch reports it too.

This macro should be in Kconfig.

And there is no single word anywhere that you can use wget and save data 
directly to SPI. That should be visible at lease in commit message.


> +	int i, rc = 0;
> +
> +	for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
> +		/* start address in flash? */
> +		if (image_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)(image_load_addr + offset), len);
> +		if (rc) {
> +			flash_perror(rc);
> +			return -1;
> +		}
> +	} else {
> +#endif /* CONFIG_SYS_DIRECT_FLASH_WGET */
> +
> +		ptr = map_sysmem(image_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;

newline here.

> +	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                  = retry_action;
> +	unsigned int tcp_ack_num   = retry_tcp_ack_num;
> +	unsigned int tcp_seq_num   = retry_tcp_seq_num;
> +	int len                    = retry_len;
> +	uchar *ptr;
> +	uchar *offset;

put it on single line and fix that indentation.

> +
> +	tcp_ack_num = tcp_ack_num + len;

isn't it easier to add it directly above?

If not tcp_ack_num += len; but I prefer to add it directly above.

> +
> +	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;

indentation is not correct and + should be on previous line.

> +		offset = ptr;
> +
> +		memcpy(offset, &bootfile1, strlen(bootfile1));
> +		offset = offset + strlen(bootfile1);

offset +=

> +
> +		memcpy(offset, image_url, strlen(image_url));
> +		offset = offset + strlen(image_url);

ditto

> +
> +		memcpy(offset, &bootfile3, strlen(bootfile3));
> +		offset = offset + strlen(bootfile3);

ditto

> +		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)
> +{
> +	retry_action      = action;
> +	retry_tcp_ack_num = tcp_ack_num;
> +	retry_tcp_seq_num = tcp_seq_num;
> +	retry_len         = len;

newline here and if indenation above.

> +	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");

Why to use this style? Just print it directly with single line.

> +	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;

pack it.

> +
> +	pkt[len] = '\0';
> +	pos = strstr((char *)pkt, http_eom);
> +
> +	if (pos == 0) {

if (!pos) {




> +		debug_cond(DEBUG_WGET,
> +			   "wget: Connected, data before Header %p\n", pkt);
> +		pkt_in_q = (void *)image_load_addr + 0x20000 + (pkt_q_idx * 0x800);

here are 2 magic values - create macros for them.

> +		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;

this is magic 1

> +		pos  = strstr((char *)pkt, linefeed);
> +		if (pos > 0)
> +			i = pos - (char *)pkt;
> +		else
> +			i = hlen;
> +		printf("%.*s", i,  pkt);
> +
> +		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;

pos +=

> +				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);

newline.

> +			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);

weird indenation.

> +			}
> +		}
> +	}
> +	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.
> + */

use kernel-doc here and aligned it with function below.

> +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) {

!len

> +			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 {
> +				printf("%.*s", len,  pkt);
> +				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) {

!len

> +			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);
> +			return;
> +		}
> +
> +		switch (wget_tcp_state) {
> +		case TCP_FIN_WAIT_2:
> +			wget_send(TCP_ACK, tcp_seq_num, tcp_ack_num, len);

do you really want to fallthrough here? If yes label it.

> +		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;
> +	}
> +}
> +
> +/*
> + * make port a little random (1024-17407)
> + * This keeps the math somewhat trivial to compute, and seems to work with
> + * all supported protocols/clients/servers
> + */

kernel-doc please.

> +static unsigned int random_port(void)
> +{
> +	return 1024 + (get_timer(0) % 0x4000);

And magic values.

> +}
> +
> +void wget_start(void)
> +{
> +	image_url = strchr(net_boot_file_name, ':');
> +	if (image_url > 0) {
> +		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);

magic value 9.

> +		print_size(net_boot_file_expected_size_in_blocks << 9, "");
> +	}
> +	debug_cond(DEBUG_WGET,
> +		   "\nwget:Load address: 0x%lx\nLoading: *\b", image_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 to forece apr resolution in case
> +	 * the server ip for the previous u-boot commsnd, for exanple dns
> +	 * is not the same as the web server ip.
> +	 */
> +
> +	memset(net_server_ethaddr, 0, 6);
> +
> +	wget_send(TCP_SYN, 0, 0, 0);
> +}

M

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

* Re: [PATCH v14 0/2] add TCP and HTTP for downloading images
  2022-04-21 16:54 [PATCH v14 0/2] add TCP and HTTP for downloading images Ying-Chun Liu
  2022-04-21 16:54 ` [PATCH v14 1/2] net: Add TCP protocol Ying-Chun Liu
  2022-04-21 16:54 ` [PATCH v14 2/2] net: Add wget application Ying-Chun Liu
@ 2022-05-18  9:53 ` Michal Simek
  2022-05-18 14:51   ` Paul Liu
  2 siblings, 1 reply; 8+ messages in thread
From: Michal Simek @ 2022-05-18  9:53 UTC (permalink / raw)
  To: Ying-Chun Liu, u-boot; +Cc: Grant.Likely, Ying-Chun Liu (PaulLiu)

Hi,

On 4/21/22 18:54, Ying-Chun Liu wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> 
> This patch is a refresh from previous patches made by
> Duncan Hare <DuncanCHare@yahoo.com>. I've contacted him and
> continue to work on this patch.
> 
> This patch introduce a TCP stack with SACK. And a simple wget command
> to download images from http server.
> 
> v13: Fix some issues which is reviewed by Christian
> v14: Add options to enable/disable SACK.

What did you change from v1 to v12?

I think you should clean up the code and in this cover letter I would expect 
more description what you have tried. I have seen demo with downloading efi 
application.
Is there any way to run any OS installer? Or for that you need to have https?

Thanks,
Michal




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

* Re: [PATCH v14 0/2] add TCP and HTTP for downloading images
  2022-05-18  9:53 ` [PATCH v14 0/2] add TCP and HTTP for downloading images Michal Simek
@ 2022-05-18 14:51   ` Paul Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Liu @ 2022-05-18 14:51 UTC (permalink / raw)
  To: Michal Simek; +Cc: Ying-Chun Liu, u-boot, Grant.Likely

Hi Michal,

Thanks for reviewing. I'll make a fix based on your review soon.

However I don't know the change from v1 to v12. The previous version are
made by Duncan, and the patches are one year ago so I lost track of the
changes from v1 to v12.

For booting iso images (installer) from distros I think I'll need to try
it. But it is definitely not related to HTTPS.
What I know is the distro release ISO images. And we need to extract all
efi (EFI shell, grub..) and initrd/kernel from ISO
image's eltorito section. I need to figure that out how to do it by
commands if we load the iso images to the memory.

Yours,
Paul


On Wed, 18 May 2022 at 17:54, Michal Simek <michal.simek@xilinx.com> wrote:

> Hi,
>
> On 4/21/22 18:54, Ying-Chun Liu wrote:
> > From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> >
> > This patch is a refresh from previous patches made by
> > Duncan Hare <DuncanCHare@yahoo.com>. I've contacted him and
> > continue to work on this patch.
> >
> > This patch introduce a TCP stack with SACK. And a simple wget command
> > to download images from http server.
> >
> > v13: Fix some issues which is reviewed by Christian
> > v14: Add options to enable/disable SACK.
>
> What did you change from v1 to v12?
>
> I think you should clean up the code and in this cover letter I would
> expect
> more description what you have tried. I have seen demo with downloading
> efi
> application.
> Is there any way to run any OS installer? Or for that you need to have
> https?
>
> Thanks,
> Michal
>
>
>
>

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

end of thread, other threads:[~2022-05-18 14:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 16:54 [PATCH v14 0/2] add TCP and HTTP for downloading images Ying-Chun Liu
2022-04-21 16:54 ` [PATCH v14 1/2] net: Add TCP protocol Ying-Chun Liu
2022-04-21 16:59   ` Ying-Chun Liu (PaulLiu)
2022-05-18  9:05   ` Michal Simek
2022-04-21 16:54 ` [PATCH v14 2/2] net: Add wget application Ying-Chun Liu
2022-05-18  9:51   ` Michal Simek
2022-05-18  9:53 ` [PATCH v14 0/2] add TCP and HTTP for downloading images Michal Simek
2022-05-18 14:51   ` Paul Liu

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.