Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR followsSACK when bundling

From: Vlad Yasevich
Date: Thu Aug 02 2012 - 17:17:18 EST


On 07/31/2012 02:51 AM, xufeng zhang wrote:
Sorry, please ignore the above patch, there was an paste error.
Please check the following patch.
============================================
I'm wondering if the below solution is fine to you which is based on
your changes.
BTW, I have verified this patch and it works ok for all the situation,
but only one problem persists:
there is a potential that commands will exceeds SCTP_MAX_NUM_COMMANDS
which happens during sending lots of small error DATA chunks.


I started thinking along the same vein, but was thinking that maybe it makes sense to make error list more generic. I need to check the spec on the ordering of ERROR chunks. If they are always after other control chunks, then maybe make an error list and queue all errors there. Then when sending control chunks, drain the control queue first, then the error queue, and finally the data queue.

BTW, the patch below doesn't include the code to queue the error chunk onto the new error queue.

-vlad

Thanks,
Xufeng Zhang

---
include/net/sctp/command.h | 1 +
include/net/sctp/structs.h | 3 +++
net/sctp/outqueue.c | 7 +++++++
net/sctp/sm_sideeffect.c | 16 ++++++++++++++++
net/sctp/sm_statefuns.c | 17 ++++++++++++++---
5 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
index 712b3be..62c34f5 100644
--- a/include/net/sctp/command.h
+++ b/include/net/sctp/command.h
@@ -110,6 +110,7 @@ typedef enum {
SCTP_CMD_SEND_NEXT_ASCONF, /* Send the next ASCONF after ACK */
SCTP_CMD_PURGE_ASCONF_QUEUE, /* Purge all asconf queues.*/
SCTP_CMD_SET_ASOC, /* Restore association context */
+ SCTP_CMD_GEN_BAD_STREAM, /* Invalid Stream errors happened
command */
SCTP_CMD_LAST
} sctp_verb_t;

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index fc5e600..3d218e0 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1183,6 +1183,9 @@ struct sctp_outq {
*/
struct list_head abandoned;

+ /* Put Invalid Stream error chunks on this list */
+ struct list_head bad_stream_err;
+
/* How many unackd bytes do we have in-flight? */
__u32 outstanding_bytes;

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index e7aa177..1e87b0b 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -211,6 +211,7 @@ void sctp_outq_init(struct sctp_association *asoc,
struct sctp_outq *q)
INIT_LIST_HEAD(&q->retransmit);
INIT_LIST_HEAD(&q->sacked);
INIT_LIST_HEAD(&q->abandoned);
+ INIT_LIST_HEAD(&q->bad_stream_err);

q->fast_rtx = 0;
q->outstanding_bytes = 0;
@@ -283,6 +284,12 @@ void sctp_outq_teardown(struct sctp_outq *q)
list_del_init(&chunk->list);
sctp_chunk_free(chunk);
}
+
+ /* Throw away any pending Invalid Stream error chunks */
+ list_for_each_entry_safe(chunk, tmp,&q->bad_stream_err, list) {
+ list_del_init(&chunk->list);
+ sctp_chunk_free(chunk);
+ }
}

/* Free the outqueue structure and any related pending chunks. */
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index fe99628..4698593 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1060,6 +1060,18 @@ static void sctp_cmd_send_asconf(struct
sctp_association *asoc)
}
}

+static void sctp_cmd_make_inv_stream_err(sctp_cmd_seq_t *commands,
+ struct sctp_association *asoc)
+{
+ struct sctp_chunk *err, *tmp;
+ struct sctp_outq *q =&asoc->outqueue;
+
+ list_for_each_entry_safe(err, tmp,&q->bad_stream_err, list) {
+ list_del_init(&err->list);
+ sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
+ SCTP_CHUNK(err));
+ }
+}

/* These three macros allow us to pull the debugging code out of the
* main flow of sctp_do_sm() to keep attention focused on the real
@@ -1724,6 +1736,10 @@ static int sctp_cmd_interpreter(sctp_event_t
event_type,
asoc = cmd->obj.asoc;
break;

+ case SCTP_CMD_GEN_BAD_STREAM:
+ sctp_cmd_make_inv_stream_err(commands, asoc);
+ break;
+
default:
pr_warn("Impossible command: %u, %p\n",
cmd->verb, cmd->obj.ptr);
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 9fca103..1c1bcd9 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2967,8 +2967,14 @@ discard_force:
return SCTP_DISPOSITION_DISCARD;

discard_noforce:
- if (chunk->end_of_packet)
+ if (chunk->end_of_packet) {
+ struct sctp_outq *q =&asoc->outqueue;
sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
+ /* Queue the INVALID STREAM error after the SACK if one
is needed. */
+ if (!list_empty(&q->bad_stream_err))
+ sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
+ SCTP_NULL());
+ }

return SCTP_DISPOSITION_DISCARD;
consume:
@@ -3037,11 +3043,16 @@ sctp_disposition_t
sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep,
* with a SACK, a SHUTDOWN chunk, and restart the T2-shutdown
timer
*/
if (chunk->end_of_packet) {
+ struct sctp_outq *q =&asoc->outqueue;
/* We must delay the chunk creation since the cumulative
* TSN has not been updated yet.
*/
sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN,
SCTP_NULL());
sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK,
SCTP_FORCE());
+ /* Queue the INVALID STREAM error after the SACK if one
is needed. */
+ if (!list_empty(&q->bad_stream_err))
+ sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
+ SCTP_NULL());
sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
}
@@ -6136,6 +6147,7 @@ static int sctp_eat_data(const struct
sctp_association *asoc,
*/
sid = ntohs(data_hdr->stream);
if (sid>= asoc->c.sinit_max_instreams) {
+ struct sctp_outq *q =&asoc->outqueue;
/* Mark tsn as received even though we drop it */
sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN,
SCTP_U32(tsn));

@@ -6144,8 +6156,7 @@ static int sctp_eat_data(const struct
sctp_association *asoc,
sizeof(data_hdr->stream),
sizeof(u16));
if (err)
- sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
- SCTP_CHUNK(err));
+ list_add_tail(&err->list,&q->bad_stream_err);
return SCTP_IERROR_BAD_STREAM;
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/