[U-Boot] [PATCH] wget and enough TCP stack for fast netboot
Simon Glass
sjg at chromium.org
Sun Nov 5 16:49:04 UTC 2017
Hi Duncan,
On 29 September 2017 at 12:48, Duncan Hare <dh at synoia.com> wrote:
> Boot with udp is relatively slow on a local net, and very slow over a WAN.
> TCP is a very effective protocol for fast file, or stream, transfers.
> http (used by wget) is a very efficient protocol, there is one message to
> retrieve the file, and acks from u-boot to server TCP thereafter.
>
> The code was built & tested on a raspberry pi, LAN and WAN with
> the Jan 2017 version of u-boot. This code is an update for the current
> master, September 2017 version of u-boot, and cleaned up to pass
> clearpatch standards, but not tested, because the current Sept
> version of u-boot appears broken, rainbow screen of death, on raspberry pi.
>
> This is a limited TCP implementation for the wget command only.
> And all the code newly written, without viewing any copyrighted source.
>
> Complied binary size: When using wget, tftp and nfs can be omitted
> from the build. This has not been measured. When not using wget, the TCP
> code is eliminted from the build.
>
> All interfaces, funtion calls, in use are preserved. This reults in an ugly
> "repurposing" of varaibles in the packet-in calls to wget. This requires
> advice and guidence on the best implementation.
>
> Kconfig is used in ./cmd and ./net to control wget and tcp respectively.
> Makefile in ./net appears to have to be configured by hand.
>
> Possible future additions include passing the TCP connection to the kernel
> to complete boot, adding SSH to improve security, and sending device
> MAC addresses or serial numbers (purportedly unique to a device)
> for registration purposes.
>
> Signed-off-by: Duncan Hare <Dh at Synoia.com>
> ---
>
> cmd/Kconfig | 5 +
> cmd/net.c | 13 +
> include/net.h | 241 +++++++++++++++-
> net/Kconfig | 5 +
> net/Makefile | 2 +-
> net/net.c | 871
> ++++++++++++++++++++++++++++++++++++++++++++++++++--------
> net/ping.c | 9 +-
> net/wget.c | 354 ++++++++++++++++++++++++
> net/wget.h | 6 +
> 9 files changed, 1365 insertions(+), 141 deletions(-)
> create mode 100644 net/wget.c
> create mode 100644 net/wget.h
There is something odd about this patch. Did you send it as plain
text? I does not apply for me:
pwclient git-am 820061
Applying patch #820061 using 'git am'
Description: [U-Boot] wget and enough TCP stack for fast netboot
Applying: wget and enough TCP stack for fast netboot
error: corrupt patch at line 21
Patch failed at 0001 wget and enough TCP stack for fast netboot
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
If you haven't already, I encourage you to set up 'git send-email' so
that it works on your machine. You can then use patman to build, check
and send your patches, and it takes the email program munging (and
drudgery) out of the process.
With your original email I think I suggested some ways to shrink this
patch as it seems to touch a lot of things at once. I don't think that
made any sense to you at the time. So I'm going to try to explain this
a bit better here and in comments below.
Note that most people will not review a patch like this. It is too
long and does too many things. It could never be applied as it since
it touches everything at once. This is the difference I suppose
between implementing a new feature and turning it into patches.
You probably already have a fair bit of git expertise. But if not, the
main skill to learn is to be able to build up patches with separate
parts of your work. In this case you might be able to use 'git add -p'
repeatedly to create each patch. Another approach is to use 'git
rebase -i', go back before your patch, and then use 'git diff
<branch>' to pull over changes one by one and then 'git commit' to
commit those as a patch. Keep doing this until you have a number of
patches. You'll want to set up git to use an external diff (like meld)
if it doesn't do that already. There are other GUI tools for git like
git-colla or stuff at https://git-scm.com/download/gui/linux
I very much encourage you to split this into patches and send a v2. I
will happily review it and I would very much like this series in
U-Boot. I suspect many others would find it useful also. You have
written the code, you just need to get it split into patches and
landed. Next time it will be much easier as you will be experienced
with the process.
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index d6d130edfa..a1596260bf 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1022,6 +1022,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..bbf199e9f6 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -24,7 +24,15 @@
> * 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
number
Here you appear to be updating an existing comment to add more detail.
Is that right? In that case it should be in a separate patch. You can
combine many changes of this sort into one patch.
> + * 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
> @@ -47,7 +55,7 @@ struct in_addr {
> __be32 s_addr;
> };
>
> -/**
Why are you changing this? The /** notation is used by docbook to
indicate that this is a function / struct that has documentation,
> +/*
> * An incoming packet handler.
> * @param pkt pointer to the application packet
> * @param dport destination UDP port
> @@ -59,7 +67,7 @@ typedef void rxhand_f(uchar *pkt, unsigned dport,
> struct in_addr sip, unsigned sport,
> unsigned len);
>
> -/**
> +/*
> * An incoming ICMP packet handler.
> * @param type ICMP type
> * @param code ICMP code
> @@ -308,7 +316,7 @@ struct ethernet_hdr {
> u8 et_dest[ARP_HLEN]; /* Destination node */
> u8 et_src[ARP_HLEN]; /* Source node */
> u16 et_protlen; /* Protocol or length */
> -} __attribute__((packed));
> +} __packed;
This should be in a separate patch too, e.g. "Use __packed in network headers"
>
> /* Ethernet header size */
> #define ETHER_HDR_SIZE (sizeof(struct ethernet_hdr))
> @@ -326,7 +334,7 @@ struct e802_hdr {
> u8 et_snap2;
> u8 et_snap3;
> u16 et_prot; /* 802 protocol */
> -} __attribute__((packed));
> +} __packed;
>
> /* 802 + SNAP + ethernet header size */
> #define E802_HDR_SIZE (sizeof(struct e802_hdr))
> @@ -340,7 +348,7 @@ struct vlan_ethernet_hdr {
> u16 vet_vlan_type; /* PROT_VLAN */
> u16 vet_tag; /* TAG of VLAN */
> u16 vet_type; /* protocol type */
> -} __attribute__((packed));
> +} __packed;
>
> /* VLAN Ethernet header size */
> #define VLAN_ETHER_HDR_SIZE (sizeof(struct vlan_ethernet_hdr))
> @@ -353,6 +361,7 @@ struct vlan_ethernet_hdr {
> #define PROT_PPP_SES 0x8864 /* PPPoE session messages */
>
> #define IPPROTO_ICMP 1 /* Internet Control Message Protocol */
> +#define IPPROTO_TCP 6 /* Transmission Control Protocol */
It seems to me that adding TCP is a thing in itself and that would
also help you break up the series. All of the code that adds TCP
support could go in one patch, perhaps, if it doesn't make sense to
split it further.
> #define IPPROTO_UDP 17 /* User Datagram Protocol */
>
> /*
> @@ -369,7 +378,7 @@ struct ip_hdr {
> u16 ip_sum; /* checksum */
> struct in_addr ip_src; /* Source IP address */
> struct in_addr ip_dst; /* Destination IP address */
> -} __attribute__((packed));
> +} __packed;
>
> #define IP_OFFS 0x1fff /* ip offset *= 8 */
> #define IP_FLAGS 0xe000 /* first 3 bits */
> @@ -397,11 +406,196 @@ struct ip_udp_hdr {
> u16 udp_dst; /* UDP destination port */
> u16 udp_len; /* Length of UDP packet */
> u16 udp_xsum; /* Checksum */
> -} __attribute__((packed));
> +} __packed;
>
> #define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr))
> #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
>
> +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 Acknowledgement 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 /* Sybch (start) flag */
> +#define TCP_RST 0x04 /* reset flag */
> +#define TCP_PUSH 0x08 /* Push - Notify app */
> +#define TCP_ACK 0x10 /* Acknowledgement 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_O_END 0x00 /* End of option list */
> +#define TCP_1_NOP 0x01 /* Single padding NOP */
> +#define TCP_O_NOP 0x01010101 /* NOPs pad to 32 bit boundary */
> +#define TCP_O_MSS 0x02 /* MSS Size option */
> +#define TCP_O_SCL 0x03 /* Window Scale option */
> +#define TCP_P_SACK 0x04 /* SACK permitted */
> +#define TCP_V_SACK 0x05 /* SACK values */
> +#define TCP_O_TS 0x08 /* Timestanp option */
> +#define TCP_OPT_LEN_2 0x02
> +#define TCP_OPT_LEN_3 0x03
> +#define TCP_OPT_LEN_4 0x04
> +#define TCP_OPT_LEN_6 0x06
> +#define TCP_OPT_LEN_8 0x08
> +#define TCP_OPT_LEN_A 0x0a /* Timestamp Length */
> +
> +/*
> + * Please reviw the warning in net.c about these two paraeters.
> + * They are part of a promise of RX buffer size to the sending TCP
> + */
> +
> +#define TCP_MSS 1460 /* Max segment size - 1460 */
> +#define TCP_SCALE 0x01 /* Scale 1 */
> +
> +struct tcp_mss { /* TCP Mex Segment size */
The comments on these structures are welcome but IMO are not very
explanatory. You could expand them to explain what a segment is and
what the various fields mean.
> + u8 kind; /* 0x02 */
> + u8 len; /* 0x04 */
> + u16 mss; /* 1460 - Max segment size */
> +} __packed;
> +
> +struct tcp_scale { /* TCP Windows Scale */
> + u8 kind; /* 0x03 */
> + u8 len; /* 0x03 */
> + u8 scale; /* win shift fat fast networks */
> +} __packed;
> +
> +struct tcp_sack_p { /* SACK permitted */
> + u8 kind; /* 0x04 */
> + u8 len; /* Length */
> +} __packed;
> +
> +struct sack_edges {
> + u32 l;
> + u32 r;
> +} __packed;
> +
> +#define TCP_SACK_HOLES 3
> +
> +struct tcp_sack_v {
> + u8 kind; /* 0x05 */
> + u8 len; /* Length */
> + struct sack_edges hole[TCP_SACK_HOLES]; /* L & R widow edges */
> +} __packed;
> +
> +struct tcp_TSopt { /* TCP time stamps option */
> + u8 kind; /* 0x08 */
> + u8 len; /* 0x0a */
> + u32 TSsnd; /* Sender timestamp */
> + u32 TSrcv; /* Receiver timestamp */
> +} __packed;
> +
> +#define TCP_TSOPT_SIZE (sizeof(struct tcp_TSopt))
> +
> +/*
> + * 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_TSopt TSopt;
> + 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_TSopt TSopt;
> + struct tcp_sack_v sack_v;
> + u8 end;
> +} __packed;
> +
> +#define IP_TCP_SACK_SIZE (sizeof(struct ip_tcp_hdr_s))
> +
> +/*
> + * TCP psuedo header definitions
> + */
> +#define PSUEDO_PAD_SIZE 8
> +
> +struct psuedo_hdr {
> + u8 padding[PSUEDO_PAD_SIZE]; /* psuedo header size */
> + /* = ip_tcp hdr size */
> + struct in_addr p_src;
> + struct in_addr p_dst;
> + u8 rsvd;
> + u8 p;
> + u16 len;
> +} __packed;
> +
> +#define PSUEDO_HDR_SIZE (sizeof(struct psuedo_hdr)) - PSUEDO_PAD_SIZE
> +
> +/*
> + * union for building IP/TCP packet.
> + * build Psuedo header in packed bufferfirst, calculate TCP checksum
> + * then build IP header in packe buffer.
> + */
> +
> +union tcp_build_pkt {
> + struct psuedo_hdr ph;
> + struct ip_tcp_hdr_o ip;
> + struct ip_tcp_hdr_s sack;
> + uchar raw[1600];
Is the 1600 available in a constand anywhere?
> +} __packed;
> +
> +/*
> + * TCP STATE MACHINE STATES FOR SOCKET
Lower case
Also I think this can be a single line comment:
/** TCP State machine states for sockets */
> + */
> +
> +enum TCP_STATE {
> + TCP_CLOSED, /* Need to send SYN to connect */
> + TCP_SYN_SENT, /* Trying to connect, waiting for SYN ACK */
> + TCP_ESTABLISHED, /* server and client have open connection */
> + TCP_CLOSE_WAIT, /* Received FIN, passed to app for */
> + TCP_CLOSING, /* Received FIN, sent FIN, ACK wait for ACK */
> + TCP_FIN_WAIT_1, /* Sendt FIN waiting for response */
> + TCP_FIN_WAIT_2 /* Received ACK from FIN sent, wait for FIN */
> +};
> +
> +enum TCP_STATE net_get_tcp_state(void);
> +int net_find_in_buffer(uchar raw[], int payload_len, uchar field[],
> + int field_len);
> +void net_print_buffer(uchar raw[], int pkt_len, int payload_len,
> + int hdr_len, bool hide);
Please comment these.
> +
> +/*
> + * TCP incoming packet handler
> + */
> +
Again, please use a /** comment. For this one you need to comment the args:
/**
* rxhand_tcp() - TCP incoming-packet handler
*
* Called by .. to ...
*
* @pkt: ...
* @dport: ...
*/
> +typedef void rxhand_tcp(uchar *pkt, unsigned int dport,
> + struct in_addr sip, unsigned int sport,
> + unsigned int len, enum TCP_STATE tcp_state);
> +
> /*
> * Address Resolution Protocol (ARP) header.
> */
> @@ -435,7 +629,7 @@ struct arp_hdr {
> u8 ar_tha[]; /* Target hardware address */
> u8 ar_tpa[]; /* Target protocol address */
> #endif /* 0 */
> -} __attribute__((packed));
> +} __packed;
>
> #define ARP_HDR_SIZE (8+20) /* Size assuming ethernet */
>
> @@ -470,7 +664,7 @@ struct icmp_hdr {
> } frag;
> u8 data[0];
> } un;
> -} __attribute__((packed));
> +} __packed;
>
> #define ICMP_HDR_SIZE (sizeof(struct icmp_hdr))
> #define IP_ICMP_HDR_SIZE (IP_HDR_SIZE + ICMP_HDR_SIZE)
> @@ -538,7 +732,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
Here you are actually adding WGET, so I think this goes in the 'wget' patch.
> };
>
> extern char net_boot_file_name[1024];/* Boot File name */
> @@ -596,9 +790,22 @@ 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);
> +int net_set_tcp_header(uchar *pkt, int len, u8 action, u32 tcp_seq_num,
> + u32 tcp_ack_seq_num);
> +
> void net_set_udp_header(uchar *pkt, struct in_addr dest, int dport,
> - int sport, int len);
> + int sport, int len);
> +/* Set ports */
> +void net_set_ports(int server_port, int our_port);
> +
> +/* Print packet or messsage */
> +void net_print_buffer(uchar raw[], int pkt_len, int payload_len,
> + int hdr_len, bool hide);
> +/* Find string in buffer */
> +int net_find_in_buffer(uchar raw[], int payload_len,
> + uchar field[], int field_len);
Please add function comments for those. If you are updating an
existing function, then it should really go in a separate patch which
updates the function and all users.
All exported functions and any non-trivial/obvious static functions
should have full comments with purpose (maybe just one line), params
and return value.
>
> /**
> * compute_ip_checksum() - Compute IP checksum
> @@ -631,6 +838,8 @@ unsigned add_ip_checksums(unsigned offset, unsigned sum,
> unsigned new_sum);
> int ip_checksum_ok(const void *addr, unsigned nbytes);
>
> /* Callbacks */
> +rxhand_f *net_get_tcp_handler(void); /* Get TCP RX packet handler */
> +void net_set_tcp_handler(rxhand_f *); /* Set TCP RX packet handler */
> rxhand_f *net_get_udp_handler(void); /* Get UDP RX packet handler */
> void net_set_udp_handler(rxhand_f *); /* Set UDP RX packet handler */
> rxhand_f *net_get_arp_handler(void); /* Get ARP RX packet handler */
> @@ -670,6 +879,10 @@ 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(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);
>
> @@ -678,8 +891,8 @@ void net_process_received_packet(uchar *in_packet, int
> len);
>
> #ifdef CONFIG_NETCONSOLE
> void nc_start(void);
> -int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned dest_port,
> - unsigned src_port, unsigned len);
> +int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned int
> dest_port,
> + unsigned int src_port, unsigned int len);
> #endif
>
> static __always_inline int eth_is_on_demand_init(void)
> 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.
Here you should explain what TCP is and why you would use it. I don't
think this is just for wget. Other things might use it.
> +
> endif # if NET
> diff --git a/net/Makefile b/net/Makefile
> index ae54eee5af..c788e2027b 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -25,7 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
> obj-$(CONFIG_CMD_RARP) += rarp.o
> obj-$(CONFIG_CMD_SNTP) += sntp.o
> obj-$(CONFIG_CMD_NET) += tftp.o
> -
> +obj-$(CONFIG_CMD_WGET) += wget.o
> # Disable this warning as it is triggered by:
> # sprintf(buf, index ? "foo%d" : "foo", index)
> # and this is intentional usage.
> diff --git a/net/net.c b/net/net.c
> index 4259c9e321..f940cda8f9 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1,3 +1,5 @@
> +/* */
> +
> /*
> * Copied from Linux Monitor (LiMon) - Networking.
> *
> @@ -6,6 +8,7 @@
> * Copyright 2000 Roland Borde
> * Copyright 2000 Paolo Scaffardi
> * Copyright 2000-2002 Wolfgang Denk, wd at denx.de
> + * Copyright 2017 Duncan Hare, dh at synoia.com
This file is already 1500 lines. Can you please try to split it or put
your stuff in a new file? E.g. net_udp.c / net_tcp.c or something like
that?
> * SPDX-License-Identifier: GPL-2.0
> */
>
> @@ -78,9 +81,18 @@
> * - own IP address
> * We want: - network time
> * Next step: none
> + *
> + * HTTP/TCP Receiver:
> + *
> + * Prequeisites: - own ethernet adress
> + * - own IP address
> + * - Server IP address
> + * - HTP client
> + * - Bootfile path & name
> + * We want: - Load the Boot file
> + * Next Step HTTPS?
> */
> -
> -
> +#define DEBUG_TCP_PKT 0 /* 1 to print debug messages, 0 to suppress
> */
> #include <common.h>
> #include <command.h>
> #include <console.h>
> @@ -107,6 +119,7 @@
> #if defined(CONFIG_CMD_SNTP)
> #include "sntp.h"
> #endif
> +#include "wget.h"
Can this be <wget.h> ?
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -137,12 +150,16 @@ u8 net_server_ethaddr[6];
> struct in_addr net_ip;
> /* Server IP addr (0 = unknown) */
> struct in_addr net_server_ip;
> +/* Ports */
> +int dport;
> +int sport;
> +
> /* Current receive packet */
> uchar *net_rx_packet;
> /* Current rx packet length */
> int net_rx_packet_len;
> /* IP packet ID */
> -static unsigned net_ip_id;
> +static unsigned int net_ip_id;
> /* Ethernet bcast address */
> const u8 net_bcast_ethaddr[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> const u8 net_null_ethaddr[6];
> @@ -178,9 +195,11 @@ struct in_addr net_ntp_server;
> int net_ntp_time_offset;
> #endif
>
> -static uchar net_pkt_buf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
> +static uchar net_pkt_buf[(PKTBUFSRX + 1) * PKTSIZE_ALIGN + PKTALIGN];
> /* Receive packets */
> uchar *net_rx_packets[PKTBUFSRX];
> +/* Current TCP RX packet handler */
> +static rxhand_f *tcp_packet_handler;
> /* Current UDP RX packet handler */
> static rxhand_f *udp_packet_handler;
> /* Current ARP RX packet handler */
> @@ -198,6 +217,33 @@ static ulong time_delta;
> /* THE transmit packet */
> uchar *net_tx_packet;
>
> +#if defined(CONFIG_TCP)
> +
> +/* TCP sliding window control */
> +/* used by us to request re-TX */
> +static u32 tcp_next_expected_seq_num;
> +static struct tcp_sack_v tcp_lost;
> +
> +/* TCP option timestamp */
> +static u32 loc_timestamp;
> +static u32 rmt_timestamp;
> +
> +/* TCP connection state */
> +static enum TCP_STATE tcp_state;
> +
> +/*
> + * An incoming TCP packet handler for the TCP protocol.
> + * There is also a dymanic function pointer for TCP based commads to
> + * receive incoming traffic after the TCP protocol code has done its work.
Great, but please document args also. E.g. what is does @len mean
(packet length in bytes including all headers?). Also please generally
avoid single-char variable names since we can't search for them - try
@pkt instead.
> + */
> +
> +void rxhand_tcp_f(union tcp_build_pkt *b, unsigned int len);
> +
> +/* Current TCP RX packet handler */
> +static rxhand_f *tcp_packet_handler;
> +
> +#endif
> +
> static int net_check_prereq(enum proto_t protocol);
>
> static int net_try_count;
> @@ -207,7 +253,7 @@ int __maybe_unused net_busy_flag;
> /**********************************************************************/
>
> static int on_bootfile(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -224,10 +270,11 @@ static int on_bootfile(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(bootfile, on_bootfile);
>
> static int on_ipaddr(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -236,10 +283,11 @@ static int on_ipaddr(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(ipaddr, on_ipaddr);
>
> static int on_gatewayip(const char *name, const char *value, enum env_op
> op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -248,10 +296,11 @@ static int on_gatewayip(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
This whitespace change seems unrelated to your patch. It should go in
a separate patch, e.g. 'net: Add blank lines before U_BOOT_ENV
callbacks'. At present people have to accept this change with the
others. IMO we don't want this change, but if you put it in a separate
patch at least people can consider it.
> U_BOOT_ENV_CALLBACK(gatewayip, on_gatewayip);
>
> static int on_netmask(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -260,10 +309,11 @@ static int on_netmask(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(netmask, on_netmask);
>
> static int on_serverip(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -272,10 +322,11 @@ static int on_serverip(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(serverip, on_serverip);
>
> static int on_nvlan(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -284,10 +335,11 @@ static int on_nvlan(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(nvlan, on_nvlan);
>
> static int on_vlan(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
Again these are all whitespace changes. It looks like you are fixing
the style in this file, which is great, but needs its own patch.
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -296,11 +348,12 @@ static int on_vlan(const char *name, const char
> *value, enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(vlan, on_vlan);
>
> #if defined(CONFIG_CMD_DNS)
> static int on_dnsip(const char *name, const char *value, enum env_op op,
> - int flags)
> + int flags)
> {
> if (flags & H_PROGRAMMATIC)
> return 0;
> @@ -309,6 +362,7 @@ static int on_dnsip(const char *name, const char *value,
> enum env_op op,
>
> return 0;
> }
> +
> U_BOOT_ENV_CALLBACK(dnsip, on_dnsip);
> #endif
>
> @@ -344,12 +398,11 @@ static void net_init_loop(void)
> {
> if (eth_get_dev())
> memcpy(net_ethaddr, eth_get_ethaddr(), 6);
> -
> - return;
> }
>
> static void net_clear_handlers(void)
> {
> + net_set_tcp_handler(NULL);
> net_set_udp_handler(NULL);
> net_set_arp_handler(NULL);
> net_set_timeout_handler(0, NULL);
> @@ -360,6 +413,12 @@ static void net_cleanup_loop(void)
> net_clear_handlers();
> }
>
> +void net_set_ports(int server_port, int our_port)
> +{
> + dport = server_port;
> + sport = our_port;
> +}
> +
> void net_init(void)
> {
> static int first_call = 1;
> @@ -381,8 +440,10 @@ void net_init(void)
>
> /* Only need to setup buffer pointers once. */
> first_call = 0;
> +#if defined(CONFIG_TCP)
> + tcp_state = TCP_CLOSED;
> +#endif
> }
> -
> net_init_loop();
> }
>
> @@ -398,7 +459,7 @@ int net_loop(enum proto_t protocol)
> net_restarted = 0;
> net_dev_exists = 0;
> net_try_count = 1;
> - debug_cond(DEBUG_INT_STATE, "--- net_loop Entry\n");
> + debug_cond(DEBUG_INT_STATE, "%s--- Entry\n", __func__);
>
> bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
> net_init();
> @@ -424,7 +485,7 @@ restart:
> * here on, this code is a state machine driven by received
> * packets and timer events.
> */
> - debug_cond(DEBUG_INT_STATE, "--- net_loop Init\n");
> + debug_cond(DEBUG_INT_STATE, "%s--- Init\n", __func__);
> net_init_loop();
>
> switch (net_check_prereq(protocol)) {
> @@ -479,11 +540,16 @@ restart:
> ping_start();
> break;
> #endif
> -#if defined(CONFIG_CMD_NFS)
> +#if defined(CONFIG_CMD_NS)
Why this change?
> case NFS:
> nfs_start();
> break;
> #endif
> +#if defined(CONFIG_CMD_WGET)
> + case WGET:
> + wget_start();
> + break;
> +#endif
> #if defined(CONFIG_CMD_CDP)
> case CDP:
> cdp_start();
> @@ -566,9 +632,9 @@ restart:
> eth_set_last_protocol(BOOTP);
>
> puts("\nAbort\n");
> - /* include a debug print as well incase the debug
> - messages are directed to stderr */
> - debug_cond(DEBUG_INT_STATE, "--- net_loop Abort!\n");
> + /* include a debug print as well incase the debug */
> + /* messages are directed to stderr */
> + debug_cond(DEBUG_INT_STATE, "%s--- Aborty\n", __func__);
More changes that should be pulled out into some sort of refactoring
on the debug statements?
> ret = -EINTR;
> goto done;
> }
> @@ -597,7 +663,8 @@ restart:
> CONFIG_LED_STATUS_ON);
> #endif /* CONFIG_SYS_FAULT_ECHO_LINK_DOWN, ... */
> #endif /* CONFIG_MII, ... */
> - debug_cond(DEBUG_INT_STATE, "--- net_loop timeout\n");
> + debug_cond(DEBUG_INT_STATE,
> + "%s--- Timeout\n", __func__);
> x = time_handler;
> time_handler = (thand_f *)0;
> (*x)();
> @@ -627,14 +694,15 @@ restart:
> eth_set_last_protocol(protocol);
>
> ret = net_boot_file_size;
> - debug_cond(DEBUG_INT_STATE, "--- net_loop Success!\n");
> + debug_cond(DEBUG_INT_STATE,
> + "%s--- Success!\n", __func__);
> goto done;
>
> case NETLOOP_FAIL:
> net_cleanup_loop();
> /* Invalidate the last protocol */
> eth_set_last_protocol(BOOTP);
> - debug_cond(DEBUG_INT_STATE, "--- net_loop Fail!\n");
> + debug_cond(DEBUG_INT_STATE, "%s--- Fail\n", __func__);
> goto done;
>
> case NETLOOP_CONTINUE:
> @@ -720,12 +788,21 @@ int net_start_again(void)
> * Miscelaneous bits.
> */
>
> -static void dummy_handler(uchar *pkt, unsigned dport,
> - struct in_addr sip, unsigned sport,
> - unsigned len)
> +static void dummy_handler(uchar *pkt, unsigned int dport,
> + struct in_addr sip, unsigned int sport,
> + unsigned int len)
> {
> }
>
> +void net_set_tcp_handler(rxhand_f *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;
> +}
> +
> rxhand_f *net_get_udp_handler(void)
> {
> return udp_packet_handler;
> @@ -734,7 +811,7 @@ rxhand_f *net_get_udp_handler(void)
> void net_set_udp_handler(rxhand_f *f)
> {
> debug_cond(DEBUG_INT_STATE, "--- net_loop UDP handler set (%p)\n", f);
> - if (f == NULL)
> + if (!f)
Put this in your style fix-up patch.
> udp_packet_handler = dummy_handler;
> else
> udp_packet_handler = f;
> @@ -776,39 +853,61 @@ 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 net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
> + int sport, int payload_len)
> +{
> + net_set_ports(dport, sport);
> + return net_send_ip_packet(payload_len, IPPROTO_UDP,
> + IPPROTO_UDP, 0, 0);
> +}
> +
> +int net_send_ip_packet(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;
> + uchar *ether = net_server_ethaddr;
>
> - /* make sure the net_tx_packet is initialized (net_init() was called)
> */
> + /* make sure the net_tx_packet is initialized (net_init() was called)*/
> assert(net_tx_packet != NULL);
> - if (net_tx_packet == NULL)
> + if (!net_tx_packet)
> return -1;
>
> /* convert to new style broadcast */
> - if (dest.s_addr == 0)
> - dest.s_addr = 0xFFFFFFFF;
> + if (net_server_ip.s_addr == 0)
> + net_server_ip.s_addr = 0xFFFFFFFF;
We try to use lower-case hex in new code.
>
> /* if broadcast, make the ether address a broadcast and don't do ARP */
> - if (dest.s_addr == 0xFFFFFFFF)
> + if (net_server_ip.s_addr == 0xFFFFFFFF)
> ether = (uchar *)net_bcast_ethaddr;
>
> 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, net_server_ip,
> + 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 if (IS_ENABLED(CONFIG_TCP)) {
This allows us to avoid a build-time branch.
> + } else {
> + pkt_hdr_size = eth_hdr_size +
> + net_set_tcp_header(pkt + eth_hdr_size,
> + 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) {
> - debug_cond(DEBUG_DEV_PKT, "sending ARP for %pI4\n", &dest);
> + debug_cond(DEBUG_DEV_PKT, "sending ARP for %pI4\n",
> + &net_server_ip);
>
> /* save the ip and eth addr for the packet to send after arp */
> - net_arp_wait_packet_ip = dest;
> + net_arp_wait_packet_ip = net_server_ip;
Here you are changing common code, right? How come dest changes to
net_server_ip? Probably needs a comment, or even a separate patch to
move the existing code over to using net_server_ip.
> arp_wait_packet_ethaddr = ether;
>
> /* size of the waiting packet */
> @@ -821,7 +920,7 @@ int net_send_udp_packet(uchar *ether, struct in_addr
> dest, int dport, int sport,
> return 1; /* waiting */
> } else {
> debug_cond(DEBUG_DEV_PKT, "sending UDP to %pI4/%pM\n",
> - &dest, ether);
> + &net_server_ip, ether);
> net_send_packet(net_tx_packet, pkt_hdr_size + payload_len);
> return 0; /* transmitted */
> }
> @@ -868,7 +967,6 @@ static struct ip_udp_hdr *__net_defragment(struct
> ip_udp_hdr *ip, int *lenp)
> thisfrag = payload + offset8;
> start = offset8 * 8;
> len = ntohs(ip->ip_len) - IP_HDR_SIZE;
> -
> if (start + len > IP_MAXUDP) /* fragment extends too far */
> return NULL;
>
> @@ -973,9 +1071,10 @@ static struct ip_udp_hdr *__net_defragment(struct
> ip_udp_hdr *ip, int *lenp)
> }
>
> static inline struct ip_udp_hdr *net_defragment(struct ip_udp_hdr *ip,
> - int *lenp)
> + int *lenp)
> {
> u16 ip_off = ntohs(ip->ip_off);
> +
> if (!(ip_off & (IP_OFFS | IP_FLAGS_MFRAG)))
> return ip; /* not a fragment */
> return __net_defragment(ip, lenp);
> @@ -984,9 +1083,10 @@ static inline struct ip_udp_hdr *net_defragment(struct
> ip_udp_hdr *ip,
> #else /* !CONFIG_IP_DEFRAG */
>
> static inline struct ip_udp_hdr *net_defragment(struct ip_udp_hdr *ip,
> - int *lenp)
> + int *lenp)
> {
> u16 ip_off = ntohs(ip->ip_off);
> +
> if (!(ip_off & (IP_OFFS | IP_FLAGS_MFRAG)))
> return ip; /* not a fragment */
> return NULL;
> @@ -1000,7 +1100,7 @@ static inline struct ip_udp_hdr *net_defragment(struct
> ip_udp_hdr *ip,
> * @parma ip IP packet containing the ICMP
> */
> static void receive_icmp(struct ip_udp_hdr *ip, int len,
> - struct in_addr src_ip, struct ethernet_hdr *et)
> + struct in_addr src_ip, struct ethernet_hdr *et)
> {
> struct icmp_hdr *icmph = (struct icmp_hdr *)&ip->udp_src;
>
> @@ -1157,9 +1257,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");
> @@ -1210,70 +1307,81 @@ void net_process_received_packet(uchar *in_packet,
> int len)
> return;
> } else if (ip->ip_p != IPPROTO_UDP) { /* Only UDP packets */
> return;
> - }
> -
> - debug_cond(DEBUG_DEV_PKT,
> - "received UDP (to=%pI4, from=%pI4, len=%d)\n",
> - &dst_ip, &src_ip, len);
> + } else if (ip->ip_p == IPPROTO_UDP) {
> + debug_cond(DEBUG_DEV_PKT,
> + "received UDP (to=%pI4, from=%pI4, len=%d)\n",
> + &dst_ip, &src_ip, len);
>
> #ifdef CONFIG_UDP_CHECKSUM
> - if (ip->udp_xsum != 0) {
> - ulong xsum;
Some sort of code style change? Put it in your code style patch.
> - ushort *sumptr;
> - ushort sumlen;
> -
> - xsum = ip->ip_p;
> - xsum += (ntohs(ip->udp_len));
> - xsum += (ntohl(ip->ip_src.s_addr) >> 16) & 0x0000ffff;
> - xsum += (ntohl(ip->ip_src.s_addr) >> 0) & 0x0000ffff;
> - xsum += (ntohl(ip->ip_dst.s_addr) >> 16) & 0x0000ffff;
> - xsum += (ntohl(ip->ip_dst.s_addr) >> 0) & 0x0000ffff;
> -
> - sumlen = ntohs(ip->udp_len);
> - sumptr = (ushort *)&(ip->udp_src);
> -
> - while (sumlen > 1) {
> - ushort sumdata;
> -
> - sumdata = *sumptr++;
> - xsum += ntohs(sumdata);
> - sumlen -= 2;
> - }
> - if (sumlen > 0) {
> - ushort sumdata;
> -
> - sumdata = *(unsigned char *)sumptr;
> - sumdata = (sumdata << 8) & 0xff00;
> - xsum += sumdata;
> - }
> - while ((xsum >> 16) != 0) {
> - xsum = (xsum & 0x0000ffff) +
> +#define XFF 0x0000ffff
> + if (ip->udp_xsum != 0) {
> + ulong xsum;
> + ushort *sumptr;
> + ushort sumlen;
> +
> + xsum = ip->ip_p;
> + xsum += (ntohs(ip->udp_len));
> + xsum += (ntohl(ip->ip_src.s_addr) >> 16) & XFF;
> + xsum += (ntohl(ip->ip_src.s_addr) >> 0) & XFF;
> + xsum += (ntohl(ip->ip_dst.s_addr) >> 16) & XFF;
> + xsum += (ntohl(ip->ip_dst.s_addr) >> 0) & XFF;
> +
> + sumlen = ntohs(ip->udp_len);
> + sumptr = (ushort *)&ip->udp_src;
> +
> + while (sumlen > 1) {
> + ushort sumdata;
> +
> + sumdata = *sumptr++;
> + xsum += ntohs(sumdata);
> + sumlen -= 2;
> + }
> + if (sumlen > 0) {
> + ushort sumdata;
> +
> + sumdata = *(unsigned char *)sumptr;
> + sumdata = (sumdata << 8) & 0xff00;
> + xsum += sumdata;
> + }
> + while ((xsum >> 16) != 0) {
> + xsum = (xsum & 0x0000ffff) +
> ((xsum >> 16) & 0x0000ffff);
> + }
> + if ((xsum != 0x00000000) && (xsum != XFF)) {
> + printf(" UDP bad xsum %08lx %08x\n",
> + xsum, ntohs(ip->udp_xsum));
> + return;
> + }
> }
> - if ((xsum != 0x00000000) && (xsum != 0x0000ffff)) {
> - printf(" UDP wrong checksum %08lx %08x\n",
> - xsum, ntohs(ip->udp_xsum));
> - return;
> - }
> - }
> #endif
>
> #if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> - nc_input_packet((uchar *)ip + IP_UDP_HDR_SIZE,
> - src_ip,
> - ntohs(ip->udp_dst),
> - ntohs(ip->udp_src),
> - ntohs(ip->udp_len) - UDP_HDR_SIZE);
> + nc_input_packet((uchar *)ip + IP_UDP_HDR_SIZE,
> + src_ip,
> + ntohs(ip->udp_dst),
> + ntohs(ip->udp_src),
> + ntohs(ip->udp_len) - UDP_HDR_SIZE);
> +#endif
> + /*
> + * IP header OK. Pass packet to the current handler.
> + */
> + (*udp_packet_handler)((uchar *)ip + IP_UDP_HDR_SIZE,
> + ntohs(ip->udp_dst),
> + src_ip,
> + ntohs(ip->udp_src),
> + ntohs(ip->udp_len)
> + - UDP_HDR_SIZE);
> + }
> +#if defined(CONFIG_TCP)
> + else if (ip->ip_p == IPPROTO_TCP) { /* Only TCP packets */
> +
> + 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);
> + }
> #endif
> - /*
> - * IP header OK. Pass the packet to the current handler.
> - */
> - (*udp_packet_handler)((uchar *)ip + IP_UDP_HDR_SIZE,
> - ntohs(ip->udp_dst),
> - src_ip,
> - ntohs(ip->udp_src),
> - ntohs(ip->udp_len) - UDP_HDR_SIZE);
> - break;
> }
> }
>
> @@ -1363,10 +1471,10 @@ common:
> }
> return 0; /* OK */
> }
> +
> /**********************************************************************/
>
> -int
> -net_eth_hdr_size(void)
> +int net_eth_hdr_size(void)
> {
> ushort myvlanid;
>
> @@ -1421,12 +1529,14 @@ int net_update_ether(struct ethernet_hdr *et, uchar
> *addr, uint prot)
> } else {
> /* 802.2 + SNAP */
> struct e802_hdr *et802 = (struct e802_hdr *)et;
> +
> et802->et_prot = htons(prot);
> return E802_HDR_SIZE;
> }
> }
>
> -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;
>
> @@ -1436,19 +1546,21 @@ void net_set_ip_header(uchar *pkt, struct in_addr
> dest, struct in_addr source)
> /* 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_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,
> - int len)
> +void net_set_udp_header(uchar *pkt, struct in_addr dest,
> + int dport, int sport, int len)
> {
> struct ip_udp_hdr *ip = (struct ip_udp_hdr *)pkt;
>
> @@ -1460,10 +1572,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);
> @@ -1485,7 +1595,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
> @@ -1501,10 +1612,10 @@ void ip_to_string(struct in_addr x, char *s)
> {
> x.s_addr = ntohl(x.s_addr);
> sprintf(s, "%d.%d.%d.%d",
> - (int) ((x.s_addr >> 24) & 0xff),
> - (int) ((x.s_addr >> 16) & 0xff),
> - (int) ((x.s_addr >> 8) & 0xff),
> - (int) ((x.s_addr >> 0) & 0xff)
> + (int)((x.s_addr >> 24) & 0xff),
> + (int)((x.s_addr >> 16) & 0xff),
> + (int)((x.s_addr >> 8) & 0xff),
> + (int)((x.s_addr >> 0) & 0xff)
> );
> }
>
> @@ -1540,3 +1651,525 @@ ushort env_get_vlan(char *var)
> {
> return string_to_vlan(env_get(var));
> }
> +
> +/*
> + * TCP module
> + */
> +
> +#if defined(CONFIG_TCP)
> +
> +enum TCP_STATE net_get_tcp_state(void)
> +{
> + return tcp_state;
> +}
> +
> +void net_print_buffer(uchar raw[], int pkt_len, int payload_len,
> + int hdr_len, bool hide)
> +{
> + int i;
> +
> + for (i = pkt_len - payload_len; i < pkt_len; i++) {
> + if (i <= hdr_len)
> + printf("%02X", raw[i]);
> + else if ((raw[i] > 0x19) && (raw[i] < 0x7f))
> + putc(raw[i]);
> + else if (hide == 0)
> + putc(raw[i]);
> + else
> + printf("%02X", raw[i]);
> + }
> + printf("%s", "\n");
> +}
> +
> +int net_find_in_buffer(uchar raw[], int payload_len, uchar field[],
> + int field_len)
> +{
> + int i, j;
> +
> + for (i = 0; i < payload_len; i++) {
> + if (raw[i] == field[0]) {
> + for (j = 1; j < field_len; j++) {
> + if (raw[i + j] != field[j])
> + break;
> + if (j == field_len)
> + return i;
> + }
> + }
> + }
> + return 0;
> +}
> +
> +u16 net_set_psuedo_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;
> +
> + /*
> + * Psuedo 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 + PSUEDO_HDR_SIZE;
> +
> + debug_cond(DEBUG_DEV_PKT,
> + "TCP Psuedo Header (to=%pI4, from=%pI4, Len=%d)\n",
> + &b->ph.p_dst, &b->ph.p_src, checksum_len);
> +
> + return compute_ip_checksum(pkt + PSUEDO_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.TSopt.kind = TCP_O_TS;
> + b->sack.TSopt.len = TCP_OPT_LEN_A;
> + b->sack.TSopt.TSsnd = htons(loc_timestamp);
> + b->sack.TSopt.TSrcv = rmt_timestamp;
> + b->sack.sack_v.kind = 0x01;
> + b->sack.sack_v.len = 0x00;
> +
> + if (tcp_lost.len > 0) {
> + b->sack.sack_v.len = tcp_lost.len;
> + b->sack.sack_v.kind = TCP_V_SACK;
> + b->sack.sack_v.hole[0].l = htonl(tcp_lost.hole[1].l);
> + b->sack.sack_v.hole[0].r = htonl(tcp_lost.hole[1].r);
> +
> + /*
> + * These fields are initialized with NOPs to
> + * provide TCP header alignment padding
> + */
> +
> + b->sack.sack_v.hole[1].l = htonl(tcp_lost.hole[1].l);
> + b->sack.sack_v.hole[1].r = htonl(tcp_lost.hole[1].r);
> + b->sack.sack_v.hole[2].l = htonl(tcp_lost.hole[2].l);
> + b->sack.sack_v.hole[2].r = htonl(tcp_lost.hole[2].r);
> +
> + tcp_lost.len = 0;
> + }
> + b->sack.hdr.tcp_hlen = (((TCP_HDR_SIZE + TCP_TSOPT_SIZE
> + + tcp_lost.len + 3) >> 2) << 4);
> +
> + return b->sack.hdr.tcp_hlen >> 2;
> +}
> +
> +void net_set_syn_options(union tcp_build_pkt *b)
> +{
> + tcp_lost.len = 0;
> + b->ip.hdr.tcp_hlen = 0xa0;
> +
> + b->ip.mss.kind = TCP_O_MSS;
> + b->ip.mss.len = TCP_OPT_LEN_4;
> + b->ip.mss.mss = htons(TCP_MSS);
> + b->ip.scale.kind = TCP_O_SCL;
> + b->ip.scale.scale = TCP_SCALE;
> + b->ip.scale.len = TCP_OPT_LEN_3;
> + b->ip.sack_p.kind = TCP_P_SACK;
> + b->ip.sack_p.len = TCP_OPT_LEN_2;
> + b->ip.TSopt.kind = TCP_O_TS;
> + b->ip.TSopt.len = TCP_OPT_LEN_A;
> + loc_timestamp = get_ticks() % 3072;
> + rmt_timestamp = 0x00000000;
> + b->ip.TSopt.TSsnd = 0;
> + b->ip.TSopt.TSrcv = 0x00000000;
> + b->ip.end = TCP_O_END;
> +}
> +
> +int net_set_tcp_header(uchar *pkt, int payload_len, u8 action,
> + u32 tcp_seq_num, u32 tcp_ack_num)
> +{
> + union tcp_build_pkt *b = (union tcp_build_pkt *)pkt;
> + int pkt_hdr_len;
> + int pkt_len;
> + int tcp_len;
> +
> + b->ip.hdr.tcp_flags = action;
> + pkt_hdr_len = IP_TCP_HDR_SIZE;
> + b->ip.hdr.tcp_hlen = 0x50; /* Header is 5 32 bit words */
> + /* 4 bits TCP header Length/4*/
> + /* 4 bits Reserved */
> + /* For options */
> + switch (action) {
> + case TCP_SYN:
> + debug_cond(DEBUG_TCP_PKT,
> + "TCP Hdr:SYN (%pI4, %pI4, sq=%d, ak=%d)\n",
> + &net_server_ip, &net_ip,
> + tcp_seq_num, tcp_ack_num);
> +
> + net_set_syn_options(b);
> + tcp_seq_num = 0;
> + tcp_ack_num = 0;
> + pkt_hdr_len = IP_TCP_O_SIZE;
> + if (tcp_state == TCP_SYN_SENT) { /* Too many sins */
> + action = TCP_FIN;
> + tcp_state = TCP_FIN_WAIT_1;
> + } else {
> + tcp_state = TCP_SYN_SENT;
> + }
> + break;
> + case TCP_ACK:
> + pkt_hdr_len = IP_HDR_SIZE + net_set_ack_options(b);
> + b->ip.hdr.tcp_flags = action;
> + debug_cond(DEBUG_TCP_PKT,
> + "TCP Hdr:ACK (%pI4, %pI4, s=%d, a=%d, A=%x)\n",
> + &net_server_ip, &net_ip, tcp_seq_num, tcp_ack_num,
> + b->ip.hdr.tcp_flags);
> + break;
> + case TCP_FIN:
> + debug_cond(DEBUG_TCP_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;
> + tcp_seq_num++;
> + break;
> + case (TCP_FIN | TCP_ACK): /* Fall thru */
> + case (TCP_FIN | TCP_ACK | TCP_PUSH): /* Fall thru */
> + if (tcp_state == TCP_CLOSE_WAIT)
> + tcp_state = TCP_CLOSING;
> + 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_TCP_PKT,
> + "TCP Hdr:dft (%pI4, %pI4, s=%d, a=%d, A=%x)\n",
> + &net_server_ip, &net_ip,
> + tcp_seq_num, tcp_ack_num, b->ip.hdr.tcp_flags);
> + break;
> + }
> +
> + pkt_len = pkt_hdr_len + payload_len;
> + tcp_len = pkt_len - IP_HDR_SIZE;
> +
> + /*
> + * TCP Header
> + */
> + b->ip.hdr.tcp_ack = htonl(tcp_ack_num);
> + 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
> + * overruun, 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 langth.
> + * 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 = net_set_psuedo_header(pkt, net_ip, net_server_ip,
> + tcp_len, pkt_len);
> +
> + /*
> + * IP Header
> + */
> +
> + net_set_ip_header((uchar *)&b->ip, net_server_ip, net_ip,
> + pkt_len, IPPROTO_TCP);
> +
> + return pkt_hdr_len;
> +}
> +
> +void tcp_hole_create(u32 tcp_left, u32 tcp_right)
> +{
> + debug_cond(DEBUG_TCP_PKT, "Hole Created Left=%d\n", tcp_left);
> +
> + tcp_lost.len = tcp_lost.len + TCP_OPT_LEN_A;
> + tcp_lost.hole[0].l = tcp_left;
> + tcp_lost.hole[0].l = tcp_right;
> +
> + tcp_lost.hole[1].l = TCP_O_NOP;
> + tcp_lost.hole[1].r = TCP_O_NOP;
> + tcp_lost.hole[2].l = TCP_O_NOP;
> + tcp_lost.hole[2].r = TCP_O_NOP;
> +}
> +
> +void tcp_parse_options(uchar *o, int o_len)
> +{
> + struct tcp_TSopt *tsopt;
> + uchar *p = o;
> +
> + for (p = o; p < (o + o_len); (*p = *o + o_len)) {
> + if (*p == TCP_O_NOP)
> + p++;
> + switch (*p) {
> + case TCP_O_END:
> + return;
> + case TCP_O_MSS:
> + break;
> + case TCP_O_SCL:
> + break;
> + case TCP_P_SACK:
> + break;
> + case TCP_V_SACK:
> + break;
> + case TCP_O_TS:
> + tsopt = (struct tcp_TSopt *)p;
> + rmt_timestamp = tsopt->TSsnd;
> + return;
> + break;
> + }
> + }
> +}
> +
> +u8 tcp_state_machine(u8 tcp_flags, u32 *tcp_seq_num, int payload_len)
> +{
> + u8 tcp_fin = tcp_flags & TCP_FIN;
> + u8 tcp_syn = tcp_flags & TCP_SYN;
> + u8 tcp_rst = tcp_flags & TCP_RST;
> + u8 tcp_push = tcp_flags & TCP_PUSH;
> + u8 tcp_ack = tcp_flags & TCP_ACK;
> + u8 action = TCP_DATA;
> +
> + /*
> + * tcp_flags are examined to determine TX action in a given state
> + * tcp_push is intrepreted 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_TCP_PKT, "TCP STATE ENTRY (%x)\n", action);
> + if (tcp_rst) {
> + action = TCP_DATA;
> + tcp_state = TCP_CLOSED;
> + net_set_state(NETLOOP_FAIL);
> + debug_cond(DEBUG_TCP_PKT, "TCP Reset (%x)\n", tcp_flags);
> + return TCP_RST;
> + }
> +
> + switch (tcp_state) {
> + case TCP_CLOSED:
> + debug_cond(DEBUG_TCP_PKT, "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_TCP_PKT, "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;
> + if (tcp_ack) {
> + *tcp_seq_num = *tcp_seq_num + 1;
> + tcp_state = TCP_ESTABLISHED;
> + action = action | TCP_PUSH;
> + tcp_next_expected_seq_num = *tcp_seq_num;
> + }
> + } else {
> + if (tcp_ack)
> + action = TCP_DATA;
> + }
> + break;
> + case TCP_ESTABLISHED:
> + debug_cond(DEBUG_TCP_PKT,
> + "TCP_ESTABLISHED (%x)\n", tcp_flags);
> + if (tcp_fin) {
> + *tcp_seq_num = *tcp_seq_num + 1;
> + tcp_next_expected_seq_num++;
> + 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;
> +
> + if (*tcp_seq_num > tcp_next_expected_seq_num) {
> + tcp_hole_create(tcp_next_expected_seq_num,
> + *tcp_seq_num);
> + debug_cond(DEBUG_TCP_PKT,
> + "TCP_ESTABLISHED Seq=%x, Exp=%x, A= %x\n",
> + *tcp_seq_num,
> + tcp_next_expected_seq_num, action);
> + }
> + if (*tcp_seq_num >= tcp_next_expected_seq_num)
> + tcp_next_expected_seq_num = *tcp_seq_num + payload_len;
> +
> + debug_cond(DEBUG_TCP_PKT, "TCP_ESTABLISHED (%x), %d\n",
> + action, *tcp_seq_num);
> + break;
> + case TCP_CLOSE_WAIT:
> + debug_cond(DEBUG_TCP_PKT, "TCP_CLOSE_WAIT (%x)\n", tcp_flags);
> + action = TCP_DATA; /* Wait for app */
> + break;
> + case TCP_FIN_WAIT_2:
> + debug_cond(DEBUG_TCP_PKT, "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;
> + }
> + break;
> + case TCP_FIN_WAIT_1:
> + debug_cond(DEBUG_TCP_PKT, "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_TCP_PKT, "TCP_CLOSING (%x)\n", tcp_flags);
> + if (tcp_fin)
> + action = TCP_DATA;
> + if (tcp_syn)
> + action = TCP_RST;
> + if (tcp_ack) {
> + action = TCP_DATA;
> + tcp_state = TCP_CLOSED;
> + }
> + 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;
> + u8 net_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_TCP_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_TCP_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_TCP_PKT,
> + "TCP RX IP xum 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 != net_set_psuedo_header((uchar *)b, b->ip.hdr.ip_src,
> + b->ip.hdr.ip_dst, tcp_len,
> + pkt_len)) {
> + debug_cond(DEBUG_TCP_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);
> +
> + 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 date 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, and the
> + * other variable "repurposed" (or misused) to carry sequence numbers
> + * and TCP state.
> + *
> + * TCP_PUSH from the state machine with a payload length of 0 is a
> + * connect or disconnect event
> + */
> +
> + if ((tcp_action && TCP_PUSH) || (payload_len > 0)) {
> + debug_cond(DEBUG_TCP_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_TCP_PKT,
> + "TCP Action (action=%x,Seq=%d,Ack=%d,Pay=%d)\n",
> + tcp_action, tcp_seq_num, tcp_ack_num, payload_len);
> +
> + /*
> + * Warning: Incoming Seq & Ack sequence numbers are transposed here
> + * to outgoing Seq & Ack sequence numbers
> + */
> +
> + net_action = (tcp_action & (~TCP_PUSH));
> + net_send_ip_packet(0, IPPROTO_TCP, net_action,
> + tcp_ack_num, tcp_seq_num);
> + }
> +}
> +#endif
All this code looks pretty good to me. I think you should try to put
it in tcp.c or similar.
> 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);
Refactoring of existing code -> new patch "net: Adjust ping to use
net_set_ip_header()"
>
> icmp->type = ICMP_ECHO_REQUEST;
> icmp->code = 0;
> diff --git a/net/wget.c b/net/wget.c
> new file mode 100644
> index 0000000000..cc5ef4f0c2
> --- /dev/null
> +++ b/net/wget.c
This can be the last patch of your series, to add the actual wget logic
> @@ -0,0 +1,354 @@
> +/*
> + * FILE support driver - based on etherboot and U-BOOT's tftp.c
> + *
> + * Copyright Duncan Hare <dh at synoia.com> 2017
> + *
> + * SPDX-License-Identifier:<TAB>GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fat.h> //FAT
Drop that comment. Also there must be something wrong with your
workflow for it to allow C++ comments. Try using 'patman' which will
take care of all of this.
> +#include <net.h>
> +#include <mapmem.h>
> +#define FILE_TEST 0 /* Set to 1 for debug messges */
> +#define SERVER_PORT 8081
> +#define FILE_RETRY_COUNT 30
> +#define FILE_TIMEOUT 2000UL
> +
> +char bootfile1[10] = "GET ";
> +char bootfile3[20] = " HTTP/1.1\r\n\r\n";
> +uchar content[6] = "<html>";
> +uchar error404[3] = "404";
Should that be a variable rather than a constant / #define?
> +static struct in_addr file_server_ip;
> +static int file_timeout_count;
> +int packets;
> +static u32 initial_data_seq_num;
> +
> +enum FILE_STATE {
enum file_state
Also needs comment
> + FILE_CLOSED,
> + FILE_CONNECTING,
> + FILE_CONNECTED,
> + FILE_TRANSFERRING,
> + FILE_TRANSFERRED};
> +
> +static enum FILE_STATE file_state;
> +
> +static char *file_filename;
> +static char *file_path;
> +static char file_path_buff[2048];
> +static unsigned int file_timeout = FILE_TIMEOUT;
> +static void file_timeout_handler(void);
I do wonder whether these could be all in a struct and only existing
for the duration of the wget? Maybe too hard though.
> +void file_fail(char *error_message, unsigned int tcp_seq_num,
> + unsigned int tcp_ack_num, u8 action);
> +
> +static inline int store_block(uchar *src, unsigned int offset, unsigned int
> len)
> +{
> + ulong newsize = offset + len;
> +
> +#ifdef CONFIG_SYS_DIRECT_FLASH_WGET
What is this option? I did not see it in the Kconfig. You should
really drop the SYS part. Some things use it but IMO the distinction
is meaningless.
> + int i, rc = 0;
> +
> + for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
> + /* start address in flash? */
> + if (load_addr + offset >= flash_info[i].start[0]) {
> + rc = 1;
> + break;
> + }
> + }
> +
> + if (rc) { /* Flash is destination for this packet */
> + rc = flash_write((uchar *)src,
> + (ulong)(load_addr + offset), len);
> + if (rc) {
> + flash_perror(rc);
> + return -1;
> + }
> + } else {
> +#endif /* CONFIG_SYS_DIRECT_FLASH_NFS */
> +
> + uchar *ptr = map_sysmem(load_addr + offset, len);
> +
> + memcpy(ptr, src, len);
> + unmap_sysmem(ptr);
> +
> +#ifdef CONFIG_SYS_DIRECT_FLASH_WGET
> + }
> +#endif
> + if (net_boot_file_size < (offset + len))
> + net_boot_file_size = newsize;
> + return 0;
> +}
> +
> +/*
> + * File request dispatcher
> + * WARNING, This, and only this, is the place where
> + * SEQUENCE NUMBERS are swapped betweeen incoming (RX) and outgoing (TX)
> + * What is in procedure file_handler() is correct for RX traffic.
> + */
> +static void file_send(u8 action, unsigned int tcp_ack_num,
int action. Should not use non-native types as args unless you have to
> + unsigned int tcp_seq_num, int len)
> +{
> + uchar *ptr;
> + uchar *offset;
> +
> + tcp_ack_num = tcp_ack_num + len;
> +
> + switch (file_state) {
> + case FILE_CLOSED:
> + debug_cond(FILE_TEST, "File send: send SYN\n");
> + file_state = FILE_CONNECTING;
> + net_send_ip_packet(0, IPPROTO_TCP, action,
> + tcp_seq_num, tcp_ack_num);
> + packets = 0;
> + break;
> + case FILE_CONNECTING:
> + debug_cond(FILE_TEST, "File send: send HTTP GET\n");
> +
> + ptr = net_tx_packet + net_eth_hdr_size()
> + + IP_TCP_HDR_SIZE + TCP_TSOPT_SIZE + 2;
> + offset = ptr;
> +
> + memcpy(offset, &bootfile1, strlen(bootfile1));
> + offset = offset + strlen(bootfile1);
> +
> + memcpy(offset, file_path, strlen(file_path));
> + offset = offset + strlen(file_path);
> +
> + memcpy(offset, &bootfile3, strlen(bootfile3));
> + offset = offset + strlen(bootfile3);
> + net_send_ip_packet((offset - ptr), IPPROTO_TCP,
> + TCP_DATA, tcp_seq_num, tcp_ack_num);
> + file_state = FILE_CONNECTED;
> + break;
> + case FILE_CONNECTED:
> + debug_cond(FILE_TEST, "File send: Header\n");
> + file_state = FILE_TRANSFERRING;
> + break;
> + case FILE_TRANSFERRING:
> + debug_cond(FILE_TEST, "File send:Transferring\n");
> + net_send_ip_packet(0, IPPROTO_TCP, action,
> + tcp_seq_num, tcp_ack_num);
> + break;
> + case FILE_TRANSFERRED:
> +
> + debug_cond(FILE_TEST, "File send:Transferred\n");
> + net_send_ip_packet(0, IPPROTO_TCP, action,
> + tcp_seq_num, tcp_ack_num);
> + break;
> + }
> +}
> +
> +void file_success(u8 action, unsigned int tcp_seq_num,
> + unsigned int tcp_ack_num, int len, int packets)
> +{
> + file_state = FILE_TRANSFERRED;
> + net_set_state(NETLOOP_SUCCESS);
> + net_set_timeout_handler(0, NULL);
> + debug_cond(1, "Packets received %d, File send:SUCCESS!!!\n", packets);
> + file_send(action, tcp_seq_num, tcp_ack_num, len);
> +}
> +
> +void file_fail(char *error_message, unsigned int tcp_seq_num,
> + unsigned int tcp_ack_num, u8 action)
> +{
> + printf("%s", error_message);
> + printf("%s", "File Fail\n");
> + file_state = FILE_TRANSFERRED;
> + net_set_timeout_handler(0, NULL);
> + net_set_state(NETLOOP_FAIL);
> + file_send(action | TCP_FIN, tcp_seq_num, tcp_ack_num, 0);
> +}
> +
> +/*
> + * Interfaces of U-BOOT
U-Boot
Although this comment seems unnecessary since we are in U-Boot
> + */
> +static void file_timeout_handler(void)
> +{
> + if (++file_timeout_count > FILE_RETRY_COUNT) {
> + puts("\nRetry count exceeded; starting again\n");
> + net_start_again();
> + } else {
> + puts("T ");
> + net_set_timeout_handler(file_timeout +
> + FILE_TIMEOUT * file_timeout_count,
> + file_timeout_handler);
> + file_send(TCP_DATA, 0, 0, 0);
> + }
> +}
> +
> + /*
> + * In the "application push" invocation the TCP header with all its
> + * information is the packet pointer, and the other variable
> + * "repurposed" (or misused) to carry sequence numbers and TCP state.
> + *
> + */
> +
> +static void file_handler(uchar *pkt, unsigned int tcp_seq_num,
> + struct in_addr action_and_state,
> + unsigned int tcp_ack_num, unsigned int len)
> +{
> + enum TCP_STATE file_tcp_state = net_get_tcp_state();
> + u8 action = action_and_state.s_addr;
> + int offset;
> +
> + net_set_timeout_handler(file_timeout, file_timeout_handler);
> + packets++;
> +
> + switch (file_state) {
> + case FILE_CLOSED:
> + debug_cond(FILE_TEST, "File Handler: Error!, State wrong\n");
> + break;
> + case FILE_CONNECTING:
> + debug_cond(FILE_TEST,
> + "File Connecting In len=%d, Seq=%d, Ack=%d\n",
> + len, tcp_seq_num, tcp_ack_num);
> + if (len == 0) {
> + if (file_tcp_state == TCP_ESTABLISHED) {
> + debug_cond(FILE_TEST,
> + "File Cting, send, len=%d\n", len);
> + file_send(action, tcp_seq_num, tcp_ack_num, len);
> + } else {
> + file_fail("File Handler Connected Fail\n",
> + tcp_seq_num, tcp_ack_num, action);
> + }
> + }
> + break;
> + case FILE_CONNECTED:
> + debug_cond(FILE_TEST, "File Cted In len=%d, seq=%d, ack=%d\n",
> + len, tcp_seq_num, tcp_ack_num);
> + if (len == 0) {
> + file_fail("File not found, no data returned/n",
> + tcp_seq_num, tcp_ack_num, action);
> + } else {
> + if (net_find_in_buffer(pkt, len, error404, 3) > 0) {
> + offset = net_find_in_buffer(pkt, len,
> + content, 6);
> + net_print_buffer(pkt, offset - 4,
> + offset - 4, -1, 0);
> + file_fail((char *)pkt, tcp_seq_num,
> + tcp_ack_num, action);
> + } else {
> + net_print_buffer(pkt, len, len, -1, 0);
> + file_state = FILE_TRANSFERRING;
> + initial_data_seq_num = tcp_seq_num + len;
> + file_send(action, tcp_seq_num,
> + tcp_ack_num, len);
> + }
> + }
> + break;
> + case FILE_TRANSFERRING:
> + debug_cond(FILE_TEST,
> + "File Transferring, seq=%d, ack=%d,len=%d\n",
> + tcp_seq_num, tcp_ack_num, len);
> +
> + if (len == 0) {
> + file_send(action, tcp_seq_num, tcp_ack_num, len);
> + } else {
> + offset = tcp_seq_num - initial_data_seq_num;
> + if (store_block(pkt, offset, len) != 0) {
> + file_fail("File store error\n",
> + tcp_seq_num,
> + tcp_ack_num,
> + action);
> + } else {
> + switch (file_tcp_state) {
> + case TCP_SYN_SENT:
> + case TCP_CLOSING:
> + case TCP_FIN_WAIT_1:
> + case TCP_FIN_WAIT_2:
> + case TCP_CLOSED:
> + net_set_state(NETLOOP_FAIL);
> + break;
> + case TCP_ESTABLISHED:
> + file_send(TCP_ACK, tcp_seq_num,
> + tcp_ack_num, len);
> + break;
> + case TCP_CLOSE_WAIT: /* End of file */
> + file_success(action | TCP_ACK,
> + tcp_seq_num, tcp_ack_num,
> + len, packets);
> + break;
> + }
> + }
> + }
> + break;
> + case FILE_TRANSFERRED:
> + file_send(TCP_ACK, tcp_seq_num, tcp_ack_num, len);
> + break;
> + }
> +}
> +
> +void wget_start(void)
> +{
> + debug("%s\n", __func__);
> +
> + file_server_ip = net_server_ip;
> + file_path = (char *)file_path_buff;
> +
> + if (!file_path) {
> + net_set_state(NETLOOP_FAIL);
> + debug("*** ERROR: Fail allocate memory\n");
> + return;
> + }
> +
> + if (net_boot_file_name[0] == '\0') {
> + sprintf(file_path, "/fileroot/%02X%02X%02X%02X.img",
> + net_ip.s_addr & 0xFF,
> + (net_ip.s_addr >> 8) & 0xFF,
> + (net_ip.s_addr >> 16) & 0xFF,
> + (net_ip.s_addr >> 24) & 0xFF);
> +
> + debug("*** Warning: no boot file name; using '%s'\n",
> + file_path);
> + } else {
> + char *p = net_boot_file_name;
> +
> + p = strchr(p, ':');
> +
> + if (!p) {
> + file_server_ip = string_to_ip(net_boot_file_name);
> + ++p;
> + strcpy(file_path, p);
> + } else {
> + strcpy(file_path, net_boot_file_name);
> + }
> + }
> +
> + debug_cond(FILE_TEST, "Using %s device\n", eth_get_name());
> + debug_cond(FILE_TEST, "File transfer HTTP Server %pI4; our IP %pI4\n",
> + &file_server_ip, &net_ip);
> +
> + /* Check if we need to send across this subnet */
> + if (net_gateway.s_addr && net_netmask.s_addr) {
> + struct in_addr our_net;
> + struct in_addr server_net;
> +
> + our_net.s_addr = net_ip.s_addr & net_netmask.s_addr;
> + server_net.s_addr = net_server_ip.s_addr & net_netmask.s_addr;
> + if (our_net.s_addr != server_net.s_addr)
> + debug("; sending through gateway %pI4",
> + &net_gateway);
> + }
> + debug_cond(FILE_TEST, "Filename '%s, %s'.\n", file_path,
> file_filename);
> +
> + if (net_boot_file_expected_size_in_blocks) {
> + debug(" Size is 0x%x Bytes = ",
> + net_boot_file_expected_size_in_blocks << 9);
> + print_size(net_boot_file_expected_size_in_blocks << 9, "");
> + }
> + debug("\nLoad address: 0x%lx\nLoading: *\b", load_addr);
> +
> + net_set_timeout_handler(file_timeout, file_timeout_handler);
> + net_set_tcp_handler(file_handler);
> +
> + file_timeout_count = 0;
> + file_state = FILE_CLOSED;
> +
> + net_set_ports(SERVER_PORT, 4096 + (get_ticks() % 3072));
> +
> + /* zero out server ether in case the server ip has changed */
> + memset(net_server_ethaddr, 0, 6);
> +
> + file_send(TCP_SYN, 0, 0, 0);
> +}
> diff --git a/net/wget.h b/net/wget.h
> new file mode 100644
> index 0000000000..80e6400247
> --- /dev/null
> +++ b/net/wget.h
> @@ -0,0 +1,6 @@
> +/*
> + * Duncan Hare Copyright 2017
> +*/
> +
> +void wget_start(void); /* Begin wget */
> +
> --
> 2.11.0
>
Again this code seems pretty good. I suppose it will get more review
when you send the v2 series, but it seems fine to me.
Regards,
Simon
More information about the U-Boot
mailing list