diff mbox

[3/5] tcp/dccp: Consolidate common code for RFC 3390 conversion

Message ID 1283145794-5615-4-git-send-email-gerrit@erg.abdn.ac.uk
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Gerrit Renker Aug. 30, 2010, 5:23 a.m. UTC 
This patch consolidates initial-window code common to TCP and CCID-2:
 * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and
 * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341).

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 include/net/tcp.h      |   15 +++++++++++++++
 net/dccp/ccids/ccid2.c |    8 ++------
 net/ipv4/tcp_input.c   |   17 ++---------------
 3 files changed, 19 insertions(+), 21 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ilpo Järvinen Aug. 30, 2010, 12:59 p.m. UTC | #1 
On Mon, 30 Aug 2010, Gerrit Renker wrote:

> This patch consolidates initial-window code common to TCP and CCID-2:
>  * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and
>  * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341).
> 
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
>  include/net/tcp.h      |   15 +++++++++++++++
>  net/dccp/ccids/ccid2.c |    8 ++------
>  net/ipv4/tcp_input.c   |   17 ++---------------
>  3 files changed, 19 insertions(+), 21 deletions(-)
> 
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -789,6 +789,21 @@ static inline __u32 tcp_current_ssthresh(const struct sock *sk)
>  /* Use define here intentionally to get WARN_ON location shown at the caller */
>  #define tcp_verify_left_out(tp)	WARN_ON(tcp_left_out(tp) > tp->packets_out)
>  
> +/*
> + * Convert RFC 3390 larger initial window into an equivalent number of packets.
> + *
> + * John Heffner states:
> + *
> + *	The RFC specifies a window of no more than 4380 bytes
> + *	unless 2*MSS > 4380.  Reading the pseudocode in the RFC
> + *	is a bit misleading because they use a clamp at 4380 bytes
> + *	rather than a multiplier in the relevant range.
> + */
> +static inline u32 rfc3390_bytes_to_packets(const u32 smss)
> +{
> +	return smss <= 1095 ? 4 : (smss > 1460 ? 2 : 3);
> +}
> +
>  extern void tcp_enter_cwr(struct sock *sk, const int set_ssthresh);
>  extern __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst);
>  
> --- a/net/dccp/ccids/ccid2.c
> +++ b/net/dccp/ccids/ccid2.c
> @@ -641,12 +641,8 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk)
>  	/* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */
>  	hc->tx_ssthresh = ~0U;
>  
> -	/*
> -	 * RFC 4341, 5: "The cwnd parameter is initialized to at most four
> -	 * packets for new connections, following the rules from [RFC3390]".
> -	 * We need to convert the bytes of RFC3390 into the packets of RFC 4341.
> -	 */
> -	hc->tx_cwnd = clamp(4380U / dp->dccps_mss_cache, 2U, 4U);
> +	/* Use larger initial windows (RFC 4341, section 5). */
> +	hc->tx_cwnd = rfc3390_bytes_to_packets(dp->dccps_mss_cache);
>  
>  	/* Make sure that Ack Ratio is enabled and within bounds. */
>  	max_ratio = DIV_ROUND_UP(hc->tx_cwnd, 2);
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -805,25 +805,12 @@ void tcp_update_metrics(struct sock *sk)
>  	}
>  }
>  
> -/* Numbers are taken from RFC3390.
> - *
> - * John Heffner states:
> - *
> - *	The RFC specifies a window of no more than 4380 bytes
> - *	unless 2*MSS > 4380.  Reading the pseudocode in the RFC
> - *	is a bit misleading because they use a clamp at 4380 bytes
> - *	rather than use a multiplier in the relevant range.
> - */
>  __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst)
>  {
>  	__u32 cwnd = (dst ? dst_metric(dst, RTAX_INITCWND) : 0);
>  
> -	if (!cwnd) {
> -		if (tp->mss_cache > 1460)
> -			cwnd = 2;
> -		else
> -			cwnd = (tp->mss_cache > 1095) ? 3 : 4;
> -	}
> +	if (!cwnd)
> +		cwnd = rfc3390_bytes_to_packets(tp->mss_cache);
>  	return min_t(__u32, cwnd, tp->snd_cwnd_clamp);
>  }

What is spelled out in tcp_select_initial_window might also need to follow 
the same logic?
Gerrit Renker Sept. 1, 2010, 5:23 a.m. UTC | #2 
| > This patch consolidates initial-window code common to TCP and CCID-2:
| >  * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and
| >  * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341).
...
| > +static inline u32 rfc3390_bytes_to_packets(const u32 smss)
| > +{
| > +	return smss <= 1095 ? 4 : (smss > 1460 ? 2 : 3);
| > +}
| > +
...
| 
| What is spelled out in tcp_select_initial_window might also need to follow 
| the same logic?
| 
Thank you for paying attention, can you please have a look at the subsequent
patch. I went through this yesterday and found that in 4 different places 
there are 4 different interpretations of what RFC 3390 means.

Having asked someone more familiar with IETF issues, it seems that the RFC 5681
interpretation of RFC 3390 is now the 'official' one.

+------------+--------------------+--------------------+--------------------+--------------------+
| Condition  |     RFC 2414       |        Linux       |       Linux        |     RFC 5681       |
|            |     RFC 3390       |      tcp_input     |     tcp_output     |                    |
+------------+--------------------+--------------------+--------------------+--------------------+
| 2 segments | 2190 < MSS         | 1460 < MSS         | 4380 < MSS         | 2190 < MSS         |
| 3 segments | 1095 < MSS <= 1460 | 1095 < MSS <= 1460 | 1460 < MSS <= 4380 | 1095 < MSS <= 2190 |
| 4 segments |        MSS <= 1095 |        MSS <= 1095 |        MSS <= 1460 |        MSS <= 1095 |
+------------+--------------------+--------------------+--------------------+--------------------+
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Zimmermann Sept. 1, 2010, 7:05 a.m. UTC | #3 
Hi Gerrit,

