[asterisk-bugs] [JIRA] (ASTERISK-25950) [patch]SIP channel does not send PeerStatus events for autocreated peers

Richard Mudgett (JIRA) noreply at issues.asterisk.org
Mon Apr 25 17:28:56 CDT 2016


    [ https://issues.asterisk.org/jira/browse/ASTERISK-25950?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=230398#comment-230398 ] 

Richard Mudgett commented on ASTERISK-25950:
--------------------------------------------

# You do need to publish the review if you put it up as a draft.  If there are any findings then you need to correct them and put the review up again.  Git allows you to modify commits locally until the patch is how you want it.  Then you put the review up again.  The gerrit change-id is used by gerrit to associate the patch with the specific review and should not be changed between resubmissions.  Once a patch is accepted and merged into the release branch it cannot be altered any more.
# There is a separate review for each branch.  It helps if you cherry pick the review to the appropriate branches.  That way the reviews will go into the branches a the same time.

https://wiki.asterisk.org/wiki/display/AST/Commit+Messages

There are plenty of review examples up on gerrit.

> [patch]SIP channel does not send PeerStatus events for autocreated peers
> ------------------------------------------------------------------------
>
>                 Key: ASTERISK-25950
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-25950
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Channels/chan_sip/General, Channels/chan_sip/Registration
>    Affects Versions: 12.0.0, 13.0.0, 13.8.2
>         Environment: Any
>            Reporter: Kirill Katsnelson
>
> Autocreated peers on the SIP channel never send any PeerStatus events. Discovered while trying to upgrade from 1.8.8.
> The reason is that autocreated peers are, well, autocreated using the temp_peer() function. There is a stanza in register_verify() which currently (13.8) looks like 
> {code}
>     if (!peer && sip_cfg.autocreatepeer != AUTOPEERS_DISABLED) {
>         /* Create peer if we have autocreate mode enabled */
>         peer = temp_peer(name);
>         if (peer) {
> {code}
> Since the transition to Stasis endpoints for reporting, an endpoint is initialized in the function build_peer(), but not in temp_peer(). So the autocreated peers do not have their `sip_peer.endpoint` field set.
> The temp_peer is a horrific misnomer, because those peers are just good normal peers, only autocreated. They are not inferior in any sense. I am mentioning that because next comes the commit 0b83761, AKA SVN changeset r394795, which attempted to fix crashes caused by the unset reporting endpoint--by simply disabling all events from these peers! I would not be surprised if the author was confused by the name of the function and implemented an incorrect fix.
> The feature has been broken since the introduction of Stasis (12.0?).
> Since  temp_peer() has been also recruited to create an authentication "(Bogus peer)" (which I do not fully understand), it is probably more correct to just allocate an endpoint for autocreated peers at the site of the call to temp_peer in the above code snippet. Additionally, rolling back 0b83761 is also an option, as long as it can be proven that the "(Bogus peer)" never causes any Stasis communication.



--
This message was sent by Atlassian JIRA
(v6.2#6252)



More information about the asterisk-bugs mailing list