diff options
author | Vlad Yasevich <vladislav.yasevich@hp.com> | 2007-10-24 15:59:16 -0400 |
---|---|---|
committer | Vlad Yasevich <vladislav.yasevich@hp.com> | 2007-11-07 11:39:27 -0500 |
commit | b6157d8e03e1e780660a328f7183bcbfa4a93a19 (patch) | |
tree | cff4da4725b1bb0c5b603dc07204697dd0623ad5 | |
parent | f3830ccc2ea503ab37d605f6c313d61423ddd94e (diff) |
SCTP: Fix difference cases of retransmit.
Commit d0ce92910bc04e107b2f3f2048f07e94f570035d broke several retransmit
cases including fast retransmit. The reason is that we should
only delay by rto while doing retranmists as a result of a timeout.
Retransmit as a result of path mtu discover, fast retransmit, or
other evernts that should trigger immidiate retransmissions got broken.
Also, since rto is doubled prior to marking of packets elegable for
retransmission, we never marked correct chunks anyway.
The fix is provide a reason for a given retransmission so that we
can mark chunks appropriately and to save the old rto value to do
comparisons against.
All regressions tests passed with this code.
Spotted by Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
-rw-r--r-- | include/net/sctp/command.h | 1 | ||||
-rw-r--r-- | include/net/sctp/constants.h | 1 | ||||
-rw-r--r-- | include/net/sctp/sctp.h | 1 | ||||
-rw-r--r-- | include/net/sctp/structs.h | 5 | ||||
-rw-r--r-- | net/sctp/outqueue.c | 33 | ||||
-rw-r--r-- | net/sctp/sm_sideeffect.c | 10 | ||||
-rw-r--r-- | net/sctp/sm_statefuns.c | 2 | ||||
-rw-r--r-- | net/sctp/transport.c | 5 |
8 files changed, 36 insertions, 22 deletions
diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h index b8733364557f..c1f797673571 100644 --- a/include/net/sctp/command.h +++ b/include/net/sctp/command.h | |||
@@ -103,6 +103,7 @@ typedef enum { | |||
103 | SCTP_CMD_ASSOC_CHANGE, /* generate and send assoc_change event */ | 103 | SCTP_CMD_ASSOC_CHANGE, /* generate and send assoc_change event */ |
104 | SCTP_CMD_ADAPTATION_IND, /* generate and send adaptation event */ | 104 | SCTP_CMD_ADAPTATION_IND, /* generate and send adaptation event */ |
105 | SCTP_CMD_ASSOC_SHKEY, /* generate the association shared keys */ | 105 | SCTP_CMD_ASSOC_SHKEY, /* generate the association shared keys */ |
106 | SCTP_CMD_T1_RETRAN, /* Mark for retransmission after T1 timeout */ | ||
106 | SCTP_CMD_LAST | 107 | SCTP_CMD_LAST |
107 | } sctp_verb_t; | 108 | } sctp_verb_t; |
108 | 109 | ||
diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h index da8354e8e33c..73fbdf6a24f8 100644 --- a/include/net/sctp/constants.h +++ b/include/net/sctp/constants.h | |||
@@ -407,6 +407,7 @@ typedef enum { | |||
407 | SCTP_RTXR_T3_RTX, | 407 | SCTP_RTXR_T3_RTX, |
408 | SCTP_RTXR_FAST_RTX, | 408 | SCTP_RTXR_FAST_RTX, |
409 | SCTP_RTXR_PMTUD, | 409 | SCTP_RTXR_PMTUD, |
410 | SCTP_RTXR_T1_RTX, | ||
410 | } sctp_retransmit_reason_t; | 411 | } sctp_retransmit_reason_t; |
411 | 412 | ||
412 | /* Reasons to lower cwnd. */ | 413 | /* Reasons to lower cwnd. */ |
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 93eb708609e7..70827305f501 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h | |||
@@ -267,6 +267,7 @@ enum | |||
267 | SCTP_MIB_T5_SHUTDOWN_GUARD_EXPIREDS, | 267 | SCTP_MIB_T5_SHUTDOWN_GUARD_EXPIREDS, |
268 | SCTP_MIB_DELAY_SACK_EXPIREDS, | 268 | SCTP_MIB_DELAY_SACK_EXPIREDS, |
269 | SCTP_MIB_AUTOCLOSE_EXPIREDS, | 269 | SCTP_MIB_AUTOCLOSE_EXPIREDS, |
270 | SCTP_MIB_T1_RETRANSMITS, | ||
270 | SCTP_MIB_T3_RETRANSMITS, | 271 | SCTP_MIB_T3_RETRANSMITS, |
271 | SCTP_MIB_PMTUD_RETRANSMITS, | 272 | SCTP_MIB_PMTUD_RETRANSMITS, |
272 | SCTP_MIB_FAST_RETRANSMITS, | 273 | SCTP_MIB_FAST_RETRANSMITS, |
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index ef892e00c833..482c2aab3d67 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h | |||
@@ -873,10 +873,11 @@ struct sctp_transport { | |||
873 | * address list derived from the INIT or INIT ACK chunk, a | 873 | * address list derived from the INIT or INIT ACK chunk, a |
874 | * number of data elements needs to be maintained including: | 874 | * number of data elements needs to be maintained including: |
875 | */ | 875 | */ |
876 | __u32 rtt; /* This is the most recent RTT. */ | ||
877 | |||
878 | /* RTO : The current retransmission timeout value. */ | 876 | /* RTO : The current retransmission timeout value. */ |
879 | unsigned long rto; | 877 | unsigned long rto; |
878 | unsigned long last_rto; | ||
879 | |||
880 | __u32 rtt; /* This is the most recent RTT. */ | ||
880 | 881 | ||
881 | /* RTTVAR : The current RTT variation. */ | 882 | /* RTTVAR : The current RTT variation. */ |
882 | __u32 rttvar; | 883 | __u32 rttvar; |
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c index e315c6c756ca..99a3db5d5fae 100644 --- a/net/sctp/outqueue.c +++ b/net/sctp/outqueue.c | |||
@@ -382,7 +382,7 @@ static void sctp_insert_list(struct list_head *head, struct list_head *new) | |||
382 | /* Mark all the eligible packets on a transport for retransmission. */ | 382 | /* Mark all the eligible packets on a transport for retransmission. */ |
383 | void sctp_retransmit_mark(struct sctp_outq *q, | 383 | void sctp_retransmit_mark(struct sctp_outq *q, |
384 | struct sctp_transport *transport, | 384 | struct sctp_transport *transport, |
385 | __u8 fast_retransmit) | 385 | __u8 reason) |
386 | { | 386 | { |
387 | struct list_head *lchunk, *ltemp; | 387 | struct list_head *lchunk, *ltemp; |
388 | struct sctp_chunk *chunk; | 388 | struct sctp_chunk *chunk; |
@@ -412,20 +412,20 @@ void sctp_retransmit_mark(struct sctp_outq *q, | |||
412 | continue; | 412 | continue; |
413 | } | 413 | } |
414 | 414 | ||
415 | /* If we are doing retransmission due to a fast retransmit, | 415 | /* If we are doing retransmission due to a timeout or pmtu |
416 | * only the chunk's that are marked for fast retransmit | 416 | * discovery, only the chunks that are not yet acked should |
417 | * should be added to the retransmit queue. If we are doing | 417 | * be added to the retransmit queue. |
418 | * retransmission due to a timeout or pmtu discovery, only the | ||
419 | * chunks that are not yet acked should be added to the | ||
420 | * retransmit queue. | ||
421 | */ | 418 | */ |
422 | if ((fast_retransmit && (chunk->fast_retransmit > 0)) || | 419 | if ((reason == SCTP_RTXR_FAST_RTX && |
423 | (!fast_retransmit && !chunk->tsn_gap_acked)) { | 420 | (chunk->fast_retransmit > 0)) || |
421 | (reason != SCTP_RTXR_FAST_RTX && !chunk->tsn_gap_acked)) { | ||
424 | /* If this chunk was sent less then 1 rto ago, do not | 422 | /* If this chunk was sent less then 1 rto ago, do not |
425 | * retransmit this chunk, but give the peer time | 423 | * retransmit this chunk, but give the peer time |
426 | * to acknowlege it. | 424 | * to acknowlege it. Do this only when |
425 | * retransmitting due to T3 timeout. | ||
427 | */ | 426 | */ |
428 | if ((jiffies - chunk->sent_at) < transport->rto) | 427 | if (reason == SCTP_RTXR_T3_RTX && |
428 | (jiffies - chunk->sent_at) < transport->last_rto) | ||
429 | continue; | 429 | continue; |
430 | 430 | ||
431 | /* RFC 2960 6.2.1 Processing a Received SACK | 431 | /* RFC 2960 6.2.1 Processing a Received SACK |
@@ -467,10 +467,10 @@ void sctp_retransmit_mark(struct sctp_outq *q, | |||
467 | } | 467 | } |
468 | } | 468 | } |
469 | 469 | ||
470 | SCTP_DEBUG_PRINTK("%s: transport: %p, fast_retransmit: %d, " | 470 | SCTP_DEBUG_PRINTK("%s: transport: %p, reason: %d, " |
471 | "cwnd: %d, ssthresh: %d, flight_size: %d, " | 471 | "cwnd: %d, ssthresh: %d, flight_size: %d, " |
472 | "pba: %d\n", __FUNCTION__, | 472 | "pba: %d\n", __FUNCTION__, |
473 | transport, fast_retransmit, | 473 | transport, reason, |
474 | transport->cwnd, transport->ssthresh, | 474 | transport->cwnd, transport->ssthresh, |
475 | transport->flight_size, | 475 | transport->flight_size, |
476 | transport->partial_bytes_acked); | 476 | transport->partial_bytes_acked); |
@@ -484,7 +484,6 @@ void sctp_retransmit(struct sctp_outq *q, struct sctp_transport *transport, | |||
484 | sctp_retransmit_reason_t reason) | 484 | sctp_retransmit_reason_t reason) |
485 | { | 485 | { |
486 | int error = 0; | 486 | int error = 0; |
487 | __u8 fast_retransmit = 0; | ||
488 | 487 | ||
489 | switch(reason) { | 488 | switch(reason) { |
490 | case SCTP_RTXR_T3_RTX: | 489 | case SCTP_RTXR_T3_RTX: |
@@ -499,16 +498,18 @@ void sctp_retransmit(struct sctp_outq *q, struct sctp_transport *transport, | |||
499 | case SCTP_RTXR_FAST_RTX: | 498 | case SCTP_RTXR_FAST_RTX: |
500 | SCTP_INC_STATS(SCTP_MIB_FAST_RETRANSMITS); | 499 | SCTP_INC_STATS(SCTP_MIB_FAST_RETRANSMITS); |
501 | sctp_transport_lower_cwnd(transport, SCTP_LOWER_CWND_FAST_RTX); | 500 | sctp_transport_lower_cwnd(transport, SCTP_LOWER_CWND_FAST_RTX); |
502 | fast_retransmit = 1; | ||
503 | break; | 501 | break; |
504 | case SCTP_RTXR_PMTUD: | 502 | case SCTP_RTXR_PMTUD: |
505 | SCTP_INC_STATS(SCTP_MIB_PMTUD_RETRANSMITS); | 503 | SCTP_INC_STATS(SCTP_MIB_PMTUD_RETRANSMITS); |
506 | break; | 504 | break; |
505 | case SCTP_RTXR_T1_RTX: | ||
506 | SCTP_INC_STATS(SCTP_MIB_T1_RETRANSMITS); | ||
507 | break; | ||
507 | default: | 508 | default: |
508 | BUG(); | 509 | BUG(); |
509 | } | 510 | } |
510 | 511 | ||
511 | sctp_retransmit_mark(q, transport, fast_retransmit); | 512 | sctp_retransmit_mark(q, transport, reason); |
512 | 513 | ||
513 | /* PR-SCTP A5) Any time the T3-rtx timer expires, on any destination, | 514 | /* PR-SCTP A5) Any time the T3-rtx timer expires, on any destination, |
514 | * the sender SHOULD try to advance the "Advanced.Peer.Ack.Point" by | 515 | * the sender SHOULD try to advance the "Advanced.Peer.Ack.Point" by |
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c index bbdc938da86f..78d1a8a49bd0 100644 --- a/net/sctp/sm_sideeffect.c +++ b/net/sctp/sm_sideeffect.c | |||
@@ -453,6 +453,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc, | |||
453 | * maximum value discussed in rule C7 above (RTO.max) may be | 453 | * maximum value discussed in rule C7 above (RTO.max) may be |
454 | * used to provide an upper bound to this doubling operation. | 454 | * used to provide an upper bound to this doubling operation. |
455 | */ | 455 | */ |
456 | transport->last_rto = transport->rto; | ||
456 | transport->rto = min((transport->rto * 2), transport->asoc->rto_max); | 457 | transport->rto = min((transport->rto * 2), transport->asoc->rto_max); |
457 | } | 458 | } |
458 | 459 | ||
@@ -1267,6 +1268,12 @@ static int sctp_cmd_interpreter(sctp_event_t event_type, | |||
1267 | sctp_ootb_pkt_free(packet); | 1268 | sctp_ootb_pkt_free(packet); |
1268 | break; | 1269 | break; |
1269 | 1270 | ||
1271 | case SCTP_CMD_T1_RETRAN: | ||
1272 | /* Mark a transport for retransmission. */ | ||
1273 | sctp_retransmit(&asoc->outqueue, cmd->obj.transport, | ||
1274 | SCTP_RTXR_T1_RTX); | ||
1275 | break; | ||
1276 | |||
1270 | case SCTP_CMD_RETRAN: | 1277 | case SCTP_CMD_RETRAN: |
1271 | /* Mark a transport for retransmission. */ | 1278 | /* Mark a transport for retransmission. */ |
1272 | sctp_retransmit(&asoc->outqueue, cmd->obj.transport, | 1279 | sctp_retransmit(&asoc->outqueue, cmd->obj.transport, |
@@ -1393,7 +1400,8 @@ static int sctp_cmd_interpreter(sctp_event_t event_type, | |||
1393 | list_for_each(pos, &asoc->peer.transport_addr_list) { | 1400 | list_for_each(pos, &asoc->peer.transport_addr_list) { |
1394 | t = list_entry(pos, struct sctp_transport, | 1401 | t = list_entry(pos, struct sctp_transport, |
1395 | transports); | 1402 | transports); |
1396 | sctp_retransmit_mark(&asoc->outqueue, t, 0); | 1403 | sctp_retransmit_mark(&asoc->outqueue, t, |
1404 | SCTP_RTXR_T1_RTX); | ||
1397 | } | 1405 | } |
1398 | 1406 | ||
1399 | sctp_add_cmd_sf(commands, | 1407 | sctp_add_cmd_sf(commands, |
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index f01b408508ff..a66075a70f29 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c | |||
@@ -2305,7 +2305,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(const struct sctp_endpoint *ep, | |||
2305 | /* If we've sent any data bundled with COOKIE-ECHO we will need to | 2305 | /* If we've sent any data bundled with COOKIE-ECHO we will need to |
2306 | * resend | 2306 | * resend |
2307 | */ | 2307 | */ |
2308 | sctp_add_cmd_sf(commands, SCTP_CMD_RETRAN, | 2308 | sctp_add_cmd_sf(commands, SCTP_CMD_T1_RETRAN, |
2309 | SCTP_TRANSPORT(asoc->peer.primary_path)); | 2309 | SCTP_TRANSPORT(asoc->peer.primary_path)); |
2310 | 2310 | ||
2311 | /* Cast away the const modifier, as we want to just | 2311 | /* Cast away the const modifier, as we want to just |
diff --git a/net/sctp/transport.c b/net/sctp/transport.c index 5f467c914f80..d55ce83a020b 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c | |||
@@ -74,8 +74,8 @@ static struct sctp_transport *sctp_transport_init(struct sctp_transport *peer, | |||
74 | * given destination transport address, set RTO to the protocol | 74 | * given destination transport address, set RTO to the protocol |
75 | * parameter 'RTO.Initial'. | 75 | * parameter 'RTO.Initial'. |
76 | */ | 76 | */ |
77 | peer->last_rto = peer->rto = msecs_to_jiffies(sctp_rto_initial); | ||
77 | peer->rtt = 0; | 78 | peer->rtt = 0; |
78 | peer->rto = msecs_to_jiffies(sctp_rto_initial); | ||
79 | peer->rttvar = 0; | 79 | peer->rttvar = 0; |
80 | peer->srtt = 0; | 80 | peer->srtt = 0; |
81 | peer->rto_pending = 0; | 81 | peer->rto_pending = 0; |
@@ -385,6 +385,7 @@ void sctp_transport_update_rto(struct sctp_transport *tp, __u32 rtt) | |||
385 | tp->rto = tp->asoc->rto_max; | 385 | tp->rto = tp->asoc->rto_max; |
386 | 386 | ||
387 | tp->rtt = rtt; | 387 | tp->rtt = rtt; |
388 | tp->last_rto = tp->rto; | ||
388 | 389 | ||
389 | /* Reset rto_pending so that a new RTT measurement is started when a | 390 | /* Reset rto_pending so that a new RTT measurement is started when a |
390 | * new data chunk is sent. | 391 | * new data chunk is sent. |
@@ -578,7 +579,7 @@ void sctp_transport_reset(struct sctp_transport *t) | |||
578 | */ | 579 | */ |
579 | t->cwnd = min(4*asoc->pathmtu, max_t(__u32, 2*asoc->pathmtu, 4380)); | 580 | t->cwnd = min(4*asoc->pathmtu, max_t(__u32, 2*asoc->pathmtu, 4380)); |
580 | t->ssthresh = asoc->peer.i.a_rwnd; | 581 | t->ssthresh = asoc->peer.i.a_rwnd; |
581 | t->rto = asoc->rto_initial; | 582 | t->last_rto = t->rto = asoc->rto_initial; |
582 | t->rtt = 0; | 583 | t->rtt = 0; |
583 | t->srtt = 0; | 584 | t->srtt = 0; |
584 | t->rttvar = 0; | 585 | t->rttvar = 0; |