netdev - Re: [PATCH] net: sctp: recover a tranport when an ack comes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5286A4D4.10905@gmail.com>
Date:	Fri, 15 Nov 2013 17:48:52 -0500
From:	Vlad Yasevich <vyasevich@...il.com>
To:	Chang <changxiangzhong@...il.com>, nhorman@...driver.com
CC:	davem@...emloft.net, linux-sctp@...r.kernel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	dreibh@...ula.no, ernstgr@...ula.no
Subject: Re: [PATCH] net: sctp: recover a tranport when an ack comes

On 11/15/2013 05:04 PM, Chang wrote:
>
> On 11/15/2013 03:34 AM, Vlad Yasevich wrote:
>> I don't think this is right.  The spec states:
>>  8.  ACKs for retransmissions do not transition a PF destination back
>>        to Active state, since a sender cannot disambiguate whether the
>>        ack was for the original transmission or the retransmission(s).
>>
> Could you please reconsider my proposal?
>
> In the rule 8, it clearly specifies ACKs for *retransmission* do not
> transition ... But those chunks were not retransmitted!
>
> Every transport maintains its own [transport->transmitted] queue, when
> retransmit happens (no matter time_out or fast_retransmit). The chunk
> would be removed from the queue and moved to the sctp_outq->retransmit.
>

Yes,  this is their temporary holding area.  Once the chunks are
transmitted again, they are placed on the transports transmitted list.
See sctp_outq_flush_rtx().

> static void sctp_check_transmitted(...) {
> ...
>     if (transport) {  /*<=======This proves that its not the
> outq->retransmit (the retransmitted queue)*/

Retransmit queue may hold more data then can be drained in a single
push.  Thus is usually holds data that is pending retransmission, but
is not retransmitted yet.  If a late SACK arrives acknowledging this
data, we need to properly mark it.  That is what this code tries
to do.

-vlad
>         if (bytes_acked) {
>         ...
>         if((transport->state in [INACTIVE, UNCONFIRMED, PF]...)
>             sctp_assoc_control_transport(..., SCTP_TRANSPORT_UP).
>
>
> In addition, if its not appropriate to transition PF->ACTIVE, why is it
> appropriate to transition INACTIVE->ACTIVE (the original implementation).
>>
>> Now, the proper way to this would would be modify
>> sctp_assoc_control_transport() to transition the transport state to
>> ACTIVE if it was PF transport that was chosen to send data.
>>
>> -vlad
>>
>>> ---
>>>   net/sctp/outqueue.c |    1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>>> index 94df758..2557fa5 100644
>>> --- a/net/sctp/outqueue.c
>>> +++ b/net/sctp/outqueue.c
>>> @@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct
>>> sctp_outq *q,
>>>                * active if it is not so marked.
>>>                */
>>>               if ((transport->state == SCTP_INACTIVE ||
>>> +                 transport->state == SCTP_PF ||
>>>                    transport->state == SCTP_UNCONFIRMED) &&
>>>                   sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
>>>                   sctp_assoc_control_transport(
>>>
>>
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html