Re: [PATCH mptcp-net v7 4/5] mptcp: avoid processing packet if a subflow reset

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Fri, 25 Jun 2021, Jianguo Wu wrote:

>
>
> On 2021/6/25 17:07, Matthieu Baerts wrote:
>> Hi Jianguo,
>>
>> Thank you for working on that!
>>
>> On 25/06/2021 10:25, wujianguo106@163.com wrote:
>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>
>>> If check_fully_established() causes a subflow reset, it should not
>>> continue to process the packet in tcp_data_queue().
>>> Add a return value to mptcp_incoming_options(), and return 0 if a
>>> subflow has been reset, else return 1.
>>
>> A small detail but it looks strange to me to return +1.
>> Maybe clearer to return -1 in case of error or return a boolean?
>>
>
> Hi Mat and Geliang,
>
> Based on your comments, I will use boolean type as return value, return false if a subflow has been reset.
> Thanks for your review!

Thanks - I think the 'bool' fits better here than -1/0.

-Mat


>
>> If you decide to return -1, please check for '< 0':
>>
>>   if (mptcp_incoming_options(...) < 0) // error: we discard
>>       goto discard;
>>
>> and not:
>>
>>   if (mptcp_incoming_options(...)) // no error but we discard??
>>       goto discard;
>>
>> (Or replace subflow_is_rst by subflow_is_accepted and return this
>> variable but it might be strange to return that if you don't have a DSS,
>> etc.)
>>
>> But if it is only me who find "strange" to return 0/+1, fine not to
>> change but it looks uncommon and maybe a source of misinterpretation :)
>>
>> Cheers,
>> Matt
>>
>
>

--
Mat Martineau
Intel