[PATCH 2/3] sctp: fix association hangs due to reassembly/ordering logic

[PATCH 2/3] sctp: fix association hangs due to reassembly/ordering logic

[Roberts, Lee A.]

RnJvbTogTGVlIEEuIFJvYmVydHMgPGxlZS5yb2JlcnRzQGhwLmNvbT4NCg0KUmVzb2x2ZSBTQ1RQ
IGFzc29jaWF0aW9uIGhhbmdzIG9ic2VydmVkIGR1cmluZyBTQ1RQIHN0cmVzcw0KdGVzdGluZy4g
IE9ic2VydmFibGUgc3ltcHRvbXMgaW5jbHVkZSBjb21tdW5pY2F0aW9ucyBoYW5ncw0Kd2l0aCBk
YXRhIGJlaW5nIGhlbGQgaW4gdGhlIGFzc29jaWF0aW9uIHJlYXNzZW1ibHkgYW5kL29yIGxvYmJ5
DQoob3JkZXJpbmcpIHF1ZXVlcy4gIENsb3NlIGV4YW1pbmF0aW9uIG9mIHJlYXNzZW1ibHkgcXVl
dWUgc2hvd3MNCm1pc3NpbmcgcGFja2V0cy4NCg0KSW4gc2N0cF91bHBxX3JlbmVnZV9saXN0KCks
IGRvIG5vdCByZW5lZ2UgcGFja2V0cyBiZWxvdyB0aGUNCmN1bXVsYXRpdmUgVFNOIEFDSyBwb2lu
dC4gIEV2ZW50cyBiZWluZyByZW5lZ2VkIGZyb20gdGhlDQpvcmRlcmluZyBxdWV1ZSBtYXkgY29y
cmVzcG9uZCB0byBtdWx0aXBsZSBUU05zOyBpZGVudGlmeQ0KYW5kIHJlbmVnZSBhbGwgYWZmZWN0
ZWQgcGFja2V0cyBmcm9tIHRoZSB0c25tYXAuDQoNClBhdGNoIGFwcGxpZXMgdG8gbGludXgtMy44
IGtlcm5lbC4NCg0KU2lnbmVkLW9mZi1ieTogTGVlIEEuIFJvYmVydHMgPGxlZS5yb2JlcnRzQGhw
LmNvbT4NCi0tLQ0KIG5ldC9zY3RwL3VscHF1ZXVlLmMgfCAgIDMwICsrKysrKysrKysrKysrKysr
KysrKysrKystLS0tLQ0KIDEgZmlsZSBjaGFuZ2VkLCAyNSBpbnNlcnRpb25zKCspLCA1IGRlbGV0
aW9ucygtKQ0KDQpkaWZmIC11cHJOIC1YIGxpbnV4LTMuOC12YW5pbGxhL0RvY3VtZW50YXRpb24v
ZG9udGRpZmYgbGludXgtMy44LVNDVFANCisxL25ldC9zY3RwL3VscHF1ZXVlLmMgbGludXgtMy44
LVNDVFArMi9uZXQvc2N0cC91bHBxdWV1ZS5jDQotLS0gbGludXgtMy44LVNDVFArMS9uZXQvc2N0
cC91bHBxdWV1ZS5jCTIwMTMtMDItMTggMTY6NTg6MzQuMDAwMDAwMDAwDQotMDcwMA0KKysrIGxp
bnV4LTMuOC1TQ1RQKzIvbmV0L3NjdHAvdWxwcXVldWUuYwkyMDEzLTAyLTIwIDA4OjE3OjUzLjY3
OTIzMzM2NQ0KLTA3MDANCkBAIC05NjIsMjAgKzk2Miw0MCBAQCBzdGF0aWMgX191MTYgc2N0cF91
bHBxX3JlbmVnZV9saXN0KHN0cnVjDQogCQlzdHJ1Y3Qgc2tfYnVmZl9oZWFkICpsaXN0LCBfX3Ux
NiBuZWVkZWQpDQogew0KIAlfX3UxNiBmcmVlZCA9IDA7DQotCV9fdTMyIHRzbjsNCi0Jc3RydWN0
IHNrX2J1ZmYgKnNrYjsNCisJX191MzIgdHNuLCBsYXN0X3RzbjsNCisJc3RydWN0IHNrX2J1ZmYg
KnNrYiwgKmZsaXN0LCAqbGFzdDsNCiAJc3RydWN0IHNjdHBfdWxwZXZlbnQgKmV2ZW50Ow0KIAlz
dHJ1Y3Qgc2N0cF90c25tYXAgKnRzbm1hcDsNCiANCiAJdHNubWFwID0gJnVscHEtPmFzb2MtPnBl
ZXIudHNuX21hcDsNCiANCi0Jd2hpbGUgKChza2IgPSBfX3NrYl9kZXF1ZXVlX3RhaWwobGlzdCkp
ICE9IE5VTEwpIHsNCi0JCWZyZWVkICs9IHNrYl9oZWFkbGVuKHNrYik7DQorCXdoaWxlICgoc2ti
ID0gc2tiX3BlZWtfdGFpbChsaXN0KSkgIT0gTlVMTCkgew0KIAkJZXZlbnQgPSBzY3RwX3NrYjJl
dmVudChza2IpOw0KIAkJdHNuID0gZXZlbnQtPnRzbjsNCiANCisJCS8qIERvbid0IHJlbmVnZSBi
ZWxvdyB0aGUgQ3VtdWxhdGl2ZSBUU04gQUNLIFBvaW50LiAqLw0KKwkJaWYgKFRTTl9sdGUodHNu
LCBzY3RwX3Rzbm1hcF9nZXRfY3Rzbih0c25tYXApKSkNCisJCQlicmVhazsNCisNCisJCS8qIEV2
ZW50cyBpbiBvcmRlcmluZyBxdWV1ZSBtYXkgaGF2ZSBtdWx0aXBsZSBmcmFnbWVudHMNCisJCSAq
IGNvcnJlc3BvbmRpbmcgdG8gYWRkaXRpb25hbCBUU05zLiAgRmluZCB0aGUgbGFzdCBvbmUuDQor
CQkgKi8NCisJCWZsaXN0ID0gc2tiX3NoaW5mbyhza2IpLT5mcmFnX2xpc3Q7DQorCQlmb3IgKGxh
c3QgPSBmbGlzdDsgZmxpc3Q7IGZsaXN0ID0gZmxpc3QtPm5leHQpDQorCQkJbGFzdCA9IGZsaXN0
Ow0KKwkJaWYgKGxhc3QpDQorCQkJbGFzdF90c24gPSBzY3RwX3NrYjJldmVudChsYXN0KS0+dHNu
Ow0KKwkJZWxzZQ0KKwkJCWxhc3RfdHNuID0gdHNuOw0KKw0KKwkJLyogVW5saW5rIHRoZSBldmVu
dCwgdGhlbiByZW5lZ2UgYWxsIGFwcGxpY2FibGUgVFNOcy4gKi8NCisJCV9fc2tiX3VubGluayhz
a2IsIGxpc3QpOw0KKwkJZnJlZWQgKz0gc2tiX2hlYWRsZW4oc2tiKTsNCiAJCXNjdHBfdWxwZXZl
bnRfZnJlZShldmVudCk7DQotCQlzY3RwX3Rzbm1hcF9yZW5lZ2UodHNubWFwLCB0c24pOw0KKwkJ
d2hpbGUgKFRTTl9sdGUodHNuLCBsYXN0X3RzbikpIHsNCisJCQlzY3RwX3Rzbm1hcF9yZW5lZ2Uo
dHNubWFwLCB0c24pOw0KKwkJCXRzbisrOw0KKwkJfQ0KIAkJaWYgKGZyZWVkID49IG5lZWRlZCkN
CiAJCQlyZXR1cm4gZnJlZWQ7DQogCX0NCg0K

