netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
@ 2013-02-21 16:45 Roberts, Lee A.
  2013-02-21 20:25 ` Vlad Yasevich
  0 siblings, 1 reply; 7+ messages in thread
From: Roberts, Lee A. @ 2013-02-21 16:45 UTC (permalink / raw)
  To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

From: Lee A. Roberts <lee.roberts@hp.com>

Resolve SCTP association hangs observed during SCTP stress
testing.  Observable symptoms include communications hangs
with data being held in the association reassembly and/or lobby
(ordering) queues.  Close examination of reassembly queue shows
missing packets.

In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
correct message reassembly logic for SCTP partial delivery.
Change logic to ensure that as much data as possible is sent
with the initial partial delivery and that following partial
deliveries contain all available data.

In sctp_ulpq_partial_delivery(), attempt partial delivery only
if the data on the head of the reassembly queue is at or before
the cumulative TSN ACK point.

In sctp_ulpq_renege(), adjust logic to enter partial delivery
only if the incoming chunk remains on the reassembly queue
after processing by sctp_ulpq_tail_data().  If the incoming
chunk has been delivered and data remains on the reassembly
queue, attempt to drain the queue.  Remove call to
sctp_tsnmap_mark(), as this is handled correctly in call to
sctp_ulpq_tail_data().

Patch applies to linux-3.8 kernel.

Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
 net/sctp/ulpqueue.c |   53 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 8 deletions(-)

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+3/net/sctp/ulpqueue.c linux-3.8-SCTP+4/net/sctp/ulpqueue.c
--- linux-3.8-SCTP+3/net/sctp/ulpqueue.c	2013-02-21 07:55:32.817713326 -0700
+++ linux-3.8-SCTP+4/net/sctp/ulpqueue.c	2013-02-21 08:07:41.562212475 -0700
@@ -540,14 +540,19 @@ static struct sctp_ulpevent *sctp_ulpq_r
 		ctsn = cevent->tsn;
 
 		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
+		case SCTP_DATA_FIRST_FRAG:
+			if (!first_frag)
+				return NULL;
+			goto done;
 		case SCTP_DATA_MIDDLE_FRAG:
 			if (!first_frag) {
 				first_frag = pos;
 				next_tsn = ctsn + 1;
 				last_frag = pos;
-			} else if (next_tsn == ctsn)
+			} else if (next_tsn == ctsn) {
 				next_tsn++;
-			else
+				last_frag = pos;
+			} else
 				goto done;
 			break;
 		case SCTP_DATA_LAST_FRAG:
@@ -651,6 +656,14 @@ static struct sctp_ulpevent *sctp_ulpq_r
 			} else
 				goto done;
 			break;
+
+		case SCTP_DATA_LAST_FRAG:
+			if (!first_frag)
+				return NULL;
+			else
+				goto done;
+			break;
+
 		default:
 			return NULL;
 		}
@@ -1025,16 +1038,28 @@ void sctp_ulpq_partial_delivery(struct s
 	struct sctp_ulpevent *event;
 	struct sctp_association *asoc;
 	struct sctp_sock *sp;
+	__u32 ctsn;
+	struct sk_buff *skb;
 
 	asoc = ulpq->asoc;
 	sp = sctp_sk(asoc->base.sk);
 
 	/* If the association is already in Partial Delivery mode
-	 * we have noting to do.
+	 * we have nothing to do.
 	 */
 	if (ulpq->pd_mode)
 		return;
 
+	/* Data must be at or below the Cumulative TSN ACK Point to
+	 * start partial delivery.
+	 */
+	skb = skb_peek(&asoc->ulpq.reasm);
+	if (skb != NULL) {
+		ctsn = sctp_skb2event(skb)->tsn;
+		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
+			return;
+	}
+
 	/* If the user enabled fragment interleave socket option,
 	 * multiple associations can enter partial delivery.
 	 * Otherwise, we can only enter partial delivery if the
@@ -1057,6 +1082,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *
 		      gfp_t gfp)
 {
 	struct sctp_association *asoc;
+	struct sk_buff *skb;
 	__u16 needed, freed;
 
 	asoc = ulpq->asoc;
@@ -1077,12 +1103,23 @@ void sctp_ulpq_renege(struct sctp_ulpq *
 	}
 	/* If able to free enough room, accept this chunk. */
 	if (chunk && (freed >= needed)) {
-		__u32 tsn;
+		__u32 tsn, ctsn;
 		tsn = ntohl(chunk->subh.data_hdr->tsn);
-		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
-		sctp_ulpq_tail_data(ulpq, chunk, gfp);
-
-		sctp_ulpq_partial_delivery(ulpq, gfp);
+		if (sctp_ulpq_tail_data(ulpq, chunk, gfp) == 0) {
+			/*
+			 * Enter partial delivery if chunk is still on
+			 * reassembly queue; otherwise, drain the queue.
+			 */
+			skb = skb_peek(&ulpq->reasm);
+			if (skb != NULL) {
+				ctsn = sctp_skb2event(skb)->tsn;
+				if (TSN_lte(ctsn, tsn))
+					sctp_ulpq_partial_delivery(ulpq, chunk,
+						gfp);
+				else
+					sctp_ulpq_reasm_drain(ulpq);
+			}
+		}
 	}
 
 	sk_mem_reclaim(asoc->base.sk);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
  2013-02-21 16:45 [PATCH 4/4] sctp: fix association hangs due to partial delivery errors Roberts, Lee A.
@ 2013-02-21 20:25 ` Vlad Yasevich
  0 siblings, 0 replies; 7+ messages in thread
