[U-Boot] [PATCH v8 1/3] Adding TCP and wget into u-boot

[Calvin Johnson]

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of
> DH at synoia.com
> Sent: Thursday, March 8, 2018 10:14 AM
> To: DuncanCHare at yahoo.com
> Cc: Duncan Hare <DHare at synoia.com>; Joe Hershberger
> <joe.hershberger at ni.com>; u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH v8 1/3] Adding TCP and wget into u-boot

<snip>

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

It would be good to have this cosmetic change into a separate patch. 

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

IMO, better place for this definition and associated explanation would be above the comment
describing PKTBUFSRX, i.e after immediately after below line.
#define DEBUG_INT_STATE 0       /* Internal network state changes */

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

Better to sort IPPROTO in ascending order

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

Why do you need this change in the set_udp_header? 

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

Place it above comments for net_send_udp_packet.

Regards
Calvin