Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752720Ab2HCCPk (ORCPT ); Thu, 2 Aug 2012 22:15:40 -0400 Received: from mail1.windriver.com ([147.11.146.13]:64896 "EHLO mail1.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751460Ab2HCCPh (ORCPT ); Thu, 2 Aug 2012 22:15:37 -0400 Message-ID: <501B3643.5000605@windriver.com> Date: Fri, 3 Aug 2012 10:24:03 +0800 From: xufeng zhang User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.10) Gecko/20100512 Thunderbird/3.0.5 ThunderBrowse/3.3.5 MIME-Version: 1.0 To: Vlad Yasevich CC: Xufeng Zhang , Neil Horman , , , , , Subject: Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling References: <1342677450-21810-1-git-send-email-xufengzhang.main@gmail.com> <75bdd73e-1c47-4fd6-bc29-5b86ee6da69a@email.android.com> <50100A29.8010705@gmail.com> <8ea83e51-81d1-407a-be06-98d2c99c876d@email.android.com> <5013069F.3080306@gmail.com> <50178061.4010709@windriver.com> <501AEE5E.8090303@gmail.com> In-Reply-To: <501AEE5E.8090303@gmail.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8218 Lines: 210 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 linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/