| Message ID | 1384387106-8105-1-git-send-email-changxiangzhong@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 11/13/2013 06:58 PM, Chang Xiangzhong wrote: > When a transport recovers due to the new coming sack, SCTP should > iterate all of its transport_list to locate the __two__ most recently used > transport and set to active_path and retran_path respectively. The exising > code does not find the two properly - In case of the following list: > > [most-recent] -> [2nd-most-recent] -> ... > > Both active_path and retran_path would be set to the 1st element. > > The bug happens when: > 1) multi-homing > 2) failure/partial_failure transport recovers > Both active_path and retran_path would be set to the same most-recent one, in > other words, retran_path would not take its role - an end user might not even > notice this issue. > > Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com> Acked-by: Vlad Yasevich <vyasevich@gmail.com> -vlad > --- > net/sctp/associola.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/associola.c b/net/sctp/associola.c > index ab67efc..8f26276 100644 > --- a/net/sctp/associola.c > +++ b/net/sctp/associola.c > @@ -913,8 +913,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc, > if (!first || t->last_time_heard > first->last_time_heard) { > second = first; > first = t; > - } > - if (!second || t->last_time_heard > second->last_time_heard) > + } else if (!second || > + t->last_time_heard > second->last_time_heard) > second = t; > } > > @@ -935,6 +935,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc, > first = asoc->peer.primary_path; > } > > + if (!second) > + second = first; > /* If we failed to find a usable transport, just camp on the > * primary, even if it is inactive. > */ > -- 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
May I ask what shall I do next? Shall I send it to someone else to have him/her merge it to the Linux source tree? Cheers On 11/14/2013 02:42 AM, Vlad Yasevich wrote: > On 11/13/2013 06:58 PM, Chang Xiangzhong wrote: >> When a transport recovers due to the new coming sack, SCTP should >> iterate all of its transport_list to locate the __two__ most recently >> used >> transport and set to active_path and retran_path respectively. The >> exising >> code does not find the two properly - In case of the following list: >> >> [most-recent] -> [2nd-most-recent] -> ... >> >> Both active_path and retran_path would be set to the 1st element. >> >> The bug happens when: >> 1) multi-homing >> 2) failure/partial_failure transport recovers >> Both active_path and retran_path would be set to the same most-recent >> one, in >> other words, retran_path would not take its role - an end user might >> not even >> notice this issue. >> >> Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com> > > Acked-by: Vlad Yasevich <vyasevich@gmail.com> > > -vlad > >> --- >> net/sctp/associola.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/net/sctp/associola.c b/net/sctp/associola.c >> index ab67efc..8f26276 100644 >> --- a/net/sctp/associola.c >> +++ b/net/sctp/associola.c >> @@ -913,8 +913,8 @@ void sctp_assoc_control_transport(struct >> sctp_association *asoc, >> if (!first || t->last_time_heard > first->last_time_heard) { >> second = first; >> first = t; >> - } >> - if (!second || t->last_time_heard > second->last_time_heard) >> + } else if (!second || >> + t->last_time_heard > second->last_time_heard) >> second = t; >> } >> >> @@ -935,6 +935,8 @@ void sctp_assoc_control_transport(struct >> sctp_association *asoc, >> first = asoc->peer.primary_path; >> } >> >> + if (!second) >> + second = first; >> /* If we failed to find a usable transport, just camp on the >> * primary, even if it is inactive. >> */ >> > -- 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
From: Chang <changxiangzhong@gmail.com> Date: Thu, 14 Nov 2013 09:12:37 +0100 > May I ask what shall I do next? Shall I send it to someone else to > have him/her merge it to the Linux source tree? I will apply your patch, you just need to be patient. -- 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
So great! And It's a pleasure to work with all you guys!
On 11/14/2013 09:20 AM, David Miller wrote:
> I will apply your patch, you just need to be patient.
--
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 Thu, Nov 14, 2013 at 12:58:26AM +0100, Chang Xiangzhong wrote: > When a transport recovers due to the new coming sack, SCTP should > iterate all of its transport_list to locate the __two__ most recently used > transport and set to active_path and retran_path respectively. The exising > code does not find the two properly - In case of the following list: > > [most-recent] -> [2nd-most-recent] -> ... > > Both active_path and retran_path would be set to the 1st element. > > The bug happens when: > 1) multi-homing > 2) failure/partial_failure transport recovers > Both active_path and retran_path would be set to the same most-recent one, in > other words, retran_path would not take its role - an end user might not even > notice this issue. > > Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com> Acked-by: Neil Horman <nhorman@tuxdriver.com> -- 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
From: Chang Xiangzhong <changxiangzhong@gmail.com> Date: Thu, 14 Nov 2013 00:58:26 +0100 > When a transport recovers due to the new coming sack, SCTP should > iterate all of its transport_list to locate the __two__ most recently used > transport and set to active_path and retran_path respectively. The exising > code does not find the two properly - In case of the following list: > > [most-recent] -> [2nd-most-recent] -> ... > > Both active_path and retran_path would be set to the 1st element. > > The bug happens when: > 1) multi-homing > 2) failure/partial_failure transport recovers > Both active_path and retran_path would be set to the same most-recent one, in > other words, retran_path would not take its role - an end user might not even > notice this issue. > > Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com> Applied, thanks. -- 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/associola.c b/net/sctp/associola.c index ab67efc..8f26276 100644 --- a/net/sctp/associola.c +++ b/net/sctp/associola.c @@ -913,8 +913,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc, if (!first || t->last_time_heard > first->last_time_heard) { second = first; first = t; - } - if (!second || t->last_time_heard > second->last_time_heard) + } else if (!second || + t->last_time_heard > second->last_time_heard) second = t; } @@ -935,6 +935,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc, first = asoc->peer.primary_path; } + if (!second) + second = first; /* If we failed to find a usable transport, just camp on the * primary, even if it is inactive. */
When a transport recovers due to the new coming sack, SCTP should iterate all of its transport_list to locate the __two__ most recently used transport and set to active_path and retran_path respectively. The exising code does not find the two properly - In case of the following list: [most-recent] -> [2nd-most-recent] -> ... Both active_path and retran_path would be set to the 1st element. The bug happens when: 1) multi-homing 2) failure/partial_failure transport recovers Both active_path and retran_path would be set to the same most-recent one, in other words, retran_path would not take its role - an end user might not even notice this issue. Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com> --- net/sctp/associola.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)