[PATCH] SCTP: enable cookie-echo retransmission transport switch

[PATCH] SCTP: enable cookie-echo retransmission transport switch

[Gui Jianfeng]

Vlad,
This patch enables cookie-echo retransmission transport switch
feature. If COOKIE-ECHO retransmission happens, it will be sent 
to the address other than the one last sent to.

Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
 include/net/sctp/command.h |    1 +
 include/net/sctp/structs.h |    6 ++++++
 net/sctp/associola.c       |   27 +++++++++++++++++++++++++++
 net/sctp/sm_sideeffect.c   |    8 +++++++-
 net/sctp/sm_statefuns.c    |    4 ++++
 5 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
index 35b1e83..02d4810 100644
--- a/include/net/sctp/command.h
+++ b/include/net/sctp/command.h
@@ -105,6 +105,7 @@ typedef enum {
 	SCTP_CMD_ASSOC_SHKEY,    /* generate the association shared keys */
 	SCTP_CMD_T1_RETRAN,	 /* Mark for retransmission after T1 timeout  */
 	SCTP_CMD_UPDATE_INITTAG, /* Update peer inittag */
+	SCTP_CMD_COOKIEECHO_CHOOSE_TRANSPORT, /* Choose transport for a COOKIE-ECHO */
 	SCTP_CMD_LAST
 } sctp_verb_t;
 
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 9c827a7..66baa96 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1664,6 +1664,9 @@ struct sctp_association {
 	/* Transport to which INIT chunk was last sent. */
 	struct sctp_transport *init_last_sent_to;
 
+	/* Transport to which COOKIE-ECHO chunk was last sent. */
+	struct sctp_transport *cookie_echo_last_sent_to;
+
 	/* Next TSN    : The next TSN number to be assigned to a new
 	 *	       : DATA chunk.  This is sent in the INIT or INIT
 	 *	       : ACK chunk to the peer and incremented each
@@ -1962,6 +1965,9 @@ int sctp_cmp_addr_exact(const union sctp_addr *ss1,
 			const union sctp_addr *ss2);
 struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc);
 
+struct sctp_transport *sctp_assoc_choose_cookie_echo_transport(
+	struct sctp_association *);
+
 /* A convenience structure to parse out SCTP specific CMSGs. */
 typedef struct sctp_cmsgs {
 	struct sctp_initmsg *init;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index d29f792..94cbb34 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1539,3 +1539,30 @@ struct sctp_chunk *sctp_assoc_lookup_asconf_ack(
 
 	return NULL;
 }
+
+/* Choose the transport for cookie-echo retransmission.  */
+struct sctp_transport *sctp_assoc_choose_cookie_echo_transport(
+	struct sctp_association *asoc)
+{
+	struct sctp_transport *t;
+
+	if (!asoc->cookie_echo_last_sent_to) {
+		/* This will never happen, because this routine is
+		 * only invoked when cookie-echo retransmission.
+		 */
+		t = asoc->peer.active_path;
+	} else {
+		if (asoc->cookie_echo_last_sent_to == asoc->peer.retran_path)
+			sctp_assoc_update_retran_path(asoc);
+		t = asoc->peer.retran_path;
+	}
+
+	SCTP_DEBUG_PRINTK_IPADDR("sctp_assoc_update_retran_path:association"
+				 " %p addr: ",
+				 " port: %d\n",
+				 asoc,
+				 (&t->ipaddr),
+				 ntohs(t->ipaddr.v4.sin_port));
+
+	return t;
+}
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index a4763fd..fd2f7aa 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1188,6 +1188,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
 					sctp_chunk_free(cmd->obj.ptr);
 				goto nomem;
 			}
+			asoc->cookie_echo_last_sent_to = new_obj->transport;
 			sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
 					SCTP_CHUNK(new_obj));
 
@@ -1539,7 +1540,12 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
 		case SCTP_CMD_UPDATE_INITTAG:
 			asoc->peer.i.init_tag = cmd->obj.u32;
 			break;
