Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
In-Reply-To: <20191220161756.GE5058@localhost.localdomain>
References: <20191220044703.88-1-qdkevin.kou@gmail.com> <20191220161756.GE5058@localhost.localdomain>
Thread-Topic: Re: [PATCH] sctp: do trace_sctp_probe after SACK validation and check
User-Agent: Android
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
 charset=UTF-8
Subject: Re: [PATCH] sctp: do trace_sctp_probe after SACK validation and check
From:   kevin kou <qdkevin.kou@gmail.com>
Date:   Sat, 21 Dec 2019 13:52:22 +0800
To:     Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
CC:     vyasevich@gmail.com, nhorman@tuxdriver.com, davem@davemloft.net,
        linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Message-ID: <2f449128-7175-4759-b1cb-ccf7d2edadb5@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

As this trace used to trace the changes of SCTP association state in respon=
se to incoming packets(SACK)=2E It is used for debugging SCTP congestion co=
ntrol algorithms, so according to the code in include/trace/events/sctp=2Eh=
, this trace event only prints the below information, and seems it is hard =
to point out if the SACK is duplicate one=2E=C2=A0


TP_printk("asoc=3D%#l=
lx%s ipaddr=3D%pISpc state=3D%u cwnd=3D%u ssthresh=3D%u " "flight_size=3D%u=
 partial_bytes_acked=3D%u pathmtu=3D%u", __entry->asoc, __entry->primary ? =
"(*)" : "", __entry->ipaddr, __entry->state, __entry->cwnd, __entry->ssthre=
sh, __entry->flight_size, __entry->partial_bytes_acked, __entry->pathmtu)

=

2019=E5=B9=B412=E6=9C=8821=E6=97=A5 00:18, 00:18=EF=BC=8C=E5=9C=A8 Marcelo=
 Ricardo Leitner <marcelo=2Eleitner@gmail=2Ecom> =E5=B7=B2=E5=86=99:
>On Fr=
i, Dec 20, 2019 at 04:47:03AM +0000, Kevin Kou wrote:
>> The function sctp_=
sf_eat_sack_6_2 now performs
>> the Verification Tag validation, Chunk leng=
th validation, Bogu check,
>> and also the detection of out-of-order SACK b=
ased on the RFC2960
>> Section 6=2E2 at the beginning, and finally performs=
 the further
>> processing of SACK=2E The trace_sctp_probe now triggered be=
fore
>> the above necessary validation and check=2E
>> 
>> This patch is to=
 do the trace_sctp_probe after the necessary check
>> and validation to SAC=
K=2E
>> 
>> Signed-off-by: Kevin Kou <qdkevin=2Ekou@gmail=2Ecom>
>> ---
>> =
 net/sctp/sm_statefuns=2Ec | 3 ++-
>>  1 file changed, 2 insertions(+), 1 d=
eletion(-)
>> 
>> diff --git a/net/sctp/sm_statefuns=2Ec b/net/sctp/sm_stat=
efuns=2Ec
>> index 42558fa=2E=2Eb4a54df 100644
>> --- a/net/sctp/sm_statefu=
ns=2Ec
>> +++ b/net/sctp/sm_statefuns=2Ec
>> @@ -3281,7 +3281,6 @@ enum sct=
p_disposition
>sctp_sf_eat_sack_6_2(struct net *net,
>>  	struct sctp_sackh=
dr *sackh;
>>  	__u32 ctsn;
>>  
>> -	trace_sctp_probe(ep, asoc, chunk);
>>=
  
>>  	if (!sctp_vtag_verify(chunk, asoc))
>>  		return sctp_sf_pdiscard(n=
et, ep, asoc, type, arg, commands);
>> @@ -3319,6 +3318,8 @@ enum sctp_disp=
osition
>sctp_sf_eat_sack_6_2(struct net *net,
>>  	if (!TSN_lt(ctsn, asoc-=
>next_tsn))
>>  		return sctp_sf_violation_ctsn(net, ep, asoc, type, arg, c=
ommands);
>>  
>> +	trace_sctp_probe(ep, asoc, chunk);
>> +
>
>Moving it he=
re will be after the check against ctsn_ack_point, which
>could cause dupli=
cated SACKs to be missed from the log=2E
>
>Yes, from the sender-side CC we=
 don't care about it (yet), but it
>helps to spot probably avoidable retran=
smissions=2E
>
>I think this is cleaning up the noise too much=2E I can agr=
ee with
>moving it to after the chunk sanity tests, though=2E
>
>>  	/* Ret=
urn this SACK for further processing=2E  */
>>  	sctp_add_cmd_sf(commands, =
SCTP_CMD_PROCESS_SACK,
>SCTP_CHUNK(chunk));
>>  
>> -- 
>> 1=2E8=2E3=2E1
>>=