From: Vlad Yasevich @ 2013-02-21 20:25 UTC (permalink / raw)
  To: Roberts, Lee A.
  Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

On 02/21/2013 11:45 AM, Roberts, Lee A. wrote:
> From: Lee A. Roberts <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing.  Observable symptoms include communications hangs
> with data being held in the association reassembly and/or lobby
> (ordering) queues.  Close examination of reassembly queue shows
> missing packets.
>
> In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
> correct message reassembly logic for SCTP partial delivery.
> Change logic to ensure that as much data as possible is sent
> with the initial partial delivery and that following partial
> deliveries contain all available data.
>
> In sctp_ulpq_partial_delivery(), attempt partial delivery only
> if the data on the head of the reassembly queue is at or before
> the cumulative TSN ACK point.
>
> In sctp_ulpq_renege(), adjust logic to enter partial delivery
> only if the incoming chunk remains on the reassembly queue
> after processing by sctp_ulpq_tail_data().  If the incoming
> chunk has been delivered and data remains on the reassembly
> queue, attempt to drain the queue.  Remove call to
> sctp_tsnmap_mark(), as this is handled correctly in call to
> sctp_ulpq_tail_data().
>
> Patch applies to linux-3.8 kernel.
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>

Ok, we still have the weirdness of when partial deliver is started and 
that can be another patch.  This looks better.  I am still not crazy 
about all the checking of the TSNs at the head of the reassembly queue, 
but this is better then before.

I really might be better to re-work the the return value from 
sctp_ulpq_tail_data().  If we can return 1 if EOR was set, we can remove 
the TSN check in sctp_ulpq_renege().  It would convert that code to 
something like

retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
if (retval <= 0)
	sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
else if (retval > 0)
	sctp_ulpq_reasm_drain(ulpq);

Just a suggestion.

-vlad

