From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vlad Yasevich Subject: Re: [PATCH] net: sctp: recover a tranport when an ack comes Date: Fri, 15 Nov 2013 15:35:14 -0500 Message-ID: <52868582.2020509@gmail.com> References: <1384461624-17636-1-git-send-email-changxiangzhong@gmail.com> <5285884F.8030907@gmail.com> <20131115123055.GA29877@hmsreliant.think-freely.org> <5286291A.7060507@gmail.com> <20131115145609.GC29877@hmsreliant.think-freely.org> <52867D2E.2090907@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, linux-sctp@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dreibh@simula.no, ernstgr@simula.no To: Chang , Neil Horman Return-path: Received: from mail-qa0-f41.google.com ([209.85.216.41]:37416 "EHLO mail-qa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751385Ab3KOUfS (ORCPT ); Fri, 15 Nov 2013 15:35:18 -0500 In-Reply-To: <52867D2E.2090907@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: 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 >>>>>> 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@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >