| Message ID | 5013069F.3080306@gmail.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 7/28/12, Vlad Yasevich <vyasevich@gmail.com> wrote: > here is an untested prototype of what I was talking about. This should > handle multiple data chunks. Yes, it works if only the end of the DATA chunk in a packet has invalid stream identifier and I have verified this patch by my test case, but what happens if there are multiple DATA chunks which have invalid stream identifier in a packet? Consider the below example: A packet has several chunks bundling together: "COOKIE_ECHO DATA DATA", both of the two DATA chunks have invalid stream identifier, then the response will be "COOKIE_ACK ERROR SACK ERROR", right? Thanks, Xufeng Zhang > > -vlad > > --- > include/net/sctp/command.h | 1 + > net/sctp/sm_sideeffect.c | 22 ++++++++++++++++++++++ > net/sctp/sm_statefuns.c | 18 ++++++++++-------- > 3 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h > index 712b3be..4043445 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, /* Issue an Invalid Stream error */ > SCTP_CMD_LAST > } sctp_verb_t; > > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c > index 1ff51c9..c5a1322 100644 > --- a/net/sctp/sm_sideeffect.c > +++ b/net/sctp/sm_sideeffect.c > @@ -1036,6 +1036,22 @@ 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 *chunk, > + struct sctp_datahdr *data_hdr) > +{ > + struct sctp_chunk *err; > + > + err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM, > + &data_hdr->stream, > + sizeof(data_hdr->stream), > + sizeof(u16)); > + if (err) > + 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 > @@ -1700,6 +1716,12 @@ 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, chunk, > + (struct sctp_datahdr *)cmd->obj.ptr); > + 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 891f5db..57532e3 100644 > --- a/net/sctp/sm_statefuns.c > +++ b/net/sctp/sm_statefuns.c > @@ -2972,6 +2972,12 @@ discard_noforce: > if (chunk->end_of_packet) > sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force); > > + /* Queue the INVALID STREAM error after the SACK if one is needed. */ > + if (SCTP_IERROR_BAD_STREAM == error) { > + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM, > + SCTP_PTR(chunk->subh.data_hdr)); > + } > + > return SCTP_DISPOSITION_DISCARD; > consume: > return SCTP_DISPOSITION_CONSUME; > @@ -3044,6 +3050,10 @@ sctp_disposition_t > sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep, > */ > sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN, SCTP_NULL()); > sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE()); > + if (SCTP_IERROR_BAD_STREAM == error) { > + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM, > + SCTP_PTR(chunk->subh.data_hdr)); > + } > sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART, > SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN)); > } > @@ -6140,14 +6150,6 @@ static int sctp_eat_data(const struct > sctp_association *asoc, > if (sid >= asoc->c.sinit_max_instreams) { > /* Mark tsn as received even though we drop it */ > sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn)); > - > - err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM, > - &data_hdr->stream, > - sizeof(data_hdr->stream), > - sizeof(u16)); > - if (err) > - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, > - SCTP_CHUNK(err)); > return SCTP_IERROR_BAD_STREAM; > } > > -- 1.7.7.6 > > > -- 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 7/30/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote: > On 7/28/12, Vlad Yasevich <vyasevich@gmail.com> wrote: >> here is an untested prototype of what I was talking about. This should >> handle multiple data chunks. > > Yes, it works if only the end of the DATA chunk in a packet has > invalid stream identifier > and I have verified this patch by my test case, but what happens if > there are multiple > DATA chunks which have invalid stream identifier in a packet? > > Consider the below example: > A packet has several chunks bundling together: "COOKIE_ECHO DATA DATA", > both > of the two DATA chunks have invalid stream identifier, then the > response will be > "COOKIE_ACK ERROR SACK ERROR", right? I just wrote a test case for my above assumption and have verified that SACK always bundled before the end of the ERROR chunk if multiple error DATA chunks happened. So this patch didn't handle all the situations and this is really what I suspected before. Thanks, Xufeng Zhang > > > > Thanks, > Xufeng Zhang > >> >> -vlad >> >> --- >> include/net/sctp/command.h | 1 + >> net/sctp/sm_sideeffect.c | 22 ++++++++++++++++++++++ >> net/sctp/sm_statefuns.c | 18 ++++++++++-------- >> 3 files changed, 33 insertions(+), 8 deletions(-) >> >> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h >> index 712b3be..4043445 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, /* Issue an Invalid Stream error */ >> SCTP_CMD_LAST >> } sctp_verb_t; >> >> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c >> index 1ff51c9..c5a1322 100644 >> --- a/net/sctp/sm_sideeffect.c >> +++ b/net/sctp/sm_sideeffect.c >> @@ -1036,6 +1036,22 @@ 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 *chunk, >> + struct sctp_datahdr *data_hdr) >> +{ >> + struct sctp_chunk *err; >> + >> + err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM, >> + &data_hdr->stream, >> + sizeof(data_hdr->stream), >> + sizeof(u16)); >> + if (err) >> + 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 >> @@ -1700,6 +1716,12 @@ 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, chunk, >> + (struct sctp_datahdr *)cmd->obj.ptr); >> + 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 891f5db..57532e3 100644 >> --- a/net/sctp/sm_statefuns.c >> +++ b/net/sctp/sm_statefuns.c >> @@ -2972,6 +2972,12 @@ discard_noforce: >> if (chunk->end_of_packet) >> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force); >> >> + /* Queue the INVALID STREAM error after the SACK if one is needed. */ >> + if (SCTP_IERROR_BAD_STREAM == error) { >> + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM, >> + SCTP_PTR(chunk->subh.data_hdr)); >> + } >> + >> return SCTP_DISPOSITION_DISCARD; >> consume: >> return SCTP_DISPOSITION_CONSUME; >> @@ -3044,6 +3050,10 @@ sctp_disposition_t >> sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep, >> */ >> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN, SCTP_NULL()); >> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE()); >> + if (SCTP_IERROR_BAD_STREAM == error) { >> + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM, >> + SCTP_PTR(chunk->subh.data_hdr)); >> + } >> sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART, >> SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN)); >> } >> @@ -6140,14 +6150,6 @@ static int sctp_eat_data(const struct >> sctp_association *asoc, >> if (sid >= asoc->c.sinit_max_instreams) { >> /* Mark tsn as received even though we drop it */ >> sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn)); >> - >> - err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM, >> - &data_hdr->stream, >> - sizeof(data_hdr->stream), >> - sizeof(u16)); >> - if (err) >> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, >> - SCTP_CHUNK(err)); >> return SCTP_IERROR_BAD_STREAM; >> } >> >> -- 1.7.7.6 >> >> >> > -- 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
diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h index 712b3be..4043445 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, /* Issue an Invalid Stream error */ SCTP_CMD_LAST } sctp_verb_t; diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c index 1ff51c9..c5a1322 100644 --- a/net/sctp/sm_sideeffect.c +++ b/net/sctp/sm_sideeffect.c @@ -1036,6 +1036,22 @@ 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 *chunk, + struct sctp_datahdr *data_hdr) +{ + struct sctp_chunk *err; + + err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM, + &data_hdr->stream, + sizeof(data_hdr->stream), + sizeof(u16)); + if (err) + 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 @@ -1700,6 +1716,12 @@ 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, chunk, + (struct sctp_datahdr *)cmd->obj.ptr); + 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 891f5db..57532e3 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -2972,6 +2972,12 @@ discard_noforce: if (chunk->end_of_packet) sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force); + /* Queue the INVALID STREAM error after the SACK if one is needed. */ + if (SCTP_IERROR_BAD_STREAM == error) { + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM, + SCTP_PTR(chunk->subh.data_hdr)); + } + return SCTP_DISPOSITION_DISCARD; consume: return SCTP_DISPOSITION_CONSUME; @@ -3044,6 +3050,10 @@ sctp_disposition_t sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep, */ sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN, SCTP_NULL()); sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE()); + if (SCTP_IERROR_BAD_STREAM == error) { + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM, + SCTP_PTR(chunk->subh.data_hdr)); + } sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART, SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
here is an untested prototype of what I was talking about. This should handle multiple data chunks. -vlad --- include/net/sctp/command.h | 1 + net/sctp/sm_sideeffect.c | 22 ++++++++++++++++++++++ net/sctp/sm_statefuns.c | 18 ++++++++++-------- 3 files changed, 33 insertions(+), 8 deletions(-) } @@ -6140,14 +6150,6 @@ static int sctp_eat_data(const struct sctp_association *asoc, if (sid >= asoc->c.sinit_max_instreams) { /* Mark tsn as received even though we drop it */ sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn)); - - err = sctp_make_op_error(asoc, chunk, SCTP_ERROR_INV_STRM, - &data_hdr->stream, - sizeof(data_hdr->stream), - sizeof(u16)); - if (err) - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, - SCTP_CHUNK(err)); return SCTP_IERROR_BAD_STREAM; } -- 1.7.7.6 -- 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