* Re: [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans'.
2014-04-25 17:06 [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans' Karl Heiss
@ 2014-04-25 17:21 ` Karl Heiss
2014-04-25 17:34 ` Vlad Yasevich
2014-04-25 17:22 ` Vlad Yasevich
2014-04-25 18:40 ` Neil Horman
2 siblings, 1 reply; 5+ messages in thread
From: Karl Heiss @ 2014-04-25 17:21 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, davem, Vladislav Yasevich, Neil Horman
On Fri, Apr 25, 2014 at 1:06 PM, Karl Heiss <kheiss@gmail.com> wrote:
> Don't transition to the PF state on every strike after 'Path.Max.Retrans'.
> Per draft-ietf-tsvwg-sctp-failover-03 Section 5.1.6:
>
> Additional (PMR - PFMR) consecutive timeouts on a PF destination
> confirm the path failure, upon which the destination transitions to the
> Inactive state. As described in [RFC4960], the sender (i) SHOULD notify
> ULP about this state transition, and (ii) transmit heartbeats to the
> Inactive destination at a lower frequency as described in Section 8.3 of
> [RFC4960].
>
> This also prevents sending SCTP_ADDR_UNREACHABLE to the user as the state
> bounces between SCTP_INACTIVE and SCTP_PF for each subsequent strike.
>
> Signed-off-by: Karl Heiss <kheiss@gmail.com>
> ---
> Changes in v2:
> - Flesh out commit message
>
> Changes in v3:
> - Compare transport state instead of pathmaxrxt
>
> net/sctp/sm_sideeffect.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 5d6883f..7805623 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -496,11 +496,12 @@ static void sctp_do_8_2_transport_strike(sctp_cmd_seq_t *commands,
>
> /* If the transport error count is greater than the pf_retrans
> * threshold, and less than pathmaxrtx, and if the current state
> - * is not SCTP_UNCONFIRMED, then mark this transport as Partially
> - * Failed, see SCTP Quick Failover Draft, section 5.1
> + * is not SCTP_UNCONFIRMED or SCTP_INACTIVE, then mark this transport
> + * as Partially Failed, see SCTP Quick Failover Draft, section 5.1
> */
> if ((transport->state != SCTP_PF) &&
> (transport->state != SCTP_UNCONFIRMED) &&
> + (transport->state != SCTP_INACTIVE) &&
On second thought, would this not make more sense to be:
if ((transport->state == SCTP_ACTIVE) &&
Since the only valid transition into PF is from ACTIVE?
> (asoc->pf_retrans < transport->pathmaxrxt) &&
> (transport->error_count > asoc->pf_retrans)) {
>
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans'.
2014-04-25 17:21 ` Karl Heiss
@ 2014-04-25 17:34 ` Vlad Yasevich
0 siblings, 0 replies; 5+ messages in thread
From: Vlad Yasevich @ 2014-04-25 17:34 UTC (permalink / raw)
To: Karl Heiss, netdev; +Cc: linux-sctp, davem, Neil Horman
On 04/25/2014 01:21 PM, Karl Heiss wrote:
> On Fri, Apr 25, 2014 at 1:06 PM, Karl Heiss <kheiss@gmail.com> wrote:
>> Don't transition to the PF state on every strike after 'Path.Max.Retrans'.
>> Per draft-ietf-tsvwg-sctp-failover-03 Section 5.1.6:
>>
>> Additional (PMR - PFMR) consecutive timeouts on a PF destination
>> confirm the path failure, upon which the destination transitions to the
>> Inactive state. As described in [RFC4960], the sender (i) SHOULD notify
>> ULP about this state transition, and (ii) transmit heartbeats to the
>> Inactive destination at a lower frequency as described in Section 8.3 of
>> [RFC4960].
>>
>> This also prevents sending SCTP_ADDR_UNREACHABLE to the user as the state
>> bounces between SCTP_INACTIVE and SCTP_PF for each subsequent strike.
>>
>> Signed-off-by: Karl Heiss <kheiss@gmail.com>
>> ---
>> Changes in v2:
>> - Flesh out commit message
>>
>> Changes in v3:
>> - Compare transport state instead of pathmaxrxt
>>
>> net/sctp/sm_sideeffect.c | 5 +++--
>> 1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 5d6883f..7805623 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -496,11 +496,12 @@ static void sctp_do_8_2_transport_strike(sctp_cmd_seq_t *commands,
>>
>> /* If the transport error count is greater than the pf_retrans
>> * threshold, and less than pathmaxrtx, and if the current state
>> - * is not SCTP_UNCONFIRMED, then mark this transport as Partially
>> - * Failed, see SCTP Quick Failover Draft, section 5.1
>> + * is not SCTP_UNCONFIRMED or SCTP_INACTIVE, then mark this transport
>> + * as Partially Failed, see SCTP Quick Failover Draft, section 5.1
>> */
>> if ((transport->state != SCTP_PF) &&
>> (transport->state != SCTP_UNCONFIRMED) &&
>> + (transport->state != SCTP_INACTIVE) &&
>
> On second thought, would this not make more sense to be:
>
> if ((transport->state == SCTP_ACTIVE) &&
>
> Since the only valid transition into PF is from ACTIVE?
Hmm.. we that UNKNOWN state, but it doesn't look like it survives
past the handshake...
So, yes, this would simplify things.
-vlad
>
>> (asoc->pf_retrans < transport->pathmaxrxt) &&
>> (transport->error_count > asoc->pf_retrans)) {
>>
>> --
>> 1.7.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans'.
2014-04-25 17:06 [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans' Karl Heiss
2014-04-25 17:21 ` Karl Heiss
@ 2014-04-25 17:22 ` Vlad Yasevich
2014-04-25 18:40 ` Neil Horman
2 siblings, 0 replies; 5+ messages in thread
From: Vlad Yasevich @ 2014-04-25 17:22 UTC (permalink / raw)
To: Karl Heiss, netdev; +Cc: linux-sctp, davem, nhorman
On 04/25/2014 01:06 PM, Karl Heiss wrote:
> Don't transition to the PF state on every strike after 'Path.Max.Retrans'.
> Per draft-ietf-tsvwg-sctp-failover-03 Section 5.1.6:
>
> Additional (PMR - PFMR) consecutive timeouts on a PF destination
> confirm the path failure, upon which the destination transitions to the
> Inactive state. As described in [RFC4960], the sender (i) SHOULD notify
> ULP about this state transition, and (ii) transmit heartbeats to the
> Inactive destination at a lower frequency as described in Section 8.3 of
> [RFC4960].
>
> This also prevents sending SCTP_ADDR_UNREACHABLE to the user as the state
> bounces between SCTP_INACTIVE and SCTP_PF for each subsequent strike.
>
> Signed-off-by: Karl Heiss <kheiss@gmail.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> Changes in v2:
> - Flesh out commit message
>
> Changes in v3:
> - Compare transport state instead of pathmaxrxt
>
> net/sctp/sm_sideeffect.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 5d6883f..7805623 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -496,11 +496,12 @@ static void sctp_do_8_2_transport_strike(sctp_cmd_seq_t *commands,
>
> /* If the transport error count is greater than the pf_retrans
> * threshold, and less than pathmaxrtx, and if the current state
> - * is not SCTP_UNCONFIRMED, then mark this transport as Partially
> - * Failed, see SCTP Quick Failover Draft, section 5.1
> + * is not SCTP_UNCONFIRMED or SCTP_INACTIVE, then mark this transport
> + * as Partially Failed, see SCTP Quick Failover Draft, section 5.1
> */
> if ((transport->state != SCTP_PF) &&
> (transport->state != SCTP_UNCONFIRMED) &&
> + (transport->state != SCTP_INACTIVE) &&
> (asoc->pf_retrans < transport->pathmaxrxt) &&
> (transport->error_count > asoc->pf_retrans)) {
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans'.
2014-04-25 17:06 [PATCH v3 net] net: sctp: Don't transition to PF state when transport has exhausted 'Path.Max.Retrans' Karl Heiss
2014-04-25 17:21 ` Karl Heiss
2014-04-25 17:22 ` Vlad Yasevich
@ 2014-04-25 18:40 ` Neil Horman
2 siblings, 0 replies; 5+ messages in thread
From: Neil Horman @ 2014-04-25 18:40 UTC (permalink / raw)
To: Karl Heiss; +Cc: netdev, linux-sctp, davem, vyasevich
On Fri, Apr 25, 2014 at 01:06:10PM -0400, Karl Heiss wrote:
> Don't transition to the PF state on every strike after 'Path.Max.Retrans'.
> Per draft-ietf-tsvwg-sctp-failover-03 Section 5.1.6:
>
> Additional (PMR - PFMR) consecutive timeouts on a PF destination
> confirm the path failure, upon which the destination transitions to the
> Inactive state. As described in [RFC4960], the sender (i) SHOULD notify
> ULP about this state transition, and (ii) transmit heartbeats to the
> Inactive destination at a lower frequency as described in Section 8.3 of
> [RFC4960].
>
> This also prevents sending SCTP_ADDR_UNREACHABLE to the user as the state
> bounces between SCTP_INACTIVE and SCTP_PF for each subsequent strike.
>
> Signed-off-by: Karl Heiss <kheiss@gmail.com>
> ---
> Changes in v2:
> - Flesh out commit message
>
> Changes in v3:
> - Compare transport state instead of pathmaxrxt
>
> net/sctp/sm_sideeffect.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 5d6883f..7805623 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -496,11 +496,12 @@ static void sctp_do_8_2_transport_strike(sctp_cmd_seq_t *commands,
>
> /* If the transport error count is greater than the pf_retrans
> * threshold, and less than pathmaxrtx, and if the current state
> - * is not SCTP_UNCONFIRMED, then mark this transport as Partially
> - * Failed, see SCTP Quick Failover Draft, section 5.1
> + * is not SCTP_UNCONFIRMED or SCTP_INACTIVE, then mark this transport
> + * as Partially Failed, see SCTP Quick Failover Draft, section 5.1
> */
> if ((transport->state != SCTP_PF) &&
> (transport->state != SCTP_UNCONFIRMED) &&
> + (transport->state != SCTP_INACTIVE) &&
> (asoc->pf_retrans < transport->pathmaxrxt) &&
> (transport->error_count > asoc->pf_retrans)) {
>
> --
> 1.7.1
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 5+ messages in thread