| Message ID | 1381757623-12803-1-git-send-email-changxiangzhong@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 10/14/2013 09:33 AM, Chang Xiangzhong wrote: > For for each TSN t being newly acked (Not only cumulatively, > but also SELECTIVELY) cacc_saw_newack should be set to 1. > > Signed-off-by: Xiangzhong Chang <changxiangzhong@gmail.com> > --- > net/sctp/outqueue.c | 42 +++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c > index 94df758..d86032b 100644 > --- a/net/sctp/outqueue.c > +++ b/net/sctp/outqueue.c > @@ -1398,6 +1398,27 @@ static void sctp_check_transmitted(struct sctp_outq *q, > forward_progress = true; > } > > + if (!tchunk->tsn_gap_acked) { You can remove this test since the block just above already performs it. Just fold this code into the block above. -vlad > + /* > + * SFR-CACC algorithm: > + * 2) If the SACK contains gap acks > + * and the flag CHANGEOVER_ACTIVE is > + * set the receiver of the SACK MUST > + * take the following action: > + * > + * B) For each TSN t being acked that > + * has not been acked in any SACK so > + * far, set cacc_saw_newack to 1 for > + * the destination that the TSN was > + * sent to. > + */ > + if (transport && > + sack->num_gap_ack_blocks && > + q->asoc->peer.primary_path->cacc. > + changeover_active) > + transport->cacc.cacc_saw_newack = 1; > + } > + > if (TSN_lte(tsn, sack_ctsn)) { > /* RFC 2960 6.3.2 Retransmission Timer Rules > * > @@ -1411,27 +1432,6 @@ static void sctp_check_transmitted(struct sctp_outq *q, > restart_timer = 1; > forward_progress = true; > > - if (!tchunk->tsn_gap_acked) { > - /* > - * SFR-CACC algorithm: > - * 2) If the SACK contains gap acks > - * and the flag CHANGEOVER_ACTIVE is > - * set the receiver of the SACK MUST > - * take the following action: > - * > - * B) For each TSN t being acked that > - * has not been acked in any SACK so > - * far, set cacc_saw_newack to 1 for > - * the destination that the TSN was > - * sent to. > - */ > - if (transport && > - sack->num_gap_ack_blocks && > - q->asoc->peer.primary_path->cacc. > - changeover_active) > - transport->cacc.cacc_saw_newack > - = 1; > - } > > list_add_tail(&tchunk->transmitted_list, > &q->sacked); > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/15/2013 04:11 PM, Vlad Yasevich wrote: > On 10/14/2013 09:33 AM, Chang Xiangzhong wrote: >> For for each TSN t being newly acked (Not only cumulatively, >> but also SELECTIVELY) cacc_saw_newack should be set to 1. >> >> Signed-off-by: Xiangzhong Chang <changxiangzhong@gmail.com> >> --- >> net/sctp/outqueue.c | 42 +++++++++++++++++++++--------------------- >> 1 file changed, 21 insertions(+), 21 deletions(-) >> >> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >> index 94df758..d86032b 100644 >> --- a/net/sctp/outqueue.c >> +++ b/net/sctp/outqueue.c >> @@ -1398,6 +1398,27 @@ static void sctp_check_transmitted(struct >> sctp_outq *q, >> forward_progress = true; >> } >> >> + if (!tchunk->tsn_gap_acked) { > > You can remove this test since the block just above already performs > it. Just fold this code into the block above. > > -vlad > Sorry, I'm not sure if I fully understand you. There are code blocks which checking the tchunk->tsn_gap_acked. In addition, they check other states as well. >> + /* >> + * SFR-CACC algorithm: >> + * 2) If the SACK contains gap acks >> + * and the flag CHANGEOVER_ACTIVE is >> + * set the receiver of the SACK MUST >> + * take the following action: >> + * >> + * B) For each TSN t being acked that >> + * has not been acked in any SACK so >> + * far, set cacc_saw_newack to 1 for >> + * the destination that the TSN was >> + * sent to. >> + */ >> + if (transport && >> + sack->num_gap_ack_blocks && >> + q->asoc->peer.primary_path->cacc. >> + changeover_active) >> + transport->cacc.cacc_saw_newack = 1; >> + } >> + >> if (TSN_lte(tsn, sack_ctsn)) { >> /* RFC 2960 6.3.2 Retransmission Timer Rules >> * >> @@ -1411,27 +1432,6 @@ static void sctp_check_transmitted(struct >> sctp_outq *q, >> restart_timer = 1; >> forward_progress = true; >> >> - if (!tchunk->tsn_gap_acked) { >> - /* >> - * SFR-CACC algorithm: >> - * 2) If the SACK contains gap acks >> - * and the flag CHANGEOVER_ACTIVE is >> - * set the receiver of the SACK MUST >> - * take the following action: >> - * >> - * B) For each TSN t being acked that >> - * has not been acked in any SACK so >> - * far, set cacc_saw_newack to 1 for >> - * the destination that the TSN was >> - * sent to. >> - */ >> - if (transport && >> - sack->num_gap_ack_blocks && >> - q->asoc->peer.primary_path->cacc. >> - changeover_active) >> - transport->cacc.cacc_saw_newack >> - = 1; >> - } >> >> list_add_tail(&tchunk->transmitted_list, >> &q->sacked); >> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/15/2013 10:27 AM, Chang wrote: > > On 10/15/2013 04:11 PM, Vlad Yasevich wrote: >> On 10/14/2013 09:33 AM, Chang Xiangzhong wrote: >>> For for each TSN t being newly acked (Not only cumulatively, >>> but also SELECTIVELY) cacc_saw_newack should be set to 1. >>> >>> Signed-off-by: Xiangzhong Chang <changxiangzhong@gmail.com> >>> --- >>> net/sctp/outqueue.c | 42 +++++++++++++++++++++--------------------- >>> 1 file changed, 21 insertions(+), 21 deletions(-) >>> >>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>> index 94df758..d86032b 100644 >>> --- a/net/sctp/outqueue.c >>> +++ b/net/sctp/outqueue.c >>> @@ -1398,6 +1398,27 @@ static void sctp_check_transmitted(struct >>> sctp_outq *q, >>> forward_progress = true; >>> } >>> >>> + if (!tchunk->tsn_gap_acked) { >> >> You can remove this test since the block just above already performs >> it. Just fold this code into the block above. >> >> -vlad >> > Sorry, I'm not sure if I fully understand you. There are code blocks > which checking the tchunk->tsn_gap_acked. In addition, they check other > states as well. The flow is: if (sctp_acked(sack, tsn)) { ... if (transport) { .... } if (!tchunk->tsn_gap_acked) { .... } if (TSN_lte(tsn, sack_ctsn)) { .... /* SFR-CACC ... } Since you are moving this up, you can simply re-use the if (!tchunk->tsn_gap_acked) immediately above. >>> + /* >>> + * SFR-CACC algorithm: >>> + * 2) If the SACK contains gap acks >>> + * and the flag CHANGEOVER_ACTIVE is >>> + * set the receiver of the SACK MUST >>> + * take the following action: >>> + * >>> + * B) For each TSN t being acked that >>> + * has not been acked in any SACK so >>> + * far, set cacc_saw_newack to 1 for >>> + * the destination that the TSN was >>> + * sent to. >>> + */ >>> + if (transport && >>> + sack->num_gap_ack_blocks && >>> + q->asoc->peer.primary_path->cacc. >>> + changeover_active) >>> + transport->cacc.cacc_saw_newack = 1; ^^^^ Don't need that many spaces... -vlad >>> + } >>> + >>> if (TSN_lte(tsn, sack_ctsn)) { >>> /* RFC 2960 6.3.2 Retransmission Timer Rules >>> * >>> @@ -1411,27 +1432,6 @@ static void sctp_check_transmitted(struct >>> sctp_outq *q, >>> restart_timer = 1; >>> forward_progress = true; >>> >>> - if (!tchunk->tsn_gap_acked) { >>> - /* >>> - * SFR-CACC algorithm: >>> - * 2) If the SACK contains gap acks >>> - * and the flag CHANGEOVER_ACTIVE is >>> - * set the receiver of the SACK MUST >>> - * take the following action: >>> - * >>> - * B) For each TSN t being acked that >>> - * has not been acked in any SACK so >>> - * far, set cacc_saw_newack to 1 for >>> - * the destination that the TSN was >>> - * sent to. >>> - */ >>> - if (transport && >>> - sack->num_gap_ack_blocks && >>> - q->asoc->peer.primary_path->cacc. >>> - changeover_active) >>> - transport->cacc.cacc_saw_newack >>> - = 1; >>> - } >>> >>> list_add_tail(&tchunk->transmitted_list, >>> &q->sacked); >>> >> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks, I've got it and will submit a new patch later. On 10/15/2013 04:34 PM, Vlad Yasevich wrote: > On 10/15/2013 10:27 AM, Chang wrote: >> >> On 10/15/2013 04:11 PM, Vlad Yasevich wrote: >>> On 10/14/2013 09:33 AM, Chang Xiangzhong wrote: >>>> For for each TSN t being newly acked (Not only cumulatively, >>>> but also SELECTIVELY) cacc_saw_newack should be set to 1. >>>> >>>> Signed-off-by: Xiangzhong Chang <changxiangzhong@gmail.com> >>>> --- >>>> net/sctp/outqueue.c | 42 >>>> +++++++++++++++++++++--------------------- >>>> 1 file changed, 21 insertions(+), 21 deletions(-) >>>> >>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>>> index 94df758..d86032b 100644 >>>> --- a/net/sctp/outqueue.c >>>> +++ b/net/sctp/outqueue.c >>>> @@ -1398,6 +1398,27 @@ static void sctp_check_transmitted(struct >>>> sctp_outq *q, >>>> forward_progress = true; >>>> } >>>> >>>> + if (!tchunk->tsn_gap_acked) { >>> >>> You can remove this test since the block just above already performs >>> it. Just fold this code into the block above. >>> >>> -vlad >>> >> Sorry, I'm not sure if I fully understand you. There are code blocks >> which checking the tchunk->tsn_gap_acked. In addition, they check other >> states as well. > > The flow is: > > if (sctp_acked(sack, tsn)) { > ... > if (transport) { > .... > } > > if (!tchunk->tsn_gap_acked) { > .... > } > > if (TSN_lte(tsn, sack_ctsn)) { > .... > /* SFR-CACC ... > } > > Since you are moving this up, you can simply re-use > the if (!tchunk->tsn_gap_acked) immediately above. > >>>> + /* >>>> + * SFR-CACC algorithm: >>>> + * 2) If the SACK contains gap acks >>>> + * and the flag CHANGEOVER_ACTIVE is >>>> + * set the receiver of the SACK MUST >>>> + * take the following action: >>>> + * >>>> + * B) For each TSN t being acked that >>>> + * has not been acked in any SACK so >>>> + * far, set cacc_saw_newack to 1 for >>>> + * the destination that the TSN was >>>> + * sent to. >>>> + */ >>>> + if (transport && >>>> + sack->num_gap_ack_blocks && >>>> + q->asoc->peer.primary_path->cacc. >>>> + changeover_active) >>>> + transport->cacc.cacc_saw_newack = 1; > ^^^^ > > Don't need that many spaces... > > -vlad >>>> + } >>>> + >>>> if (TSN_lte(tsn, sack_ctsn)) { >>>> /* RFC 2960 6.3.2 Retransmission Timer Rules >>>> * >>>> @@ -1411,27 +1432,6 @@ static void sctp_check_transmitted(struct >>>> sctp_outq *q, >>>> restart_timer = 1; >>>> forward_progress = true; >>>> >>>> - if (!tchunk->tsn_gap_acked) { >>>> - /* >>>> - * SFR-CACC algorithm: >>>> - * 2) If the SACK contains gap acks >>>> - * and the flag CHANGEOVER_ACTIVE is >>>> - * set the receiver of the SACK MUST >>>> - * take the following action: >>>> - * >>>> - * B) For each TSN t being acked that >>>> - * has not been acked in any SACK so >>>> - * far, set cacc_saw_newack to 1 for >>>> - * the destination that the TSN was >>>> - * sent to. >>>> - */ >>>> - if (transport && >>>> - sack->num_gap_ack_blocks && >>>> - q->asoc->peer.primary_path->cacc. >>>> - changeover_active) >>>> - transport->cacc.cacc_saw_newack >>>> - = 1; >>>> - } >>>> >>>> list_add_tail(&tchunk->transmitted_list, >>>> &q->sacked); >>>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c index 94df758..d86032b 100644 --- a/net/sctp/outqueue.c +++ b/net/sctp/outqueue.c @@ -1398,6 +1398,27 @@ static void sctp_check_transmitted(struct sctp_outq *q, forward_progress = true; } + if (!tchunk->tsn_gap_acked) { + /* + * SFR-CACC algorithm: + * 2) If the SACK contains gap acks + * and the flag CHANGEOVER_ACTIVE is + * set the receiver of the SACK MUST + * take the following action: + * + * B) For each TSN t being acked that + * has not been acked in any SACK so + * far, set cacc_saw_newack to 1 for + * the destination that the TSN was + * sent to. + */ + if (transport && + sack->num_gap_ack_blocks && + q->asoc->peer.primary_path->cacc. + changeover_active) + transport->cacc.cacc_saw_newack = 1; + } + if (TSN_lte(tsn, sack_ctsn)) { /* RFC 2960 6.3.2 Retransmission Timer Rules * @@ -1411,27 +1432,6 @@ static void sctp_check_transmitted(struct sctp_outq *q, restart_timer = 1; forward_progress = true; - if (!tchunk->tsn_gap_acked) { - /* - * SFR-CACC algorithm: - * 2) If the SACK contains gap acks - * and the flag CHANGEOVER_ACTIVE is - * set the receiver of the SACK MUST - * take the following action: - * - * B) For each TSN t being acked that - * has not been acked in any SACK so - * far, set cacc_saw_newack to 1 for - * the destination that the TSN was - * sent to. - */ - if (transport && - sack->num_gap_ack_blocks && - q->asoc->peer.primary_path->cacc. - changeover_active) - transport->cacc.cacc_saw_newack - = 1; - } list_add_tail(&tchunk->transmitted_list, &q->sacked);
For for each TSN t being newly acked (Not only cumulatively, but also SELECTIVELY) cacc_saw_newack should be set to 1. Signed-off-by: Xiangzhong Chang <changxiangzhong@gmail.com> --- net/sctp/outqueue.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-)