diff mbox

[1/2] sctp: HEARTBEAT negotiation after ASCONF

Message ID 9C3219F3-AD39-40CF-8422-241D26ECA4D7@sfc.wide.ad.jp
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Michio Honda June 16, 2011, 8:56 a.m. UTC 
From 6780e27850126dc200a8700a0f8c3105971b8978 Mon Sep 17 00:00:00 2001
From: Michio Honda <micchie@sfc.wide.ad.jp>
Date: Thu, 16 Jun 2011 10:54:23 +0900
Subject: [PATCH 1/2] sctp: HEARTBEAT negotiation after ASCONF

This patch fixes BUG that the ASCONF receiver transmits DATA chunks
to the newly added UNCONFIRMED destination.

Signed-off-by: Michio Honda <micchie@sfc.wide.ad.jp>
---
 net/sctp/outqueue.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

David Miller June 19, 2011, 11:10 p.m. UTC | #1 
From: Michio Honda <micchie@sfc.wide.ad.jp>
Date: Thu, 16 Jun 2011 17:56:42 +0900

> @@ -989,6 +991,8 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>  			    ((new_transport->state == SCTP_INACTIVE) ||
>  			     (new_transport->state == SCTP_UNCONFIRMED)))
>  				new_transport = asoc->peer.active_path;
> +			if (new_transport->state == SCTP_UNCONFIRMED) 
> +				continue;
>  
>  			/* Change packets if necessary.  */
>  			if (new_transport != transport) {

This code path uses the asoc->peer.active_path is the new_transport
is in the SCTP_UNCONFIRMED state.  Are you sure your new check is
correct in that case?

Shouldn't you be testing the variable "new_transport" instead?

--
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
Michio Honda June 20, 2011, 1:17 a.m. UTC | #2 
Yes, this code covers such case and ensures chunks never transmitted to UNCONFIRMED destination.  

Thanks,
- Michio 

On Jun 20, 2011, at 8:10 , David Miller wrote:

> From: Michio Honda <micchie@sfc.wide.ad.jp>
> Date: Thu, 16 Jun 2011 17:56:42 +0900
> 
>> @@ -989,6 +991,8 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
>> 			    ((new_transport->state == SCTP_INACTIVE) ||
>> 			     (new_transport->state == SCTP_UNCONFIRMED)))
>> 				new_transport = asoc->peer.active_path;
>> +			if (new_transport->state == SCTP_UNCONFIRMED) 
>> +				continue;
>> 
>> 			/* Change packets if necessary.  */
>> 			if (new_transport != transport) {
> 
> This code path uses the asoc->peer.active_path is the new_transport
> is in the SCTP_UNCONFIRMED state.  Are you sure your new check is
> correct in that case?
> 
> Shouldn't you be testing the variable "new_transport" instead?
> 
> --
> 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

--
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 mbox

Patch

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index edc7532..4adfc1e 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -917,6 +917,8 @@  static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
 		 * current cwnd).
 		 */
 		if (!list_empty(&q->retransmit)) {
+			if (asoc->peer.retran_path->state == SCTP_UNCONFIRMED) 
+				goto sctp_flush_out;
 			if (transport == asoc->peer.retran_path)
 				goto retran;
 
@@ -989,6 +991,8 @@  static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)
 			    ((new_transport->state == SCTP_INACTIVE) ||
 			     (new_transport->state == SCTP_UNCONFIRMED)))
 				new_transport = asoc->peer.active_path;
+			if (new_transport->state == SCTP_UNCONFIRMED) 
+				continue;
 
 			/* Change packets if necessary.  */
 			if (new_transport != transport) {