[asterisk-dev] [Code Review]: Help mitigate reinvite glares in the SIP channel driver

[Kevin Fleming]

> On May 24, 2012, 10:55 a.m., Kevin Fleming wrote:
> > /trunk/channels/sip/include/sip.h, line 1344
> > <https://reviewboard.asterisk.org/r/1946/diff/1/?file=28246#file28246line1344>
> >
> >     Any particular reason this isn't just a flag in the existing array of flags? Could it also just be another sub-option of the existing 'directmedia' option, something like:
> >     
> >     directmedia=(no|yes|nonat)[,update][,outgoing]
> 
> Mark Michelson wrote:
>     The biggest reason that I didn't make it a flag is that the direct media flags are crunched together with all the other SIP flags with no room to expand in place. I would have to move the entire set of direct media flags to page 3 of the SIP flags. which means that I would leave a weird gap in the page 1 flags and that I would have to touch a lot more code in order to accomplish my goal.
>     
>     Regarding the question of why it isn't included as an add-on to the directmedia option, I just hadn't thought to do it that way :) I can certainly change the configuration to work that way with little issue if that would be a better way to approach it.
> 
> Mark Michelson wrote:
>     Actually, I guess I could have just defined the flag non-contiguously from the other direct media flags. I don't think there's a mask that marks the beginning and end of direct media settings, so that may have worked. I'll do some checking about it.
> 
> Mark Michelson wrote:
>     A follow-up:
>     
>     The directmedia_outgoing setting can only be set per peer, whereas the other directmedia settings may be set in the general section or per peer. This makes it much more difficult to code accurately. So unless it's just deemed to be absolutely awful to define directmedia_outgoing as its own setting, then I'm going to leave it that way.
>     
>     I experimented with using a flag instead of having a new int in the sip_peer and sip_pvt structures and it seems to work fine. Whether that's actually any better, I can't say.

Hmm... we probably shouldn't do that. I know the policy is that new configuration options should not be set in [general] unless they make sense there (and there's a possibility this setting does, since it's possible to dial SIP URIs from the dialplan without them being associated with any peer), but since this option is really modifying the behavior of an existing option, it seems reasonable to me to make an exception in this case. It would be much easier to document and understand as an additional parameter to the existing option.

We already decided years ago to switch to flags instead of using char/int structure members to hold boolean flags, so by rule, it's better :-)


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1946/#review6307
-----------------------------------------------------------


On May 23, 2012, 5:46 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1946/
> -----------------------------------------------------------
> 
> (Updated May 23, 2012, 5:46 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> There are times where multiple Asterisk servers are peered together over SIP. In such situations, it is possible for both Asterisk servers to attempt to send direct media reinvites to each other simultaneously. This results in a glare situation in which each of the Asterisk servers sends a 491 to the other. After a waiting period, the reinvites are re-attempted. This waiting period can potentially be distracting since it can cause the media to take multiple seconds to finalize, especially if more than 2 Asterisk servers are involved.
> 
> This patch introduces a new SIP peer option called "directmedia_outgoing". If enabled, then when communicating with the peer, Asterisk will only attempt to send reinvites if the call direction is outgoing. The assumption is that the peer Asterisk server will also have this setting enabled. This way, when the two Asterisk servers communicate, they will never attempt to send direct media reinvites to each other. Instead, it will always be the peer that placed the call that will send the direct media reinvite.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 367417 
>   /trunk/channels/sip/include/sip.h 367417 
>   /trunk/configs/sip.conf.sample 367417 
> 
> Diff: https://reviewboard.asterisk.org/r/1946/diff
> 
> 
> Testing
> -------
> 
> I have tested this by running two Asterisk servers and ensuring that the option was honored and that the media streams were still set up properly.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120525/259941a6/attachment.htm>