sctp: COOKIE-ACK should back to where the COOKIE-ECHO
Subsystems (2)
PATCH 2/2 sctp: COOKIE-ACK should back to where the COOKIE-ECHO
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK, etc.) to the same destination transport address from which it received the DATA or control chunk to which it is replying.
But if endpoint is multi-homed, retransmit COOKIE-ECHO will cause COOKIE-ACK be sent back to the destination transport address from which the INIT chunk is received. This patch fixed it while retransmit COOKIE-ECHO is because of either COOKIE-ECHO lost or COOKIE-ACK lost.
--- net/sctp/sm_statefuns.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
net/sctp/sm_statefuns.c +9 -1
@@ -724,10 +724,15 @@ sctp_disposition_t sctp_sf_do_5_1D_ce(const struct sctp_endpoint *ep, peer_init = &chunk->subh.cookie_hdr->c.peer_init[0 ];if (!sctp_process_init (new_asoc, chunk->chunk_hdr->type, - &chunk->subh.cookie_hdr->c.peer_addr, +sctp_source (chunk), peer_init,GFP_ATOMIC )) goto nomem_init; +/* The peer's original address may not appear in address parameters */ +if (!sctp_assoc_add_peer (new_asoc, &chunk->subh.cookie_hdr->c.peer_addr, +GFP_ATOMIC ,SCTP_UNCONFIRMED )) + goto nomem_init; +/* SCTP-AUTH: Now that we've populate required fields in * sctp_process_init, set up the assocaition shared keys as * necessary so that we can potentially authenticate the ACK
@@ -1914,6 +1919,9 @@ static sctp_disposition_t sctp_sf_do_dupcook_d(const struct sctp_endpoint *ep, } } +sctp_add_cmd_sf (commands,SCTP_CMD_TRANSPORT_ON , +SCTP_TRANSPORT (chunk->transport)); + repl =sctp_make_cookie_ack (new_asoc, chunk);if (!repl) goto nomem;
Signature · 1 line
1.6.5.2
Quoted 23 lines · Wei Yongjun
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK, etc.) to the same destination transport address from which it received the DATA or control chunk to which it is replying.
But if endpoint is multi-homed, retransmit COOKIE-ECHO will cause COOKIE-ACK be sent back to the destination transport address from which the INIT chunk is received. This patch fixed it while retransmit COOKIE-ECHO is because of either COOKIE-ECHO lost or COOKIE-ACK lost.
Signed-off-by Wei Yongjun <[email protected]>--- net/sctp/sm_statefuns.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)net/sctp/sm_statefuns.c +0
-0@@ -724,10 +724,15 @@ sctp_disposition_t sctp_sf_do_5_1D_ce(const struct sctp_endpoint *ep, peer_init = &chunk->subh.cookie_hdr->c.peer_init[0 ]; if (!sctp_process_init(new_asoc, chunk->chunk_hdr->type,
net/sctp/sm_statefuns.c sctp_disposition_t sctp_sf_do_5_1D_ce(const struct sctp_endpoint *ep, · line 735
Quoted 10 lines
net/sctp/sm_statefuns.c +6
-1- &chunk->subh.cookie_hdr->c.peer_addr, + sctp_source(chunk), peer_init, GFP_ATOMIC)) goto nomem_init; +/* The peer's original address may not appear in address parameters */ +if (!sctp_assoc_add_peer (new_asoc, &chunk->subh.cookie_hdr->c.peer_addr, +GFP_ATOMIC ,SCTP_UNCONFIRMED )) + goto nomem_init; +
Actually, the interesting thing here is that here is what rfc 4960 states
5.4. Path Verification
...
the following rules are applied to all addresses of the
new association:
1) Any address passed to the sender of the INIT by its upper layer is automatically considered to be CONFIRMED.
2) For the receiver of the COOKIE ECHO, the only CONFIRMED address is the one to which the INIT-ACK was sent.
3) All other addresses not covered by rules 1 and 2 are considered UNCONFIRMED and are subject to probing for verification.
The proper way to solve this is to implement the following text:
- A COOKIE ACK MAY be sent to an UNCONFIRMED address, but it MUST be bundled with a HEARTBEAT including a nonce. An implementation that does NOT support bundling MUST NOT send a COOKIE ACK to an UNCONFIRMED address.
- A COOKIE ECHO MAY be sent to an UNCONFIRMED address, but it MUST be bundled with a HEARTBEAT including a nonce, and the packet MUST NOT exceed the path MTU. If the implementation does NOT support bundling or if the bundled COOKIE ECHO plus HEARTBEAT (including nonce) would exceed the path MTU, then the implementation MUST NOT send a COOKIE ECHO to an UNCONFIRMED address.
net/sctp/sm_statefuns.c static sctp_disposition_t sctp_sf_do_dupcook_d(const struct sctp_endpoint *ep, · line 1924
Quoted 10 lines
net/sctp/sm_statefuns.c +3
-0/* SCTP-AUTH: Now that we've populate required fields in * sctp_process_init, set up the assocaition shared keys as * necessary so that we can potentially authenticate the ACK @@ -1914,6 +1919,9 @@ static sctp_disposition_t sctp_sf_do_dupcook_d(const struct sctp_endpoint *ep, } } +sctp_add_cmd_sf (commands,SCTP_CMD_TRANSPORT_ON , +SCTP_TRANSPORT (chunk->transport)); +
sctp_cmd_transport_on() assumes that the chunk passed to it is a HB chunk. This is not the right thing to do. I am truly amazed that this worked for you.
-vlad
Quoted 3 lines
net/sctp/sm_statefuns.c +0
-0repl =sctp_make_cookie_ack (new_asoc, chunk);if (!repl) goto nomem;
PATCH 2/2 sctp: COOKIE-ACK should back to where the COOKIE-ECHO
In reply to Wei YongjunThis patch fixed to send COOKIE-ACK back to where the COOKIE-ECHO came from, and if COOKIE ACK is sent to an UNCONFIRMED address, bundled it with a HEARTBEAT including a nonce. Based on RFC4960:
Section 6.4. Multi-Homed SCTP Endpoints
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK, etc.) to the same destination transport address from which it received the DATA or control chunk to which it is replying.
Section 5.4. Path Verification:
- A COOKIE ACK MAY be sent to an UNCONFIRMED address, but it MUST be bundled with a HEARTBEAT including a nonce. An implementation that does NOT support bundling MUST NOT send a COOKIE ACK to an UNCONFIRMED address.
--- net/sctp/output.c | 22 ++++++++++++++++++++++ net/sctp/outqueue.c | 1 + net/sctp/sm_make_chunk.c | 3 +++ 3 files changed, 26 insertions(+), 0 deletions(-)
net/sctp/output.c +22 -0
@@ -260,6 +260,25 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,return retval; } +/* Try to bundle a HEARTBEAT with the packet. */ +static sctp_xmit_t sctp_packet_bundle_hb (struct sctp_packet *pkt, +struct sctp_chunk *chunk) +{ +sctp_xmit_t retval =SCTP_XMIT_OK ; + +if (pkt->transport->state =SCTP_UNCONFIRMED && + chunk->chunk_hdr->type =SCTP_CID_COOKIE_ACK ) { +struct sctp_transport *transport = pkt->transport; +struct sctp_chunk *hb; + + hb =sctp_make_heartbeat (transport->asoc, transport); +if (hb) + retval =sctp_packet_append_chunk (pkt, hb); + } + +return retval; +} +/* Append a chunk to the offered packet reporting back any inability to do * so. */
@@ -329,6 +348,9 @@ sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,list_add_tail (&chunk->list, &packet->chunk_list); packet->size += chunk_len; chunk->transport = packet->transport; + +/* Try to bundle HEARTBEAT chunk */ + retval =sctp_packet_bundle_hb (packet, chunk); finish:return retval; }
net/sctp/outqueue.c +1 -0
@@ -788,6 +788,7 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)*/ if (chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT && chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT_ACK && + chunk->chunk_hdr->type !=SCTP_CID_COOKIE_ACK && chunk->chunk_hdr->type !=SCTP_CID_ASCONF_ACK ) new_transport = asoc->peer.active_path; }
net/sctp/sm_make_chunk.c +3 -0
@@ -584,6 +584,9 @@ struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc,if (retval && chunk) retval->transport = chunk->transport; +if (retval && !retval->transport) + retval->dest = chunk->source; +return retval; }
Signature · 1 line
1.6.5.2
Hi Wei
Quoted 55 lines · Wei Yongjun
This patch fixed to send COOKIE-ACK back to where the COOKIE-ECHO came from, and if COOKIE ACK is sent to an UNCONFIRMED address, bundled it with a HEARTBEAT including a nonce. Based on RFC4960:
Section 6.4. Multi-Homed SCTP Endpoints
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK, etc.) to the same destination transport address from which it received the DATA or control chunk to which it is replying.
Section 5.4. Path Verification:
- A COOKIE ACK MAY be sent to an UNCONFIRMED address, but it MUST be bundled with a HEARTBEAT including a nonce. An implementation that does NOT support bundling MUST NOT send a COOKIE ACK to an UNCONFIRMED address.
Signed-off-by Wei Yongjun <[email protected]>--- net/sctp/output.c | 22 ++++++++++++++++++++++ net/sctp/outqueue.c | 1 + net/sctp/sm_make_chunk.c | 3 +++ 3 files changed, 26 insertions(+), 0 deletions(-)net/sctp/output.c +19
-0@@ -260,6 +260,25 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,return retval; } +/* Try to bundle a HEARTBEAT with the packet. */ +static sctp_xmit_t sctp_packet_bundle_hb (struct sctp_packet *pkt, +struct sctp_chunk *chunk) +{ +sctp_xmit_t retval =SCTP_XMIT_OK ; + +if (pkt->transport->state =SCTP_UNCONFIRMED && + chunk->chunk_hdr->type =SCTP_CID_COOKIE_ACK ) { +struct sctp_transport *transport = pkt->transport; +struct sctp_chunk *hb; + + hb =sctp_make_heartbeat (transport->asoc, transport); +if (hb) + retval =sctp_packet_append_chunk (pkt, hb); + } + +return retval; +} +/* Append a chunk to the offered packet reporting back any inability to do * so. */
net/sctp/output.c sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet, · line 353
Quoted 6 lines
net/sctp/output.c +3
-0@@ -329,6 +348,9 @@ sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,list_add_tail (&chunk->list, &packet->chunk_list); packet->size += chunk_len; chunk->transport = packet->transport; + +/* Try to bundle HEARTBEAT chunk */ + retval =sctp_packet_bundle_hb (packet, chunk);
Not sure if this is the right place. Also, this will add the HB after cookie-ACK.
I am wondering if it should go before. Seems a more correct thing to do is
to elicit HB-ACK before any further data might flow over the transport.
Also, what about all the HB related things like error counters, timers, etc... Seems that we should also do that.
May be we can just cue the HB to the control queue if we are not replying to the source of INIT-ACK?
Quoted 20 lines
net/sctp/output.c +0
-0finish:return retval; }net/sctp/outqueue.c +1
-0@@ -788,6 +788,7 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)*/ if (chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT && chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT_ACK && + chunk->chunk_hdr->type !=SCTP_CID_COOKIE_ACK && chunk->chunk_hdr->type !=SCTP_CID_ASCONF_ACK ) new_transport = asoc->peer.active_path; }net/sctp/sm_make_chunk.c +0
-0
net/sctp/sm_make_chunk.c struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc, · line 589
Quoted 6 lines
net/sctp/sm_make_chunk.c +3
-0@@ -584,6 +584,9 @@ struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc,if (retval && chunk) retval->transport = chunk->transport; +if (retval && !retval->transport) + retval->dest = chunk->source; +
This one is good, but I am thinking that transport assignment may be incorrect.
Like I said before, when we are restarting, the transport assigned is from the
existing association, but the packet is transmitted on the temporary association that's about to vanish. Not sure if this is ok.
Quoted 3 lines
net/sctp/sm_make_chunk.c +0
-0return retval; }
Quoted 62 lines
Hi Wei
Quoted 57 lines · Wei Yongjun
This patch fixed to send COOKIE-ACK back to where the COOKIE-ECHO came from, and if COOKIE ACK is sent to an UNCONFIRMED address, bundled it with a HEARTBEAT including a nonce. Based on RFC4960:
Section 6.4. Multi-Homed SCTP Endpoints
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK, etc.) to the same destination transport address from which it received the DATA or control chunk to which it is replying.
Section 5.4. Path Verification:
- A COOKIE ACK MAY be sent to an UNCONFIRMED address, but it MUST be bundled with a HEARTBEAT including a nonce. An implementation that does NOT support bundling MUST NOT send a COOKIE ACK to an UNCONFIRMED address.
Signed-off-by Wei Yongjun <[email protected]>--- net/sctp/output.c | 22 ++++++++++++++++++++++ net/sctp/outqueue.c | 1 + net/sctp/sm_make_chunk.c | 3 +++ 3 files changed, 26 insertions(+), 0 deletions(-)net/sctp/output.c +19
-0@@ -260,6 +260,25 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,return retval; } +/* Try to bundle a HEARTBEAT with the packet. */ +static sctp_xmit_t sctp_packet_bundle_hb (struct sctp_packet *pkt, +struct sctp_chunk *chunk) +{ +sctp_xmit_t retval =SCTP_XMIT_OK ; + +if (pkt->transport->state =SCTP_UNCONFIRMED && + chunk->chunk_hdr->type =SCTP_CID_COOKIE_ACK ) { +struct sctp_transport *transport = pkt->transport; +struct sctp_chunk *hb; + + hb =sctp_make_heartbeat (transport->asoc, transport); +if (hb) + retval =sctp_packet_append_chunk (pkt, hb); + } + +return retval; +} +/* Append a chunk to the offered packet reporting back any inability to do * so. */ @@ -329,6 +348,9 @@ sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,list_add_tail (&chunk->list, &packet->chunk_list); packet->size += chunk_len;
Quoted 9 lines
net/sctp/output.c sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet, · line 354
Quoted 5 lines
net/sctp/output.c +3
-0chunk->transport = packet->transport; + +/* Try to bundle HEARTBEAT chunk */ + retval =sctp_packet_bundle_hb (packet, chunk);Not sure if this is the right place. Also, this will add the HB after cookie-ACK.
I am wondering if it should go before. Seems a more correct thing to do is
to elicit HB-ACK before any further data might flow over the transport.
I add the HB after cookie-ACK is because that the COOKIE-ECHO on UNCONFIRMED address must also be bundled with a HEARTBEAT. If HB is before COOKIE-ECHO, the peer is in CLOSED state and the HB will be treat as OOTB chunk. And I think COOKIE-ACK need to do the same thing.
Quoted 3 lines
Also, what about all the HB related things like error counters, timers, etc... Seems that we should also do that.
Not sure about that, the first HB may be sent then RTO is expired. Do this may change the orig HB's action.
Quoted 3 lines
May be we can just cue the HB to the control queue if we are not replying to the source of INIT-ACK?
If the state is UNCONFIRMED, the transport should not be the source of INIT-ACK, since INIT-ACK is sent to the source of INIT, and it should be in ACTIVE state.
Quoted 24 lines
Quoted 24 lines
net/sctp/output.c +0
-0finish:return retval;}
net/sctp/outqueue.c +1
-0@@ -788,6 +788,7 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)*/ if (chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT && chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT_ACK && + chunk->chunk_hdr->type !=SCTP_CID_COOKIE_ACK && chunk->chunk_hdr->type !=SCTP_CID_ASCONF_ACK ) new_transport = asoc->peer.active_path; }net/sctp/sm_make_chunk.c +0
-0@@ -584,6 +584,9 @@ struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc,if (retval && chunk) retval->transport = chunk->transport;
Quoted 9 lines
net/sctp/sm_make_chunk.c struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc, · line 590
Quoted 4 lines
net/sctp/sm_make_chunk.c +3
-0+if (retval && !retval->transport) + retval->dest = chunk->source; +This one is good, but I am thinking that transport assignment may be incorrect.
Like I said before, when we are restarting, the transport assigned is from the
existing association, but the packet is transmitted on the temporary association that's about to vanish. Not sure if this is ok.
It can works, but may be the transport should not be set, the only thing we should do is set the retval->dest.
Quoted 13 lines
net/sctp/sm_make_chunk.c struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc, · line 592
Quoted 6 lines
net/sctp/sm_make_chunk.c +0
-0return retval; }--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 71 lines · Wei Yongjun
Quoted 61 lines
Hi Wei
Quoted 57 lines · Wei Yongjun
This patch fixed to send COOKIE-ACK back to where the COOKIE-ECHO came from, and if COOKIE ACK is sent to an UNCONFIRMED address, bundled it with a HEARTBEAT including a nonce. Based on RFC4960:
Section 6.4. Multi-Homed SCTP Endpoints
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK, etc.) to the same destination transport address from which it received the DATA or control chunk to which it is replying.
Section 5.4. Path Verification:
- A COOKIE ACK MAY be sent to an UNCONFIRMED address, but it MUST be bundled with a HEARTBEAT including a nonce. An implementation that does NOT support bundling MUST NOT send a COOKIE ACK to an UNCONFIRMED address.
Signed-off-by Wei Yongjun <[email protected]>--- net/sctp/output.c | 22 ++++++++++++++++++++++ net/sctp/outqueue.c | 1 + net/sctp/sm_make_chunk.c | 3 +++ 3 files changed, 26 insertions(+), 0 deletions(-)net/sctp/output.c +19
-0@@ -260,6 +260,25 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,return retval; } +/* Try to bundle a HEARTBEAT with the packet. */ +static sctp_xmit_t sctp_packet_bundle_hb (struct sctp_packet *pkt, +struct sctp_chunk *chunk) +{ +sctp_xmit_t retval =SCTP_XMIT_OK ; + +if (pkt->transport->state =SCTP_UNCONFIRMED && + chunk->chunk_hdr->type =SCTP_CID_COOKIE_ACK ) { +struct sctp_transport *transport = pkt->transport; +struct sctp_chunk *hb; + + hb =sctp_make_heartbeat (transport->asoc, transport); +if (hb) + retval =sctp_packet_append_chunk (pkt, hb); + } + +return retval; +} +/* Append a chunk to the offered packet reporting back any inability to do * so. */ @@ -329,6 +348,9 @@ sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,list_add_tail (&chunk->list, &packet->chunk_list); packet->size += chunk_len;Quoted 9 lines
net/sctp/output.c sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet, · line 354
Quoted 5 lines
net/sctp/output.c +3
-0chunk->transport = packet->transport; + +/* Try to bundle HEARTBEAT chunk */ + retval =sctp_packet_bundle_hb (packet, chunk);Not sure if this is the right place. Also, this will add the HB after cookie-ACK.
I am wondering if it should go before. Seems a more correct thing to do is
to elicit HB-ACK before any further data might flow over the transport.
Quoted 5 lines
I add the HB after cookie-ACK is because that the COOKIE-ECHO on UNCONFIRMED address must also be bundled with a HEARTBEAT. If HB is before COOKIE-ECHO, the peer is in CLOSED state and the HB will be treat as OOTB chunk. And I think COOKIE-ACK need to do the same thing.
Not really. Heartbeats are allows in the COOKIE_ECHOED state. See section 8.3.
So, it would be perfectly valid to include a HB first and attach a cookie-ack to it.
Quoted 9 lines
Quoted 5 lines
Also, what about all the HB related things like error counters, timers, etc... Seems that we should also do that.
Not sure about that, the first HB may be sent then RTO is expired. Do this may change the orig HB's action.
Well, consider this scenario. If we send a HB+Cookie_ACK to the destination and it gets lost, we wouldn't count this as a strike against the destination, while in reality, it is.
Looks like the timers are taken care of, but a stike should be recorded.
-vlad
Quoted 61 lines
Quoted 4 lines
May be we can just cue the HB to the control queue if we are not replying to the source of INIT-ACK?
If the state is UNCONFIRMED, the transport should not be the source of INIT-ACK, since INIT-ACK is sent to the source of INIT, and it should be in ACTIVE state.
Quoted 24 lines
Quoted 24 lines
net/sctp/output.c +0
-0finish:return retval;}
net/sctp/outqueue.c +1
-0@@ -788,6 +788,7 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout)*/ if (chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT && chunk->chunk_hdr->type !=SCTP_CID_HEARTBEAT_ACK && + chunk->chunk_hdr->type !=SCTP_CID_COOKIE_ACK && chunk->chunk_hdr->type !=SCTP_CID_ASCONF_ACK ) new_transport = asoc->peer.active_path; }net/sctp/sm_make_chunk.c +0
-0@@ -584,6 +584,9 @@ struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc,if (retval && chunk) retval->transport = chunk->transport;Quoted 9 lines
net/sctp/sm_make_chunk.c struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc, · line 590
Quoted 4 lines
net/sctp/sm_make_chunk.c +3
-0+if (retval && !retval->transport) + retval->dest = chunk->source; +This one is good, but I am thinking that transport assignment may be incorrect.
Like I said before, when we are restarting, the transport assigned is from the
existing association, but the packet is transmitted on the temporary association that's about to vanish. Not sure if this is ok.It can works, but may be the transport should not be set, the only thing we should do is set the
retval->dest.Quoted 12 lines
net/sctp/sm_make_chunk.c struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc, · line 592
Quoted 5 lines
net/sctp/sm_make_chunk.c +0
-0return retval; }--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html