diff mbox series

[net,3/5] sctp: only allow the asoc reset when the asoc outq is empty

Message ID 49b01e67e4cc1f8a0be85ffefdee51dad6ae3286.1511614961.git.lucien.xin@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series sctp: a bunch of fixes for stream reconfig | expand

Commit Message

Xin Long Nov. 25, 2017, 1:05 p.m. UTC
As it says in rfc6525#section5.1.4, before sending the request,

   C2:  The sender has either no outstanding TSNs or considers all
        outstanding TSNs abandoned.

Prior to this patch, it tried to consider all outstanding TSNs abandoned
by dropping all chunks in all outqs with sctp_outq_free (even including
sacked, retransmit and transmitted queues) when doing this reset, which
is too aggressive.

To make it work gently, this patch will only allow the asoc reset when
the sender has no outstanding TSNs by checking if unsent, transmitted
and retransmit are all empty with sctp_outq_is_empty before sending
and processing the request.

Fixes: 692787cef651 ("sctp: implement receiver-side procedures for the SSN/TSN Reset Request Parameter")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/stream.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

David Laight Nov. 27, 2017, 10:06 a.m. UTC | #1
From: Xin Long
> Sent: 25 November 2017 13:06
> 
> As it says in rfc6525#section5.1.4, before sending the request,
> 
>    C2:  The sender has either no outstanding TSNs or considers all
>         outstanding TSNs abandoned.
> 
> Prior to this patch, it tried to consider all outstanding TSNs abandoned
> by dropping all chunks in all outqs with sctp_outq_free (even including
> sacked, retransmit and transmitted queues) when doing this reset, which
> is too aggressive.
> 
> To make it work gently, this patch will only allow the asoc reset when
> the sender has no outstanding TSNs by checking if unsent, transmitted
> and retransmit are all empty with sctp_outq_is_empty before sending
> and processing the request.

Doesn't that rather defeat the point of a reset?

	David
Xin Long Nov. 27, 2017, 10:58 a.m. UTC | #2
On Mon, Nov 27, 2017 at 6:06 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Xin Long
>> Sent: 25 November 2017 13:06
>>
>> As it says in rfc6525#section5.1.4, before sending the request,
>>
>>    C2:  The sender has either no outstanding TSNs or considers all
>>         outstanding TSNs abandoned.
>>
>> Prior to this patch, it tried to consider all outstanding TSNs abandoned
>> by dropping all chunks in all outqs with sctp_outq_free (even including
>> sacked, retransmit and transmitted queues) when doing this reset, which
>> is too aggressive.
>>
>> To make it work gently, this patch will only allow the asoc reset when
>> the sender has no outstanding TSNs by checking if unsent, transmitted
>> and retransmit are all empty with sctp_outq_is_empty before sending
>> and processing the request.
>
> Doesn't that rather defeat the point of a reset?
No, reset here means to reset the next tsn/ssn, not with the cost of
losing outstanding data, though RFC6525 allows to consider them
abandoned.
Marcelo Ricardo Leitner Nov. 27, 2017, 1:36 p.m. UTC | #3
On Sat, Nov 25, 2017 at 09:05:34PM +0800, Xin Long wrote:
> As it says in rfc6525#section5.1.4, before sending the request,
> 
>    C2:  The sender has either no outstanding TSNs or considers all
>         outstanding TSNs abandoned.
> 
> Prior to this patch, it tried to consider all outstanding TSNs abandoned
> by dropping all chunks in all outqs with sctp_outq_free (even including
> sacked, retransmit and transmitted queues) when doing this reset, which
> is too aggressive.
> 
> To make it work gently, this patch will only allow the asoc reset when
> the sender has no outstanding TSNs by checking if unsent, transmitted
> and retransmit are all empty with sctp_outq_is_empty before sending
> and processing the request.
> 
> Fixes: 692787cef651 ("sctp: implement receiver-side procedures for the SSN/TSN Reset Request Parameter")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/stream.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index b209037..f3b7d27 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -406,6 +406,9 @@ int sctp_send_reset_assoc(struct sctp_association *asoc)
>  	if (asoc->strreset_outstanding)
>  		return -EINPROGRESS;
>  
> +	if (!sctp_outq_is_empty(&asoc->outqueue))
> +		return -EAGAIN;
> +
>  	chunk = sctp_make_strreset_tsnreq(asoc);
>  	if (!chunk)
>  		return -ENOMEM;
> @@ -728,6 +731,12 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
>  		}
>  		goto err;
>  	}
> +
> +	if (!sctp_outq_is_empty(&asoc->outqueue)) {
> +		result = SCTP_STRRESET_IN_PROGRESS;
> +		goto err;
> +	}
> +
>  	asoc->strreset_inseq++;
>  
>  	if (!(asoc->strreset_enable & SCTP_ENABLE_RESET_ASSOC_REQ))
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
diff mbox series

Patch

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index b209037..f3b7d27 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -406,6 +406,9 @@  int sctp_send_reset_assoc(struct sctp_association *asoc)
 	if (asoc->strreset_outstanding)
 		return -EINPROGRESS;
 
+	if (!sctp_outq_is_empty(&asoc->outqueue))
+		return -EAGAIN;
+
 	chunk = sctp_make_strreset_tsnreq(asoc);
 	if (!chunk)
 		return -ENOMEM;
@@ -728,6 +731,12 @@  struct sctp_chunk *sctp_process_strreset_tsnreq(
 		}
 		goto err;
 	}
+
+	if (!sctp_outq_is_empty(&asoc->outqueue)) {
+		result = SCTP_STRRESET_IN_PROGRESS;
+		goto err;
+	}
+
 	asoc->strreset_inseq++;
 
 	if (!(asoc->strreset_enable & SCTP_ENABLE_RESET_ASSOC_REQ))