Thread (29 messages) 29 messages, 5 authors, 2012-08-03

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

From: Xufeng Zhang <hidden>
Date: 2012-07-30 05:47:25
Also in: linux-sctp, lkml

On 7/30/12, Xufeng Zhang [off-list ref] wrote:
On 7/28/12, Vlad Yasevich [off-list ref] wrote:
quoted
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
quoted
-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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help