[asterisk-dev] [Code Review] 3050: PJSIP: Add Path header support

[Mark Michelson]

> On Dec. 11, 2013, 3:22 p.m., Matt Jordan wrote:
> > /trunk/res/res_pjsip_path.c, lines 105-126
> > <https://reviewboard.asterisk.org/r/3050/diff/1/?file=49103#file49103line105>
> >
> >     Because this is using an ao2_callback to get the contact, it's going to be rather expensive. If we were searching based on the contact's typical hash key this wouldn't be a problem - but here, this is an O(n) search.
> >     
> >     I'm not sure how we typically use the contacts on an AOR, but if we're iterating over the container a lot, an ao2 container may not be the best choice of storage. ao2 containers with linear searches perform much worse than the linkedlist macros.
> >     
> >     Also: why OBJ_UNLINK? Why would you want to remove the contact from the AOR when all you're trying to get is the Path header?

Switching away from an ao2_container here would be nontrivial. The operation to retrieve contacts is pretty much a wrapper around a sorcery retrieval of objects, and sorcery returns an ao2_container of objects. There's also no "typical hash key" for objects returned from sorcery since the container is built on the fly when certain objects are requested, and the container allocation does not specify a hash or comparison function.

If this presents a performance problem, the problem will need to be addressed at a lower layer than here.


- Mark


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


On Dec. 5, 2013, 7:06 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3050/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2013, 7:06 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21084
>     https://issues.asterisk.org/jira/browse/ASTERISK-21084
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds Path support to chan_pjsip in res_pjsip_path.c with minimal additions in res_pjsip_registrar.c to store the path and additions in res_pjsip_outbound_registration.c to enable advertisement of path support to registrars and intervening proxies.
> 
> Path information is stored on contacts and is enabled via Address of Record (AoRs) and Registration configuration sections.
> 
> While adding path support, it became necessary to be able to add SIP supplements that handled messages outside of sessions, so a framework for handling these types of hooks was added in parallel to the already-existing session supplements and several senders of out-of-dialog requests were refactored as a result.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_pjsip_t38.c 403395 
>   /trunk/res/res_pjsip_registrar.c 403395 
>   /trunk/res/res_pjsip_refer.c 403395 
>   /trunk/res/res_pjsip_path.c PRE-CREATION 
>   /trunk/res/res_pjsip_outbound_registration.c 403395 
>   /trunk/res/res_pjsip_notify.c 403395 
>   /trunk/res/res_pjsip_mwi.c 403395 
>   /trunk/res/res_pjsip_messaging.c 403395 
>   /trunk/res/res_pjsip_header_funcs.c 403395 
>   /trunk/res/res_pjsip_diversion.c 403395 
>   /trunk/res/res_pjsip_caller_id.c 403395 
>   /trunk/res/res_pjsip/pjsip_options.c 403395 
>   /trunk/res/res_pjsip/pjsip_distributor.c 403395 
>   /trunk/res/res_pjsip/location.c 403395 
>   /trunk/res/res_pjsip.c 403395 
>   /trunk/include/asterisk/res_pjsip_session.h 403395 
>   /trunk/include/asterisk/res_pjsip.h 403395 
>   /trunk/channels/chan_pjsip.c 403395 
> 
> Diff: https://reviewboard.asterisk.org/r/3050/diff/
> 
> 
> Testing
> -------
> 
> This passes the tests covered by the review at https://reviewboard.asterisk.org/r/3051/
> 
> These tests were converted to work with chan_pjsip from their chan_sip counterparts.
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131211/afc812ac/attachment-0001.html>