> ---
>   net/sctp/ulpqueue.c |   53 +++++++++++++++++++++++++++++++++++-------
>   1 file changed, 45 insertions(+), 8 deletions(-)
>
> diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+3/net/sctp/ulpqueue.c linux-3.8-SCTP+4/net/sctp/ulpqueue.c
> --- linux-3.8-SCTP+3/net/sctp/ulpqueue.c	2013-02-21 07:55:32.817713326 -0700
> +++ linux-3.8-SCTP+4/net/sctp/ulpqueue.c	2013-02-21 08:07:41.562212475 -0700
> @@ -540,14 +540,19 @@ static struct sctp_ulpevent *sctp_ulpq_r
>   		ctsn = cevent->tsn;
>
>   		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
> +		case SCTP_DATA_FIRST_FRAG:
> +			if (!first_frag)
> +				return NULL;
> +			goto done;
>   		case SCTP_DATA_MIDDLE_FRAG:
>   			if (!first_frag) {
>   				first_frag = pos;
>   				next_tsn = ctsn + 1;
>   				last_frag = pos;
> -			} else if (next_tsn == ctsn)
> +			} else if (next_tsn == ctsn) {
>   				next_tsn++;
> -			else
> +				last_frag = pos;
> +			} else
>   				goto done;
>   			break;
>   		case SCTP_DATA_LAST_FRAG:
> @@ -651,6 +656,14 @@ static struct sctp_ulpevent *sctp_ulpq_r
>   			} else
>   				goto done;
>   			break;
> +
> +		case SCTP_DATA_LAST_FRAG:
> +			if (!first_frag)
> +				return NULL;
> +			else
> +				goto done;
> +			break;
> +
>   		default:
>   			return NULL;
>   		}
> @@ -1025,16 +1038,28 @@ void sctp_ulpq_partial_delivery(struct s
>   	struct sctp_ulpevent *event;
>   	struct sctp_association *asoc;
>   	struct sctp_sock *sp;
> +	__u32 ctsn;
> +	struct sk_buff *skb;
>
>   	asoc = ulpq->asoc;
>   	sp = sctp_sk(asoc->base.sk);
>
>   	/* If the association is already in Partial Delivery mode
> -	 * we have noting to do.
> +	 * we have nothing to do.
>   	 */
>   	if (ulpq->pd_mode)
>   		return;
>
> +	/* Data must be at or below the Cumulative TSN ACK Point to
> +	 * start partial delivery.
> +	 */
> +	skb = skb_peek(&asoc->ulpq.reasm);
> +	if (skb != NULL) {
> +		ctsn = sctp_skb2event(skb)->tsn;
> +		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
> +			return;
> +	}
> +
>   	/* If the user enabled fragment interleave socket option,
>   	 * multiple associations can enter partial delivery.
>   	 * Otherwise, we can only enter partial delivery if the
> @@ -1057,6 +1082,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *
>   		      gfp_t gfp)
>   {
>   	struct sctp_association *asoc;
> +	struct sk_buff *skb;
>   	__u16 needed, freed;
>
>   	asoc = ulpq->asoc;
> @@ -1077,12 +1103,23 @@ void sctp_ulpq_renege(struct sctp_ulpq *
>   	}
>   	/* If able to free enough room, accept this chunk. */
>   	if (chunk && (freed >= needed)) {
> -		__u32 tsn;
> +		__u32 tsn, ctsn;
>   		tsn = ntohl(chunk->subh.data_hdr->tsn);
> -		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> -		sctp_ulpq_tail_data(ulpq, chunk, gfp);
> -
> -		sctp_ulpq_partial_delivery(ulpq, gfp);
> +		if (sctp_ulpq_tail_data(ulpq, chunk, gfp) == 0) {
> +			/*
> +			 * Enter partial delivery if chunk is still on
> +			 * reassembly queue; otherwise, drain the queue.
> +			 */
> +			skb = skb_peek(&ulpq->reasm);
> +			if (skb != NULL) {
> +				ctsn = sctp_skb2event(skb)->tsn;
> +				if (TSN_lte(ctsn, tsn))
> +					sctp_ulpq_partial_delivery(ulpq, chunk,
> +						gfp);
> +				else
> +					sctp_ulpq_reasm_drain(ulpq);
> +			}
> +		}
>   	}
>
>   	sk_mem_reclaim(asoc->base.sk);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
  2013-02-26 14:36 [PATCH 0/4] sctp: fix association hangs due to reassembly/ordering logic Lee A. Roberts
@ 2013-02-26 14:36 ` Lee A. Roberts
  2013-02-27  2:34   ` Vlad Yasevich
  2013-02-27 13:53   ` Vlad Yasevich
  0 siblings, 2 replies; 7+ messages in thread
From: Lee A. Roberts @ 2013-02-26 14:36 UTC (permalink / raw)
  To: netdev; +Cc: lee.roberts

From: "Lee A. Roberts" <lee.roberts@hp.com>

Resolve SCTP association hangs observed during SCTP stress
testing.  Observable symptoms include communications hangs
with data being held in the association reassembly and/or lobby
(ordering) queues.  Close examination of reassembly queue shows
missing packets.

In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
a complete event (with MSG_EOR set) was delivered.  A return value
of -ENOMEM continues to indicate an out-of-memory condition was
encountered.

In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
correct message reassembly logic for SCTP partial delivery.
Change logic to ensure that as much data as possible is sent
with the initial partial delivery and that following partial
deliveries contain all available data.

In sctp_ulpq_partial_delivery(), attempt partial delivery only
if the data on the head of the reassembly queue is at or before
the cumulative TSN ACK point.

In sctp_ulpq_renege(), use the modified return values from
sctp_ulpq_tail_data() to choose whether to attempt partial
delivery or to attempt to drain the reassembly queue as a
means to reduce memory pressure.  Remove call to
sctp_tsnmap_mark(), as this is handled correctly in call to
sctp_ulpq_tail_data().

Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
 net/sctp/ulpqueue.c |   54 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index f221fbb..482e3ea 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
 {
 	struct sk_buff_head temp;
 	struct sctp_ulpevent *event;
+	int event_eor = 0;
 
 	/* Create an event from the incoming chunk. */
 	event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
@@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
 	/* Send event to the ULP.  'event' is the sctp_ulpevent for
 	 * very first SKB on the 'temp' list.
 	 */
-	if (event)
+	if (event) {
+		event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
 		sctp_ulpq_tail_event(ulpq, event);
+	}
 
-	return 0;
+	return event_eor;
 }
 
 /* Add a new event for propagation to the ULP.  */
@@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq *ulpq)
 		ctsn = cevent->tsn;
 
 		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
+		case SCTP_DATA_FIRST_FRAG:
+			if (!first_frag)
+				return NULL;
+			goto done;
 		case SCTP_DATA_MIDDLE_FRAG:
 			if (!first_frag) {
 				first_frag = pos;
 				next_tsn = ctsn + 1;
 				last_frag = pos;
-			} else if (next_tsn == ctsn)
+			} else if (next_tsn == ctsn) {
 				next_tsn++;
-			else
+				last_frag = pos;
+			} else
 				goto done;
 			break;
 		case SCTP_DATA_LAST_FRAG:
@@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
 			} else
 				goto done;
 			break;
+
+		case SCTP_DATA_LAST_FRAG:
+			if (!first_frag)
+				return NULL;
+			else
+				goto done;
+			break;
+
 		default:
 			return NULL;
 		}
@@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
 	struct sctp_ulpevent *event;
 	struct sctp_association *asoc;
 	struct sctp_sock *sp;
+	__u32 ctsn;
+	struct sk_buff *skb;
 
 	asoc = ulpq->asoc;
 	sp = sctp_sk(asoc->base.sk);
 
 	/* If the association is already in Partial Delivery mode
-	 * we have noting to do.
+	 * we have nothing to do.
 	 */
 	if (ulpq->pd_mode)
 		return;
 
+	/* Data must be at or below the Cumulative TSN ACK Point to
+	 * start partial delivery.
+	 */
+	skb = skb_peek(&asoc->ulpq.reasm);
+	if (skb != NULL) {
+		ctsn = sctp_skb2event(skb)->tsn;
+		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
+			return;
+	}
+
 	/* If the user enabled fragment interleave socket option,
 	 * multiple associations can enter partial delivery.
 	 * Otherwise, we can only enter partial delivery if the
@@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
 	}
 	/* If able to free enough room, accept this chunk. */
 	if (chunk && (freed >= needed)) {
-		__u32 tsn;
-		tsn = ntohl(chunk->subh.data_hdr->tsn);
-		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
-		sctp_ulpq_tail_data(ulpq, chunk, gfp);
-
-		sctp_ulpq_partial_delivery(ulpq, gfp);
+		int retval;
+		retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
+		/*
+		 * Enter partial delivery if chunk has not been
+		 * delivered; otherwise, drain the reassembly queue.
+		 */
+		if (retval <= 0)
+			sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
+		else if (retval == 1)
+			sctp_ulpq_reasm_drain(ulpq);
 	}
 
 	sk_mem_reclaim(asoc->base.sk);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
  2013-02-26 14:36 ` [PATCH 4/4] sctp: fix association hangs due to partial delivery errors Lee A. Roberts
@ 2013-02-27  2:34   ` Vlad Yasevich
  2013-02-27  4:38     ` Roberts, Lee A.
  2013-02-27 13:53   ` Vlad Yasevich
  1 sibling, 1 reply; 7+ messages in thread
From: Vlad Yasevich @ 2013-02-27  2:34 UTC (permalink / raw)
  To: Lee A. Roberts; +Cc: netdev

On 02/26/2013 09:36 AM, Lee A. Roberts wrote:
> From: "Lee A. Roberts" <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing.  Observable symptoms include communications hangs
> with data being held in the association reassembly and/or lobby
> (ordering) queues.  Close examination of reassembly queue shows
> missing packets.
>
> In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
> a complete event (with MSG_EOR set) was delivered.  A return value
> of -ENOMEM continues to indicate an out-of-memory condition was
> encountered.
>
> In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
> correct message reassembly logic for SCTP partial delivery.
> Change logic to ensure that as much data as possible is sent
> with the initial partial delivery and that following partial
> deliveries contain all available data.
>
> In sctp_ulpq_partial_delivery(), attempt partial delivery only
> if the data on the head of the reassembly queue is at or before
> the cumulative TSN ACK point.
>
> In sctp_ulpq_renege(), use the modified return values from
> sctp_ulpq_tail_data() to choose whether to attempt partial
> delivery or to attempt to drain the reassembly queue as a
> means to reduce memory pressure.  Remove call to
> sctp_tsnmap_mark(), as this is handled correctly in call to
> sctp_ulpq_tail_data().
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> ---
>   net/sctp/ulpqueue.c |   54 ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> index f221fbb..482e3ea 100644
> --- a/net/sctp/ulpqueue.c
> +++ b/net/sctp/ulpqueue.c
> @@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>   {
>   	struct sk_buff_head temp;
>   	struct sctp_ulpevent *event;
> +	int event_eor = 0;
>
>   	/* Create an event from the incoming chunk. */
>   	event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
> @@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>   	/* Send event to the ULP.  'event' is the sctp_ulpevent for
>   	 * very first SKB on the 'temp' list.
>   	 */
> -	if (event)
> +	if (event) {
> +		event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
>   		sctp_ulpq_tail_event(ulpq, event);
> +	}

You can re-use the check just before this one to catch the EOR 
condition.   You already do
	if ((event) && (event->msg_flags & MSG_EOR)){

right after you try to get a reassembled event.

Everything else looks good.
-vlad

>
> -	return 0;
> +	return event_eor;
>   }
>
>   /* Add a new event for propagation to the ULP.  */
> @@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq *ulpq)
>   		ctsn = cevent->tsn;
>
>   		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
> +		case SCTP_DATA_FIRST_FRAG:
> +			if (!first_frag)
> +				return NULL;
> +			goto done;
>   		case SCTP_DATA_MIDDLE_FRAG:
>   			if (!first_frag) {
>   				first_frag = pos;
>   				next_tsn = ctsn + 1;
>   				last_frag = pos;
> -			} else if (next_tsn == ctsn)
> +			} else if (next_tsn == ctsn) {
>   				next_tsn++;
> -			else
> +				last_frag = pos;
> +			} else
>   				goto done;
>   			break;
>   		case SCTP_DATA_LAST_FRAG:
> @@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
>   			} else
>   				goto done;
>   			break;
> +
> +		case SCTP_DATA_LAST_FRAG:
> +			if (!first_frag)
> +				return NULL;
> +			else
> +				goto done;
> +			break;
> +
>   		default:
>   			return NULL;
>   		}
> @@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
>   	struct sctp_ulpevent *event;
>   	struct sctp_association *asoc;
>   	struct sctp_sock *sp;
> +	__u32 ctsn;
> +	struct sk_buff *skb;
>
>   	asoc = ulpq->asoc;
>   	sp = sctp_sk(asoc->base.sk);
>
>   	/* If the association is already in Partial Delivery mode
> -	 * we have noting to do.
> +	 * we have nothing to do.
>   	 */
>   	if (ulpq->pd_mode)
>   		return;
>
> +	/* Data must be at or below the Cumulative TSN ACK Point to
> +	 * start partial delivery.
> +	 */
> +	skb = skb_peek(&asoc->ulpq.reasm);
> +	if (skb != NULL) {
> +		ctsn = sctp_skb2event(skb)->tsn;
> +		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
> +			return;
> +	}
> +
>   	/* If the user enabled fragment interleave socket option,
>   	 * multiple associations can enter partial delivery.
>   	 * Otherwise, we can only enter partial delivery if the
> @@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>   	}
>   	/* If able to free enough room, accept this chunk. */
>   	if (chunk && (freed >= needed)) {
> -		__u32 tsn;
> -		tsn = ntohl(chunk->subh.data_hdr->tsn);
> -		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> -		sctp_ulpq_tail_data(ulpq, chunk, gfp);
> -
> -		sctp_ulpq_partial_delivery(ulpq, gfp);
> +		int retval;
> +		retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
> +		/*
> +		 * Enter partial delivery if chunk has not been
> +		 * delivered; otherwise, drain the reassembly queue.
> +		 */
> +		if (retval <= 0)
> +			sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
> +		else if (retval == 1)
> +			sctp_ulpq_reasm_drain(ulpq);
>   	}
>
>   	sk_mem_reclaim(asoc->base.sk);
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
  2013-02-27  2:34   ` Vlad Yasevich
@ 2013-02-27  4:38     ` Roberts, Lee A.
  2013-02-27 13:51       ` Vlad Yasevich
  0 siblings, 1 reply; 7+ messages in thread
From: Roberts, Lee A. @ 2013-02-27  4:38 UTC (permalink / raw)
  To: vyasevic@redhat.com; +Cc: netdev@vger.kernel.org

Vlad,

> -----Original Message-----
> From: Vlad Yasevich [mailto:vyasevic@redhat.com]
> Sent: Tuesday, February 26, 2013 7:35 PM
> To: Roberts, Lee A.
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
> 
> On 02/26/2013 09:36 AM, Lee A. Roberts wrote:
> > From: "Lee A. Roberts" <lee.roberts@hp.com>
> >
> > Resolve SCTP association hangs observed during SCTP stress
> > testing.  Observable symptoms include communications hangs
> > with data being held in the association reassembly and/or lobby
> > (ordering) queues.  Close examination of reassembly queue shows
> > missing packets.
> >
> > In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
> > a complete event (with MSG_EOR set) was delivered.  A return value
> > of -ENOMEM continues to indicate an out-of-memory condition was
> > encountered.
> >
> > In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
> > correct message reassembly logic for SCTP partial delivery.
> > Change logic to ensure that as much data as possible is sent
> > with the initial partial delivery and that following partial
> > deliveries contain all available data.
> >
> > In sctp_ulpq_partial_delivery(), attempt partial delivery only
> > if the data on the head of the reassembly queue is at or before
> > the cumulative TSN ACK point.
> >
> > In sctp_ulpq_renege(), use the modified return values from
> > sctp_ulpq_tail_data() to choose whether to attempt partial
> > delivery or to attempt to drain the reassembly queue as a
> > means to reduce memory pressure.  Remove call to
> > sctp_tsnmap_mark(), as this is handled correctly in call to
> > sctp_ulpq_tail_data().
> >
> > Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> > ---
> >   net/sctp/ulpqueue.c |   54 ++++++++++++++++++++++++++++++++++++++++-----------
> >   1 file changed, 43 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> > index f221fbb..482e3ea 100644
> > --- a/net/sctp/ulpqueue.c
> > +++ b/net/sctp/ulpqueue.c
> > @@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> >   {
> >   	struct sk_buff_head temp;
> >   	struct sctp_ulpevent *event;
> > +	int event_eor = 0;
> >
> >   	/* Create an event from the incoming chunk. */
> >   	event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
> > @@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> >   	/* Send event to the ULP.  'event' is the sctp_ulpevent for
> >   	 * very first SKB on the 'temp' list.
> >   	 */
> > -	if (event)
> > +	if (event) {
> > +		event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
> >   		sctp_ulpq_tail_event(ulpq, event);
> > +	}
> 
> You can re-use the check just before this one to catch the EOR
> condition.   You already do
> 	if ((event) && (event->msg_flags & MSG_EOR)){
> 
> right after you try to get a reassembled event.
> 
> Everything else looks good.
> -vlad
> 
It depends on what we want the return value to mean.  In the case where we've reneged
to be able to put in a packet, we know that the incoming packet is the next one we're
looking for.  In which case, the earlier test is correct, since this will be the next
event.

In general, the event that's being passed to sctp_ulpq_order() might not be the
next event.  In that case, sctp_ulpq_order() will call sctp_ulpq_store_ordered()
and return NULL.  Only at the last "if (event)" can we really be sure that an event
is being passed to the ULP.

In general, do we want to return 1 if a complete event is actually passed to the ULP?
Or, do we want to return 1 if we've seen a complete event come from reassembly?

                                                    - Lee

> >
> > -	return 0;
> > +	return event_eor;
> >   }
> >
> >   /* Add a new event for propagation to the ULP.  */
> > @@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq
> *ulpq)
> >   		ctsn = cevent->tsn;
> >
> >   		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
> > +		case SCTP_DATA_FIRST_FRAG:
> > +			if (!first_frag)
> > +				return NULL;
> > +			goto done;
> >   		case SCTP_DATA_MIDDLE_FRAG:
> >   			if (!first_frag) {
> >   				first_frag = pos;
> >   				next_tsn = ctsn + 1;
> >   				last_frag = pos;
> > -			} else if (next_tsn == ctsn)
> > +			} else if (next_tsn == ctsn) {
> >   				next_tsn++;
> > -			else
> > +				last_frag = pos;
> > +			} else
> >   				goto done;
> >   			break;
> >   		case SCTP_DATA_LAST_FRAG:
> > @@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
> >   			} else
> >   				goto done;
> >   			break;
> > +
> > +		case SCTP_DATA_LAST_FRAG:
> > +			if (!first_frag)
> > +				return NULL;
> > +			else
> > +				goto done;
> > +			break;
> > +
> >   		default:
> >   			return NULL;
> >   		}
> > @@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
> >   	struct sctp_ulpevent *event;
> >   	struct sctp_association *asoc;
> >   	struct sctp_sock *sp;
> > +	__u32 ctsn;
> > +	struct sk_buff *skb;
> >
> >   	asoc = ulpq->asoc;
> >   	sp = sctp_sk(asoc->base.sk);
> >
> >   	/* If the association is already in Partial Delivery mode
> > -	 * we have noting to do.
> > +	 * we have nothing to do.
> >   	 */
> >   	if (ulpq->pd_mode)
> >   		return;
> >
> > +	/* Data must be at or below the Cumulative TSN ACK Point to
> > +	 * start partial delivery.
> > +	 */
> > +	skb = skb_peek(&asoc->ulpq.reasm);
> > +	if (skb != NULL) {
> > +		ctsn = sctp_skb2event(skb)->tsn;
> > +		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
> > +			return;
> > +	}
> > +
> >   	/* If the user enabled fragment interleave socket option,
> >   	 * multiple associations can enter partial delivery.
> >   	 * Otherwise, we can only enter partial delivery if the
> > @@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> >   	}
> >   	/* If able to free enough room, accept this chunk. */
> >   	if (chunk && (freed >= needed)) {
> > -		__u32 tsn;
> > -		tsn = ntohl(chunk->subh.data_hdr->tsn);
> > -		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> > -		sctp_ulpq_tail_data(ulpq, chunk, gfp);
> > -
> > -		sctp_ulpq_partial_delivery(ulpq, gfp);
> > +		int retval;
> > +		retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
> > +		/*
> > +		 * Enter partial delivery if chunk has not been
> > +		 * delivered; otherwise, drain the reassembly queue.
> > +		 */
> > +		if (retval <= 0)
> > +			sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
> > +		else if (retval == 1)
> > +			sctp_ulpq_reasm_drain(ulpq);
> >   	}
> >
> >   	sk_mem_reclaim(asoc->base.sk);
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
  2013-02-27  4:38     ` Roberts, Lee A.
@ 2013-02-27 13:51       ` Vlad Yasevich
  0 siblings, 0 replies; 7+ messages in thread
