From patchwork Wed Jun 29 13:57:04 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Graf X-Patchwork-Id: 102606 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 60220B6F59 for ; Wed, 29 Jun 2011 23:57:15 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754883Ab1F2N5J (ORCPT ); Wed, 29 Jun 2011 09:57:09 -0400 Received: from merlin.infradead.org ([205.233.59.134]:57772 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752182Ab1F2N5H (ORCPT ); Wed, 29 Jun 2011 09:57:07 -0400 Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QbvGL-0001Vw-2O; Wed, 29 Jun 2011 13:57:05 +0000 Received: from tgr by canuck.infradead.org with local (Exim 4.76 #1 (Red Hat Linux)) id 1QbvGK-0007DC-P3; Wed, 29 Jun 2011 13:57:04 +0000 Date: Wed, 29 Jun 2011 09:57:04 -0400 From: Thomas Graf To: netdev@vger.kernel.org Cc: davem@davemloft.net, Wei Yongjun , Vlad Yasevich , Sridhar Samudrala , linux-sctp@vger.kernel.org Subject: [PATCH] sctp: Enforce maximum retransmissions during shutdown Message-ID: <20110629135704.GB10085@canuck.infradead.org> Mail-Followup-To: netdev@vger.kernel.org, davem@davemloft.net, Wei Yongjun , Vlad Yasevich , Sridhar Samudrala , linux-sctp@vger.kernel.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-08-17) X-SRS-Rewrite: SMTP reverse-path rewritten from by canuck.infradead.org See http://www.infradead.org/rpr.html Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When initiating a graceful shutdown while having data chunks on the retransmission queue with a peer which is in zero window mode the shutdown is never completed because the retransmission error count is reset periodically by the following two rules: - Do not timeout association while doing zero window probe. - Reset overall error count when a heartbeat request has been acknowledged. The graceful shutdown will wait for all outstanding TSN to be acknowledged before sending the SHUTDOWN request. This never happens due to the peer's zero window not acknowledging the continuously retransmitted data chunks. Although the error counter is incremented for each failed retransmission done via the T3-rtx timer, the receiving of the SACK sent in return to the retransmission, announcing the zero window, clears the error count again immediately. Also heartbeat requests continue to be sent periodically. The peer acknowledges these requests causing the error counter to be reset as well. This patch changes behaviour to only reset the overall error counter for the above rules while not in shutdown. This means that if already queued data can't be transmitted in max_retrans attempts we ABORT because a graceful shutdown is obviously not possible. The issue can be easily reproduced by establishing a sctp association over the loopback device, constantly queueing data at the sender while not reading any at the receiver. Wait for the window to reach zero, then initiate a shutdown by killing both processes simultaneously. The association will never be freed and the chunks on the retransmission queue will be retransmitted indefinitely. Signed-off-by: Thomas Graf --- 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 diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c index 1c88c89..14a5295 100644 --- a/net/sctp/outqueue.c +++ b/net/sctp/outqueue.c @@ -1629,10 +1629,15 @@ static void sctp_check_transmitted(struct sctp_outq *q, * A sender is doing zero window probing when the * receiver's advertised window is zero, and there is * only one data chunk in flight to the receiver. + * + * Allow the association to timeout if SHUTDOWN is + * pending. We have no interest in keeping the + * association around forever. */ if (!q->asoc->peer.rwnd && !list_empty(&tlist) && - (sack_ctsn+2 == q->asoc->next_tsn)) { + (sack_ctsn+2 == q->asoc->next_tsn) && + !(q->asoc->state >= SCTP_STATE_SHUTDOWN_PENDING)) { SCTP_DEBUG_PRINTK("%s: SACK received for zero " "window probe: %u\n", __func__, sack_ctsn); diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c index 534c2e5..fa92f4d6 100644 --- a/net/sctp/sm_sideeffect.c +++ b/net/sctp/sm_sideeffect.c @@ -670,10 +670,21 @@ static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds, /* 8.3 Upon the receipt of the HEARTBEAT ACK, the sender of the * HEARTBEAT should clear the error counter of the destination * transport address to which the HEARTBEAT was sent. - * The association's overall error count is also cleared. */ t->error_count = 0; - t->asoc->overall_error_count = 0; + + /* + * Although RFC2960 and RFC4460 specify that the overall error + * count must be cleared when a HEARTBEAT ACK is received this + * behaviour may prevent the maximum retransmission count from + * being reached while in SHUTDOWN. If the peer keeps its window + * closed not acknowledging any outstanding TSN we may rely on + * reaching the max_retrans limit via the T3-rtx timer to close + * the association which will never happen if the error count is + * reset every heartbeat interval. + */ + if (!(t->asoc->state >= SCTP_STATE_SHUTDOWN_PENDING)) + t->asoc->overall_error_count = 0; /* Clear the hb_sent flag to signal that we had a good * acknowledgement.