[U-Boot] [PATCH v8 1/3] Adding TCP and wget into u-boot
Joe Hershberger
joe.hershberger at ni.com
Thu Mar 8 18:52:53 UTC 2018
Hi Duncan,
Still some issues, but getting closer to parsable.
The subject of this should be something like, "net: Adjust UDP
implementation to prepare for TCP support"
On Wed, Mar 7, 2018 at 10:43 PM, <DH at synoia.com> wrote:
> From: Duncan Hare <DuncanCHare at yahoo.com>
>
>>>>>>
> <<<<
I think these are coming from your commit log. You should remove them.
>
> cover-letter:
You need to use the exact tags in the documentation. That means that
you need to capitalize the 'C'.
> Why netboot:
The first line here is the subject for the series, so you should have
the line that you used to have as the patches' titles in v7.
> Central management, including logs and change control,
> coupled with with enhanced security and unauthorized
> change detection and remediation by exposing a
> small attack surface.
>
> Why TCP:
>
> Currently file transfer are done using tftp or NFS both
> over udp. This requires a request to be sent from client
> (u-boot) to the boot server.
>
> For a 4 Mbyte kernel, with a 1k block size this requires
> 4,000 request for a block.
>
> Using a large block size, one greater than the Ethernet
> maximum frame size limitation, would require fragmentation,
> which u-boot supports. However missing fragment recovery
> requires timeout detection and re-transmission requests
> for missing fragments.
>
> UDP is ideally suited to fast single packet exchanges,
> inquiry/response, for example dns, becuse of the lack of
> connection overhead.
>
> UDP as a file transport mechanism is slow, even in low
> latency networks, because file transfer with udp requires
> poll/response mechanism to provide transfer integrity.
>
> In networks with large latency, for example: the internet,
> UDP is even slower. What is a 30 second transfer on a local
> boot server and LAN increase to over 3 minutes, because of
> all the requests/response traffic.
>
> This was anticipated in the evolution of the IP protocols
> and TCP was developed and then enhanced for high latency high
> bandwidth networks.
>
> The current standard is TCP with selective acknowledgment.
>
> In our testing we have reduce kernel transmission time to
> around 0.4 seconds for a 4Mbyte kernel, with a 100 Mbps
> downlink.
>
> Why http and wget:
>
> HTTP is the most efficient file retrieval protocol in common
> use. The client send a single request, after TCP connection,
> to receive a file of any length.
>
> WGET is the application which implements http file transfer
> outside browsers as a file transfer protocol. Versions of
> wget exists on many operating systems.
> END
>
This stuff is great for a cover letter. I would recommend that you
also include the relevant TCP section and wget section in those
patches.
> Signed-off-by: Duncan Hare <DHare at synoia.com>
> ---
>
> Changed in this patch:
> include/net.h
> net/net.c
This isn't really needed since the patch already lists the files
changed... see below about 12 lines.
>
> Added a protocol parameter to ip packet sending in net.c
> Added UDP protocol for current applications to minimize
> code changes to existing net apps.
This stuff should not be in a patman tag. The default (untagged) is
what ends up in the commit message. In other words, don't put this
stuff in the "Commit-notes:" tag. In patman, "notes" means that you
are providing notes to the reviewer that you do not want to end up in
git.
>
> All the code is new, and not copied from any source.
>
>
> Changes in v8:
> Initial changes for adding TCP
>
> include/net.h | 25 +++++++++++++++++++------
> net/net.c | 52 ++++++++++++++++++++++++++++++++++------------------
> net/ping.c | 9 ++-------
> 3 files changed, 55 insertions(+), 31 deletions(-)
>
> diff --git a/include/net.h b/include/net.h
> index 455b48f6c7..7e5f5a6a5b 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -15,17 +15,26 @@
> #include <asm/cache.h>
> #include <asm/byteorder.h> /* for nton* / ntoh* stuff */
>
> -#define DEBUG_LL_STATE 0 /* Link local state machine changes */
> -#define DEBUG_DEV_PKT 0 /* Packets or info directed to the device */
> -#define DEBUG_NET_PKT 0 /* Packets on info on the network at large */
> +#define DEBUG_LL_STATE 0 /* Link local state machine changes */
> +#define DEBUG_DEV_PKT 0 /* Packets or info directed to the device */
> +#define DEBUG_NET_PKT 0 /* Packets on info on the network at large */
> #define DEBUG_INT_STATE 0 /* Internal network state changes */
>
> /*
> * The number of receive packet buffers, and the required packet buffer
> * alignment in memory.
> *
> + * The number of buffers for TCP is used to calculate a static TCP window
> + * size, becuse TCP window size is a promise to the sending TCP to be able
> + * to buffer up to the window size of data.
> + * When the sending TCP has a window size of outstanding unacknowledged
> + * data, the sending TCP will stop sending.
> */
>
> +#if defined(CONFIG_TCP)
> +#define CONFIG_SYS_RX_ETH_BUFFER 12 /* For TCP */
> +#endif
> +
> #ifdef CONFIG_SYS_RX_ETH_BUFFER
> # define PKTBUFSRX CONFIG_SYS_RX_ETH_BUFFER
> #else
> @@ -354,6 +363,7 @@ struct vlan_ethernet_hdr {
>
> #define IPPROTO_ICMP 1 /* Internet Control Message Protocol */
> #define IPPROTO_UDP 17 /* User Datagram Protocol */
> +#define IPPROTO_TCP 6 /* Transmission Control Protocol */
>
> /*
> * Internet Protocol (IP) header.
> @@ -596,10 +606,10 @@ int net_set_ether(uchar *xet, const uchar *dest_ethaddr, uint prot);
> int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot);
>
> /* Set IP header */
> -void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source);
> +void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
> + u16 pkt_len, u8 prot);
> void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport,
> - int sport, int len);
> -
> + int sport, int len);
> /**
> * compute_ip_checksum() - Compute IP checksum
> *
> @@ -670,6 +680,9 @@ static inline void net_send_packet(uchar *pkt, int len)
> * @param sport Source UDP port
> * @param payload_len Length of data after the UDP header
> */
> +int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> + int payload_len, int proto);
> +
> int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
> int sport, int payload_len);
>
> diff --git a/net/net.c b/net/net.c
> index 4259c9e321..df4e7317e7 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -181,6 +181,7 @@ int net_ntp_time_offset;
> static uchar net_pkt_buf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
> /* Receive packets */
> uchar *net_rx_packets[PKTBUFSRX];
> +
> /* Current UDP RX packet handler */
> static rxhand_f *udp_packet_handler;
> /* Current ARP RX packet handler */
> @@ -777,11 +778,23 @@ void net_set_timeout_handler(ulong iv, thand_f *f)
> }
>
> int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> - int payload_len)
> + int payload_len)
> +{
> + return net_send_ip_packet(ether, dest, dport, sport, payload_len,
> + IPPROTO_UDP);
> +}
> +
> +int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> + int payload_len, int proto)
> {
> uchar *pkt;
> int eth_hdr_size;
> int pkt_hdr_size;
> + if (proto == IPPROTO_UDP) {
> + debug_cond(DEBUG_DEV_PKT,
> + "UDP Send (to=%pI4, from=%pI4, len=%d)\n",
> + &dest, &net_ip, payload_len);
> + }
>
> /* make sure the net_tx_packet is initialized (net_init() was called) */
> assert(net_tx_packet != NULL);
> @@ -799,9 +812,15 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
> pkt = (uchar *)net_tx_packet;
>
> eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
> - pkt += eth_hdr_size;
> - net_set_udp_header(pkt, dest, dport, sport, payload_len);
> - pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
> + switch (proto) {
> + case IPPROTO_UDP:
> + net_set_udp_header(pkt + eth_hdr_size, dest,
> + dport, sport, payload_len);
> + pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
> + break;
> + default:
> + return -1;
> + }
>
> /* if MAC address was not discovered yet, do an ARP request */
> if (memcmp(ether, net_null_ethaddr, 6) == 0) {
> @@ -1157,9 +1176,6 @@ void net_process_received_packet(uchar *in_packet, int len)
> /* Can't deal with anything except IPv4 */
> if ((ip->ip_hl_v & 0xf0) != 0x40)
> return;
> - /* Can't deal with IP options (headers != 20 bytes) */
> - if ((ip->ip_hl_v & 0x0f) > 0x05)
> - return;
> /* Check the Checksum of the header */
> if (!ip_checksum_ok((uchar *)ip, IP_HDR_SIZE)) {
> debug("checksum bad\n");
> @@ -1205,6 +1221,7 @@ void net_process_received_packet(uchar *in_packet, int len)
> * we send a tftp packet to a dead connection, or when
> * there is no server at the other end.
> */
> +
> if (ip->ip_p == IPPROTO_ICMP) {
> receive_icmp(ip, len, src_ip, et);
> return;
> @@ -1365,8 +1382,7 @@ common:
> }
> /**********************************************************************/
>
> -int
> -net_eth_hdr_size(void)
> +int net_eth_hdr_size(void)
> {
> ushort myvlanid;
>
> @@ -1426,25 +1442,28 @@ int net_update_ether(struct ethernet_hdr *et, uchar *addr, uint prot)
> }
> }
>
> -void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source)
> +void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
> + u16 pkt_len, u8 prot)
> {
> struct ip_udp_hdr *ip = (struct ip_udp_hdr *)pkt;
>
> /*
> - * Construct an IP header.
> + * Construct an IP header.
> */
> /* IP_HDR_SIZE / 4 (not including UDP) */
> ip->ip_hl_v = 0x45;
> ip->ip_tos = 0;
> - ip->ip_len = htons(IP_HDR_SIZE);
> + ip->ip_len = htons(pkt_len);
> ip->ip_id = htons(net_ip_id++);
> - ip->ip_off = htons(IP_FLAGS_DFRAG); /* Don't fragment */
> + ip->ip_off = htons(IP_FLAGS_DFRAG); /* Don't fragment */
> ip->ip_ttl = 255;
> + ip->ip_p = prot;
> ip->ip_sum = 0;
> /* already in network byte order */
> net_copy_ip((void *)&ip->ip_src, &source);
> /* already in network byte order */
> net_copy_ip((void *)&ip->ip_dst, &dest);
> + ip->ip_sum = compute_ip_checksum(ip, IP_HDR_SIZE);
> }
>
> void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
> @@ -1460,11 +1479,8 @@ void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport, int sport,
> if (len & 1)
> pkt[IP_UDP_HDR_SIZE + len] = 0;
>
> - net_set_ip_header(pkt, dest, net_ip);
> - ip->ip_len = htons(IP_UDP_HDR_SIZE + len);
> - ip->ip_p = IPPROTO_UDP;
> - ip->ip_sum = compute_ip_checksum(ip, IP_HDR_SIZE);
> -
> + net_set_ip_header(pkt, dest, net_ip, IP_UDP_HDR_SIZE + len,
> + IPPROTO_UDP);
> ip->udp_src = htons(sport);
> ip->udp_dst = htons(dport);
> ip->udp_len = htons(UDP_HDR_SIZE + len);
> diff --git a/net/ping.c b/net/ping.c
> index 9508cf1160..254b646193 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -20,16 +20,11 @@ struct in_addr net_ping_ip;
> static void set_icmp_header(uchar *pkt, struct in_addr dest)
> {
> /*
> - * Construct an IP and ICMP header.
> + * Construct an ICMP header.
> */
> - struct ip_hdr *ip = (struct ip_hdr *)pkt;
> struct icmp_hdr *icmp = (struct icmp_hdr *)(pkt + IP_HDR_SIZE);
>
> - net_set_ip_header(pkt, dest, net_ip);
> -
> - ip->ip_len = htons(IP_ICMP_HDR_SIZE);
> - ip->ip_p = IPPROTO_ICMP;
> - ip->ip_sum = compute_ip_checksum(ip, IP_HDR_SIZE);
> + net_set_ip_header(pkt, dest, net_ip, IP_ICMP_HDR_SIZE, IPPROTO_ICMP);
>
> icmp->type = ICMP_ECHO_REQUEST;
> icmp->code = 0;
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list