From: Vlad Yasevich @ 2013-02-27 13:51 UTC (permalink / raw)
  To: Roberts, Lee A.; +Cc: netdev@vger.kernel.org

On 02/26/2013 11:38 PM, Roberts, Lee A. wrote:
> Vlad,
>
>> -----Original Message-----
>> From: Vlad Yasevich [mailto:vyasevic@redhat.com]
>> Sent: Tuesday, February 26, 2013 7:35 PM
>> To: Roberts, Lee A.
>> Cc: netdev@vger.kernel.org
>> Subject: Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
>>
>> On 02/26/2013 09:36 AM, Lee A. Roberts wrote:
>>> From: "Lee A. Roberts" <lee.roberts@hp.com>
>>>
>>> Resolve SCTP association hangs observed during SCTP stress
>>> testing.  Observable symptoms include communications hangs
>>> with data being held in the association reassembly and/or lobby
>>> (ordering) queues.  Close examination of reassembly queue shows
>>> missing packets.
>>>
>>> In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
>>> a complete event (with MSG_EOR set) was delivered.  A return value
>>> of -ENOMEM continues to indicate an out-of-memory condition was
>>> encountered.
>>>
>>> In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
>>> correct message reassembly logic for SCTP partial delivery.
>>> Change logic to ensure that as much data as possible is sent
>>> with the initial partial delivery and that following partial
>>> deliveries contain all available data.
>>>
>>> In sctp_ulpq_partial_delivery(), attempt partial delivery only
>>> if the data on the head of the reassembly queue is at or before
>>> the cumulative TSN ACK point.
>>>
>>> In sctp_ulpq_renege(), use the modified return values from
>>> sctp_ulpq_tail_data() to choose whether to attempt partial
>>> delivery or to attempt to drain the reassembly queue as a
>>> means to reduce memory pressure.  Remove call to
>>> sctp_tsnmap_mark(), as this is handled correctly in call to
>>> sctp_ulpq_tail_data().
>>>
>>> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
>>> ---
>>>    net/sctp/ulpqueue.c |   54 ++++++++++++++++++++++++++++++++++++++++-----------
>>>    1 file changed, 43 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
>>> index f221fbb..482e3ea 100644
>>> --- a/net/sctp/ulpqueue.c
>>> +++ b/net/sctp/ulpqueue.c
>>> @@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>>>    {
>>>    	struct sk_buff_head temp;
>>>    	struct sctp_ulpevent *event;
>>> +	int event_eor = 0;
>>>
>>>    	/* Create an event from the incoming chunk. */
>>>    	event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
>>> @@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>>>    	/* Send event to the ULP.  'event' is the sctp_ulpevent for
>>>    	 * very first SKB on the 'temp' list.
>>>    	 */
>>> -	if (event)
>>> +	if (event) {
>>> +		event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
>>>    		sctp_ulpq_tail_event(ulpq, event);
>>> +	}
>>
>> You can re-use the check just before this one to catch the EOR
>> condition.   You already do
>> 	if ((event) && (event->msg_flags & MSG_EOR)){
>>
>> right after you try to get a reassembled event.
>>
>> Everything else looks good.
>> -vlad
>>
> It depends on what we want the return value to mean.  In the case where we've reneged
> to be able to put in a packet, we know that the incoming packet is the next one we're
> looking for.  In which case, the earlier test is correct, since this will be the next
> event.
>
> In general, the event that's being passed to sctp_ulpq_order() might not be the
> next event.  In that case, sctp_ulpq_order() will call sctp_ulpq_store_ordered()
> and return NULL.  Only at the last "if (event)" can we really be sure that an event
> is being passed to the ULP.
>
> In general, do we want to return 1 if a complete event is actually passed to the ULP?
> Or, do we want to return 1 if we've seen a complete event come from reassembly?

You are right.  We want it set based on deliver to ULP.  There is a 
probability (if PARTIAL_DELIVERY_POINT is set) that we might get
a partial message here without EOR and skip ordering.  So, your check
is correct.

-vlad
>
>                                                      - Lee
>
>>>
>>> -	return 0;
>>> +	return event_eor;
>>>    }
>>>
>>>    /* Add a new event for propagation to the ULP.  */
>>> @@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq
>> *ulpq)
>>>    		ctsn = cevent->tsn;
>>>
>>>    		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
>>> +		case SCTP_DATA_FIRST_FRAG:
>>> +			if (!first_frag)
>>> +				return NULL;
>>> +			goto done;
>>>    		case SCTP_DATA_MIDDLE_FRAG:
>>>    			if (!first_frag) {
>>>    				first_frag = pos;
>>>    				next_tsn = ctsn + 1;
>>>    				last_frag = pos;
>>> -			} else if (next_tsn == ctsn)
>>> +			} else if (next_tsn == ctsn) {
>>>    				next_tsn++;
>>> -			else
>>> +				last_frag = pos;
>>> +			} else
>>>    				goto done;
>>>    			break;
>>>    		case SCTP_DATA_LAST_FRAG:
>>> @@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
>>>    			} else
>>>    				goto done;
>>>    			break;
>>> +
>>> +		case SCTP_DATA_LAST_FRAG:
>>> +			if (!first_frag)
>>> +				return NULL;
>>> +			else
>>> +				goto done;
>>> +			break;
>>> +
>>>    		default:
>>>    			return NULL;
>>>    		}
>>> @@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
>>>    	struct sctp_ulpevent *event;
>>>    	struct sctp_association *asoc;
>>>    	struct sctp_sock *sp;
>>> +	__u32 ctsn;
>>> +	struct sk_buff *skb;
>>>
>>>    	asoc = ulpq->asoc;
>>>    	sp = sctp_sk(asoc->base.sk);
>>>
>>>    	/* If the association is already in Partial Delivery mode
>>> -	 * we have noting to do.
>>> +	 * we have nothing to do.
>>>    	 */
>>>    	if (ulpq->pd_mode)
>>>    		return;
>>>
>>> +	/* Data must be at or below the Cumulative TSN ACK Point to
>>> +	 * start partial delivery.
>>> +	 */
>>> +	skb = skb_peek(&asoc->ulpq.reasm);
>>> +	if (skb != NULL) {
>>> +		ctsn = sctp_skb2event(skb)->tsn;
>>> +		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
>>> +			return;
>>> +	}
>>> +
>>>    	/* If the user enabled fragment interleave socket option,
>>>    	 * multiple associations can enter partial delivery.
>>>    	 * Otherwise, we can only enter partial delivery if the
>>> @@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>>>    	}
>>>    	/* If able to free enough room, accept this chunk. */
>>>    	if (chunk && (freed >= needed)) {
>>> -		__u32 tsn;
>>> -		tsn = ntohl(chunk->subh.data_hdr->tsn);
>>> -		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
>>> -		sctp_ulpq_tail_data(ulpq, chunk, gfp);
>>> -
>>> -		sctp_ulpq_partial_delivery(ulpq, gfp);
>>> +		int retval;
>>> +		retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
>>> +		/*
>>> +		 * Enter partial delivery if chunk has not been
>>> +		 * delivered; otherwise, drain the reassembly queue.
>>> +		 */
>>> +		if (retval <= 0)
>>> +			sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
>>> +		else if (retval == 1)
>>> +			sctp_ulpq_reasm_drain(ulpq);
>>>    	}
>>>
>>>    	sk_mem_reclaim(asoc->base.sk);
>>>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
  2013-02-26 14:36 ` [PATCH 4/4] sctp: fix association hangs due to partial delivery errors Lee A. Roberts
  2013-02-27  2:34   ` Vlad Yasevich
