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] [thread-next>] [day] [month] [year] [list]
Message-ID: <52868582.2020509@gmail.com>
Date:	Fri, 15 Nov 2013 15:35:14 -0500
From:	Vlad Yasevich <vyasevich@...il.com>
To:	Chang <changxiangzhong@...il.com>,
	Neil Horman <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 02:59 PM, Chang wrote:
> I've tried to catch you guys up~
>
> Here's a quick question:
>
> Where are the default [transport->pf_retrans] and
> [transport->pathmaxrtx] set?

They are initially set through the sctp_net_init.  Later
they can be changed through sysctl or via socket options.

> I could figure out that they could be set
> through setsockopt(SCTP_PEER_ADDR_THLDS, ...) (but it seems like the
> SCTP library has not supported such option yet, maybe that's due to my
> library is out of date).  So by default both of the two threshold are zero.

pf_retrans starts at 0.  pathmaxrtx has always started at 5.

You should be able to use the socket option directly, but you might need
a newer header.  You can always add the value to your application as well.

>
> Here's my question: Does it go conflict with "the recommended value for
> Path.Max.Retrans in the standard is 5"?

Path.Max.Retrans is set to 5 (at least on my system...)

-vlad

>
> Thanks!
>
> On 11/15/2013 03:56 PM, Neil Horman wrote:
>> On Fri, Nov 15, 2013 at 09:00:58AM -0500, Vlad Yasevich wrote:
>>> On 11/15/2013 07:30 AM, Neil Horman wrote:
>>>> On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
>>>>> On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
>>>>>> Expected Behavior:
>>>>>> When hearing an ack from a tranport/path, set its state to
>>>>>> normal/on if it's
>>>>>> in abnormal(__partial_failure__ or inactive) state.
>>>>>>
>>>>>> state machine of tranport->state
>>>>>> Whenever a T3_RTX timer expires, then transport->error_count++.
>>>>>> When (association->pf_retrans < transport->error_count <
>>>>>> tranport->pathmaxrtx)
>>>>>>     transport->state = SCTP_PF //partial failure
>>>>>>
>>>>>> When a heartbeat-ack comes or conventional ack acknowledged its
>>>>>> availability,
>>>>>>     transport->state = SCTP_ON
>>>>>>
>>>>>> Signed-off-by: Chang Xiangzhong <changxiangzhong@...il.com>
>>>>>> Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from
>>>>>> tsvwg")
>>>>> 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).
>>>>>
>>>>>
>>>>> 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
>>>>>
>>>> I agree, this patch doesn't agree with the spec, the only time we
>>>> transition
>>> >from PF to ACTIVE should be on receipt of ack of new data.
>>>
>>> You mean HB ACK, right?  The 02 spec that see on the ietf site doesn't
>>> mention anything about transition on SACKs.  Also, there is no way to
>>> tell right now if the ack is for new or retransmitted data.  We could
>>> mark chunks that are retransmitted though.
>>>
>> Yes, sorry I wasn't clear, I was speaknig about HB Acks.
>>
>>>> I'm not even sure if
>>>> we should allow PF transports to be selected to send new data.
>>>> Currently a
>>>> potentially failed transport will get ignored when specified, and
>>>> the stack will
>>>> use the active path in its place.  Only if all transports are PF
>>>> will a PF
>>>> transport be chosen.
>>> Not even that :(.  If all transports are PF, we are going to camp on
>>> the primary path instead of choosing a PF transport with the lowest
>>> error count.
>>>
>> Yes, we don't do the smallest error count check, though I have to
>> wonder if
>> thats really worthwhile.  If all your transports are PF, you're a step
>> away from
>> a connection reset anyway.
>> Neil
>>
>>> -vlad
>>>
>>>> Neil
>>>>
>>>>>> ---
>>>>>>   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 linux-sctp" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>

--
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