netdev - Re: [PATCH] net: sctp: fix a cacc_saw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <525D525E.8010508@gmail.com>
Date:	Tue, 15 Oct 2013 10:34:06 -0400
From:	Vlad Yasevich <vyasevich@...il.com>
To:	Chang <changxiangzhong@...il.com>, nhorman@...driver.com
CC:	davem@...emloft.net, linux-sctp@...r.kernel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: sctp: fix a cacc_saw_newack missetting issue

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