| Message ID | 50178061.4010709@windriver.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
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 netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/2012 05:17 AM, Vlad Yasevich wrote: > 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. I didn't find the explicitly bundling order description in spec for other ERROR chunks, but it's reasonable to do this. Anyway, I'll wait for your final solution. Thanks, Xufeng Zhang > > 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 netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
============================================ 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. 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; }