Am 01.09.2010 um 07:23 schrieb Gerrit Renker:

> | > This patch consolidates initial-window code common to TCP and CCID-2:
> | >  * TCP uses RFC 3390 in a packet-oriented manner (tcp_input.c) and
> | >  * CCID-2 uses RFC 3390 in packet-oriented manner (RFC 4341).
> ...
> | > +static inline u32 rfc3390_bytes_to_packets(const u32 smss)
> | > +{
> | > +	return smss <= 1095 ? 4 : (smss > 1460 ? 2 : 3);
> | > +}
> | > +
> ...
> | 
> | What is spelled out in tcp_select_initial_window might also need to follow 
> | the same logic?
> | 
> Thank you for paying attention, can you please have a look at the subsequent
> patch. I went through this yesterday and found that in 4 different places 
> there are 4 different interpretations of what RFC 3390 means.
> 
> Having asked someone more familiar with IETF issues, it seems that the RFC 5681
> interpretation of RFC 3390 is now the 'official' one.
> 
> +------------+--------------------+--------------------+--------------------+--------------------+
> | Condition  |     RFC 2414       |        Linux       |       Linux        |     RFC 5681       |
> |            |     RFC 3390       |      tcp_input     |     tcp_output     |                    |
> +------------+--------------------+--------------------+--------------------+--------------------+
> | 2 segments | 2190 < MSS         | 1460 < MSS         | 4380 < MSS         | 2190 < MSS         |
> | 3 segments | 1095 < MSS <= 1460 | 1095 < MSS <= 1460 | 1460 < MSS <= 4380 | 1095 < MSS <= 2190 |

                              ^^^^^

Are you sure about this?

RFC3390 says (page 2):
 If (1095 bytes < MSS < 2190 bytes) then win <= 4380;

So, IMO it should be 1095 < MSS < 2190 for 3 segments, and 2190 <= MSS for 2 segments
in the "RFC 3390" column.

BTW: I think that we should not reference RFC2414, since RFC3390 obsoletes it.

Alex


> | 4 segments |        MSS <= 1095 |        MSS <= 1095 |        MSS <= 1460 |        MSS <= 1095 |
> +------------+--------------------+--------------------+--------------------+--------------------+
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

//
// Dipl.-Inform. Alexander Zimmermann
// Department of Computer Science, Informatik 4
// RWTH Aachen University
// Ahornstr. 55, 52056 Aachen, Germany
// phone: (49-241) 80-21422, fax: (49-241) 80-22221
// email: zimmermann@cs.rwth-aachen.de
// web: http://www.umic-mesh.net
//
diff mbox

Patch

--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -789,6 +789,21 @@  static inline __u32 tcp_current_ssthresh(const struct sock *sk)
 /* Use define here intentionally to get WARN_ON location shown at the caller */
 #define tcp_verify_left_out(tp)	WARN_ON(tcp_left_out(tp) > tp->packets_out)
 
+/*
+ * Convert RFC 3390 larger initial window into an equivalent number of packets.
+ *
+ * John Heffner states:
+ *
+ *	The RFC specifies a window of no more than 4380 bytes
+ *	unless 2*MSS > 4380.  Reading the pseudocode in the RFC
+ *	is a bit misleading because they use a clamp at 4380 bytes
+ *	rather than a multiplier in the relevant range.
+ */
+static inline u32 rfc3390_bytes_to_packets(const u32 smss)
+{
+	return smss <= 1095 ? 4 : (smss > 1460 ? 2 : 3);
+}
+
 extern void tcp_enter_cwr(struct sock *sk, const int set_ssthresh);
 extern __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst);
 
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -641,12 +641,8 @@  static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk)
 	/* RFC 4341, 5: initialise ssthresh to arbitrarily high (max) value */
 	hc->tx_ssthresh = ~0U;
 
-	/*
-	 * RFC 4341, 5: "The cwnd parameter is initialized to at most four
-	 * packets for new connections, following the rules from [RFC3390]".
-	 * We need to convert the bytes of RFC3390 into the packets of RFC 4341.
-	 */
-	hc->tx_cwnd = clamp(4380U / dp->dccps_mss_cache, 2U, 4U);
+	/* Use larger initial windows (RFC 4341, section 5). */
+	hc->tx_cwnd = rfc3390_bytes_to_packets(dp->dccps_mss_cache);
 
 	/* Make sure that Ack Ratio is enabled and within bounds. */
 	max_ratio = DIV_ROUND_UP(hc->tx_cwnd, 2);
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -805,25 +805,12 @@  void tcp_update_metrics(struct sock *sk)
 	}
 }
 
-/* Numbers are taken from RFC3390.
- *
- * John Heffner states:
- *
- *	The RFC specifies a window of no more than 4380 bytes
- *	unless 2*MSS > 4380.  Reading the pseudocode in the RFC
- *	is a bit misleading because they use a clamp at 4380 bytes
- *	rather than use a multiplier in the relevant range.
- */
 __u32 tcp_init_cwnd(struct tcp_sock *tp, struct dst_entry *dst)
 {
 	__u32 cwnd = (dst ? dst_metric(dst, RTAX_INITCWND) : 0);
 
-	if (!cwnd) {
-		if (tp->mss_cache > 1460)
-			cwnd = 2;
-		else
-			cwnd = (tp->mss_cache > 1095) ? 3 : 4;
-	}
+	if (!cwnd)
+		cwnd = rfc3390_bytes_to_packets(tp->mss_cache);
 	return min_t(__u32, cwnd, tp->snd_cwnd_clamp);
 }