a couple of fixes for chunks abandoned in prsctp

Now outstanding_bytes is only increased when appending chunks into one packet and sending it at 1st time, while decreased when it is about to move into retransmit queue. It means outstanding_bytes value is already decreased for all chunks in retransmit queue.

However sctp_prsctp_prune_sent is a common function to check the chunks in both transmitted and retransmit queue, it decrease outstanding_bytes when moving a chunk into abandoned queue from either of them.

It could cause outstanding_bytes underflow, as it also decreases it's value for the chunks in retransmit queue.

This patch fixes it by only updating outstanding_bytes for transmitted queue when pruning queues for prsctp prio policy, the same fix is also needed in sctp_check_transmitted.

Fixes 8dbdf1f5b09c ("sctp: implement prsctp PRIO policy")

Signed-off-by Xin Long <[email protected]>

---
 net/sctp/outqueue.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

net/sctp/outqueue.c +4 -2

index 4db012a..7029f8b 100644
@@ -377,7 +377,8 @@ static int sctp_prsctp_prune_sent(struct sctp_association *asoc,
 		asoc->abandoned_sent[SCTP_PR_INDEX(PRIO)]++;
 		streamout->ext->abandoned_sent[SCTP_PR_INDEX(PRIO)]++;
 
-		if (!chk->tsn_gap_acked) {
+		if (queue != &asoc->outqueue.retransmit &&
+		    !chk->tsn_gap_acked) {
 			if (chk->transport)
 				chk->transport->flight_size ->  						sctp_data_size(chk);

@@ -1434,7 +1435,8 @@ static void sctp_check_transmitted(struct sctp_outq *q,
 			/* If this chunk has not been acked, stop
 			 * considering it as 'outstanding'.
 			 */
-			if (!tchunk->tsn_gap_acked) {
+			if (transmitted_queue != &q->retransmit &&
+			    !tchunk->tsn_gap_acked) {
 				if (tchunk->transport)
 					tchunk->transport->flight_size ->  							sctp_data_size(tchunk);

Signature · 6 lines

2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

As rfc3758#section-3.1 demands:

A3) When a TSN is "abandoned", if it is part of a fragmented message,
all other TSN's within that fragmented message MUST be abandoned at the same time.

Besides, if it couldn't handle this, the rest frags would never get assembled in peer side.

This patch supports it by adding abandoned flag in sctp_datamsg, when one chunk is being abandoned, set chunk->msg->abandoned as well. Next time when checking for abandoned, go checking chunk->msg->abandoned first.

Signed-off-by Xin Long <[email protected]>

---
 include/net/sctp/structs.h |  3 ++-
 net/sctp/chunk.c           |  7 +++++++
 net/sctp/outqueue.c        | 12 ++++++++----
 3 files changed, 17 insertions(+), 5 deletions(-)

include/net/sctp/structs.h +2 -1

index 16f949e..2f8f93d 100644
@@ -503,7 +503,8 @@ struct sctp_datamsg {
 	/* Did the messenge fail to send? */
 	int send_error;
 	u8 send_failed:1,
-	   can_delay;	    /* should this message be Nagle delayed */
+	   can_delay:1,	/* should this message be Nagle delayed */
+	   abandoned:1;	/* should this message be abandoned */
 };
 
 struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *,

net/sctp/chunk.c +4 -0

index 7b261af..9213805 100644
@@ -53,6 +53,7 @@ static void sctp_datamsg_init(struct sctp_datamsg *msg)
 	msg->send_failed = 0;
 	msg->send_error = 0;
 	msg->can_delay = 1;
+	msg->abandoned = 0;
 	msg->expires_at = 0;
 	INIT_LIST_HEAD(&msg->chunks);
 }

@@ -304,6 +305,9 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 	if (!chunk->asoc->peer.prsctp_capable)
 		return 0;
 
+	if (chunk->msg->abandoned)
+		return 1;
+
 	if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
 	    time_after(jiffies, chunk->msg->expires_at)) {
 		struct sctp_stream_out *streamout > @@ -316,6 +320,7 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)

net/sctp/chunk.c +3 -0

 			chunk->asoc->abandoned_unsent[SCTP_PR_INDEX(TTL)]++;
 			streamout->ext->abandoned_unsent[SCTP_PR_INDEX(TTL)]++;
 		}
+		chunk->msg->abandoned = 1;
 		return 1;
 	} else if (SCTP_PR_RTX_ENABLED(chunk->sinfo.sinfo_flags) &&
 		   chunk->sent_count > chunk->sinfo.sinfo_timetolive) {

@@ -324,10 +329,12 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 
 		chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
 		streamout->ext->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
+		chunk->msg->abandoned = 1;
 		return 1;
 	} else if (!SCTP_PR_POLICY(chunk->sinfo.sinfo_flags) &&
 		   chunk->msg->expires_at &&
 		   time_after(jiffies, chunk->msg->expires_at)) {
+		chunk->msg->abandoned = 1;
 		return 1;
 	}
 	/* PRIO policy is processed by sendmsg, not here */

net/sctp/outqueue.c +8 -4

index 7029f8b..4ab164b 100644
@@ -364,10 +364,12 @@ static int sctp_prsctp_prune_sent(struct sctp_association *asoc,
 	list_for_each_entry_safe(chk, temp, queue, transmitted_list) {
 		struct sctp_stream_out *streamout;
 
-		if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
-		    chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
+		if (!chk->msg->abandoned &&
+		    (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
+		     chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive))
 			continue;
 
+		chk->msg->abandoned = 1;
 		list_del_init(&chk->transmitted_list);
 		sctp_insert_list(&asoc->outqueue.abandoned,
 				 &chk->transmitted_list);

@@ -404,10 +406,12 @@ static int sctp_prsctp_prune_unsent(struct sctp_association *asoc,
 	q->sched->unsched_all(&asoc->stream);
 
 	list_for_each_entry_safe(chk, temp, &q->out_chunk_list, list) {
-		if (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
-		    chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive)
+		if (!chk->msg->abandoned &&
+		    (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
+		     chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive))
 			continue;
 
+		chk->msg->abandoned = 1;
 		sctp_sched_dequeue_common(q, chk);
 		asoc->sent_cnt_removable--;
 		asoc->abandoned_unsent[SCTP_PR_INDEX(PRIO)]++;

Signature · 6 lines

2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Now for the abandoned chunks in unsent outq, it would just free the chunks. Because no tsn is assigned to them yet, there's no need to send fwd tsn to peer, unlike for the abandoned chunks in sent outq.

The problem is when parts of the msg have been sent and the other frags are still in unsent outq, if they are abandoned/dropped, the peer would never get this msg reassembled.

So these frags in unsent outq can't be dropped if this msg already has outstanding frags.

This patch does the check in sctp_chunk_abandoned and sctp_prsctp_prune_unsent.

Signed-off-by Xin Long <[email protected]>

---
 net/sctp/chunk.c    | 4 ++++
 net/sctp/outqueue.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

net/sctp/chunk.c +4 -0

index 9213805..7f8baa4 100644
@@ -308,6 +308,10 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
 	if (chunk->msg->abandoned)
 		return 1;
 
+	if (!chunk->has_tsn &&
+	    !(chunk->chunk_hdr->flags & SCTP_DATA_FIRST_FRAG))
+		return 0;
+
 	if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
 	    time_after(jiffies, chunk->msg->expires_at)) {
 		struct sctp_stream_out *streamout > diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c

index 4ab164b..7d67fee 100644

net/sctp/outqueue.c +2 -1

@@ -407,7 +407,8 @@ static int sctp_prsctp_prune_unsent(struct sctp_association *asoc,
 
 	list_for_each_entry_safe(chk, temp, &q->out_chunk_list, list) {
 		if (!chk->msg->abandoned &&
-		    (!SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
+		    (!(chk->chunk_hdr->flags & SCTP_DATA_FIRST_FRAG) ||
+		     !SCTP_PR_PRIO_ENABLED(chk->sinfo.sinfo_flags) ||
 		     chk->sinfo.sinfo_timetolive <= sinfo->sinfo_timetolive))
 			continue;
 

Signature · 6 lines

2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Now when abandoning chunks in prsctp, it doesn't consider for frags in one msg, which would cause peer can never receive the whole frags for one msg to get them reassembled, these pieces of this msg will stay in the reasm queue forever and block the following chunks' receiving.

This patchset is to fix them in patch 2 and 3, and also fix another issue for prsctp in patch 1.

Xin Long (3):
sctp: only update outstanding_bytes for transmitted queue when doing
prsctp_prune
sctp: abandon the whole msg if one part of a fragmented message is
abandoned
sctp: do not abandon the other frags in unsent outq if one msg has
outstanding frags

 include/net/sctp/structs.h |  3 ++-
 net/sctp/chunk.c           | 11 +++++++++++
 net/sctp/outqueue.c        | 19 +++++++++++++------
 3 files changed, 26 insertions(+), 7 deletions(-)

Signature · 1 line

2.1.0

Now when abandoning chunks in prsctp, it doesn't consider for frags in one msg, which would cause peer can never receive the whole frags for one msg to get them reassembled, these pieces of this msg will stay in the reasm queue forever and block the following chunks' receiving.

This patchset is to fix them in patch 2 and 3, and also fix another issue for prsctp in patch 1.