@ 2013-02-27 13:53   ` Vlad Yasevich
  1 sibling, 0 replies; 7+ messages in thread
From: Vlad Yasevich @ 2013-02-27 13:53 UTC (permalink / raw)
  To: Lee A. Roberts; +Cc: netdev

On 02/26/2013 09:36 AM, Lee A. Roberts wrote:
> From: "Lee A. Roberts" <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing.  Observable symptoms include communications hangs
> with data being held in the association reassembly and/or lobby
> (ordering) queues.  Close examination of reassembly queue shows
> missing packets.
>
> In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
> a complete event (with MSG_EOR set) was delivered.  A return value
> of -ENOMEM continues to indicate an out-of-memory condition was
> encountered.
>
> In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
> correct message reassembly logic for SCTP partial delivery.
> Change logic to ensure that as much data as possible is sent
> with the initial partial delivery and that following partial
> deliveries contain all available data.
>
> In sctp_ulpq_partial_delivery(), attempt partial delivery only
> if the data on the head of the reassembly queue is at or before
> the cumulative TSN ACK point.
>
> In sctp_ulpq_renege(), use the modified return values from
> sctp_ulpq_tail_data() to choose whether to attempt partial
> delivery or to attempt to drain the reassembly queue as a
> means to reduce memory pressure.  Remove call to
> sctp_tsnmap_mark(), as this is handled correctly in call to
> sctp_ulpq_tail_data().
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

-vlad

> ---
>   net/sctp/ulpqueue.c |   54 ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> index f221fbb..482e3ea 100644
> --- a/net/sctp/ulpqueue.c
> +++ b/net/sctp/ulpqueue.c
> @@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>   {
>   	struct sk_buff_head temp;
>   	struct sctp_ulpevent *event;
> +	int event_eor = 0;
>
>   	/* Create an event from the incoming chunk. */
>   	event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
> @@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>   	/* Send event to the ULP.  'event' is the sctp_ulpevent for
>   	 * very first SKB on the 'temp' list.
>   	 */
> -	if (event)
> +	if (event) {
> +		event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
>   		sctp_ulpq_tail_event(ulpq, event);
> +	}
>
> -	return 0;
> +	return event_eor;
>   }
>
>   /* Add a new event for propagation to the ULP.  */
> @@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq *ulpq)
>   		ctsn = cevent->tsn;
>
>   		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
> +		case SCTP_DATA_FIRST_FRAG:
> +			if (!first_frag)
> +				return NULL;
> +			goto done;
>   		case SCTP_DATA_MIDDLE_FRAG:
>   			if (!first_frag) {
>   				first_frag = pos;
>   				next_tsn = ctsn + 1;
>   				last_frag = pos;
> -			} else if (next_tsn == ctsn)
> +			} else if (next_tsn == ctsn) {
>   				next_tsn++;
> -			else
> +				last_frag = pos;
> +			} else
>   				goto done;
>   			break;
>   		case SCTP_DATA_LAST_FRAG:
> @@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
>   			} else
>   				goto done;
>   			break;
> +
> +		case SCTP_DATA_LAST_FRAG:
> +			if (!first_frag)
> +				return NULL;
> +			else
> +				goto done;
> +			break;
> +
>   		default:
>   			return NULL;
>   		}
> @@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
>   	struct sctp_ulpevent *event;
>   	struct sctp_association *asoc;
>   	struct sctp_sock *sp;
> +	__u32 ctsn;
> +	struct sk_buff *skb;
>
>   	asoc = ulpq->asoc;
>   	sp = sctp_sk(asoc->base.sk);
>
>   	/* If the association is already in Partial Delivery mode
> -	 * we have noting to do.
> +	 * we have nothing to do.
>   	 */
>   	if (ulpq->pd_mode)
>   		return;
>
> +	/* Data must be at or below the Cumulative TSN ACK Point to
> +	 * start partial delivery.
> +	 */
> +	skb = skb_peek(&asoc->ulpq.reasm);
> +	if (skb != NULL) {
> +		ctsn = sctp_skb2event(skb)->tsn;
> +		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
> +			return;
> +	}
> +
>   	/* If the user enabled fragment interleave socket option,
>   	 * multiple associations can enter partial delivery.
>   	 * Otherwise, we can only enter partial delivery if the
> @@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
>   	}
>   	/* If able to free enough room, accept this chunk. */
>   	if (chunk && (freed >= needed)) {
> -		__u32 tsn;
> -		tsn = ntohl(chunk->subh.data_hdr->tsn);
> -		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> -		sctp_ulpq_tail_data(ulpq, chunk, gfp);
> -
> -		sctp_ulpq_partial_delivery(ulpq, gfp);
> +		int retval;
> +		retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
> +		/*
> +		 * Enter partial delivery if chunk has not been
> +		 * delivered; otherwise, drain the reassembly queue.
> +		 */
> +		if (retval <= 0)
> +			sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
> +		else if (retval == 1)
> +			sctp_ulpq_reasm_drain(ulpq);
>   	}
>
>   	sk_mem_reclaim(asoc->base.sk);
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-02-27 13:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-21 16:45 [PATCH 4/4] sctp: fix association hangs due to partial delivery errors Roberts, Lee A.
2013-02-21 20:25 ` Vlad Yasevich
  -- strict thread matches above, loose matches on Subject: below --
2013-02-26 14:36 [PATCH 0/4] sctp: fix association hangs due to reassembly/ordering logic Lee A. Roberts
2013-02-26 14:36 ` [PATCH 4/4] sctp: fix association hangs due to partial delivery errors Lee A. Roberts
2013-02-27  2:34   ` Vlad Yasevich
2013-02-27  4:38     ` Roberts, Lee A.
2013-02-27 13:51       ` Vlad Yasevich
2013-02-27 13:53   ` Vlad Yasevich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).