-
+		case SCTP_CMD_COOKIEECHO_CHOOSE_TRANSPORT:
+			chunk = cmd->obj.ptr;
+			t = sctp_assoc_choose_cookie_echo_transport(asoc);
+			asoc->cookie_echo_last_sent_to = t;
+			chunk->transport = t;
+			break;
 		default:
 			printk(KERN_WARNING "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 07194c2..ce40f29 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -5291,6 +5291,10 @@ sctp_disposition_t sctp_sf_t1_cookie_timer_expire(const struct sctp_endpoint *ep
 		if (!repl)
 			return SCTP_DISPOSITION_NOMEM;
 
+		/* Choose transport for COOKIE-ECHO */
+		sctp_add_cmd_sf(commands, SCTP_CMD_COOKIEECHO_CHOOSE_TRANSPORT,
+				SCTP_CHUNK(repl));
+
 		/* Issue a sideeffect to do the needed accounting. */
 		sctp_add_cmd_sf(commands, SCTP_CMD_COOKIEECHO_RESTART,
 				SCTP_TO(SCTP_EVENT_TIMEOUT_T1_COOKIE));
-- 
1.5.3

Re: [PATCH] SCTP: enable cookie-echo retransmission transport switch

[Vlad Yasevich]

Gui Jianfeng wrote:
> Vlad,
> This patch enables cookie-echo retransmission transport switch
> feature. If COOKIE-ECHO retransmission happens, it will be sent 
> to the address other than the one last sent to.
> 

NAK.

You can't blindly choose a different transport since they could
be unconfirmed and can't really be used until we confirm them
with HBs.  So, you can only do this when the user issued an
sctp_connectx() and we have multiple confirmed transports.

In this case only confirmed transports are allowed, otherwise
there is a possibility of hijacking associations.


Also, looking at this, the same problem exists in current
code for selection INIT transports.

We don't correctly treat peers passed to connectx() as confirmed
and don't select the correct transport.

Once you fix that above, you can just re-use the function and
re-use init_last_sent_to.

-vlad

Re: [PATCH] SCTP: enable cookie-echo retransmission transport switch

[Gui Jianfeng]

Vlad,

  Thanks for your comments.

Vlad Yasevich wrote:
> Gui Jianfeng wrote:
>> Vlad,
>> This patch enables cookie-echo retransmission transport switch
>> feature. If COOKIE-ECHO retransmission happens, it will be sent to the
>> address other than the one last sent to.
>>
> 
> NAK.
> 
> You can't blindly choose a different transport since they could
> be unconfirmed and can't really be used until we confirm them
> with HBs.  So, you can only do this when the user issued an
> sctp_connectx() and we have multiple confirmed transports.
> 
> In this case only confirmed transports are allowed, otherwise
> there is a possibility of hijacking associations.
> 
  Choosing a transport here just gives a suggestion for sctp_outq_flush(),
  sctp_outq_flush() will judge again. If the selected transport is in 
  INACTIVE or UNCONFIRMED, active_path will be used.
  So COOKIE-ECHO won't be sent to a unconfirmed address.

> 
> Also, looking at this, the same problem exists in current
> code for selection INIT transports.
> 
> We don't correctly treat peers passed to connectx() as confirmed
> and don't select the correct transport.
> 
> Once you fix that above, you can just re-use the function and
> re-use init_last_sent_to.

  Will do.

> 
> -vlad
> 



> 
> 

-- 
Regards
Gui Jianfeng

Re: [PATCH] SCTP: enable cookie-echo retransmission transport switch

[Vlad Yasevich]

Gui Jianfeng wrote:
> Vlad,
> 
>   Thanks for your comments.
> 
> Vlad Yasevich wrote:
>> Gui Jianfeng wrote:
>>> Vlad,
>>> This patch enables cookie-echo retransmission transport switch
>>> feature. If COOKIE-ECHO retransmission happens, it will be sent to the
>>> address other than the one last sent to.
>>>
>> NAK.
>>
>> You can't blindly choose a different transport since they could
>> be unconfirmed and can't really be used until we confirm them
>> with HBs.  So, you can only do this when the user issued an
>> sctp_connectx() and we have multiple confirmed transports.
>>
>> In this case only confirmed transports are allowed, otherwise
>> there is a possibility of hijacking associations.
>>
>   Choosing a transport here just gives a suggestion for sctp_outq_flush(),
>   sctp_outq_flush() will judge again. If the selected transport is in 
>   INACTIVE or UNCONFIRMED, active_path will be used.
>   So COOKIE-ECHO won't be sent to a unconfirmed address.
> 

Yep, and this completely nullifies the thing you are trying to do.  So, instead
of trying other available transports, we end up just sending on the active path
is at this stage of association is the same as primary.

>> Also, looking at this, the same problem exists in current
>> code for selection INIT transports.
>>
>> We don't correctly treat peers passed to connectx() as confirmed
>> and don't select the correct transport.
>>
>> Once you fix that above, you can just re-use the function and
>> re-use init_last_sent_to.
> 
>   Will do.

Just an FYI, here is a way BSD handles this:
	a.  they assume all destinations passed to connectx() are
	    reachable/confirmed.
	b.  when processing INIT-ACK, they tag all the existing destinations
	    with a flag, and clear the flag if the destination is in INIT-ACK.
	c.  they free all destinations that have the flag set at the end of
	    INIT-ACK.

I think a simpler way would to add another state to the transport that allows 
sending handshakes and correctly transitions to confirmed when association is
fully formed.  In BSD, they end up walking the destination chain 3 times, which
is just a waste.

-vlad
> 
>> -vlad
>>
> 
> 
> 
>>
>