All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chang <changxiangzhong@gmail.com>
To: Vlad Yasevich <vyasevich@gmail.com>, nhorman@tuxdriver.com
Cc: davem@davemloft.net, linux-sctp@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dreibh@simula.no, ernstgr@simula.no
Subject: Re: [PATCH] net: sctp: recover a tranport when an ack comes
Date: Fri, 15 Nov 2013 22:04:28 +0000	[thread overview]
Message-ID: <52869A6C.8060900@gmail.com> (raw)
In-Reply-To: <5285884F.8030907@gmail.com>


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.

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


WARNING: multiple messages have this Message-ID (diff)
From: Chang <changxiangzhong@gmail.com>
To: Vlad Yasevich <vyasevich@gmail.com>, nhorman@tuxdriver.com
Cc: davem@davemloft.net, linux-sctp@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dreibh@simula.no, ernstgr@simula.no
Subject: Re: [PATCH] net: sctp: recover a tranport when an ack comes
Date: Fri, 15 Nov 2013 23:04:28 +0100	[thread overview]
Message-ID: <52869A6C.8060900@gmail.com> (raw)
In-Reply-To: <5285884F.8030907@gmail.com>


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.

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


  parent reply	other threads:[~2013-11-15 22:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-14 20:40 [PATCH] net: sctp: recover a tranport when an ack comes Chang Xiangzhong
2013-11-14 20:40 ` Chang Xiangzhong
2013-11-15  2:34 ` Vlad Yasevich
2013-11-15  2:34   ` Vlad Yasevich
2013-11-15 12:30   ` Neil Horman
2013-11-15 12:30     ` Neil Horman
2013-11-15 14:00     ` Vlad Yasevich
2013-11-15 14:00       ` Vlad Yasevich
2013-11-15 14:56       ` Neil Horman
2013-11-15 14:56         ` Neil Horman
2013-11-15 19:59         ` Chang
2013-11-15 19:59           ` Chang
2013-11-15 20:25           ` Neil Horman
2013-11-15 20:25             ` Neil Horman
2013-11-15 20:35           ` Vlad Yasevich
2013-11-15 20:35             ` Vlad Yasevich
2013-11-15 20:38             ` Chang
2013-11-15 20:38               ` Chang
2013-11-15 22:04   ` Chang [this message]
2013-11-15 22:04     ` Chang
2013-11-15 22:48     ` Vlad Yasevich
2013-11-15 22:48       ` Vlad Yasevich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52869A6C.8060900@gmail.com \
    --to=changxiangzhong@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dreibh@simula.no \
    --cc=ernstgr@simula.no \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.