Wei Yongjun wrote:
> Vlad Yasevich wrote:
>> Wei Yongjun wrote:
>>
>>> If there is still data waiting to retransmit and remain in
>>> retransmit queue, while doing the next retransmit, if the
>>> chunk is abandoned, we should move it to abandoned list.
>>>
>> I think it might be better to move this to sctp_outq_flush_rtx().
>> Then we can reduce this to just the sctp_chunk_abandoned() check.
>>
> Maybe both place need this check?
>
> Check in sctp_retransmit_mark() can early indicating the retransmit chunk
> is abandoned when we do retransmit and send the FORWARD TSN chunk.
> And check in sctp_retransmit_mark() only miss the process when new DATA
> send is coming and still have retransmit chunk.
>
> If we received SACK, this expired chunks can also be remove to
> abandoned list when we do sctp_outq_sack().
>
> Check in sctp_outq_flush_rtx() aim to check only when new DATA is coming
> and we send the retransmit chunk before the new DATA.
Not only. This will happen on SACKs and timeouts, but I see what you are
trying to catch.
If we have just timeout retransmits and we have more then 1 MTU worth to
retransmit, then it's possible for the retransmit list to hold abandoned
data.
I guess what I don't like is that the code you added has nothing to do
with retransmission. I am OK with the code, but I think it deserves its
own function. It will make it clearer.
-vlad
>
> Just move this code to sctp_outq_flush_rtx() may have other issue. The
> FORWARD TSN chunk is sent before we do sctp_outq_flush_rtx(), so the
> retransmit chunk will remain in abandoned list until next rtx timeout or
> received SACK. If all of the DATA chunks are abandoned, nothing will be
> sent in this timeout.
>
>
>> Remember, retransmit_mark is not executed that often. Also, if
>> we have move then one chunk on the retransmit queue, some part of
>> the queue may expire without us ever running retransmit_mark() again.
>>
>>
>> However, when we flush the queue, we should check for timed out chunks
>> just like when we flush the outqueue.
>>
>
> The outqueue does not need to send the FORWARD TSN chunk
> because it does not hold an TSN. But retransmit queue need to
> send the FORWARD TSN chunk.
>
>
> [PATCH v2] sctp: move chunk from retransmit queue to abandoned list
>
> If there is still data waiting to retransmit and remain in
> retransmit queue, while doing the next retransmit, if the
> chunk is abandoned, we should move it to abandoned list.
>
> Signed-off-by: Wei Yongjun <yjwei@xxxxxxxxxxxxxx>
> ---
> net/sctp/outqueue.c | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index 229690f..d36aea4 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -390,6 +390,18 @@ void sctp_retransmit_mark(struct sctp_outq *q,
> struct list_head *lchunk, *ltemp;
> struct sctp_chunk *chunk;
>
> + /* Walk through the retransmit queue */
> + list_for_each_safe(lchunk, ltemp, &q->retransmit) {
> + chunk = list_entry(lchunk, struct sctp_chunk,
> + transmitted_list);
> +
> + /* If the chunk is abandoned, move it to abandoned list. */
> + if (sctp_chunk_abandoned(chunk)) {
> + list_del_init(lchunk);
> + sctp_insert_list(&q->abandoned, lchunk);
> + }
> + }
> +
> /* Walk through the specified transmitted queue. */
> list_for_each_safe(lchunk, ltemp, &transport->transmitted) {
> chunk = list_entry(lchunk, struct sctp_chunk,
> @@ -578,6 +590,12 @@ static int sctp_outq_flush_rtx(struct sctp_outq *q, struct sctp_packet *pkt,
> * try to send as much as possible.
> */
> list_for_each_entry_safe(chunk, chunk1, lqueue, transmitted_list) {
> + /* If the chunk is abandoned, move it to abandoned list. */
> + if (!rtx_timeout && sctp_chunk_abandoned(chunk)) {
> + list_del_init(&chunk->transmitted_list);
> + sctp_insert_list(&q->abandoned,
> + &chunk->transmitted_list);
> + }
>
> /* Make sure that Gap Acked TSNs are not retransmitted. A
> * simple approach is just to move such TSNs out of the
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
