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

[Vlad Yasevich]

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@xxxxxxxxx>
> > > > > > 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@xxxxxxxxxxxxxxx
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/