[PATCH 2/3] sctp: fix association hangs due to reassembly/ordering logic

[Roberts, Lee A.]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2451 bytes --]

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_renege_list(), do not renege packets below the
cumulative TSN ACK point.  Events being reneged from the
ordering queue may correspond to multiple TSNs; identify
and renege all affected packets from the tsnmap.

Patch applies to linux-3.8 kernel.

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

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP
+1/net/sctp/ulpqueue.c linux-3.8-SCTP+2/net/sctp/ulpqueue.c
--- linux-3.8-SCTP+1/net/sctp/ulpqueue.c	2013-02-18 16:58:34.000000000
-0700
+++ linux-3.8-SCTP+2/net/sctp/ulpqueue.c	2013-02-20 08:17:53.679233365
-0700
@@ -962,20 +962,40 @@ static __u16 sctp_ulpq_renege_list(struc
 		struct sk_buff_head *list, __u16 needed)
 {
 	__u16 freed = 0;
-	__u32 tsn;
-	struct sk_buff *skb;
+	__u32 tsn, last_tsn;
+	struct sk_buff *skb, *flist, *last;
 	struct sctp_ulpevent *event;
 	struct sctp_tsnmap *tsnmap;
 
 	tsnmap = &ulpq->asoc->peer.tsn_map;
 
-	while ((skb = __skb_dequeue_tail(list)) != NULL) {
-		freed += skb_headlen(skb);
+	while ((skb = skb_peek_tail(list)) != NULL) {
 		event = sctp_skb2event(skb);
 		tsn = event->tsn;
 
+		/* Don't renege below the Cumulative TSN ACK Point. */
+		if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
+			break;
+
+		/* Events in ordering queue may have multiple fragments
+		 * corresponding to additional TSNs.  Find the last one.
+		 */
+		flist = skb_shinfo(skb)->frag_list;
+		for (last = flist; flist; flist = flist->next)
+			last = flist;
+		if (last)
+			last_tsn = sctp_skb2event(last)->tsn;
+		else
+			last_tsn = tsn;
+
+		/* Unlink the event, then renege all applicable TSNs. */
+		__skb_unlink(skb, list);
+		freed += skb_headlen(skb);
 		sctp_ulpevent_free(event);
-		sctp_tsnmap_renege(tsnmap, tsn);
+		while (TSN_lte(tsn, last_tsn)) {
+			sctp_tsnmap_renege(tsnmap, tsn);
+			tsn++;
+		}
 		if (freed >= needed)
 			return freed;
 	}

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH 2/3] sctp: fix association hangs due to reassembly/ordering logic

[Roberts, Lee A.]

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_renege_list(), do not renege packets below the
cumulative TSN ACK point.  Events being reneged from the
ordering queue may correspond to multiple TSNs; identify
and renege all affected packets from the tsnmap.

Patch applies to linux-3.8 kernel.

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

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP
+1/net/sctp/ulpqueue.c linux-3.8-SCTP+2/net/sctp/ulpqueue.c
--- linux-3.8-SCTP+1/net/sctp/ulpqueue.c	2013-02-18 16:58:34.000000000
-0700
+++ linux-3.8-SCTP+2/net/sctp/ulpqueue.c	2013-02-20 08:17:53.679233365
-0700
@@ -962,20 +962,40 @@ static __u16 sctp_ulpq_renege_list(struc
 		struct sk_buff_head *list, __u16 needed)
 {
 	__u16 freed = 0;
-	__u32 tsn;
-	struct sk_buff *skb;
+	__u32 tsn, last_tsn;
+	struct sk_buff *skb, *flist, *last;
 	struct sctp_ulpevent *event;
 	struct sctp_tsnmap *tsnmap;
 
 	tsnmap = &ulpq->asoc->peer.tsn_map;
 
-	while ((skb = __skb_dequeue_tail(list)) != NULL) {
-		freed += skb_headlen(skb);
+	while ((skb = skb_peek_tail(list)) != NULL) {
 		event = sctp_skb2event(skb);
 		tsn = event->tsn;
 
+		/* Don't renege below the Cumulative TSN ACK Point. */
+		if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
+			break;
+
+		/* Events in ordering queue may have multiple fragments
+		 * corresponding to additional TSNs.  Find the last one.
+		 */
+		flist = skb_shinfo(skb)->frag_list;
+		for (last = flist; flist; flist = flist->next)
+			last = flist;
+		if (last)
+			last_tsn = sctp_skb2event(last)->tsn;
+		else
+			last_tsn = tsn;
+
+		/* Unlink the event, then renege all applicable TSNs. */
+		__skb_unlink(skb, list);
+		freed += skb_headlen(skb);
 		sctp_ulpevent_free(event);
-		sctp_tsnmap_renege(tsnmap, tsn);
+		while (TSN_lte(tsn, last_tsn)) {
+			sctp_tsnmap_renege(tsnmap, tsn);
+			tsn++;
+		}
 		if (freed >= needed)
 			return freed;
 	}

Re: [PATCH 2/3] sctp: fix association hangs due to reassembly/ordering logic

[Vlad Yasevich]

On 02/20/2013 10:55 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_renege_list(), do not renege packets below the
> cumulative TSN ACK point.  Events being reneged from the
> ordering queue may correspond to multiple TSNs; identify
> and renege all affected packets from the tsnmap.
>
> Patch applies to linux-3.8 kernel.
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> ---
>   net/sctp/ulpqueue.c |   30 +++++++++++++++++++++++++-----
>   1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP
> +1/net/sctp/ulpqueue.c linux-3.8-SCTP+2/net/sctp/ulpqueue.c
> --- linux-3.8-SCTP+1/net/sctp/ulpqueue.c	2013-02-18 16:58:34.000000000
> -0700
> +++ linux-3.8-SCTP+2/net/sctp/ulpqueue.c	2013-02-20 08:17:53.679233365
> -0700
> @@ -962,20 +962,40 @@ static __u16 sctp_ulpq_renege_list(struc
>   		struct sk_buff_head *list, __u16 needed)
>   {
>   	__u16 freed = 0;
> -	__u32 tsn;
> -	struct sk_buff *skb;
> +	__u32 tsn, last_tsn;
> +	struct sk_buff *skb, *flist, *last;
>   	struct sctp_ulpevent *event;
>   	struct sctp_tsnmap *tsnmap;
>
>   	tsnmap = &ulpq->asoc->peer.tsn_map;
>
> -	while ((skb = __skb_dequeue_tail(list)) != NULL) {
> -		freed += skb_headlen(skb);
> +	while ((skb = skb_peek_tail(list)) != NULL) {
>   		event = sctp_skb2event(skb);
>   		tsn = event->tsn;
>
> +		/* Don't renege below the Cumulative TSN ACK Point. */
> +		if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
> +			break;
> +
> +		/* Events in ordering queue may have multiple fragments
> +		 * corresponding to additional TSNs.  Find the last one.
> +		 */
> +		flist = skb_shinfo(skb)->frag_list;
> +		for (last = flist; flist; flist = flist->next)
> +			last = flist;
> +		if (last)
> +			last_tsn = sctp_skb2event(last)->tsn;
> +		else
> +			last_tsn = tsn;
> +
> +		/* Unlink the event, then renege all applicable TSNs. */
> +		__skb_unlink(skb, list);
> +		freed += skb_headlen(skb);


This is no longer correct.  You are actually freeing more space if you 
are reneging a reassembled event from the the ordered queue.

Please separate the 2 patches since they fix 2 distinct bugs.

Thanks
-vlad

>   		sctp_ulpevent_free(event);
> -		sctp_tsnmap_renege(tsnmap, tsn);
> +		while (TSN_lte(tsn, last_tsn)) {
> +			sctp_tsnmap_renege(tsnmap, tsn);
> +			tsn++;
> +		}
>   		if (freed >= needed)
>   			return freed;
>   	}
>
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{���i�{ay�\x1dʇڙ�,j\a��f���h���z�\x1e�w���\f���j:+v���w�j�m����\a����zZ+��ݢj"��!tml>

Re: [PATCH 2/3] sctp: fix association hangs due to reassembly/ordering logic

[Vlad Yasevich]

On 02/20/2013 10:55 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_renege_list(), do not renege packets below the
> cumulative TSN ACK point.  Events being reneged from the
> ordering queue may correspond to multiple TSNs; identify
> and renege all affected packets from the tsnmap.
>
> Patch applies to linux-3.8 kernel.
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> ---
>   net/sctp/ulpqueue.c |   30 +++++++++++++++++++++++++-----
>   1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP
> +1/net/sctp/ulpqueue.c linux-3.8-SCTP+2/net/sctp/ulpqueue.c
> --- linux-3.8-SCTP+1/net/sctp/ulpqueue.c	2013-02-18 16:58:34.000000000
> -0700
> +++ linux-3.8-SCTP+2/net/sctp/ulpqueue.c	2013-02-20 08:17:53.679233365
> -0700
> @@ -962,20 +962,40 @@ static __u16 sctp_ulpq_renege_list(struc
>   		struct sk_buff_head *list, __u16 needed)
>   {
>   	__u16 freed = 0;
> -	__u32 tsn;
> -	struct sk_buff *skb;
> +	__u32 tsn, last_tsn;
> +	struct sk_buff *skb, *flist, *last;
>   	struct sctp_ulpevent *event;
>   	struct sctp_tsnmap *tsnmap;
>
>   	tsnmap = &ulpq->asoc->peer.tsn_map;
>
> -	while ((skb = __skb_dequeue_tail(list)) != NULL) {
> -		freed += skb_headlen(skb);
> +	while ((skb = skb_peek_tail(list)) != NULL) {
>   		event = sctp_skb2event(skb);
>   		tsn = event->tsn;
>
> +		/* Don't renege below the Cumulative TSN ACK Point. */
> +		if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
> +			break;
> +
> +		/* Events in ordering queue may have multiple fragments
> +		 * corresponding to additional TSNs.  Find the last one.
> +		 */
> +		flist = skb_shinfo(skb)->frag_list;
> +		for (last = flist; flist; flist = flist->next)
> +			last = flist;
> +		if (last)
> +			last_tsn = sctp_skb2event(last)->tsn;
> +		else
> +			last_tsn = tsn;
> +
> +		/* Unlink the event, then renege all applicable TSNs. */
> +		__skb_unlink(skb, list);
> +		freed += skb_headlen(skb);


This is no longer correct.  You are actually freeing more space if you 
are reneging a reassembled event from the the ordered queue.

Please separate the 2 patches since they fix 2 distinct bugs.

Thanks
-vlad

>   		sctp_ulpevent_free(event);
> -		sctp_tsnmap_renege(tsnmap, tsn);
> +		while (TSN_lte(tsn, last_tsn)) {
> +			sctp_tsnmap_renege(tsnmap, tsn);
> +			tsn++;
> +		}
>   		if (freed >= needed)
>   			return freed;
>   	}
>
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{���i�{ay�\x1dʇڙ�,j\a��f���h���z�\x1e�w���\f���j:+v���w�j�m����\a����zZ+��ݢj"��!tml=
>