| Summary: | ASTERISK-10752: [patch] Many retransmits when chan_sip generates multiple outstanding requests | ||
| Reporter: | Frederic LE FOLL (flefoll) | Labels: | |
| Date Opened: | 2007-11-13 08:08:45.000-0600 | Date Closed: | 2009-06-30 13:57:34 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Channels/chan_sip/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) bug11231.txt ( 1) chan_sip.c.br14.89184.patch_retransmit | |
| Description: | I opened issue 10946 in october http://bugs.digium.com/view.php?id=10946 on the same subject, but it was closed last week, together with issue 9567 and issue 10915 (sorry, I was away last week). All 3 issues were closed when chan_skip rev 89097 was committed. This revision adds support for one outgoing transaction with the new "lastnoninvite" field. So, I have given a try to modification brought by chan_skip rev 89097. But it does not solve the retransmit problem for me. Typically, try with a series of SendText(ABC) in your extensions.conf : the SIP peer will never respond quickly enough, and all (except last) MESSAGE messages will be retransmitted. Silly test, but easy to reproduce ! The problem also most probably still occurs when REFER handling provokes too many and too quick NOTIFY messages (not re-tried yet, but new "lastnoninvite" can handle one NOTIFY only). Consequently, I propose another patch : this one won't remove code :-) but it will add a few more lines instead, in order to cancel retransmits when chan_sip receives a "late" response. I know, it does not respond to file's dream of "a chan_sip with real transaction support" :-) ****** ADDITIONAL INFORMATION ****** Memo : In issue 10915, file and ramonpeek discussed the patch I proposed for issue 10946 (quite "basic", I admit) and it fixed ramonpeek's problem with NOTIFY. In issue 9567, file and atca_pres also discussed this patch, and it only fixed half of atca_pres's problem (leaving a repeated BYE). | ||
| Comments: | By: Frederic LE FOLL (flefoll) 2007-11-14 03:09:34.000-0600 I confirm that REFER handling still provokes NOTIFY's retransmits, even with new "lastnoninvite" feature. Typically : IF a *fake* "NOTIFY 183 Ringing" sent by handle_request_refer() is followed "too quickly" by a *real* "NOTIFY 200 Ok" (too quickly to enable REFER requester to repond to first NOTIFY before), THEN retransmits occur for "NOTIFY 183 Ringing", because new lastnoninvite stores CSeq for last NOTIFY, not first one (am I clear enough ? hemm ...). The patch I propose here fixes the problem, since it cancels retransmit mechanism, in case we decide not to handle the response (until chan_sip can fully handle responses when it generates multiple outstanding requests : this is another question). By: Olle Johansson (oej) 2007-12-15 04:16:01.000-0600 We need to solve this issue, not continue with patches... By: Frederic LE FOLL (flefoll) 2007-12-17 06:29:12.000-0600 Olle, I agree with the need to solve the issue. So, first : what needs to be done when receiving a response, including a late-but-correctly-ordered response after a burst of requests sent within a SIP dialog ? I did not (could not) find much information about this topic. - At the very least, I think that retransmit mechanism must be canceled (this is the only thing that is done in the the patch I proposed here). - As far as the response is in-sequence, it seems reasonable to handle it (this is what the patch I initially proposed for issue 10946 was doing) ... unless the dialog has entered a new phase since the request (closing the dialog, ...) ? Not to mention implementation difficulties, this problem is not Asterisk-specific, so we could expect to find some kind of specification/recommendation on this subject ? Any reference (RFC, discussion thread, ...) ? By: Olle Johansson (oej) 2008-01-27 05:07:31.000-0600 Ok, we now have quite a lot of outstanding bug reports on this issue, many duplicates. I think your code actually is a good way to handle this. Will have another look and test it a bit next week. By: Olle Johansson (oej) 2008-01-30 07:17:26.000-0600 Hmm. Maybe we can do the same as we did for subscription notifys. By: Frederic LE FOLL (flefoll) 2008-01-31 01:56:17.000-0600 It must be possible indeed to change the code that generates NOTIFYs after REFER, so that chan_sip does not send new NOTIFYs until it receives a response for last one (if I understand what you suggest, oej). It may be the opportunity then, to comply to RFC 3515 "The Session Initiation Protocol (SIP) Refer Method", section "3.10 Rate of Notifications" : "NOTIFYs to event refer SHOULD NOT be sent more frequently than once per second". But the "responses ignored" problem still can occur after a series of MESSAGES, and possibly other situations ? In fact, it is completely generic to chan_sip transaction (request/response) handling, and even with a fix in code that generates NOTIFYs after REFER, I keep thinking that chan_sip should never drop a "late but valid" response as it does now. Maybe an enhancement in REFER/NOTIFYs as you suggest, PLUS a retransmit cancellation for residual late responses, would be a not-so-bad solution ? By: Olle Johansson (oej) 2008-01-31 02:01:04.000-0600 I'll take a look, but there's a limit on how much time I can spend on non-customer related Asterisk issues currently. In general, my opinion is that this is just patches to an already bad code base that needs a rewrite. There's no transaction state engine which shows up in more and more bug reports. Everything we can do right now is just quick and very dirty fixes. By: Olle Johansson (oej) 2008-02-03 09:46:17.000-0600 Please test this patch for NOTIFY after REFER. Thanks. By: Frederic LE FOLL (flefoll) 2008-02-04 04:39:10.000-0600 Here are the results of my test [edit : patch tested upon Asterisk 1.4.17]. - the patch won't work as is : a "return AST_SUCCESS;" should terminate the new code "Check if we already have an open NOTIFY transaction" in transmit_notify_with_sipfrag(), otherwise we queue the new NOTIFY, but we send it anyway :-) - in my call scenarios, the 200 response to first NOTIFY is handled in the "Responses to OUTGOING SIP requests on INCOMING calls" section of handle_response(). Bad luck :-) for me. In fact, handle_response() contains a huge "if (ast_test_flag(&p->flags[0], SIP_OUTGOING)) { ... } else { ...}" where both parts of the if/else contain a "case 200" and I think that both parts should include the NOTIFY de-queuing code, or even better, the de-queuing code may be common and reside outside the incoming/outgoing alternative. But : if I add the "return" in transmit_notify_with_sipfrag() and I duplicate the de-queuing code where it is useful (although I don't like this solution very much), I obtain indeed a delayed NOTIFY transmission. By: jmls (jmls) 2008-05-03 14:21:59 ping. housekeeping. By: Leif Madsen (lmadsen) 2008-10-07 13:37:50 ping retransmit #1; progress? By: Frederic LE FOLL (flefoll) 2008-10-23 01:56:40 As described in 2008-02-04 note, I tested the patch for NOTIFY after REFER and, with a few modifications (explained in the note), it worked. But it only addresses NOTIFY after REFER scenarios, while retransmission problem is more general. As far as I'm concerned, I still use the patch I proposed in Nov. 2007, that handles "late" responses in order to cancel retransmission mechanism. By: Digium Subversion (svnbot) 2009-06-29 16:23:44 Repository: asterisk Revision: 204243 U branches/1.4/channels/chan_sip.c ------------------------------------------------------------------------ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204243 By: Digium Subversion (svnbot) 2009-06-29 16:48:55 Repository: asterisk Revision: 204247 _U trunk/ U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r204247 | mmichelson | 2009-06-29 16:48:55 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204247 By: Digium Subversion (svnbot) 2009-06-29 16:51:28 Repository: asterisk Revision: 204248 _U branches/1.6.0/ U branches/1.6.0/channels/chan_sip.c ------------------------------------------------------------------------ r204248 | mmichelson | 2009-06-29 16:51:28 -0500 (Mon, 29 Jun 2009) | 39 lines Merged revisions 204247 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204248 By: Digium Subversion (svnbot) 2009-06-29 16:53:24 Repository: asterisk Revision: 204249 _U branches/1.6.1/ U branches/1.6.1/channels/chan_sip.c ------------------------------------------------------------------------ r204249 | mmichelson | 2009-06-29 16:53:24 -0500 (Mon, 29 Jun 2009) | 39 lines Merged revisions 204247 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204249 By: Digium Subversion (svnbot) 2009-06-29 16:55:41 Repository: asterisk Revision: 204250 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c ------------------------------------------------------------------------ r204250 | mmichelson | 2009-06-29 16:55:41 -0500 (Mon, 29 Jun 2009) | 39 lines Merged revisions 204247 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204250 By: Digium Subversion (svnbot) 2009-06-30 13:55:02 Repository: asterisk Revision: 204243 U branches/1.4/channels/chan_sip.c ------------------------------------------------------------------------ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 23 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll Review: https://reviewboard.asterisk.org/r/298 ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204243 By: Digium Subversion (svnbot) 2009-06-30 13:56:37 Repository: asterisk Revision: 204247 _U trunk/ U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 34 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll Review: https://reviewboard.asterisk.org/r/298 ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204247 By: Digium Subversion (svnbot) 2009-06-30 13:56:59 Repository: asterisk Revision: 204248 _U branches/1.6.0/ U branches/1.6.0/channels/chan_sip.c ------------------------------------------------------------------------ r204248 | mmichelson | 2009-06-29 16:51:27 -0500 (Mon, 29 Jun 2009) | 41 lines Merged revisions 204247 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll Review: https://reviewboard.asterisk.org/r/298 ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204248 By: Digium Subversion (svnbot) 2009-06-30 13:57:19 Repository: asterisk Revision: 204249 _U branches/1.6.1/ U branches/1.6.1/channels/chan_sip.c ------------------------------------------------------------------------ r204249 | mmichelson | 2009-06-29 16:53:23 -0500 (Mon, 29 Jun 2009) | 41 lines Merged revisions 204247 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll Review: https://reviewboard.asterisk.org/r/298 ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204249 By: Digium Subversion (svnbot) 2009-06-30 13:57:32 Repository: asterisk Revision: 204250 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c ------------------------------------------------------------------------ r204250 | mmichelson | 2009-06-29 16:55:40 -0500 (Mon, 29 Jun 2009) | 41 lines Merged revisions 204247 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r204247 | mmichelson | 2009-06-29 16:48:54 -0500 (Mon, 29 Jun 2009) | 32 lines Merged revisions 204243,204246 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r204243 | mmichelson | 2009-06-29 16:23:43 -0500 (Mon, 29 Jun 2009) | 22 lines Fix a problem where chan_sip would ignore "old" but valid responses. chan_sip has had a problem for quite a long time that would manifest when Asterisk would send multiple SIP responses on the same dialog before receiving a response. The problem occurred because chan_sip only kept track of the highest outgoing sequence number used on the dialog. If Asterisk sent two requests out, and a response arrived for the first request sent, then Asterisk would ignore the response. The result was that Asterisk would continue retransmitting the requests and ignoring the responses until the maximum number of retransmissions had been reached. The fix here is to rearrange the code a bit so that instead of simply comparing the sequence number of the response to our latest outgoing sequence number, we walk our list of outstanding packets and determine if there is a match. If there is, we continue. If not, then we ignore the response. In doing this, I found a few completely useless variables that I have now removed. (closes issue ASTERISK-10752) Reported by: flefoll Review: https://reviewboard.asterisk.org/r/298 ........ r204246 | mmichelson | 2009-06-29 16:37:05 -0500 (Mon, 29 Jun 2009) | 3 lines Fix build oops. ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=204250 | ||