[PATCH] TCP: Add comments to (near) all functions in tcp_output.c

[PATCH] TCP: Add comments to (near) all functions in tcp_output.c

[Andi Kleen]

TCP: Add comments to (near) all functions in tcp_output.c

While looking for something else I spent some time adding
one liner comments to the tcp_output.c functions that
didn't have any. That makes the comments more consistent.

I hope I documented everything right.

No code changes.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 net/ipv4/tcp_output.c |   49 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 5 deletions(-)

Index: linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
===================================================================
--- linux-2.6.31-rc3-ak.orig/net/ipv4/tcp_output.c
+++ linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
@@ -59,6 +59,8 @@ int sysctl_tcp_base_mss __read_mostly =
 /* By default, RFC2861 behavior.  */
 int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
 
+/* Account for new data that has been sent to the network.
+ */
 static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -142,6 +144,8 @@ static void tcp_cwnd_restart(struct sock
 	tp->snd_cwnd_used = 0;
 }
 
+/* Congestion state accounting after a packet has been sent.
+ */
 static void tcp_event_data_sent(struct tcp_sock *tp,
 				struct sk_buff *skb, struct sock *sk)
 {
@@ -276,6 +280,8 @@ static u16 tcp_select_window(struct sock
 	return new_win;
 }
 
+/* Packet ECN state for a SYN-ACK
+ */
 static inline void TCP_ECN_send_synack(struct tcp_sock *tp, struct sk_buff *skb)
 {
 	TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_CWR;
@@ -283,6 +289,8 @@ static inline void TCP_ECN_send_synack(s
 		TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_ECE;
 }
 
+/* Packet ECN state for a SYN.
+ */
 static inline void TCP_ECN_send_syn(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -301,6 +309,9 @@ TCP_ECN_make_synack(struct request_sock
 		th->ece = 1;
 }
 
+/* Set up ECN state for a packet on a ESTABLISHED socket that is about to
+ * be sent.
+ */
 static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb,
 				int tcp_header_len)
 {
@@ -362,7 +373,9 @@ struct tcp_out_options {
 	__u32 tsval, tsecr;	/* need to include OPTION_TS */
 };
 
-/* Beware: Something in the Internet is very sensitive to the ordering of
+/* Write previously computed TCP options to the packet.
+ *
+ * Beware: Something in the Internet is very sensitive to the ordering of
  * TCP options, we learned this through the hard way, so be careful here.
  * Luckily we can at least blame others for their non-compliance but from
  * inter-operatibility perspective it seems that we're somewhat stuck with
@@ -445,6 +458,8 @@ static void tcp_options_write(__be32 *pt
 	}
 }
 
+/* Compute TCP options for SYN packets and store them in the socket.
+ */
 static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
 				struct tcp_out_options *opts,
 				struct tcp_md5sig_key **md5) {
@@ -493,6 +508,8 @@ static unsigned tcp_syn_options(struct s
 	return size;
 }
 
+/* Set up TCP options for SYN-ACKs and store them in the socket.
+ */
 static unsigned tcp_synack_options(struct sock *sk,
 				   struct request_sock *req,
 				   unsigned mss, struct sk_buff *skb,
@@ -541,6 +558,8 @@ static unsigned tcp_synack_options(struc
 	return size;
 }
 
+/* Compute TCP options for ESTABLISHED sockets and store them in the socket.
+ */
 static unsigned tcp_established_options(struct sock *sk, struct sk_buff *skb,
 					struct tcp_out_options *opts,
 					struct tcp_md5sig_key **md5) {
@@ -705,7 +724,7 @@ static int tcp_transmit_skb(struct sock
 	return net_xmit_eval(err);
 }
 
-/* This routine just queue's the buffer
+/* This routine just queues the buffer
  *
  * NOTE: probe0 timer is not checked, do not forget tcp_push_pending_frames,
  * otherwise socket can stall.
@@ -909,6 +928,8 @@ static void __pskb_trim_head(struct sk_b
 	skb->len = skb->data_len;
 }
 
+/* Remove acked data from a packet in the transmit queue.
+ */
 int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
 {
 	if (skb_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
@@ -937,7 +958,9 @@ int tcp_trim_head(struct sock *sk, struc
 	return 0;
 }
 
-/* Not accounting for SACKs here. */
+/* Calculate MSS
+ * Not accounting for SACKs here.
+ */
 int tcp_mtu_to_mss(struct sock *sk, int pmtu)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1143,7 +1166,8 @@ static inline unsigned int tcp_cwnd_test
 	return 0;
 }
 
-/* This must be invoked the first time we consider transmitting
+/* Intialize TSO state.
+ * This must be invoked the first time we consider transmitting
  * SKB onto the wire.
  */
 static int tcp_init_tso_segs(struct sock *sk, struct sk_buff *skb,
@@ -1242,6 +1266,8 @@ static unsigned int tcp_snd_test(struct
 	return cwnd_quota;
 }
 
+/* Test if sending is allowed right now.
+ */
 int tcp_may_send_now(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1378,6 +1404,9 @@ send_now:
 }
 
 /* Create a new MTU probe if we are ready.
+ * MTU probe is trying to increase the path MTU again after PMTU
+ * discovery initially shrunk the PMTU. This discovers routing
+ * changes.
  * Returns 0 if we should wait to probe (no cwnd available),
  *         1 if a probe was sent,
  *         -1 otherwise
@@ -1808,6 +1837,9 @@ static int tcp_can_collapse(struct sock
 	return 1;
 }
 
+/* Collapse packets in the retransmit queue to make them
+ * fit into the MSS/MTU and minimize packets on the wire.
+ */
 static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
 				     int space)
 {
@@ -1957,6 +1989,9 @@ int tcp_retransmit_skb(struct sock *sk,
 	return err;
 }
 
+/* Check if we forward retransmits are possible in the current
+ * window/congestion state.
+ */
 static int tcp_can_forward_retransmit(struct sock *sk)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2145,7 +2180,8 @@ void tcp_send_active_reset(struct sock *
 	TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
 }
 
-/* WARNING: This routine must only be called when we have already sent
+/* Send a crossed SYN-ACK during socket establishment.
+ * WARNING: This routine must only be called when we have already sent
  * a SYN packet that crossed the incoming SYN that caused this routine
  * to get called. If this assumption fails then the initial rcv_wnd
  * and rcv_wscale values will not be correct.
@@ -2493,6 +2529,9 @@ static int tcp_xmit_probe_skb(struct soc
 	return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC);
 }
 
+/* Initiate keepalive or window probe from timer.
+ * Retransmits the data on the head of the retransmit queue.
+ */
 int tcp_write_wakeup(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);

Re: [PATCH] TCP: Add comments to (near) all functions in tcp_output.c

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7907 bytes --]

On Fri, 17 Jul 2009, Andi Kleen wrote:

> TCP: Add comments to (near) all functions in tcp_output.c
> 
> While looking for something else I spent some time adding
> one liner comments to the tcp_output.c functions that
> didn't have any. That makes the comments more consistent.
> 
> I hope I documented everything right.
> 
> No code changes.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
>  net/ipv4/tcp_output.c |   49 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
> ===================================================================
> --- linux-2.6.31-rc3-ak.orig/net/ipv4/tcp_output.c
> +++ linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
> @@ -59,6 +59,8 @@ int sysctl_tcp_base_mss __read_mostly =
>  /* By default, RFC2861 behavior.  */
>  int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
>  
> +/* Account for new data that has been sent to the network.
> + */
>  static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -142,6 +144,8 @@ static void tcp_cwnd_restart(struct sock
>  	tp->snd_cwnd_used = 0;
>  }
>  
> +/* Congestion state accounting after a packet has been sent.
> + */
>  static void tcp_event_data_sent(struct tcp_sock *tp,
>  				struct sk_buff *skb, struct sock *sk)
>  {
> @@ -276,6 +280,8 @@ static u16 tcp_select_window(struct sock
>  	return new_win;
>  }
>  
> +/* Packet ECN state for a SYN-ACK
> + */
>  static inline void TCP_ECN_send_synack(struct tcp_sock *tp, struct sk_buff *skb)
>  {
>  	TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_CWR;
> @@ -283,6 +289,8 @@ static inline void TCP_ECN_send_synack(s
>  		TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_ECE;
>  }
>  
> +/* Packet ECN state for a SYN.
> + */
>  static inline void TCP_ECN_send_syn(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -301,6 +309,9 @@ TCP_ECN_make_synack(struct request_sock
>  		th->ece = 1;
>  }
>  
> +/* Set up ECN state for a packet on a ESTABLISHED socket that is about to
> + * be sent.
> + */
>  static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb,
>  				int tcp_header_len)
>  {
> @@ -362,7 +373,9 @@ struct tcp_out_options {
>  	__u32 tsval, tsecr;	/* need to include OPTION_TS */
>  };
>  
> -/* Beware: Something in the Internet is very sensitive to the ordering of
> +/* Write previously computed TCP options to the packet.
> + *
> + * Beware: Something in the Internet is very sensitive to the ordering of
>   * TCP options, we learned this through the hard way, so be careful here.
>   * Luckily we can at least blame others for their non-compliance but from
>   * inter-operatibility perspective it seems that we're somewhat stuck with
> @@ -445,6 +458,8 @@ static void tcp_options_write(__be32 *pt
>  	}
>  }
>  
> +/* Compute TCP options for SYN packets and store them in the socket.
> + */

These three (this and two below) calculate stuff not into socket but into 
a tcp_out_options struct in stack, and besides that they also find out 
the space that is needed for those options (the returned int).

>  static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
>  				struct tcp_out_options *opts,
>  				struct tcp_md5sig_key **md5) {
> @@ -493,6 +508,8 @@ static unsigned tcp_syn_options(struct s
>  	return size;
>  }
>  
> +/* Set up TCP options for SYN-ACKs and store them in the socket.
> + */
>  static unsigned tcp_synack_options(struct sock *sk,
>  				   struct request_sock *req,
>  				   unsigned mss, struct sk_buff *skb,
> @@ -541,6 +558,8 @@ static unsigned tcp_synack_options(struc
>  	return size;
>  }
>  
> +/* Compute TCP options for ESTABLISHED sockets and store them in the socket.
> + */
>  static unsigned tcp_established_options(struct sock *sk, struct sk_buff *skb,
>  					struct tcp_out_options *opts,
>  					struct tcp_md5sig_key **md5) {
> @@ -705,7 +724,7 @@ static int tcp_transmit_skb(struct sock
>  	return net_xmit_eval(err);
>  }
>  
> -/* This routine just queue's the buffer
> +/* This routine just queues the buffer
>   *
>   * NOTE: probe0 timer is not checked, do not forget tcp_push_pending_frames,
>   * otherwise socket can stall.
> @@ -909,6 +928,8 @@ static void __pskb_trim_head(struct sk_b
>  	skb->len = skb->data_len;
>  }
>  
> +/* Remove acked data from a packet in the transmit queue.
> + */
>  int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
>  {
>  	if (skb_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
> @@ -937,7 +958,9 @@ int tcp_trim_head(struct sock *sk, struc
>  	return 0;
>  }
>  
> -/* Not accounting for SACKs here. */
> +/* Calculate MSS
> + * Not accounting for SACKs here.
> + */
>  int tcp_mtu_to_mss(struct sock *sk, int pmtu)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -1143,7 +1166,8 @@ static inline unsigned int tcp_cwnd_test
>  	return 0;
>  }
>  
> -/* This must be invoked the first time we consider transmitting
> +/* Intialize TSO state.

... of skb. ?

Not some some socket or whatever global state.

> + * This must be invoked the first time we consider transmitting
>   * SKB onto the wire.
>   */
>  static int tcp_init_tso_segs(struct sock *sk, struct sk_buff *skb,
> @@ -1242,6 +1266,8 @@ static unsigned int tcp_snd_test(struct
>  	return cwnd_quota;
>  }
>  
> +/* Test if sending is allowed right now.
> + */
>  int tcp_may_send_now(struct sock *sk)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> @@ -1378,6 +1404,9 @@ send_now:
>  }
>  
>  /* Create a new MTU probe if we are ready.
> + * MTU probe is trying to increase the path MTU again after PMTU
> + * discovery initially shrunk the PMTU. This discovers routing
> + * changes.

I don't particularly like that "initially shrunk" part here. Honestly I 
don't even know what that refers to :-) and mtu probe may well work from 
1500 upwards too if your path is able... I mean that no shrunking 
occurred in that scenario?!?

>   * Returns 0 if we should wait to probe (no cwnd available),
>   *         1 if a probe was sent,
>   *         -1 otherwise
> @@ -1808,6 +1837,9 @@ static int tcp_can_collapse(struct sock
>  	return 1;
>  }
>  
> +/* Collapse packets in the retransmit queue to make them
> + * fit into the MSS/MTU and minimize packets on the wire.

small packets? collapsed up to MSS/MTU?

> + */
>  static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
>  				     int space)
>  {
> @@ -1957,6 +1989,9 @@ int tcp_retransmit_skb(struct sock *sk,
>  	return err;
>  }
>  
> +/* Check if we forward retransmits are possible in the current
> + * window/congestion state.
> + */
>  static int tcp_can_forward_retransmit(struct sock *sk)
>  {
>  	const struct inet_connection_sock *icsk = inet_csk(sk);
> @@ -2145,7 +2180,8 @@ void tcp_send_active_reset(struct sock *
>  	TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
>  }
>  
> -/* WARNING: This routine must only be called when we have already sent
> +/* Send a crossed SYN-ACK during socket establishment.
> + * WARNING: This routine must only be called when we have already sent
>   * a SYN packet that crossed the incoming SYN that caused this routine
>   * to get called. If this assumption fails then the initial rcv_wnd
>   * and rcv_wscale values will not be correct.
> @@ -2493,6 +2529,9 @@ static int tcp_xmit_probe_skb(struct soc
>  	return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC);
>  }
>  
> +/* Initiate keepalive or window probe from timer.
> + * Retransmits the data on the head of the retransmit queue.

skb = tcp_send_head(sk)? ...That's not head of the retransmit queue but 
the first unsent segment. ...And it may send just a probe packet(s) if no 
data is available.

> + */
>  int tcp_write_wakeup(struct sock *sk)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);

Other than those mentioned,

Acked-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>


-- 
 i.

[PATCH] TCP: Add comments to (near) all functions in tcp_output.c v2

[Andi Kleen]

Here's a updated version incorporating feedback.
-Andi

---

TCP: Add comments to (near) all functions in tcp_output.c v2

While looking for something else I spent some time adding
one liner comments to the tcp_output.c functions that
didn't have any. That makes the comments more consistent.

I hope I documented everything right.

No code changes.

v2: Incorporated feedback from Ilpo.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

---
 net/ipv4/tcp_output.c |   51 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 5 deletions(-)

Index: linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
===================================================================
--- linux-2.6.31-rc3-ak.orig/net/ipv4/tcp_output.c
+++ linux-2.6.31-rc3-ak/net/ipv4/tcp_output.c
@@ -59,6 +59,8 @@ int sysctl_tcp_base_mss __read_mostly =
 /* By default, RFC2861 behavior.  */
 int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
 
+/* Account for new data that has been sent to the network.
+ */
 static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -142,6 +144,8 @@ static void tcp_cwnd_restart(struct sock
 	tp->snd_cwnd_used = 0;
 }
 
+/* Congestion state accounting after a packet has been sent.
+ */
 static void tcp_event_data_sent(struct tcp_sock *tp,
 				struct sk_buff *skb, struct sock *sk)
 {
@@ -276,6 +280,8 @@ static u16 tcp_select_window(struct sock
 	return new_win;
 }
 
+/* Packet ECN state for a SYN-ACK
+ */
 static inline void TCP_ECN_send_synack(struct tcp_sock *tp, struct sk_buff *skb)
 {
 	TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_CWR;
@@ -283,6 +289,8 @@ static inline void TCP_ECN_send_synack(s
 		TCP_SKB_CB(skb)->flags &= ~TCPCB_FLAG_ECE;
 }
 
+/* Packet ECN state for a SYN.
+ */
 static inline void TCP_ECN_send_syn(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -301,6 +309,9 @@ TCP_ECN_make_synack(struct request_sock
 		th->ece = 1;
 }
 
+/* Set up ECN state for a packet on a ESTABLISHED socket that is about to
+ * be sent.
+ */
 static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb,
 				int tcp_header_len)
 {
@@ -362,7 +373,9 @@ struct tcp_out_options {
 	__u32 tsval, tsecr;	/* need to include OPTION_TS */
 };
 
-/* Beware: Something in the Internet is very sensitive to the ordering of
+/* Write previously computed TCP options to the packet.
+ *
+ * Beware: Something in the Internet is very sensitive to the ordering of
  * TCP options, we learned this through the hard way, so be careful here.
  * Luckily we can at least blame others for their non-compliance but from
  * inter-operatibility perspective it seems that we're somewhat stuck with
@@ -445,6 +458,9 @@ static void tcp_options_write(__be32 *pt
 	}
 }
 
+/* Compute TCP options for SYN packets. This is not the final
+ * network wire format yet.
+ */
 static unsigned tcp_syn_options(struct sock *sk, struct sk_buff *skb,
 				struct tcp_out_options *opts,
 				struct tcp_md5sig_key **md5) {
@@ -493,6 +509,8 @@ static unsigned tcp_syn_options(struct s
 	return size;
 }
 
+/* Set up TCP options for SYN-ACKs.
+ */
 static unsigned tcp_synack_options(struct sock *sk,
 				   struct request_sock *req,
 				   unsigned mss, struct sk_buff *skb,
@@ -541,6 +559,9 @@ static unsigned tcp_synack_options(struc
 	return size;
 }
 
+/* Compute TCP options for ESTABLISHED sockets. This is not the
+ * final wire format yet.
+ */
 static unsigned tcp_established_options(struct sock *sk, struct sk_buff *skb,
 					struct tcp_out_options *opts,
 					struct tcp_md5sig_key **md5) {
@@ -705,7 +726,7 @@ static int tcp_transmit_skb(struct sock
 	return net_xmit_eval(err);
 }
 
-/* This routine just queue's the buffer
+/* This routine just queues the buffer
  *
  * NOTE: probe0 timer is not checked, do not forget tcp_push_pending_frames,
  * otherwise socket can stall.
@@ -909,6 +930,8 @@ static void __pskb_trim_head(struct sk_b
 	skb->len = skb->data_len;
 }
 
+/* Remove acked data from a packet in the transmit queue.
+ */
 int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
 {
 	if (skb_cloned(skb) && pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
@@ -937,7 +960,9 @@ int tcp_trim_head(struct sock *sk, struc
 	return 0;
 }
 
-/* Not accounting for SACKs here. */
+/* Calculate MSS
+ * Not accounting for SACKs here.
+ */
 int tcp_mtu_to_mss(struct sock *sk, int pmtu)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1143,7 +1168,8 @@ static inline unsigned int tcp_cwnd_test
 	return 0;
 }
 
-/* This must be invoked the first time we consider transmitting
+/* Intialize TSO state of a skb.
+ * This must be invoked the first time we consider transmitting
  * SKB onto the wire.
  */
 static int tcp_init_tso_segs(struct sock *sk, struct sk_buff *skb,
@@ -1242,6 +1268,8 @@ static unsigned int tcp_snd_test(struct
 	return cwnd_quota;
 }
 
+/* Test if sending is allowed right now.
+ */
 int tcp_may_send_now(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1378,6 +1406,10 @@ send_now:
 }
 
 /* Create a new MTU probe if we are ready.
+ * MTU probe is regularly attempting to increase the path MTU by
+ * deliberately sending larger packets.  This discovers routing
+ * changes resulting in larger path MTUs.
+ *
  * Returns 0 if we should wait to probe (no cwnd available),
  *         1 if a probe was sent,
  *         -1 otherwise
@@ -1808,6 +1840,9 @@ static int tcp_can_collapse(struct sock
 	return 1;
 }
 
+/* Collapse packets in the retransmit queue to make to create
+ * less packets on the wire. This is only done on retransmission.
+ */
 static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
 				     int space)
 {
@@ -1957,6 +1992,9 @@ int tcp_retransmit_skb(struct sock *sk,
 	return err;
 }
 
+/* Check if we forward retransmits are possible in the current
+ * window/congestion state.
+ */
 static int tcp_can_forward_retransmit(struct sock *sk)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2145,7 +2183,8 @@ void tcp_send_active_reset(struct sock *
 	TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTRSTS);
 }
 
-/* WARNING: This routine must only be called when we have already sent
+/* Send a crossed SYN-ACK during socket establishment.
+ * WARNING: This routine must only be called when we have already sent
  * a SYN packet that crossed the incoming SYN that caused this routine
  * to get called. If this assumption fails then the initial rcv_wnd
  * and rcv_wscale values will not be correct.
@@ -2493,6 +2532,8 @@ static int tcp_xmit_probe_skb(struct soc
 	return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC);
 }
 
+/* Initiate keepalive or window probe from timer.
+ */
 int tcp_write_wakeup(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);

Re: [PATCH] TCP: Add comments to (near) all functions in tcp_output.c v2

[Shan Wei]

Andi Kleen said:
>  
> +/* Compute TCP options for ESTABLISHED sockets. This is not the
> + * final wire format yet.
> + */

typo, “network wire”?


> +/* Test if sending is allowed right now.
> + */
>  int tcp_may_send_now(struct sock *sk)

Is it better to provide the condition be allowed?

+ /* Check the nagle, the congestion window and 
+  * the send window, decide if sending is allowed right now.
+  */

Re: [PATCH] TCP: Add comments to (near) all functions in tcp_output.c v2

[Andi Kleen]

On Mon, Jul 20, 2009 at 08:06:53PM +0800, Shan Wei wrote:
> Andi Kleen said:
> >  
> > +/* Compute TCP options for ESTABLISHED sockets. This is not the
> > + * final wire format yet.
> > + */
> 
> typo, “network wire”?

I don't see a typo.

> 
> 
> > +/* Test if sending is allowed right now.
> > + */
> >  int tcp_may_send_now(struct sock *sk)
> 
> Is it better to provide the condition be allowed?

Such a introductionary comment is not intended to be a full
representation of the code, just a overview.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] TCP: Add comments to (near) all functions in tcp_output.c v2

[Shan Wei]

Andi Kleen said:
> On Mon, Jul 20, 2009 at 08:06:53PM +0800, Shan Wei wrote:
>> Andi Kleen said:
>>>  
>>> +/* Compute TCP options for ESTABLISHED sockets. This is not the
>>> + * final wire format yet.
>>> + */
>> typo, “network wire”?
> 
> I don't see a typo.

Just see the comment about tcp_syn_options().
"wire" vs "network wire".

> 
>>
>>> +/* Test if sending is allowed right now.
>>> + */
>>>  int tcp_may_send_now(struct sock *sk)
>> Is it better to provide the condition be allowed?
> 
> Such a introductionary comment is not intended to be a full
> representation of the code, just a overview.
> 

OK

Re: [PATCH] TCP: Add comments to (near) all functions in tcp_output.c v2

[David Miller]

From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Mon, 20 Jul 2009 20:06:53 +0800

> Andi Kleen said:
>>  
>> +/* Compute TCP options for ESTABLISHED sockets. This is not the
>> + * final wire format yet.
>> + */
> 
> typo, “network wire”?

Saying plain "wire" is canonical, the context is well understood.