From: Joe Hershberger <joe.hershberger@ni.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] TCP and wget implementation.
Date: Tue, 5 Dec 2017 14:35:29 -0600 [thread overview]
Message-ID: <CANr=Z=YEER0UWp7TMyNg8stYT=+195ws7=MKvgrKMNX1aBn4nQ@mail.gmail.com> (raw)
In-Reply-To: <20171108163407.3e8bb872@raspberrypi>
Hi Duncan,
The subject should not end with a '.' character.
Also, this should have been marked as v2. Please mark the next as v3.
On Wed, Nov 8, 2017 at 6:34 PM, Duncan Hare <dh@synoia.com> wrote:
> his is the interface and Kconfig files for introducing TCP and wget
> into u-boot.
Missing 'T' at the beginning?
> Interfaces are in net.c and net.h, ping is modified to the new ip send
> interface, and UDP and TCP have shim procedures call map the protocol
> interface to the ip interface.
I'd like to see the shim and all changes to existing stack as a
separate patch to make review simpler.
Also please make TCP and wget into 2 other separate patches.
> The UDP interface is unchanged, and the existing UDP programs need no
> changes.
>
> All the code is new, and not copied from any source.
>
> The wget command and TCP stack are separately configured in Kconfig,
> and provisioning wget without TCP will result in a number of error
> messages. It might be possible to have Kconfig handle dependencies,
> if so advice is welcome.
Yes it should... you could have wget "selects" tcp.
> Makefile in the net directory is modified by hand. It appears not to be
> generated by Kconfig. Again advice is welcome.
That's fine.
> The rationale behind this change is that UDP file transfers elapsed time
> is twice the sum of network latency x number of pcckets, and TCP file
> transfer times are about 4x network latency plus data transit time.
>
> In tests this reduces kernel trnasfer time from about 15 to 20 seconds
> with tftp on a raspberry pi to about 0.4 seconds with wget.
What settings are used in this tftp benchmark for block size? It makes
a huge difference to the performance.
> The raspberry pi as a sink for the kernel runs at about 10 Mbits/sec.
>
> Signed-off-by: Duncan Hare <DH@synoia.com>
> ---
>
> cmd/Kconfig | 5 +++
> cmd/net.c | 13 ++++++++
> include/net.h | 26 ++++++++++++---
> net/Kconfig | 5 +++
> net/Makefile | 3 +-
> net/net.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++----------
> net/ping.c | 9 ++----
> 7 files changed, 132 insertions(+), 29 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 5a6afab99b..4e5bac685e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1035,6 +1035,11 @@ config CMD_ETHSW
> operations such as enabling / disabling a port and
> viewing/maintaining the filtering database (FDB)
>
> +config CMD_WGET
> + bool "wget"
> + help
> + Download a kernel, or other files, from a web server over TCP.
> +
> endmenu
>
> menu "Misc commands"
> diff --git a/cmd/net.c b/cmd/net.c
> index d7c776aacf..f5c2d0f8ed 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -110,6 +110,19 @@ U_BOOT_CMD(
> );
> #endif
>
> +#if defined(CONFIG_CMD_WGET)
> +static int do_wget(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + return netboot_common(WGET, cmdtp, argc, argv);
> +}
> +
> +U_BOOT_CMD(
> + wget, 3, 1, do_wget,
> + "boot image via network using HTTP protocol",
> + "[loadAddress] [[hostIPaddr:]bootfilename]"
> +);
> +#endif
> +
> static void netboot_update_env(void)
> {
> char tmp[22];
> diff --git a/include/net.h b/include/net.h
> index 455b48f6c7..7787413816 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -24,8 +24,17 @@
> * The number of receive packet buffers, and the required packet buffer
> * alignment in memory.
> *
> + * The nuber of buffers for TCP is used to calculate a static TCP window
> + * size, becuse TCP window size is a promise to the sending TCP to be able
> + * to buffer up to the window size of data.
> + * When the sending TCP has a window size of outstanding unacknowledged
> + * data, the sending TCP will stop sending.
> */
>
> +#if defined(CONFIG_TCP)
> +#define CONFIG_SYS_RX_ETH_BUFFER 50 /* For TCP */
> +#endif
> +
> #ifdef CONFIG_SYS_RX_ETH_BUFFER
> # define PKTBUFSRX CONFIG_SYS_RX_ETH_BUFFER
> #else
> @@ -354,6 +363,8 @@ struct vlan_ethernet_hdr {
>
> #define IPPROTO_ICMP 1 /* Internet Control Message Protocol */
> #define IPPROTO_UDP 17 /* User Datagram Protocol */
> +#define IPPROTO_TCP 6 /* Transmission Control Protocol */
> +
>
> /*
> * Internet Protocol (IP) header.
> @@ -538,7 +549,7 @@ extern int net_restart_wrap; /* Tried all network devices */
>
> enum proto_t {
> BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
> - TFTPSRV, TFTPPUT, LINKLOCAL
> + TFTPSRV, TFTPPUT, LINKLOCAL, WGET
> };
>
> extern char net_boot_file_name[1024];/* Boot File name */
> @@ -596,10 +607,10 @@ int net_set_ether(uchar *xet, const uchar *dest_ethaddr, uint prot);
> int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot);
>
> /* Set IP header */
> -void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source);
> +void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
> + u16 pkt_len, u8 prot);
> void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport,
> - int sport, int len);
> -
> + int sport, int len);
> /**
> * compute_ip_checksum() - Compute IP checksum
> *
> @@ -670,9 +681,16 @@ static inline void net_send_packet(uchar *pkt, int len)
> * @param sport Source UDP port
> * @param payload_len Length of data after the UDP header
> */
> +int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> + int payload_len, int proto, u8 action, u32 tcp_seq_num,
> + u32 tcp_ack_num);
> +
> int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
> int sport, int payload_len);
>
> +int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
> + u32 tcp_seq_num, u32 tcp_ack_num);
> +
> /* Processes a received packet */
> void net_process_received_packet(uchar *in_packet, int len);
>
> diff --git a/net/Kconfig b/net/Kconfig
> index 414c5497c7..625ad291bb 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -45,4 +45,9 @@ config BOOTP_VCI_STRING
> default "U-Boot.arm" if ARM
> default "U-Boot"
>
> +config TCP
> + bool "Include Subset TCP stack for wget"
> + help
> + TCP protocol support for wget.
> +
> endif # if NET
> diff --git a/net/Makefile b/net/Makefile
> index ae54eee5af..f83df5b728 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -25,7 +25,8 @@ obj-$(CONFIG_CMD_PING) += ping.o
> obj-$(CONFIG_CMD_RARP) += rarp.o
> obj-$(CONFIG_CMD_SNTP) += sntp.o
> obj-$(CONFIG_CMD_NET) += tftp.o
> -
> +obj-$(CONFIG_TCP) += tcp.o
> +obj-$(CONFIG_CMD_WGET) += wget.o
It looks like you are expecting to add two new source files, but they
are not here.
> # Disable this warning as it is triggered by:
> # sprintf(buf, index ? "foo%d" : "foo", index)
> # and this is intentional usage.
> diff --git a/net/net.c b/net/net.c
> index 4259c9e321..0ce3413cfc 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -107,6 +107,12 @@
> #if defined(CONFIG_CMD_SNTP)
> #include "sntp.h"
> #endif
> +#if defined(CONFIG_TCP)
> +#include "tcp.h"
> +#endif
> +#if defined(CONFIG_CMD_WGET)
> +#include "wget.h"
> +#endif
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -181,6 +187,7 @@ int net_ntp_time_offset;
> static uchar net_pkt_buf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
> /* Receive packets */
> uchar *net_rx_packets[PKTBUFSRX];
> +
> /* Current UDP RX packet handler */
> static rxhand_f *udp_packet_handler;
> /* Current ARP RX packet handler */
> @@ -381,6 +388,9 @@ void net_init(void)
>
> /* Only need to setup buffer pointers once. */
> first_call = 0;
> +#if defined(CONFIG_TCP)
> + tcp_set_tcp_state(TCP_CLOSED);
> +#endif
> }
>
> net_init_loop();
> @@ -484,6 +494,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();
> @@ -777,11 +792,41 @@ void net_set_timeout_handler(ulong iv, thand_f *f)
> }
>
> int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> - int payload_len)
> + int payload_len)
> +{
> + return net_send_ip_packet(ether, dest, dport, sport, payload_len,
> + IPPROTO_UDP, 0, 0, 0);
> +}
> +
> +#if defined(CONFIG_TCP)
> +int net_send_tcp_packet(int payload_len, int dport, int sport, u8 action,
> + u32 tcp_seq_num, u32 tcp_ack_num)
> +{
> + return net_send_ip_packet(net_server_ethaddr, net_server_ip, dport,
> + sport, payload_len, IPPROTO_TCP, action,
> + tcp_seq_num, tcp_ack_num);
> +}
> +#endif
> +
> +int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> + int payload_len, int proto, u8 action, u32 tcp_seq_num,
> + u32 tcp_ack_num)
> {
> uchar *pkt;
> int eth_hdr_size;
> int pkt_hdr_size;
> + if (proto == IPPROTO_UDP) {
> + debug_cond(DEBUG_DEV_PKT,
> + "UDP Send (to=%pI4, from=%pI4, len=%d)\n",
> + &dest, &net_ip, payload_len);
> +#if defined(CONFIG_TCP)
> +
> + } else {
> + debug_cond(DEBUG_DEV_PKT,
> + "TCP Send (%pI4, %pI4, len=%d, A=%x)\n",
> + &dest, &net_ip, payload_len, action);
> +#endif
> + }
>
> /* make sure the net_tx_packet is initialized (net_init() was called) */
> assert(net_tx_packet != NULL);
> @@ -799,9 +844,22 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> pkt = (uchar *)net_tx_packet;
>
> eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
> - pkt += eth_hdr_size;
> - net_set_udp_header(pkt, dest, dport, sport, payload_len);
> - pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
> +#if defined(CONFIG_TCP)
> + if (proto == IPPROTO_UDP) {
> +#endif
> + net_set_udp_header(pkt + eth_hdr_size, dest,
> + dport, sport, payload_len);
> + pkt_hdr_size = IP_UDP_HDR_SIZE;
> + eth_hdr_size = eth_hdr_size + pkt_hdr_size;
> +
> +#if defined(CONFIG_TCP)
> + } else {
> + pkt_hdr_size = eth_hdr_size +
> + tcp_set_tcp_header(pkt + eth_hdr_size, dport, sport,
> + payload_len, action,
> + tcp_seq_num, tcp_ack_num);
> + }
> +#endif
>
> /* if MAC address was not discovered yet, do an ARP request */
> if (memcmp(ether, net_null_ethaddr, 6) == 0) {
> @@ -1157,9 +1215,6 @@ void net_process_received_packet(uchar *in_packet, int len)
> /* Can't deal with anything except IPv4 */
> if ((ip->ip_hl_v & 0xf0) != 0x40)
> return;
> - /* Can't deal with IP options (headers != 20 bytes) */
> - if ((ip->ip_hl_v & 0x0f) > 0x05)
> - return;
> /* Check the Checksum of the header */
> if (!ip_checksum_ok((uchar *)ip, IP_HDR_SIZE)) {
> debug("checksum bad\n");
> @@ -1205,9 +1260,19 @@ void net_process_received_packet(uchar *in_packet, int len)
> * we send a tftp packet to a dead connection, or when
> * there is no server at the other end.
> */
> +
> if (ip->ip_p == IPPROTO_ICMP) {
> receive_icmp(ip, len, src_ip, et);
> return;
> +#if defined(CONFIG_TCP)
> + } else if (ip->ip_p == IPPROTO_TCP) {
> + debug_cond(DEBUG_DEV_PKT,
> + "TCP PH (to=%pI4, from=%pI4, len=%d)\n",
> + &dst_ip, &src_ip, len);
> +
> + rxhand_tcp_f((union tcp_build_pkt *)ip, len);
> + return;
> +#endif
> } else if (ip->ip_p != IPPROTO_UDP) { /* Only UDP packets */
> return;
> }
> @@ -1426,25 +1491,28 @@ int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot)
> }
> }
>
> -void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source)
> +void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
> + u16 pkt_len, u8 prot)
> {
> struct ip_udp_hdr *ip = (struct ip_udp_hdr *)pkt;
>
> /*
> - * Construct an IP header.
> + * Construct an IP header.
> */
> /* IP_HDR_SIZE / 4 (not including UDP) */
> ip->ip_hl_v = 0x45;
> ip->ip_tos = 0;
> - ip->ip_len = htons(IP_HDR_SIZE);
> + ip->ip_len = htons(pkt_len);
> ip->ip_id = htons(net_ip_id++);
> - ip->ip_off = htons(IP_FLAGS_DFRAG); /* Don't fragment */
> + ip->ip_off = htons(IP_FLAGS_DFRAG); /* Don't fragment */
> ip->ip_ttl = 255;
> + ip->ip_p = prot;
> ip->ip_sum = 0;
> /* already in network byte order */
> net_copy_ip((void *)&ip->ip_src, &source);
> /* already in network byte order */
> net_copy_ip((void *)&ip->ip_dst, &dest);
> + ip->ip_sum = compute_ip_checksum(ip, IP_HDR_SIZE);
> }
>
> void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
> @@ -1460,11 +1528,8 @@ void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
> if (len & 1)
> pkt[IP_UDP_HDR_SIZE + len] = 0;
>
> - net_set_ip_header(pkt, dest, net_ip);
> - ip->ip_len = htons(IP_UDP_HDR_SIZE + len);
> - ip->ip_p = IPPROTO_UDP;
> - ip->ip_sum = compute_ip_checksum(ip, IP_HDR_SIZE);
> -
> + net_set_ip_header(pkt, dest, net_ip, IP_UDP_HDR_SIZE + len,
> + IPPROTO_UDP);
> ip->udp_src = htons(sport);
> ip->udp_dst = htons(dport);
> ip->udp_len = htons(UDP_HDR_SIZE + len);
> @@ -1485,7 +1550,8 @@ void copy_filename(char *dst, const char *src, int size)
>
> #if defined(CONFIG_CMD_NFS) || \
> defined(CONFIG_CMD_SNTP) || \
> - defined(CONFIG_CMD_DNS)
> + defined(CONFIG_CMD_DNS) || \
> + defined(CONFIG_CMD_WGET)
> /*
> * make port a little random (1024-17407)
> * This keeps the math somewhat trivial to compute, and seems to work with
> diff --git a/net/ping.c b/net/ping.c
> index 9508cf1160..254b646193 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -20,16 +20,11 @@ struct in_addr net_ping_ip;
> static void set_icmp_header(uchar *pkt, struct in_addr dest)
> {
> /*
> - * Construct an IP and ICMP header.
> + * Construct an ICMP header.
> */
> - struct ip_hdr *ip = (struct ip_hdr *)pkt;
> struct icmp_hdr *icmp = (struct icmp_hdr *)(pkt + IP_HDR_SIZE);
>
> - net_set_ip_header(pkt, dest, net_ip);
> -
> - ip->ip_len = htons(IP_ICMP_HDR_SIZE);
> - ip->ip_p = IPPROTO_ICMP;
> - ip->ip_sum = compute_ip_checksum(ip, IP_HDR_SIZE);
> + net_set_ip_header(pkt, dest, net_ip, IP_ICMP_HDR_SIZE, IPPROTO_ICMP);
>
> icmp->type = ICMP_ECHO_REQUEST;
> icmp->code = 0;
> --
> 2.11.0
next prev parent reply other threads:[~2017-12-05 20:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-09 0:34 [U-Boot] [PATCH] TCP and wget implementation Duncan Hare
2017-12-05 20:35 ` Joe Hershberger [this message]
[not found] ` <1512063880.2580267.1512525185991@mail.yahoo.com>
2018-01-03 21:07 ` [U-Boot] [PATCH] TCP and wget implementation v3 Duncan Hare
2018-01-03 21:23 ` Joe Hershberger
[not found] ` <1220614347.9042380.1515015734928@mail.yahoo.com>
2018-01-03 23:01 ` [U-Boot] [PATCH] TCP and wget implementation v4 Duncan Hare
2018-01-05 18:35 ` Joe Hershberger
[not found] ` <1265391460.1283367.1515183992460@mail.yahoo.com>
2018-01-05 22:10 ` Duncan Hare
2018-01-06 0:38 ` Joe Hershberger
2018-01-11 2:18 ` [U-Boot] [PATCH] TCP and wget implementation v5.1 Duncan Hare
2018-01-12 16:08 ` Joe Hershberger
2018-01-15 17:01 ` Duncan Hare
2018-01-16 19:43 ` Joe Hershberger
2017-11-23 21:53 [U-Boot] [PATCH] TCP and wget implementation Duncan Hare
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANr=Z=YEER0UWp7TMyNg8stYT=+195ws7=MKvgrKMNX1aBn4nQ@mail.gmail.com' \
--to=joe.hershberger@ni.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.