[PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang Xiangzhong]

Look for the __two__ most recently used path/transport and set to active_path
and retran_path respectively

Signed-off-by: changxiangzhong@gmail.com
---
 net/sctp/associola.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index ab67efc..070011a 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		if (!first || t->last_time_heard > first->last_time_heard) {
 			second = first;
 			first = t;
+			continue;
 		}
 		if (!second || t->last_time_heard > second->last_time_heard)
 			second = t;
 	}
 
+	if (!second)
+		second = first;
+
 	/* RFC 2960 6.4 Multi-Homed SCTP Endpoints
 	 *
 	 * By default, an endpoint should always transmit to the
-- 
1.7.9.5

[PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang Xiangzhong]

Look for the __two__ most recently used path/transport and set to active_path
and retran_path respectively

Signed-off-by: changxiangzhong@gmail.com
---
 net/sctp/associola.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index ab67efc..070011a 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		if (!first || t->last_time_heard > first->last_time_heard) {
 			second = first;
 			first = t;
+			continue;
 		}
 		if (!second || t->last_time_heard > second->last_time_heard)
 			second = t;
 	}
 
+	if (!second)
+		second = first;
+
 	/* RFC 2960 6.4 Multi-Homed SCTP Endpoints
 	 *
 	 * By default, an endpoint should always transmit to the
-- 
1.7.9.5

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Vlad Yasevich]

On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
> Look for the __two__ most recently used path/transport and set to active_path
> and retran_path respectively
>
> Signed-off-by: changxiangzhong@gmail.com
> ---
>   net/sctp/associola.c |    4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index ab67efc..070011a 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
>   		if (!first || t->last_time_heard > first->last_time_heard) {
>   			second = first;
>   			first = t;
> +			continue;
>   		}
>   		if (!second || t->last_time_heard > second->last_time_heard)
>   			second = t;

You might as well remove this bit and then you don't need a continue.

>   	}
>
> +	if (!second)
> +		second = first;
> +

This needs to move down 1 more block.  Set the second transport after we
check to see if the primary is back up and we need to go back to using it.

-vlad

>   	/* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>   	 *
>   	 * By default, an endpoint should always transmit to the
>

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Vlad Yasevich]

On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
> Look for the __two__ most recently used path/transport and set to active_path
> and retran_path respectively
>
> Signed-off-by: changxiangzhong@gmail.com
> ---
>   net/sctp/associola.c |    4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index ab67efc..070011a 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
>   		if (!first || t->last_time_heard > first->last_time_heard) {
>   			second = first;
>   			first = t;
> +			continue;
>   		}
>   		if (!second || t->last_time_heard > second->last_time_heard)
>   			second = t;

You might as well remove this bit and then you don't need a continue.

>   	}
>
> +	if (!second)
> +		second = first;
> +

This needs to move down 1 more block.  Set the second transport after we
check to see if the primary is back up and we need to go back to using it.

-vlad

>   	/* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>   	 *
>   	 * By default, an endpoint should always transmit to the
>

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>> Look for the __two__ most recently used path/transport and set to 
>> active_path
>> and retran_path respectively
>>
>> Signed-off-by: changxiangzhong@gmail.com
>> ---
>>   net/sctp/associola.c |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> index ab67efc..070011a 100644
>> --- a/net/sctp/associola.c
>> +++ b/net/sctp/associola.c
>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct 
>> sctp_association *asoc,
>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>               second = first;
>>               first = t;
>> +            continue;
>>           }
>>           if (!second || t->last_time_heard > second->last_time_heard)
>>               second = t;
>
> You might as well remove this bit and then you don't need a continue.
I don't think we could remove this bit. My understanding of these 
algorithms are to find the 1st recently used path and the 2nd, assigning 
to active_path and retran_path respectively. If we remove the 
looking-for-second block, how are we suppose to find the 2nd?
I think we can remove the continue and use else-if in the 
2nd-assignment-block.
>
>>       }
>>
>> +    if (!second)
>> +        second = first;
>> +
>
> This needs to move down 1 more block.  Set the second transport after we
> check to see if the primary is back up and we need to go back to using 
> it.
>
> -vlad
>
I agree with this change
>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>        *
>>        * By default, an endpoint should always transmit to the
>>
>

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>> Look for the __two__ most recently used path/transport and set to 
>> active_path
>> and retran_path respectively
>>
>> Signed-off-by: changxiangzhong@gmail.com
>> ---
>>   net/sctp/associola.c |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> index ab67efc..070011a 100644
>> --- a/net/sctp/associola.c
>> +++ b/net/sctp/associola.c
>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct 
>> sctp_association *asoc,
>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>               second = first;
>>               first = t;
>> +            continue;
>>           }
>>           if (!second || t->last_time_heard > second->last_time_heard)
>>               second = t;
>
> You might as well remove this bit and then you don't need a continue.
I don't think we could remove this bit. My understanding of these 
algorithms are to find the 1st recently used path and the 2nd, assigning 
to active_path and retran_path respectively. If we remove the 
looking-for-second block, how are we suppose to find the 2nd?
I think we can remove the continue and use else-if in the 
2nd-assignment-block.
>
>>       }
>>
>> +    if (!second)
>> +        second = first;
>> +
>
> This needs to move down 1 more block.  Set the second transport after we
> check to see if the primary is back up and we need to go back to using 
> it.
>
> -vlad
>
I agree with this change
>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>        *
>>        * By default, an endpoint should always transmit to the
>>
>

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Daniel Borkmann]

On 11/13/2013 03:54 AM, Chang wrote:
> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>> Look for the __two__ most recently used path/transport and set to active_path
>>> and retran_path respectively

Please also for the log, elaborate a bit more, explaining what currently
happens, and what the effects of this bug are, so that later when people
are looking through the Git log they can easily get what problem you are
trying to fix; and if possible, add:

Fixes: <12 digits SHA1> ("<commit title>")

>>> Signed-off-by: changxiangzhong@gmail.com
>>> ---
>>>   net/sctp/associola.c |    4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index ab67efc..070011a 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
>>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>>               second = first;
>>>               first = t;
>>> +            continue;
>>>           }
>>>           if (!second || t->last_time_heard > second->last_time_heard)
>>>               second = t;
>>
>> You might as well remove this bit and then you don't need a continue.
> I don't think we could remove this bit. My understanding of these algorithms are to find the 1st recently used path and the 2nd, assigning to active_path and retran_path respectively. If we remove the looking-for-second block, how are we suppose to find the 2nd?
> I think we can remove the continue and use else-if in the 2nd-assignment-block.
>>
>>>       }
>>>
>>> +    if (!second)
>>> +        second = first;
>>> +
>>
>> This needs to move down 1 more block.  Set the second transport after we
>> check to see if the primary is back up and we need to go back to using it.
>>
>> -vlad
>>
> I agree with this change
>>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>        *
>>>        * By default, an endpoint should always transmit to the
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Daniel Borkmann]

On 11/13/2013 03:54 AM, Chang wrote:
> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>> Look for the __two__ most recently used path/transport and set to active_path
>>> and retran_path respectively

Please also for the log, elaborate a bit more, explaining what currently
happens, and what the effects of this bug are, so that later when people
are looking through the Git log they can easily get what problem you are
trying to fix; and if possible, add:

Fixes: <12 digits SHA1> ("<commit title>")

>>> Signed-off-by: changxiangzhong@gmail.com
>>> ---
>>>   net/sctp/associola.c |    4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index ab67efc..070011a 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
>>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>>               second = first;
>>>               first = t;
>>> +            continue;
>>>           }
>>>           if (!second || t->last_time_heard > second->last_time_heard)
>>>               second = t;
>>
>> You might as well remove this bit and then you don't need a continue.
> I don't think we could remove this bit. My understanding of these algorithms are to find the 1st recently used path and the 2nd, assigning to active_path and retran_path respectively. If we remove the looking-for-second block, how are we suppose to find the 2nd?
> I think we can remove the continue and use else-if in the 2nd-assignment-block.
>>
>>>       }
>>>
>>> +    if (!second)
>>> +        second = first;
>>> +
>>
>> This needs to move down 1 more block.  Set the second transport after we
>> check to see if the primary is back up and we need to go back to using it.
>>
>> -vlad
>>
> I agree with this change
>>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>        *
>>>        * By default, an endpoint should always transmit to the
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Vlad Yasevich]

On 11/12/2013 09:54 PM, Chang wrote:
>
> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>> Look for the __two__ most recently used path/transport and set to
>>> active_path
>>> and retran_path respectively
>>>
>>> Signed-off-by: changxiangzhong@gmail.com
>>> ---
>>>   net/sctp/associola.c |    4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index ab67efc..070011a 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct
>>> sctp_association *asoc,
>>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>>               second = first;
>>>               first = t;
>>> +            continue;
>>>           }
>>>           if (!second || t->last_time_heard > second->last_time_heard)
>>>               second = t;
>>
>> You might as well remove this bit and then you don't need a continue.
> I don't think we could remove this bit. My understanding of these
> algorithms are to find the 1st recently used path and the 2nd, assigning
> to active_path and retran_path respectively. If we remove the
> looking-for-second block, how are we suppose to find the 2nd?
> I think we can remove the continue and use else-if in the
> 2nd-assignment-block.

Yes, you are right.  An if...else block is what we need here.

-vlad

>>
>>>       }
>>>
>>> +    if (!second)
>>> +        second = first;
>>> +
>>
>> This needs to move down 1 more block.  Set the second transport after we
>> check to see if the primary is back up and we need to go back to using
>> it.
>>
>> -vlad
>>
> I agree with this change
>>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>        *
>>>        * By default, an endpoint should always transmit to the
>>>
>>
>

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Vlad Yasevich]

On 11/12/2013 09:54 PM, Chang wrote:
>
> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>> Look for the __two__ most recently used path/transport and set to
>>> active_path
>>> and retran_path respectively
>>>
>>> Signed-off-by: changxiangzhong@gmail.com
>>> ---
>>>   net/sctp/associola.c |    4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index ab67efc..070011a 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct
>>> sctp_association *asoc,
>>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>>               second = first;
>>>               first = t;
>>> +            continue;
>>>           }
>>>           if (!second || t->last_time_heard > second->last_time_heard)
>>>               second = t;
>>
>> You might as well remove this bit and then you don't need a continue.
> I don't think we could remove this bit. My understanding of these
> algorithms are to find the 1st recently used path and the 2nd, assigning
> to active_path and retran_path respectively. If we remove the
> looking-for-second block, how are we suppose to find the 2nd?
> I think we can remove the continue and use else-if in the
> 2nd-assignment-block.

Yes, you are right.  An if...else block is what we need here.

-vlad

>>
>>>       }
>>>
>>> +    if (!second)
>>> +        second = first;
>>> +
>>
>> This needs to move down 1 more block.  Set the second transport after we
>> check to see if the primary is back up and we need to go back to using
>> it.
>>
>> -vlad
>>
> I agree with this change
>>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>        *
>>>        * By default, an endpoint should always transmit to the
>>>
>>
>

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
> On 11/13/2013 03:54 AM, Chang wrote:
>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>> Look for the __two__ most recently used path/transport and set to 
>>>> active_path
>>>> and retran_path respectively
>
> Please also for the log, elaborate a bit more, explaining what currently
> happens, and what the effects of this bug are, so that later when people
> are looking through the Git log they can easily get what problem you are
> trying to fix; and if possible, add:
>
> Fixes: <12 digits SHA1> ("<commit title>")
>
Yeah, sure, I'll elaborate that more specifically.
I assume the 12-digit SHA1 is the revision number. But may I ask where 
and how shall I add the tag "Fixes" tag? The revision number is 
generated after "git commit", how can I know that in advance?

Best Regards!

>>>> Signed-off-by: changxiangzhong@gmail.com
>>>> ---
>>>>   net/sctp/associola.c |    4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>>> index ab67efc..070011a 100644
>>>> --- a/net/sctp/associola.c
>>>> +++ b/net/sctp/associola.c
>>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct 
>>>> sctp_association *asoc,
>>>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>>>               second = first;
>>>>               first = t;
>>>> +            continue;
>>>>           }
>>>>           if (!second || t->last_time_heard > second->last_time_heard)
>>>>               second = t;
>>>
>>> You might as well remove this bit and then you don't need a continue.
>> I don't think we could remove this bit. My understanding of these 
>> algorithms are to find the 1st recently used path and the 2nd, 
>> assigning to active_path and retran_path respectively. If we remove 
>> the looking-for-second block, how are we suppose to find the 2nd?
>> I think we can remove the continue and use else-if in the 
>> 2nd-assignment-block.
>>>
>>>>       }
>>>>
>>>> +    if (!second)
>>>> +        second = first;
>>>> +
>>>
>>> This needs to move down 1 more block.  Set the second transport 
>>> after we
>>> check to see if the primary is back up and we need to go back to 
>>> using it.
>>>
>>> -vlad
>>>
>> I agree with this change
>>>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>>        *
>>>>        * By default, an endpoint should always transmit to the
>>>>
>>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
> On 11/13/2013 03:54 AM, Chang wrote:
>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>> Look for the __two__ most recently used path/transport and set to 
>>>> active_path
>>>> and retran_path respectively
>
> Please also for the log, elaborate a bit more, explaining what currently
> happens, and what the effects of this bug are, so that later when people
> are looking through the Git log they can easily get what problem you are
> trying to fix; and if possible, add:
>
> Fixes: <12 digits SHA1> ("<commit title>")
>
Yeah, sure, I'll elaborate that more specifically.
I assume the 12-digit SHA1 is the revision number. But may I ask where 
and how shall I add the tag "Fixes" tag? The revision number is 
generated after "git commit", how can I know that in advance?

Best Regards!

>>>> Signed-off-by: changxiangzhong@gmail.com
>>>> ---
>>>>   net/sctp/associola.c |    4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>>> index ab67efc..070011a 100644
>>>> --- a/net/sctp/associola.c
>>>> +++ b/net/sctp/associola.c
>>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct 
>>>> sctp_association *asoc,
>>>>           if (!first || t->last_time_heard > first->last_time_heard) {
>>>>               second = first;
>>>>               first = t;
>>>> +            continue;
>>>>           }
>>>>           if (!second || t->last_time_heard > second->last_time_heard)
>>>>               second = t;
>>>
>>> You might as well remove this bit and then you don't need a continue.
>> I don't think we could remove this bit. My understanding of these 
>> algorithms are to find the 1st recently used path and the 2nd, 
>> assigning to active_path and retran_path respectively. If we remove 
>> the looking-for-second block, how are we suppose to find the 2nd?
>> I think we can remove the continue and use else-if in the 
>> 2nd-assignment-block.
>>>
>>>>       }
>>>>
>>>> +    if (!second)
>>>> +        second = first;
>>>> +
>>>
>>> This needs to move down 1 more block.  Set the second transport 
>>> after we
>>> check to see if the primary is back up and we need to go back to 
>>> using it.
>>>
>>> -vlad
>>>
>> I agree with this change
>>>>       /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>>        *
>>>>        * By default, an endpoint should always transmit to the
>>>>
>>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Daniel Borkmann]

On 11/13/2013 08:06 PM, Chang wrote:
> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>> On 11/13/2013 03:54 AM, Chang wrote:
>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>> Look for the __two__ most recently used path/transport and set to active_path
>>>>> and retran_path respectively
>>
>> Please also for the log, elaborate a bit more, explaining what currently
>> happens, and what the effects of this bug are, so that later when people
>> are looking through the Git log they can easily get what problem you are
>> trying to fix; and if possible, add:
>>
>> Fixes: <12 digits SHA1> ("<commit title>")
>>
> Yeah, sure, I'll elaborate that more specifically.

Thanks !

> I assume the 12-digit SHA1 is the revision number. But may I ask where and how shall I add the tag "Fixes" tag? The revision number is generated after "git commit", how can I know that in advance?

Nope, it's the affected commit id from the current git log that
your patch fixes.

Have a look for example at commit:

98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Daniel Borkmann]

On 11/13/2013 08:06 PM, Chang wrote:
> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>> On 11/13/2013 03:54 AM, Chang wrote:
>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>> Look for the __two__ most recently used path/transport and set to active_path
>>>>> and retran_path respectively
>>
>> Please also for the log, elaborate a bit more, explaining what currently
>> happens, and what the effects of this bug are, so that later when people
>> are looking through the Git log they can easily get what problem you are
>> trying to fix; and if possible, add:
>>
>> Fixes: <12 digits SHA1> ("<commit title>")
>>
> Yeah, sure, I'll elaborate that more specifically.

Thanks !

> I assume the 12-digit SHA1 is the revision number. But may I ask where and how shall I add the tag "Fixes" tag? The revision number is generated after "git commit", how can I know that in advance?

Nope, it's the affected commit id from the current git log that
your patch fixes.

Have a look for example at commit:

98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
> On 11/13/2013 08:06 PM, Chang wrote:
>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>> Look for the __two__ most recently used path/transport and set to 
>>>>>> active_path
>>>>>> and retran_path respectively
>>>
>>> Please also for the log, elaborate a bit more, explaining what 
>>> currently
>>> happens, and what the effects of this bug are, so that later when 
>>> people
>>> are looking through the Git log they can easily get what problem you 
>>> are
>>> trying to fix; and if possible, add:
>>>
>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>
>> Yeah, sure, I'll elaborate that more specifically.
>
> Thanks !
>
>> I assume the 12-digit SHA1 is the revision number. But may I ask 
>> where and how shall I add the tag "Fixes" tag? The revision number is 
>> generated after "git commit", how can I know that in advance?
>
> Nope, it's the affected commit id from the current git log that
> your patch fixes.
>
> Have a look for example at commit:
>
> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
Thank you for your quick response. I'm quite green on kernel programming 
and git. So here's one question:
To find the the revision that **caused** the bug, I could use gitk to 
trace the changing of the file(s) history. Is that correct?

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
> On 11/13/2013 08:06 PM, Chang wrote:
>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>> Look for the __two__ most recently used path/transport and set to 
>>>>>> active_path
>>>>>> and retran_path respectively
>>>
>>> Please also for the log, elaborate a bit more, explaining what 
>>> currently
>>> happens, and what the effects of this bug are, so that later when 
>>> people
>>> are looking through the Git log they can easily get what problem you 
>>> are
>>> trying to fix; and if possible, add:
>>>
>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>
>> Yeah, sure, I'll elaborate that more specifically.
>
> Thanks !
>
>> I assume the 12-digit SHA1 is the revision number. But may I ask 
>> where and how shall I add the tag "Fixes" tag? The revision number is 
>> generated after "git commit", how can I know that in advance?
>
> Nope, it's the affected commit id from the current git log that
> your patch fixes.
>
> Have a look for example at commit:
>
> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
Thank you for your quick response. I'm quite green on kernel programming 
and git. So here's one question:
To find the the revision that **caused** the bug, I could use gitk to 
trace the changing of the file(s) history. Is that correct?

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 09:48 PM, Chang wrote:
>
> On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
>> On 11/13/2013 08:06 PM, Chang wrote:
>>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>>> Look for the __two__ most recently used path/transport and set 
>>>>>>> to active_path
>>>>>>> and retran_path respectively
>>>>
>>>> Please also for the log, elaborate a bit more, explaining what 
>>>> currently
>>>> happens, and what the effects of this bug are, so that later when 
>>>> people
>>>> are looking through the Git log they can easily get what problem 
>>>> you are
>>>> trying to fix; and if possible, add:
>>>>
>>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>>
>>> Yeah, sure, I'll elaborate that more specifically.
>>
>> Thanks !
>>
>>> I assume the 12-digit SHA1 is the revision number. But may I ask 
>>> where and how shall I add the tag "Fixes" tag? The revision number 
>>> is generated after "git commit", how can I know that in advance?
>>
>> Nope, it's the affected commit id from the current git log that
>> your patch fixes.
>>
>> Have a look for example at commit:
>>
>> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
> Thank you for your quick response. I'm quite green on kernel 
> programming and git. So here's one question:
> To find the the revision that **caused** the bug, I could use gitk to 
> trace the changing of the file(s) history. Is that correct?
>
Let's say the following condition
A - the initial revision
B - something was wrong that introduced a bug
C - latest revision
MyFix - fixing the bug caused by B. Shall I specify B or C in the 
"fixes:" tag?


         revision introduces a bug
          |
A -> B -> C ->MyFix
                  |
                  latest revision

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Chang]

On 11/13/2013 09:48 PM, Chang wrote:
>
> On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
>> On 11/13/2013 08:06 PM, Chang wrote:
>>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>>> Look for the __two__ most recently used path/transport and set 
>>>>>>> to active_path
>>>>>>> and retran_path respectively
>>>>
>>>> Please also for the log, elaborate a bit more, explaining what 
>>>> currently
>>>> happens, and what the effects of this bug are, so that later when 
>>>> people
>>>> are looking through the Git log they can easily get what problem 
>>>> you are
>>>> trying to fix; and if possible, add:
>>>>
>>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>>
>>> Yeah, sure, I'll elaborate that more specifically.
>>
>> Thanks !
>>
>>> I assume the 12-digit SHA1 is the revision number. But may I ask 
>>> where and how shall I add the tag "Fixes" tag? The revision number 
>>> is generated after "git commit", how can I know that in advance?
>>
>> Nope, it's the affected commit id from the current git log that
>> your patch fixes.
>>
>> Have a look for example at commit:
>>
>> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
> Thank you for your quick response. I'm quite green on kernel 
> programming and git. So here's one question:
> To find the the revision that **caused** the bug, I could use gitk to 
> trace the changing of the file(s) history. Is that correct?
>
Let's say the following condition
A - the initial revision
B - something was wrong that introduced a bug
C - latest revision
MyFix - fixing the bug caused by B. Shall I specify B or C in the 
"fixes:" tag?


         revision introduces a bug
          |
A -> B -> C ->MyFix
                  |
                  latest revision

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Daniel Borkmann]

On 11/13/2013 10:23 PM, Chang wrote:
[...]
> Let's say the following condition
> A - the initial revision
> B - something was wrong that introduced a bug
> C - latest revision
> MyFix - fixing the bug caused by B. Shall I specify B or C in the "fixes:" tag?
>
>
>          revision introduces a bug
>           |
> A -> B -> C ->MyFix
>                   |
>                   latest revision

You would specify B in the "Fixes:" tag, as B introduced this bug first.
In case there is no suitable git-blame information available in the current
tree (so that bug is pre Linux-2.6.12-rc2 which I believe on a quick look
that this seems the case), you can omit that. Then, just elaborate on the
commit message, and resubmit with feedback from Vlad included, too.

Thanks !

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Daniel Borkmann]

On 11/13/2013 10:23 PM, Chang wrote:
[...]
> Let's say the following condition
> A - the initial revision
> B - something was wrong that introduced a bug
> C - latest revision
> MyFix - fixing the bug caused by B. Shall I specify B or C in the "fixes:" tag?
>
>
>          revision introduces a bug
>           |
> A -> B -> C ->MyFix
>                   |
>                   latest revision

You would specify B in the "Fixes:" tag, as B introduced this bug first.
In case there is no suitable git-blame information available in the current
tree (so that bug is pre Linux-2.6.12-rc2 which I believe on a quick look
that this seems the case), you can omit that. Then, just elaborate on the
commit message, and resubmit with feedback from Vlad included, too.

Thanks !

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Vlad Yasevich]

On 11/13/2013 03:48 PM, Chang wrote:
>
> On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
>> On 11/13/2013 08:06 PM, Chang wrote:
>>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>>> Look for the __two__ most recently used path/transport and set to
>>>>>>> active_path
>>>>>>> and retran_path respectively
>>>>
>>>> Please also for the log, elaborate a bit more, explaining what
>>>> currently
>>>> happens, and what the effects of this bug are, so that later when
>>>> people
>>>> are looking through the Git log they can easily get what problem you
>>>> are
>>>> trying to fix; and if possible, add:
>>>>
>>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>>
>>> Yeah, sure, I'll elaborate that more specifically.
>>
>> Thanks !
>>
>>> I assume the 12-digit SHA1 is the revision number. But may I ask
>>> where and how shall I add the tag "Fixes" tag? The revision number is
>>> generated after "git commit", how can I know that in advance?
>>
>> Nope, it's the affected commit id from the current git log that
>> your patch fixes.
>>
>> Have a look for example at commit:
>>
>> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
> Thank you for your quick response. I'm quite green on kernel programming
> and git. So here's one question:
> To find the the revision that **caused** the bug, I could use gitk to
> trace the changing of the file(s) history. Is that correct?
>

A lot easier is to run 'git blame <filename>" and find the line you
are fixing.  The number that line starts with is the short commit id.

In this particular case, don't worry about it.  This is a day 1 bug

-vlad

Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers

[Vlad Yasevich]

On 11/13/2013 03:48 PM, Chang wrote:
>
> On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
>> On 11/13/2013 08:06 PM, Chang wrote:
>>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>>> Look for the __two__ most recently used path/transport and set to
>>>>>>> active_path
>>>>>>> and retran_path respectively
>>>>
>>>> Please also for the log, elaborate a bit more, explaining what
>>>> currently
>>>> happens, and what the effects of this bug are, so that later when
>>>> people
>>>> are looking through the Git log they can easily get what problem you
>>>> are
>>>> trying to fix; and if possible, add:
>>>>
>>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>>
>>> Yeah, sure, I'll elaborate that more specifically.
>>
>> Thanks !
>>
>>> I assume the 12-digit SHA1 is the revision number. But may I ask
>>> where and how shall I add the tag "Fixes" tag? The revision number is
>>> generated after "git commit", how can I know that in advance?
>>
>> Nope, it's the affected commit id from the current git log that
>> your patch fixes.
>>
>> Have a look for example at commit:
>>
>> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
> Thank you for your quick response. I'm quite green on kernel programming
> and git. So here's one question:
> To find the the revision that **caused** the bug, I could use gitk to
> trace the changing of the file(s) history. Is that correct?
>

A lot easier is to run 'git blame <filename>" and find the line you
are fixing.  The number that line starts with is the short commit id.

In this particular case, don't worry about it.  This is a day 1 bug

-vlad