Re: [asterisk-dev] [Code Review] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/#review13329 --- /branches/13/channels/chan_iax2.c https://reviewboard.asterisk.org/r/3999/#comment23811 Despite it not changing behavior I'd still use 20 here to match 12. - Joshua Colp On Sept. 16, 2014, 9:28 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/ --- (Updated Sept. 16, 2014, 9:28 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24265 https://issues.asterisk.org/jira/browse/ASTERISK-24265 Repository: Asterisk Description --- This only occurs when the chan_iax jitterbuffer options are set and no when setting jitterbuffers via diaplan or anything like that. The first time __get_from_jb is called, voiceformat has not been set on the IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. This worked previously because Asterisk 12 and prior simply modified an ast_format on the stack, so when it used ast_codec_interp_len on that format pointer there was no possibility for it to be a NULL pointer... just one that doesn't have a real format associated with it. One thing I might not be doing right here is that I'm using an interpolation value of 0 for a NULL format. Previously Asterisk would just check to see if the format was ILBC and if it was, return 30 and otherwise return 20... so it might be more appropriate to use 20 instead of 0. It doesn't appear to make a difference for the sake of behavior. Diffs - /branches/13/channels/chan_iax2.c 423149 Diff: https://reviewboard.asterisk.org/r/3999/diff/ Testing --- Ran basic call from a PJSIP peer to an IAX peer with the following: [general] ; The important parts jitterbuffer=yes forcejitterbuffer=yes [deskbox] type=friend requirecalltoken = no username = deskbox secret = secret host = dynamic transfer = no dtmfmode = auto encryption = no qualify = 300 context = default disallow=all allow=ulaw allow=alaw ; Most of this is probably unnecessary for reproduction Without the patch this would crash in 13 but work fine in 12. With the patch, behavior strongly resembles 12 with the initial call into __get_from_jb attempting to jb_get and getting JB_OK back and then later, when the call was actually answered, the voice format would change and the function would call again with the proper format and the jitterbuffer would get started properly. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3988: res_pjsip: Don't require a password when doing userpass authentication
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/#review1 --- Ship it! Ship It! - Joshua Colp On Sept. 15, 2014, 7:27 p.m., Sean Bright wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/ --- (Updated Sept. 15, 2014, 7:27 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- An empty password is valid for username/password authentication so we shouldn't barf on it. Diffs - /branches/12/res/res_pjsip/config_auth.c 422963 Diff: https://reviewboard.asterisk.org/r/3988/diff/ Testing --- Compiles. Registered a device with auth_type=userpass and no password set. Tested registration with a password which failed, and again without a password (an empty password) and it succeeds. Thanks, Sean Bright -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3994: Bridges User Documentation Update
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3994/#review13334 --- Ship it! Ship It! - Matt Jordan On Sept. 15, 2014, 10:46 a.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3994/ --- (Updated Sept. 15, 2014, 10:46 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23469 None Description --- I have updated the Bridges documentation for Asterisk 12/13. I'd love any feedback on the structure, completeness, and correctness of the information. You can find the documentation here: https://wiki.asterisk.org/wiki/display/AST/Bridges Note that this is user documentation, not developer documentation. Diffs - Diff: https://reviewboard.asterisk.org/r/3994/diff/ Testing --- N/A Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3994: Bridges User Documentation Update
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3994/ --- (Updated Sept. 18, 2014, 8:20 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Bugs: ASTERISK-23469 None Description --- I have updated the Bridges documentation for Asterisk 12/13. I'd love any feedback on the structure, completeness, and correctness of the information. You can find the documentation here: https://wiki.asterisk.org/wiki/display/AST/Bridges Note that this is user documentation, not developer documentation. Diffs - Diff: https://reviewboard.asterisk.org/r/3994/diff/ Testing --- N/A Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Wed, Sep 17, 2014 at 11:21 AM, Paul Belanger paul.belan...@polybeacon.com wrote: On Tue, Sep 16, 2014 at 6:01 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan mjor...@digium.com wrote: snip Just to echo everything Russell typed, I also recommend above. While complicated and full of moving parts, its extremely good at what it does. I have a system running both public and private for different projects I am doing. One thing that is great about it, developers can develop faster without knowing or understanding the release processes of a project. I think the issue that you'll run into the is amount of time and effort to learn the system and understand the workings. I don't know the official timelines, however if we are still talking about this at Astricon, I don't mind sitting down with people and hashing it out. I'd like to move everything over prior to starting the major work on Asterisk 14. That wouldn't occur until after AstriCon at the earliest, so we can certainly sit down and talk about this some more then. Additionally, I'd offer up my public infrastructure for a demo or POC of the asterisk testsuite. Infact, once the project was converted into git, it would only take a few minutes to import it into the process. The, people could do test commits / see how the system works. That's awesome Paul - thanks! I know there's a few small issues we'd have to work through in just the initial conversion process, but once we've done that we can test it out. I do think the next step is to see how gerritt can interact with crowd and the rest of the existing infrastructure. We can do that in parallel with other things however. Matt -- Matthew Jordan Digium, Inc. | Engineering Manager 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3993: config: bug: Fix SEGV in ast_category_insert when a matching category isn't found
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3993/ --- (Updated Sept. 18, 2014, 9:37 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423276 Repository: Asterisk Description --- If you call ast_category_insert with a match category that doesn't exist, the list traverse runs out of 'next' categories and you get a SEGV. This patch adds check for the end-of-list condition and changes the signature to return an int for success/failure indication instead of a void. The only consumer of this function is manager and it was also changed to use the return value. Diffs - branches/1.8/main/manager.c 423127 branches/1.8/main/config.c 423127 branches/1.8/include/asterisk/config.h 423127 Diff: https://reviewboard.asterisk.org/r/3993/diff/ Testing --- Tested with AMI UpdateConfig/newcat to make sure the proper value is returned in both positive and negative cases. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3972: Only install dahdi_span_config_hook if DAHDI is enabled
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3972/ --- (Updated Sept. 18, 2014, 10 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423281 Repository: Asterisk Description --- I'm a bit weird, and I configure asterisk with --prefix=/opt/asterisk, so that I can install it without running as root. The install script for the DAHDI hook scripts were hard coded to install into /usr/share, which foils my weirdness. This patch changes the install to only install the hook script if DAHDI is enabled. It also adds the script to the uninstall task, and moves the DAHDI_UDEV_HOOK_DIR variable so that it's not between the _MAKEOPTS variables and their comment. Diffs - /branches/13/makeopts.in 422582 /branches/13/Makefile 422582 Diff: https://reviewboard.asterisk.org/r/3972/diff/ Testing --- Ran ./configure make all install both with and without DAHDI, confirming that it worked as expected. Thanks, David Lee -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan mjor...@digium.com wrote: And there was much rejoicing \o/ But seriously, we all know that a lot of people have wanted to move to Git for some time. For the record, everyone at Digium has wanted to move the project to Git for some time. I swore to myself that we wouldn't do another Standard release on Subversion - after we spent at least six weeks mucking around with merge conflicts during Asterisk 12 - and with Asterisk 14 looming ever closer, the time is now to start getting something done on this. ... -- Team repos I'd recommend just using your own account on github or whatever. ... -- Process Recommendation I discussed this a good bit above, but I'm happy to answer questions. -- Russell Bryant Russell, how does Gerrit deal with submitting reviews? Are all reviews simply topic branches on the repository that Gerrit hosts? What about a pull request workflow where the repository is forked during development, can Gerrit support this in some way? Just trying to understand how team repos on Github or some other platform could be used for development purposes. -- Samuel Fortier-Galarneau Digium, Inc. | Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: www.digium.com www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes
On Sept. 18, 2014, 8:12 a.m., Joshua Colp wrote: /branches/13/channels/chan_iax2.c, line 4126 https://reviewboard.asterisk.org/r/3999/diff/1/?file=67373#file67373line4126 Despite it not changing behavior I'd still use 20 here to match 12. Alright, fixed that. The only difference in the code is that 20 is there instead of 0, so I think I'll hold off on actually posting another review for now. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/#review13329 --- On Sept. 16, 2014, 4:28 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/ --- (Updated Sept. 16, 2014, 4:28 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24265 https://issues.asterisk.org/jira/browse/ASTERISK-24265 Repository: Asterisk Description --- This only occurs when the chan_iax jitterbuffer options are set and no when setting jitterbuffers via diaplan or anything like that. The first time __get_from_jb is called, voiceformat has not been set on the IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. This worked previously because Asterisk 12 and prior simply modified an ast_format on the stack, so when it used ast_codec_interp_len on that format pointer there was no possibility for it to be a NULL pointer... just one that doesn't have a real format associated with it. One thing I might not be doing right here is that I'm using an interpolation value of 0 for a NULL format. Previously Asterisk would just check to see if the format was ILBC and if it was, return 30 and otherwise return 20... so it might be more appropriate to use 20 instead of 0. It doesn't appear to make a difference for the sake of behavior. Diffs - /branches/13/channels/chan_iax2.c 423149 Diff: https://reviewboard.asterisk.org/r/3999/diff/ Testing --- Ran basic call from a PJSIP peer to an IAX peer with the following: [general] ; The important parts jitterbuffer=yes forcejitterbuffer=yes [deskbox] type=friend requirecalltoken = no username = deskbox secret = secret host = dynamic transfer = no dtmfmode = auto encryption = no qualify = 300 context = default disallow=all allow=ulaw allow=alaw ; Most of this is probably unnecessary for reproduction Without the patch this would crash in 13 but work fine in 12. With the patch, behavior strongly resembles 12 with the initial call into __get_from_jb attempting to jb_get and getting JB_OK back and then later, when the call was actually answered, the voice format would change and the function would call again with the proper format and the jitterbuffer would get started properly. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/#review13337 --- Ship it! Ship It! - Joshua Colp On Sept. 16, 2014, 9:28 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/ --- (Updated Sept. 16, 2014, 9:28 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24265 https://issues.asterisk.org/jira/browse/ASTERISK-24265 Repository: Asterisk Description --- This only occurs when the chan_iax jitterbuffer options are set and no when setting jitterbuffers via diaplan or anything like that. The first time __get_from_jb is called, voiceformat has not been set on the IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. This worked previously because Asterisk 12 and prior simply modified an ast_format on the stack, so when it used ast_codec_interp_len on that format pointer there was no possibility for it to be a NULL pointer... just one that doesn't have a real format associated with it. One thing I might not be doing right here is that I'm using an interpolation value of 0 for a NULL format. Previously Asterisk would just check to see if the format was ILBC and if it was, return 30 and otherwise return 20... so it might be more appropriate to use 20 instead of 0. It doesn't appear to make a difference for the sake of behavior. Diffs - /branches/13/channels/chan_iax2.c 423149 Diff: https://reviewboard.asterisk.org/r/3999/diff/ Testing --- Ran basic call from a PJSIP peer to an IAX peer with the following: [general] ; The important parts jitterbuffer=yes forcejitterbuffer=yes [deskbox] type=friend requirecalltoken = no username = deskbox secret = secret host = dynamic transfer = no dtmfmode = auto encryption = no qualify = 300 context = default disallow=all allow=ulaw allow=alaw ; Most of this is probably unnecessary for reproduction Without the patch this would crash in 13 but work fine in 12. With the patch, behavior strongly resembles 12 with the initial call into __get_from_jb attempting to jb_get and getting JB_OK back and then later, when the call was actually answered, the voice format would change and the function would call again with the proper format and the jitterbuffer would get started properly. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3995: res_pjsip_endpoint_identifier_ip: Can't parse identify with match value containing CIDR
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3995/ --- (Updated Sept. 18, 2014, 11:44 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and Joshua Colp. Changes --- Committed in revision 423417 Bugs: ASTERISK-24290 https://issues.asterisk.org/jira/browse/ASTERISK-24290 Repository: Asterisk Description --- Using a value such as '10.24.0.0/16' would fail to match because ast_sockaddr_resolve can only parse the following formats: * hostname:port * host.example.com:port * a.b.c.d * a.b.c.d:port * a:b:c:...:d * [a:b:c:...:d] * [a:b:c:...:d]:port When the format doesn't match one of these, the function fails and we bail. To get around this, I simply checked for the presence of a '/' in the identify string and used ast_append_ha directly with the address if it was present. Diffs - /branches/12/res/res_pjsip_endpoint_identifier_ip.c 423062 Diff: https://reviewboard.asterisk.org/r/3995/diff/ Testing --- Used CLI command 'pjsip show endpoint 1603' with an endpoint that had the following identifier: [1603] type=identify match=10.24.18.13/16 endpoint=1603 Before, the address would fail to parse and the command would show no identifier After, the address would parse correctly and show '10.24.0.0/16' for the identifier as seen in: Endpoint: 1603/1603Not in use 0 of inf Aor: 1603 5 Contact: 1603/sip:1603@10.24.18.13:5060;obUnknown nan Identify: 10.24.0.0/16 I tried a few other things, such as not using a CIDR and using a hostname to verify that there wasn't any obvious deviation in behavior introduced by the patch. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13339 --- /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23816 This will terminate the dial masquerade datastore on the first dial completion message. Say I'm in a parallel dial. I've dialled Alice, Bob, and Charlie. I complete the dial to Alice, and publish my message. That removes the datastore. Can I be masqueraded out before I also complete the dial with Bob and Charlie? - Matt Jordan On Sept. 18, 2014, 10:59 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 10:59 a.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
On Sept. 18, 2014, 12:22 p.m., Matt Jordan wrote: /branches/12/main/stasis_channels.c, line 383 https://reviewboard.asterisk.org/r/3990/diff/3/?file=67394#file67394line383 This will terminate the dial masquerade datastore on the first dial completion message. Say I'm in a parallel dial. I've dialled Alice, Bob, and Charlie. I complete the dial to Alice, and publish my message. That removes the datastore. Can I be masqueraded out before I also complete the dial with Bob and Charlie? Good point. Perhaps what I should be doing here is removing individual entries from the existing dialstore rather than removing it wholesale. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13339 --- On Sept. 18, 2014, 10:59 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 10:59 a.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
On Sept. 18, 2014, 12:22 p.m., Matt Jordan wrote: /branches/12/main/stasis_channels.c, line 383 https://reviewboard.asterisk.org/r/3990/diff/3/?file=67394#file67394line383 This will terminate the dial masquerade datastore on the first dial completion message. Say I'm in a parallel dial. I've dialled Alice, Bob, and Charlie. I complete the dial to Alice, and publish my message. That removes the datastore. Can I be masqueraded out before I also complete the dial with Bob and Charlie? Jonathan Rose wrote: Good point. Perhaps what I should be doing here is removing individual entries from the existing dialstore rather than removing it wholesale. I've checked through your suggested scenario with some added sleeps and it looks like what you've described is technically possible. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13339 --- On Sept. 18, 2014, 10:59 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 10:59 a.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:21 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Changes --- Address mjordan's masquerade during parallel dial between events that happen with the canceled dial and the events that happen on the dial that stays alive. It's kind of weird. I addressed it by changing the function that was purging the datastore entirely to remove only items belonging to the same channel. Hopefully I didn't miss anything important. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs (updated) - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13342 --- /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23819 Oops, I need a break here. - Jonathan Rose On Sept. 18, 2014, 1:21 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:21 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:25 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Changes --- Add a break in the function to remove an entry from the datastore. I don't think it's possible to have the same channel in there for two separate events, but there's no sense in continuing to traverse past the found entry anyway. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs (updated) - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/ --- Review request for Asterisk Developers. Bugs: AFS-162 https://issues.asterisk.org/jira/browse/AFS-162 Repository: Asterisk Description --- Outgoing PJSIP calls can result in non-negotiated formats listed in the channel's native formats if video formats are listed in the endpoint's configuration. The resulting call could then use a non-negotiated format resulting in one way audio. * Simplified the update of session-req_caps in set_caps(). Why do something in five steps when only one is needed? Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 423446 /branches/13/channels/chan_pjsip.c 423446 Diff: https://reviewboard.asterisk.org/r/4000/diff/ Testing --- Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw Called from D40 with g722 among other formats enabled to a Polycom that negotiates ulaw. Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/#review13343
---
Ship it!
Ship It!
- opticron
On Sept. 15, 2014, 2:43 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 15, 2014, 2:43 p.m.)
Review request for Asterisk Developers and rmudgett.
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags Controls post-processing of the result.
AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
AST_STRSEP_STRIP does a trim then strips the outermost quotes. You may want
to trim again after the strip. Just OR both the TRIM and STRIP flags.
AST_STRSEP_UNESCAPE unescapes '\' sequences.
AST_STRSEP_ALL does all of the above processing.
\return The next token or NULL if done or if there are more than 8 levels of
nested quotes.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result and have '\' sequences unescaped.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, uint32_t flags);
Diffs
-
branches/12/tests/test_strings.c 422963
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3967: Subscriptoin state test events for review 3966
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3967/ --- (Updated Sept. 18, 2014, 1:31 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423462 Repository: Asterisk Description --- These are some simple test events that can be used by the testsuite to know the current state of a subscription in Asterisk. Since these are meant for simple tests, not much information is given except that a subscription has changed to state X. It does not attempt to indicate which subscription has changed in any way. Without these events, the tests on /r/3966 cannot run. Diffs - /branches/13/res/res_pjsip_pubsub.c 422536 Diff: https://reviewboard.asterisk.org/r/3967/diff/ Testing --- /r/3966 exercises these events and has tested that the events are sent when expected. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Thu, Sep 18, 2014 at 12:29 PM, Russell Bryant russ...@russellbryant.net wrote: On Thu, Sep 18, 2014 at 11:31 AM, Samuel Galarneau sgalarn...@digium.com wrote: On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan mjor...@digium.com wrote: And there was much rejoicing \o/ But seriously, we all know that a lot of people have wanted to move to Git for some time. For the record, everyone at Digium has wanted to move the project to Git for some time. I swore to myself that we wouldn't do another Standard release on Subversion - after we spent at least six weeks mucking around with merge conflicts during Asterisk 12 - and with Asterisk 14 looming ever closer, the time is now to start getting something done on this. ... -- Team repos I'd recommend just using your own account on github or whatever. ... -- Process Recommendation I discussed this a good bit above, but I'm happy to answer questions. -- Russell Bryant Russell, how does Gerrit deal with submitting reviews? Are all reviews simply topic branches on the repository that Gerrit hosts? Perhaps a real demonstration of workflow would help. I'll use a recent trivial fix that I did. This is a one-liner patch that needed to go into master as well as two stable branches. I headed over to my local git tree and created a branch to do the fix. $ cd openstack/nova $ git checkout -b bug/1370191 origin/master ... Hack code and commit the fix ... $ git commit -a Now I have a single patch on top of upstream master that I want to submit for review. $ git review This created https://review.openstack.org/#/c/121940/ What's actually happening is a git push to gerrit. A git repo in gerrit maintains all revisions of all patches. You can actually fetch the patch and look at it locally in your tree. A couple more comments about the magic happening here ... First, git review knows where to push based on a file checked in to the repo: $ cat .gitreview [gerrit] host=review.openstack.org port=29418 project=openstack/nova.git git review also sets up a local commit hook that adds a Change-Id header to your commit message. That Change-Id is what links multiple revisions of the same change together. So, if you edit your change and push it again, as long as the Change-Id remains the same, gerrit treats it as the same review request and not a new one. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13344 --- Ship it! Ship It! - Matt Jordan On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:25 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
A couple more comments about the magic happening here ... First, git review knows where to push based on a file checked in to the repo: $ cat .gitreview [gerrit] host=review.openstack.org port=29418 project=openstack/nova.git git review also sets up a local commit hook that adds a Change-Id header to your commit message. That Change-Id is what links multiple revisions of the same change together. So, if you edit your change and push it again, as long as the Change-Id remains the same, gerrit treats it as the same review request and not a new one. Sounds good to me. So it doesn't really matter from which repo you post a review so long as it's a clone of the original with that .gitreview file. I have another question unrelated to reviews. Does your setup make it easy to mirror a repo? In a more complicated scenario, what if someone had a private fork but they wanted to get public commits to master mirrored to their repo? Would they have to treat the original repo as upstream and manually pull changes and rebase their private branch off of it? -- Samuel Fortier-Galarneau Digium, Inc. | Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: www.digium.com www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Asterisk 11.6-cert6, 11.12.1, 12.5.1 Now Available (Security Release)
The Asterisk Development Team has announced security releases for Certified Asterisk 11.6 and Asterisk 11 and 12. The available security releases are released as versions 11.6-cert6, 11.12.1, and 12.5.1. These releases are available for immediate download at http://downloads.asterisk.org/pub/telephony/asterisk/releases Please note that the release of these versions resolves the following security vulnerability: * AST-2014-010: Remote Crash when Handling Out of Call Message in Certain Dialplan Configurations Additionally, the release of Asterisk 12.5.1 resolves the following security vulnerability: * AST-2014-009: Remote Crash Based on Malformed SIP Subscription Requests Note that the crash described in AST-2014-010 can be worked around through dialplan configuration. Given the likelihood of the issue, an advisory was deemed to be warranted. For more information about the details of these vulnerabilities, please read security advisories AST-2014-009 and AST-2014-010, which were released at the same time as this announcement. For a full list of changes in the current releases, please see the ChangeLogs: http://downloads.asterisk.org/pub/telephony/certified-asterisk/releases/ChangeLog-11.6-cert6 http://downloads.asterisk.org/pub/telephony/asterisk/releases/ChangeLog-11.12.1 http://downloads.asterisk.org/pub/telephony/asterisk/releases/ChangeLog-12.5.1 The security advisories are available at:  * http://downloads.asterisk.org/pub/security/AST-2014-009.pdf  * http://downloads.asterisk.org/pub/security/AST-2014-010.pdf Thank you for your continued support of Asterisk! -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] AST-2014-009: Remote crash based on malformed SIP subscription requests
Asterisk Project Security Advisory - AST-2014-009 ProductAsterisk SummaryRemote crash based on malformed SIP subscription requests Nature of Advisory Remotely triggered crash of Asterisk SusceptibilityRemote authenticated sessions Severity Major Exploits KnownNo Reported On 30 July, 2014 Reported By Mark Michelson Posted On 18 September, 2014 Last Updated OnSeptember 18, 2014 Advisory Contact Mark Michelson mmichelson AT digium DOT com CVE Name Pending Description It is possible to trigger a crash in Asterisk by sending a SIP SUBSCRIBE request with unexpected mixes of headers for a given event package. The crash occurs because Asterisk allocates data of one type at one layer and then interprets the data as a separate type at a different layer. The crash requires that the SUBSCRIBE be sent from a configured endpoint, and the SUBSCRIBE must pass any authentication that has been configured. Note that this crash is Asterisk's PJSIP-based res_pjsip_pubsub module and not in the old chan_sip module. Resolution Type-safety has been built into the pubsub API where it previously was absent. A test has been added to the testsuite that previously would have triggered the crash. Affected Versions Product Release Series Asterisk Open Source 1.8.x Unaffected Asterisk Open Source 11.xUnaffected Asterisk Open Source 12.x12.1.0 and up Certified Asterisk 1.8.15 Unaffected Certified Asterisk 11.6Unaffected Corrected In Product Release Asterisk Open Source12.5.1 Patches SVN URL Revision http://downloads.asterisk.org/pub/security/AST-2014-009-12.diff Asterisk 12 Links https://issues.asterisk.org/jira/browse/ASTERISK-24136 Asterisk Project Security Advisories are posted at http://www.asterisk.org/security This document may be superseded by later versions; if so, the latest version will be posted at http://downloads.digium.com/pub/security/AST-2014-009.pdf and http://downloads.digium.com/pub/security/AST-2014-009.html Revision History DateEditor Revisions Made 19 August, 2014 Mark Michelson Initial version of document Asterisk Project Security Advisory - AST-2014-009 Copyright (c) 2014 Digium, Inc. All Rights Reserved. Permission is hereby granted to distribute and publish this advisory in its original, unaltered form. -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] AST-2014-010: Remote crash when handling out of call message in certain dialplan configurations
Asterisk Project Security Advisory - AST-2014-010 ProductAsterisk SummaryRemote crash when handling out of call message in certain dialplan configurations Nature of Advisory Remotely triggered crash of Asterisk SusceptibilityRemote authenticated sessions Severity Minor Exploits KnownNo Reported On 05 September 2014 Reported By Philippe Lindheimer Posted On 18 September 2014 Last Updated OnSeptember 18, 2014 Advisory Contact Matt Jordan mjordan AT digium DOT com CVE Name Pending Description When an out of call message - delivered by either the SIP or PJSIP channel driver or the XMPP stack - is handled in Asterisk, a crash can occur if the channel servicing the message is sent into the ReceiveFax dialplan application while using the res_fax_spandsp module. Note that this crash does not occur when using the res_fax_digium module. While this crash technically occurs due to a configuration issue, as attempting to receive a fax from a channel driver that only contains textual information will never succeed, the likelihood of having it occur is sufficiently high as to warrant this advisory. Resolution The fax family of applications have been updated to handle the Message channel driver correctly. Users using the fax family of applications along with the out of call text messaging features are encouraged to upgrade their versions of Asterisk to the versions specified in this security advisory. Additionally, users of Asterisk are encouraged to use a separate dialplan context to process text messages. This avoids issues where the Message channel driver is passed to dialplan applications that assume a media stream is available. Note that the various channel drivers and stacks provide such an option; an example being the SIP channel driver's outofcall_message_context option. Affected Versions Product Release Series Asterisk Open Source 11.xAll versions Asterisk Open Source 12.xAll versions Certified Asterisk 11.6All versions Corrected In Product Release Asterisk Open Source11.12.1, 12.5.1 Certified Asterisk 11.6-cert6 Patches SVN URL Revision http://downloads.asterisk.org/pub/security/AST-2014-010-11.diff Asterisk 11 http://downloads.asterisk.org/pub/security/AST-2014-010-12.diff Asterisk 12 http://downloads.asterisk.org/pub/security/AST-2014-010-11.6.diff Certified Asterisk 11.6 Links https://issues.asterisk.org/jira/browse/ASTERISK-24301 Asterisk Project Security Advisories are posted at http://www.asterisk.org/security
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 18, 2014, 2:21 p.m.)
Status
--
This change has been marked as submitted.
Review request for Asterisk Developers and rmudgett.
Changes
---
Committed in revision 423476
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags Controls post-processing of the result.
AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
AST_STRSEP_STRIP does a trim then strips the outermost quotes. You may want
to trim again after the strip. Just OR both the TRIM and STRIP flags.
AST_STRSEP_UNESCAPE unescapes '\' sequences.
AST_STRSEP_ALL does all of the above processing.
\return The next token or NULL if done or if there are more than 8 levels of
nested quotes.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result and have '\' sequences unescaped.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, uint32_t flags);
Diffs
-
branches/12/tests/test_strings.c 422963
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3988: res_pjsip: Don't require a password when doing userpass authentication
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/ --- (Updated Sept. 18, 2014, 2:29 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423481 Repository: Asterisk Description --- An empty password is valid for username/password authentication so we shouldn't barf on it. Diffs - /branches/12/res/res_pjsip/config_auth.c 422963 Diff: https://reviewboard.asterisk.org/r/3988/diff/ Testing --- Compiles. Registered a device with auth_type=userpass and no password set. Tested registration with a password which failed, and again without a password (an empty password) and it succeeds. Thanks, Sean Bright -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3966: Testsuite: RLS batched notification tests
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3966/ --- (Updated Sept. 18, 2014, 2:31 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 5603 Bugs: ASTERISK-23874 https://issues.asterisk.org/jira/browse/ASTERISK-23874 Repository: testsuite Description --- This is a suite of tests for resource list subscriptions with batched notification. Batched notification is a process by which a time interval may be configured on a resource list. When a state change occurs for one of the resources in the list, a timer is started. While the timer is running, any further state changes to any of the resources in the list are cached. Once the timer has expired, all state changes that have occurred for resources in the list are sent in a single NOTIFY. This set of tests means to ensure that the feature works as expected. The tests are as follows: * basic: A baseline test, this creates a subscription to a resource list with two resources. A change is made to one of the resources, and we ensure that the notification of the state change occurs after the configured interval. This test also includes a resubscription and termination, ensuring that the NOTIFYs that we send in response to SUBSCRIBEs are sent immediately and do not use the configured interval. * single_resource_multiple_changes: In this test, a single resource has two rapid state changes made on it. We ensure that even though we have changed the state twice, we only receive a single NOTIFY from Asterisk with the most recent state change. * multiple_resources_single_change: In this test, two resources each have a single state change made on them. We ensure that even though we have changed two different resources' states, we receive only a single NOTIFY from Asterisk containing both resources' state changes. * resubscription_interruption: In this test, we make a state change on a resource, and then immediately resubscribe to the list. Since we must send a NOTIFY immediately in response to the SUBSCRIBE, and since this NOTIFY will reflect the latest state of the resource, we test that the batched notification that we created when the state change occurred has been canceled. We do this by waiting for several seconds after the conclusion of the test to be certain that Asterisk does not send any errant NOTIFY requests. * termination_interruption: This test is nearly identical to resubscription_interruption, except that instead of sending a resubscription to interrupt the batched notification, we send a subscription termination. Like with resubscription_interruption, we wait around after the scenario has completed in case Asterisk sends any further NOTIFY requests. * list_of_lists/batched: This is the only test for lists containing sublists. In this scenario, the outer list has batched notifications disabled, but the inner sublist has batched notifications enabled. The test ensures that the configuration for notification batching on the inner list is ignored and that notifications are not batched at all. We do this by sending two rapid state changes and ensuring that each results in a NOTIFY request being sent by Asterisk. In order to get these tests to work properly, I made a couple of changes to rls_test.py: * I added a stop_after_notifys option, True by default. For the resubscription_interruption and termination_interruption tests, I needed rls_test.py not to stop the test after the final expected NOTIFY was received. This option allows those tests to keep the test alive until they deem it okay to stop the test. * I made the call to register_scenario_started_callback() conditional. I did this initially because I was not going to use the SIPpTestCase for resubscription_interruption and termination_interruption. When not using the SIPpTestCase, an exception is thrown when trying to call register_scenario_started_callback() since it does not exist. In the end, I did end up using the SIPpTestCase for those tests, but I decided to leave the change to rls_test.py intact since it shouldn't necessarily presume that a SIPpTestCase is being used. There are two versions of each of the tests, one for presence and one for MWI. For the most part, the tests are identical except for the obvious bits (Updating MWI instead of device state in test drivers, configuring mailboxes instead of hints, etc.). However, the single_resource_multiple_state_changes test is a bit different between the presence and MWI versions. This is because while testing with presence, I discovered issue https://issues.asterisk.org/jira/browse/ASTERISK-24286 . For presence tests, I could not reliably send two rapid state changes and expect consistent results. So instead, I have to
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13345 --- /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23821 Why is the caller channel lock held for the ast_channel_publish_dial_forward() call? A dead lock can happen holding the caller lock because ast_channel_publish_dial_forward() locks caller and peer in turn. /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23825 Maybe this should be ast_strlen_zero(dialstatus) instead. There are several calls in app_queue() that pass something for both dialstring and dialstatus. /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23823 dial_masquerade() is called with several locks held: the global channels lock, old_chan, and new_chan. The calls to ast_channel_publish_dial_forward() will then try to lock cur-peer. I'm just noting this situation because locking more than one channel at a time normally has a deadlock potential. However, since the global channels container lock is held it should be safe enough. /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23820 target is leaked here. /branches/12/main/stasis_channels.c https://reviewboard.asterisk.org/r/3990/#comment23822 Allocation failure is not checked here. Though it seems like it might not cause a problem anyway. - rmudgett On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:25 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose --
Re: [asterisk-dev] [Code Review] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
On Sept. 15, 2014, 4:14 p.m., opticron wrote: This change appears to render the media_use_received_transport configuration option non-functional since it removes all checks relating to it. Joshua Colp wrote: It'll still work, the difference is on received it is always done and the outgoing check has been relaxed to not rely on the option. I don't really understand your response here, Josh. As opticron stated, all references to the option media_use_received_transport have been removed, therefore the option has no effect any longer. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/#review13301 --- On Sept. 14, 2014, 12:25 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- (Updated Sept. 14, 2014, 12:25 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs - /trunk/res/res_pjsip_sdp_rtp.c 423064 /trunk/res/res_pjsip/pjsip_configuration.c 423064 /trunk/res/res_pjsip.c 423064 /trunk/include/asterisk/res_pjsip.h 423064 /trunk/configs/samples/pjsip.conf.sample 423064 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 18, 2014, 2:42 p.m.) Review request for Asterisk Developers. Changes --- Updated summary Repository: Asterisk Description (updated) --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_register and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 422737 branches/12/main/chanvars.c 422737 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 422737 branches/12/configs/phoneprov.conf.sample 422737 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
On Sept. 18, 2014, 3:10 p.m., rmudgett wrote: /branches/12/main/stasis_channels.c, line 1257 https://reviewboard.asterisk.org/r/3990/diff/5/?file=67398#file67398line1257 dial_masquerade() is called with several locks held: the global channels lock, old_chan, and new_chan. The calls to ast_channel_publish_dial_forward() will then try to lock cur-peer. I'm just noting this situation because locking more than one channel at a time normally has a deadlock potential. However, since the global channels container lock is held it should be safe enough. On Sept. 18, 2014, 3:10 p.m., rmudgett wrote: /branches/12/main/stasis_channels.c, lines 381-394 https://reviewboard.asterisk.org/r/3990/diff/5/?file=67398#file67398line381 Why is the caller channel lock held for the ast_channel_publish_dial_forward() call? A dead lock can happen holding the caller lock because ast_channel_publish_dial_forward() locks caller and peer in turn. Well, I want to keep the lock held in case a masquerade happens before I do ast_channel_publish_dial_forward... not sure how realistic such a scenario is, probably not very, but perhaps I should just do a lock_both here instead? - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13345 --- On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:25 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev
Re: [asterisk-dev] Git Migration
On Thu, Sep 18, 2014 at 3:01 PM, Samuel Galarneau sgalarn...@digium.com wrote: A couple more comments about the magic happening here ... First, git review knows where to push based on a file checked in to the repo: $ cat .gitreview [gerrit] host=review.openstack.org port=29418 project=openstack/nova.git git review also sets up a local commit hook that adds a Change-Id header to your commit message. That Change-Id is what links multiple revisions of the same change together. So, if you edit your change and push it again, as long as the Change-Id remains the same, gerrit treats it as the same review request and not a new one. Sounds good to me. So it doesn't really matter from which repo you post a review so long as it's a clone of the original with that .gitreview file. I have another question unrelated to reviews. Does your setup make it easy to mirror a repo? In a more complicated scenario, what if someone had a private fork but they wanted to get public commits to master mirrored to their repo? Would they have to treat the original repo as upstream and manually pull changes and rebase their private branch off of it? Mirroring, sure. I don't remember exactly how we do it, but all OpenStack repos are mirrored to github for convenience, for example. Regarding private branches, git generally makes that kind of thing ***MUCH*** easier than svn. You can easily track the exact commits that are applied on top of upstream. You could either periodically rebase your changes on top of upstream (re-applying the commits, rewriting history but a cleaner history), or periodically merge from upstream. In either case, I personally wouldn't automate it. You want some sanity checking around that stuff. Conflicts happen. Really, I think this is going to be MUCH better no matter what specific infrastructure you go with. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
On Sept. 18, 2014, 3:10 p.m., rmudgett wrote: /branches/12/main/stasis_channels.c, lines 381-394 https://reviewboard.asterisk.org/r/3990/diff/5/?file=67398#file67398line381 Why is the caller channel lock held for the ast_channel_publish_dial_forward() call? A dead lock can happen holding the caller lock because ast_channel_publish_dial_forward() locks caller and peer in turn. Jonathan Rose wrote: Well, I want to keep the lock held in case a masquerade happens before I do ast_channel_publish_dial_forward... not sure how realistic such a scenario is, probably not very, but perhaps I should just do a lock_both here instead? Masquerades are pure evil as far as stasis messages that are part of a series is concerned. Surprise! I'm a different channel now. Dial Start/Stop Bridge Enter/Leave Probably have to lock both channels. Note that you may or may not have caller, but you will always have peer. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13345 --- On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 1:25 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4001: PJSIP: Prevent T38 framehook being put on wrong channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4001/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- This changes gives framehooks a reverse-direction masquerade callback in addition to chan_fixup_cb similar to the callback added to datastores to handle the same situation. The new callback provides the same parameters as the fixup callback, but is called on the new channel's framehooks before moving framehooks from the old channel to the new channel. This gives the framehooks an oppurtunity to decide whether they should remain on the new channel or be removed. This new callback is used to prevent the PJSIP T.38 framehook from remaining on a masqueraded channel if the new channel is not also a PJSIP channel. This was causing a crash when a local channel was masqueraded into a PJSIP channel and the framehook was executed on the local channel since the channel's tech private data was not structured as expected. Diffs - branches/12/res/res_pjsip_t38.c 423230 branches/12/main/framehook.c 423230 branches/12/include/asterisk/framehook.h 423230 Diff: https://reviewboard.asterisk.org/r/4001/diff/ Testing --- This corrected my reproduction of the crash and fixed the crash for the original reporter. Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 3:21 p.m.) Review request for Asterisk Developers. Changes --- Updated description and cleaned up default processing. Repository: Asterisk Description (updated) --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a template substitution map as follows: ;MAC=deadbeef4dad ;added by pp1000 ;USERNAME=myname;automatically added by 1000-auth username ;SECRET=mysecret;automatically added by 1000-auth password ;PROFILE=someprofile;added by defaults ;SERVER=myserver.example.com;added by defaults ;SERVER_PORT=5060 ;added by defaults ;MYVAR=someUSERvalue;added by defaults but overdidden by user ;CALLERID=8005551212;automatically added by 1000 callerid ;DISPLAY_NAME=My Name ;automatically added by 1000 callerid ;TIMEZONE=America/Denver;added by defaults ;TZOFFSET=252100;automatically calculated by res_phoneprov ;DST_ENABLE=1 ;automatically calculated by res_phoneprov ;DST_START_MONTH=3 ;automatically calculated by res_phoneprov ;DST_START_MDAY=9 ;automatically calculated by res_phoneprov ;DST_START_HOUR=3 ;automatically calculated by res_phoneprov ;DST_END_MONTH=11 ;automatically calculated by res_phoneprov ;DST_END_MDAY=2 ;automatically calculated by res_phoneprov ;DST_END_HOUR=1 ;automatically calculated by res_phoneprov ;ENDPOINT_ID=1000 ;automatically added by this module ;AUTH_ID=1000-auth ;automatically added by this module ;TRANSPORT_ID=transport-udp-nat ;automatically added by this module ;LABEL=1000 ;added by user Diffs (updated) -
Re: [asterisk-dev] [Code Review] 4001: PJSIP: Prevent T38 framehook being put on wrong channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4001/#review13350 --- Ship it! Ship It! - Matt Jordan On Sept. 18, 2014, 4:19 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4001/ --- (Updated Sept. 18, 2014, 4:19 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This changes gives framehooks a reverse-direction masquerade callback in addition to chan_fixup_cb similar to the callback added to datastores to handle the same situation. The new callback provides the same parameters as the fixup callback, but is called on the new channel's framehooks before moving framehooks from the old channel to the new channel. This gives the framehooks an oppurtunity to decide whether they should remain on the new channel or be removed. This new callback is used to prevent the PJSIP T.38 framehook from remaining on a masqueraded channel if the new channel is not also a PJSIP channel. This was causing a crash when a local channel was masqueraded into a PJSIP channel and the framehook was executed on the local channel since the channel's tech private data was not structured as expected. Diffs - branches/12/res/res_pjsip_t38.c 423230 branches/12/main/framehook.c 423230 branches/12/include/asterisk/framehook.h 423230 Diff: https://reviewboard.asterisk.org/r/4001/diff/ Testing --- This corrected my reproduction of the crash and fixed the crash for the original reporter. Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/#review13351 --- /branches/13/res/res_pjsip_sdp_rtp.c https://reviewboard.asterisk.org/r/4000/#comment23836 Since joint only has formats of type media_type, would specifying media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here? - Mark Michelson On Sept. 18, 2014, 6:26 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/ --- (Updated Sept. 18, 2014, 6:26 p.m.) Review request for Asterisk Developers. Bugs: AFS-162 https://issues.asterisk.org/jira/browse/AFS-162 Repository: Asterisk Description --- Outgoing PJSIP calls can result in non-negotiated formats listed in the channel's native formats if video formats are listed in the endpoint's configuration. The resulting call could then use a non-negotiated format resulting in one way audio. * Simplified the update of session-req_caps in set_caps(). Why do something in five steps when only one is needed? Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 423446 /branches/13/channels/chan_pjsip.c 423446 Diff: https://reviewboard.asterisk.org/r/4000/diff/ Testing --- Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw Called from D40 with g722 among other formats enabled to a Polycom that negotiates ulaw. Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 4:48 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Changes --- Hit rmudgett's findings. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs (updated) - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.
On Sept. 11, 2014, 3:16 p.m., Joshua Colp wrote: /branches/13/res/res_pjsip.c, lines 2488-2501 https://reviewboard.asterisk.org/r/3954/diff/4/?file=67305#file67305line2488 Looking at the pjproject code why wouldn't the following return values cover these cases for when the callback is not called: PJ_EINVAL PJ_EINVALIDOP PJ_ENOMEM Handling those as special would reduce the memory leak possibility. rmudgett wrote: Almost any value could be returned because user supplied callbacks could fail with any error code they think is reasonable. Most of those transport callback routines can return PJ_EINVAL when they sanity check the parameters. If it was going out over a websocket transport the PJ_ENOMEM value could be returned. The safeset thing that can be done is to not free anything. I'm going to suggest something that I think should ensure that memory gets freed. The issue is that the callback may or may not be called when an error occurs. Similarly, the callback may be called synchronously or asynchronously. By process of elimination, if the callback is called asynchronously, then that means that the initial call to pjsip_endpt_send_request() MUST have returned success. There's no way for pjsip_endpt_send_request() to return an error if the error happens at a separate time. Any in the async case, the callback SHOULD be called every single time. The problem can be narrowed down to the case of when the callback is called synchronously when an error occurs. Since the callback is called synchronously, it can be made possible to detect if the callback has been called when pjsip_endpt_send_request() returns. There's only one piece of data that is common to both send_out_of_dialog_request() and send_request_cb(), and that's the token. In send_out_of_dialog_request(), you can wrap token inside another structure that has a flag that indicates if the callback has been called or not. With all this information at hand, you do the following: * In send_request_cb, always free the data pased in, and be sure to mark the token's wrapper so that it indicates that the callback was called. * In send_out_of_dialog_request(), if pjsip_endpt_send_request() returns an error, then check the token wrapper to see if the callback was called. If so, do nothing. If not, then free the data. In both cases, you can return an error, and the caller of send_out_of_dialog_request() can safely free their token. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/#review13283 --- On Sept. 10, 2014, 10:18 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/ --- (Updated Sept. 10, 2014, 10:18 p.m.) Review request for Asterisk Developers. Bugs: AFS-155 and ASTERISK-24295 https://issues.asterisk.org/jira/browse/AFS-155 https://issues.asterisk.org/jira/browse/ASTERISK-24295 Repository: Asterisk Description --- The crash on the issues is a result of an invalid transport configuration change when asterisk is restarted. The attempt to send the qualify request fails and we cleaned up. However, the callback is also called which results in a double unref of the objects involved. * Fixed send_request_cb() and qualify_contact_cb() to not cleanup the token when the PJSIP event is PJSIP_EVENT_TRANSPORT_ERROR since the initial function call will do the clean up. * Made send_request_cb() able to handle repeated challenges (Up to 10). * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it. The sched entry will no longer self stop and must be externally stopped. * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c. * Fix some off-nominal ref leaks in schedule_qualify(), qualify_and_schedule(). * Reordered pjsip_options.c module start/stop code to cleanup better on error. Diffs - /branches/13/res/res_pjsip/pjsip_options.c 422963 /branches/13/res/res_pjsip.c 422963 Diff: https://reviewboard.asterisk.org/r/3954/diff/ Testing --- * With the qualify_frequency option enabled, added and removed a local_net= line in the transport section and restarted asterisk via core restart now. Before the latest patch version, asterisk would crash. With the new patch, it keeps on going. * Set the qualify_frequency option to different values and reloaded res_pjsip each time. The OPTIONS poll frequency changed, started, and stopped according to the new qualify_frequency value. Thanks, rmudgett --
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3970/#review13348
---
A once over pass thourgh of the patch.
You should go through and fixup any coding guidelines problems in the changes
you have made.
branches/12/include/asterisk/chanvars.h
https://reviewboard.asterisk.org/r/3970/#comment23834
Protect these macros with the do {} while (0) trick.
I'm not sure of the usefulness of these AST_VAR_LIST_xxx macros. You are
simply hiding the if test which is for the most part already in the code.
branches/12/include/asterisk/phoneprov.h
https://reviewboard.asterisk.org/r/3970/#comment23828
You should extern this declaration in the header and initialize it in a c
file. As it is, every file that includes this header has an initialized copy.
branches/12/main/chanvars.c
https://reviewboard.asterisk.org/r/3970/#comment23835
Rather than a relatively expensive safe traversal do this:
while ((var = AST_LIST_REMOVE_HEAD()) {
ast_var_delete(var);
}
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23829
This is an interesting way of doing the standard ao2 container callback
functions.
Did you fix the formatting from the wiki page. Directly cutting and
pasting the template function from the wiki gives you spaces instead of tabs.
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23831
curly on its own line
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23832
No need for the backslash on this line as it ends the macro.
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23833
idem
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23837
if test is redundant because you defined AST_VAR_LIST_INSERT_TAIL to have
the if test inside it.
The next 7 changes are the same as here.
I looks like just about everywhere you used the new macros that there is
now a redundant if test.
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23839
curlies
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23840
Is the cast necessary?
branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/3970/#comment23841
privider is ref leaked
Use:
for (; (provider = ao2_it_next()); ao2_ref(provider, -1))
- rmudgett
On Sept. 18, 2014, 3:42 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3970/
---
(Updated Sept. 18, 2014, 3:42 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
The big piece missing for me to finally transition to pjsip was the ability
to mirror the auto provisioning features of res_phoneprov. The first step
(this patch) is to make res_phoneprov more modular so other modules (like
pjsip) can provide configuration information instead of res_phoneprov relying
solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov
public API is now exposed which allows config providers to register
themselves, set defaults (server profile, etc) and add user extensions.
ast_phoneprov_provider_register registers the provider and provides callbacks
for loading default settings and loading users.
ast_phoneprov_provider_unregister clears the defaults and users.
ast_phoneprov_add_extension should be called once for each user/extension by
the provider's load_users callback to add them.
ast_phoneprov_delete_extension deletes one extension.
ast_phoneprov_delete_extensions deletes all extensions for the provider.
res_phoneprov actually registers itself as the provider for sip/users and is
always available and is the default.
Writing a new provider...
Since res_phoneprov is also it's own provider, examples of what a new
provider would have to do are in load_users() in res_phoneprov.c. Those
functions gather the information from users.conf and sip.conf and call the
ast_provider_register and ast_phoneprov_add_extension apis.
So...
The provider creates a callback function which calls the
ast_phoneprov_add_extension api for each user.
It then calls ast_phoneprov_provider_register with the callback.
res_phoneprov then calls the callback to cause the actual load.
During normal http server ops, all work is done by res_phoneprov and the
provider is never called again unless a reload is needed.
If the provider wants to reload it can simply unregister and reregister or it
can call its own load_users callback.
If
Re: [asterisk-dev] [Code Review] 3980: cel_odbc: Add microseconds precision in the eventtime column
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3980/#review13353 --- Ship it! Ship It! - Mark Michelson On Sept. 5, 2014, 7:54 p.m., Etienne Lessard wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3980/ --- (Updated Sept. 5, 2014, 7:54 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24283 https://issues.asterisk.org/jira/browse/ASTERISK-24283 Repository: Asterisk Description --- This patch adds microsecond precision when inserting a CEL record into a table with an eventtime column of type timestamp, instead of second precision. The documentation (configs/cel_odbc.conf.sample) was already saying that the eventtime column included microseconds precision, but that was not the case. Also, without this patch, if you had a table with an eventtime column of type varchar, you had millisecond precision. With this patch, you also get microsecond precision in this case. Diffs - /branches/11/cel/cel_odbc.c 422682 Diff: https://reviewboard.asterisk.org/r/3980/diff/ Testing --- Tested with postgres 9.1 and mysql 5.5. With postgres, with a CEL table with an eventtime column of type timestamp, you get microsecond precision. Same for a CEL table with an eventtime column of type varchar. With mysql, with a CEL table with an eventtime column of type timestamp, you still get only second precision, because mysql 5.5 don't store it ( http://dev.mysql.com/doc/refman/5.5/en/fractional-seconds.html ). That said, it's not causing any problem. For a CEL table with an eventtime column of type varchar, you do get microsecond precision. Thanks, Etienne Lessard -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3673: RLS: Nominal list tests
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3673/ --- (Updated Sept. 18, 2014, 10:32 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Bugs: ASTERISK-23870 https://issues.asterisk.org/jira/browse/ASTERISK-23870 Repository: testsuite Description --- This changeset implements the nominal resource list tests outlined on this page: https://wiki.asterisk.org/wiki/display/AST/Resource+List+Subscription+Test+Plan There are six tests: 1. Subscription Establishment: Simply ensures that Asterisk responds with a 200 OK when we subscribe to a resource list and that the 200 OK has a Require: eventlist header in it. 2. Initial NOTIFY: Validates the initial NOTIFY body that Asterisk sends when subscribing to a resource list. 3. Full State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with full state of the list. 4. Partial State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with partial state, with only the state of the resource whose state was changed. 5. Resubscription Full State: Establishes a subscription and then resubscribes. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the resubscription has full state of the list. 6. Termination Full State: Establishes a subscription and then terminates the subscription. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the termination has full state of the list. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/tests.yaml 5168 /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/rls_integrity.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/sipp/resubscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/resubscribe.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/partial_state.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/configs/ast1/extensions.conf PRE-CREATION
Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.
On Sept. 18, 2014, 4:35 p.m., Mark Michelson wrote: /branches/13/res/res_pjsip_sdp_rtp.c, line 259 https://reviewboard.asterisk.org/r/4000/diff/1/?file=67396#file67396line259 Since joint only has formats of type media_type, would specifying media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here? It's a case of six of one and half a dozen of another. It won't make any difference in this case since all formats will be appended anyway. It's a little more efficient to use the constant since the function has to test if it isn't UNKNOWN and then test to see if it is the specified type. I'll leave it as is unless there is a stronger argument for changing it. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/#review13351 --- On Sept. 18, 2014, 1:26 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/ --- (Updated Sept. 18, 2014, 1:26 p.m.) Review request for Asterisk Developers. Bugs: AFS-162 https://issues.asterisk.org/jira/browse/AFS-162 Repository: Asterisk Description --- Outgoing PJSIP calls can result in non-negotiated formats listed in the channel's native formats if video formats are listed in the endpoint's configuration. The resulting call could then use a non-negotiated format resulting in one way audio. * Simplified the update of session-req_caps in set_caps(). Why do something in five steps when only one is needed? Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 423446 /branches/13/channels/chan_pjsip.c 423446 Diff: https://reviewboard.asterisk.org/r/4000/diff/ Testing --- Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw Called from D40 with g722 among other formats enabled to a Polycom that negotiates ulaw. Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.
On Sept. 18, 2014, 9:35 p.m., Mark Michelson wrote: /branches/13/res/res_pjsip_sdp_rtp.c, line 259 https://reviewboard.asterisk.org/r/4000/diff/1/?file=67396#file67396line259 Since joint only has formats of type media_type, would specifying media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here? rmudgett wrote: It's a case of six of one and half a dozen of another. It won't make any difference in this case since all formats will be appended anyway. It's a little more efficient to use the constant since the function has to test if it isn't UNKNOWN and then test to see if it is the specified type. I'll leave it as is unless there is a stronger argument for changing it. Okay, that's good enough for me. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/#review13351 --- On Sept. 18, 2014, 6:26 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/ --- (Updated Sept. 18, 2014, 6:26 p.m.) Review request for Asterisk Developers. Bugs: AFS-162 https://issues.asterisk.org/jira/browse/AFS-162 Repository: Asterisk Description --- Outgoing PJSIP calls can result in non-negotiated formats listed in the channel's native formats if video formats are listed in the endpoint's configuration. The resulting call could then use a non-negotiated format resulting in one way audio. * Simplified the update of session-req_caps in set_caps(). Why do something in five steps when only one is needed? Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 423446 /branches/13/channels/chan_pjsip.c 423446 Diff: https://reviewboard.asterisk.org/r/4000/diff/ Testing --- Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw Called from D40 with g722 among other formats enabled to a Polycom that negotiates ulaw. Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/#review13356 --- Ship it! Ship It! - Mark Michelson On Sept. 18, 2014, 6:26 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/ --- (Updated Sept. 18, 2014, 6:26 p.m.) Review request for Asterisk Developers. Bugs: AFS-162 https://issues.asterisk.org/jira/browse/AFS-162 Repository: Asterisk Description --- Outgoing PJSIP calls can result in non-negotiated formats listed in the channel's native formats if video formats are listed in the endpoint's configuration. The resulting call could then use a non-negotiated format resulting in one way audio. * Simplified the update of session-req_caps in set_caps(). Why do something in five steps when only one is needed? Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 423446 /branches/13/channels/chan_pjsip.c 423446 Diff: https://reviewboard.asterisk.org/r/4000/diff/ Testing --- Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw Called from D40 with g722 among other formats enabled to a Polycom that negotiates ulaw. Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 5:50 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Changes --- Fix reference leak rmudgett pointed out. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs (updated) - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13358 --- Ship it! Ship It! - rmudgett On Sept. 18, 2014, 5:50 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/ --- (Updated Sept. 18, 2014, 5:50 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24237 https://issues.asterisk.org/jira/browse/ASTERISK-24237 Repository: Asterisk Description --- Reproduction: pj123 calls 1601 pj123 does a SIP blonde transfer to 1603 1603 answers FRACK occurs all are PJSIP endpoints. Basically what happens is there is a second outbound dial from another pj123 channel. Before the dial is answered, the pj123 gets masqueraded out of the picture and replaced with a channel that isn't in the dial state. This patch fixes that by advancing the incoming channel to the dial state in the channel breakdown function of a datastore on the pj123 channel. Honestly, I'm not sure this is entirely adequate, but it seems to work in all of the conditions I've tried so far and it doesn't impede normal attended transfers. I might need to try and figure out what happens if I masquerade in a channel that is already dialing though. I'm not sure if that's something we can really expect to happen under normal conditions, but that seems like something that would screw up this approach. Note that I think this patch is the right idea, I just don't know if I need to account for the possibility that the channel that masquerades into pj123's dialing channel might not be in the neutral state. Diffs - /branches/12/main/stasis_channels.c 422882 Diff: https://reviewboard.asterisk.org/r/3990/diff/ Testing --- Ran against reproduction of the above scenario. Assertion was gone and the CDR results were as follows: ,123,1601,default, 123,PJSIP/pj123-,PJSIP/1601-0001,Dial,PJSIP/1601,,tT,2014-09-11 21:48:51,2014-09-11 21:48:53,2014-09-11 21:48:57,5,4,ANSWERED,DOCUMENTATION,1410472131.0, ,123,,default, 123,PJSIP/pj123-0002,PJSIP/1603-0003,Dial,PJSIP/1603,2014-09-11 21:48:55,,2014-09-11 21:48:57,2,0,NO ANSWER,DOCUMENTATION,1410472135.6, ,1601,1603,default, 1601,PJSIP/1601-0001,PJSIP/1603-0003,AppDial,(Outgoing Line),2014-09-11 21:48:57,2014-09-11 21:48:57,2014-09-11 21:49:04,6,6,ANSWERED,DOCUMENTATION,1410472131.1, And dial events reported by AMI: http://pastebin.com/tWuwL7xa (note that there is a mismatch between the number of dial end and dial begin events... not sure if this is a problem, but I might be able to fix it just by updating the old chan, not sure what status code to use though). Ran against normal attended transfer, feature attended transfers, and blind transfers with no noticeable effect. Ran against testsuite blonde transfer tests, some attended transfer tests, some blind transfer tests, and all pjsip transfer tests (at least ones that will run on my box... a few won't due to sipp version requirements that I really need to get around to fixing eventually) for comparison purposes. All passed exhibiting the same behavior as before as far as warning messages and such go. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/#review13359 --- branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23848 Probably should be marked as extended since res_phoneprov is extended. branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23852 Concerned about the parameter names chosen. All uppercase parameter names is a bit unusual. I think there is a requirement for pjsip parameters to be snake_case for ARI. branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23851 Thge ?? branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23850 Another case where the if test burried in AST_VAR_LIST_INSERT_TAIL() is redundant. branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23847 Use: for (; (pp = ao2_iter_next()); ao2_ref(pp, -1)) This way you could use continue instead of goto cleanup. Probably should break up the while loop body into functions. It is rather large. branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23846 Check for failure. - rmudgett On Sept. 18, 2014, 4:21 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 4:21 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
On Sept. 18, 2014, 4:11 p.m., rmudgett wrote: branches/12/res/res_phoneprov.c, lines 128-129 https://reviewboard.asterisk.org/r/3970/diff/4/?file=67281#file67281line128 This is an interesting way of doing the standard ao2 container callback functions. Did you fix the formatting from the wiki page. Directly cutting and pasting the template function from the wiki gives you spaces instead of tabs. Yep, I reformatted it after pasting it. On Sept. 18, 2014, 4:11 p.m., rmudgett wrote: branches/12/res/res_phoneprov.c, line 983 https://reviewboard.asterisk.org/r/3970/diff/4/?file=67281#file67281line983 Is the cast necessary? Yes. The compiler complains note: expected ‘void *’ but argument is of type ‘ast_string_field’ - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/#review13348 --- On Sept. 18, 2014, 6:01 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 18, 2014, 6:01 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_register and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 423501 branches/12/main/chanvars.c 423501 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 423501 branches/12/configs/phoneprov.conf.sample 423501 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 18, 2014, 6:01 p.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's comments. Repository: Asterisk Description --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_register and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs (updated) - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 423501 branches/12/main/chanvars.c 423501 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 423501 branches/12/configs/phoneprov.conf.sample 423501 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Thu, Sep 18, 2014 at 5:12 PM, Russell Bryant russ...@russellbryant.net wrote: On Thu, Sep 18, 2014 at 3:01 PM, Samuel Galarneau sgalarn...@digium.com wrote: A couple more comments about the magic happening here ... First, git review knows where to push based on a file checked in to the repo: $ cat .gitreview [gerrit] host=review.openstack.org port=29418 project=openstack/nova.git git review also sets up a local commit hook that adds a Change-Id header to your commit message. That Change-Id is what links multiple revisions of the same change together. So, if you edit your change and push it again, as long as the Change-Id remains the same, gerrit treats it as the same review request and not a new one. Sounds good to me. So it doesn't really matter from which repo you post a review so long as it's a clone of the original with that .gitreview file. I have another question unrelated to reviews. Does your setup make it easy to mirror a repo? In a more complicated scenario, what if someone had a private fork but they wanted to get public commits to master mirrored to their repo? Would they have to treat the original repo as upstream and manually pull changes and rebase their private branch off of it? Mirroring, sure. I don't remember exactly how we do it, but all OpenStack repos are mirrored to github for convenience, for example. Technically, the repos on github for Openstack are the mirrors of a mirror, I know :) The basic idea is the upstream repo lives either in gerrit or zuul, then can mirror the repos to any system. Thing to remember, everything goes through gerrit and zuul. So, if you mirrored to github, PR are useless since it is only read-only. For internal projects, I host a cgit server and public I just mirror to github. Regarding private branches, git generally makes that kind of thing ***MUCH*** easier than svn. You can easily track the exact commits that are applied on top of upstream. You could either periodically rebase your changes on top of upstream (re-applying the commits, rewriting history but a cleaner history), or periodically merge from upstream. In either case, I personally wouldn't automate it. You want some sanity checking around that stuff. Conflicts happen. Really, I think this is going to be MUCH better no matter what specific infrastructure you go with. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- Paul Belanger | PolyBeacon, Inc. Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode) Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
On Sept. 18, 2014, 5:32 p.m., rmudgett wrote: branches/12/res/res_pjsip_phoneprov_provider.c, lines 71-75 https://reviewboard.asterisk.org/r/3976/diff/3/?file=67405#file67405line71 Concerned about the parameter names chosen. All uppercase parameter names is a bit unusual. I think there is a requirement for pjsip parameters to be snake_case for ARI. phoneprov variable have historically been upper case and they are case-sensitive. Having some upper and some lower would be not only confusing but it would make it impossible to use the same template file for both sip and pjsip providers. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/#review13359 --- On Sept. 18, 2014, 3:21 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 3:21 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a template substitution map as follows: ;MAC=deadbeef4dad ;added by pp1000 ;USERNAME=myname;automatically added by 1000-auth username ;SECRET=mysecret;automatically added by 1000-auth password ;PROFILE=someprofile;added by defaults ;SERVER=myserver.example.com;added by defaults ;SERVER_PORT=5060 ;added by defaults ;MYVAR=someUSERvalue;added by defaults but overdidden by user ;CALLERID=8005551212;automatically added by 1000 callerid ;DISPLAY_NAME=My Name ;automatically added by 1000
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
On Sept. 18, 2014, 5:32 p.m., rmudgett wrote: branches/12/res/res_pjsip_phoneprov_provider.c, line 174 https://reviewboard.asterisk.org/r/3976/diff/3/?file=67405#file67405line174 Another case where the if test burried in AST_VAR_LIST_INSERT_TAIL() is redundant. true but here I'm actually checking the return for the allocation failure and bailing. res_phoneprov always silently ignored the allocation failure and just didn't insert the variable into the list. Silently ignoring non-required variables can't hurt anything and if the system is so low on memory that it can't allocate a dozen bytes then you'll never see the error messages anyway. So, I guess I'll remove the check here and silently ignore. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/#review13359 --- On Sept. 18, 2014, 3:21 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 3:21 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a template substitution map as follows: ;MAC=deadbeef4dad ;added by pp1000 ;USERNAME=myname;automatically added by 1000-auth username ;SECRET=mysecret;automatically added by 1000-auth password ;PROFILE=someprofile;added by defaults ;SERVER=myserver.example.com;added by defaults ;SERVER_PORT=5060 ;added by defaults ;MYVAR=someUSERvalue;added by defaults but overdidden by user ;CALLERID=8005551212
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 6:55 p.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's comments. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a template substitution map as follows: ;MAC=deadbeef4dad ;added by pp1000 ;USERNAME=myname;automatically added by 1000-auth username ;SECRET=mysecret;automatically added by 1000-auth password ;PROFILE=someprofile;added by defaults ;SERVER=myserver.example.com;added by defaults ;SERVER_PORT=5060 ;added by defaults ;MYVAR=someUSERvalue;added by defaults but overdidden by user ;CALLERID=8005551212;automatically added by 1000 callerid ;DISPLAY_NAME=My Name ;automatically added by 1000 callerid ;TIMEZONE=America/Denver;added by defaults ;TZOFFSET=252100;automatically calculated by res_phoneprov ;DST_ENABLE=1 ;automatically calculated by res_phoneprov ;DST_START_MONTH=3 ;automatically calculated by res_phoneprov ;DST_START_MDAY=9 ;automatically calculated by res_phoneprov ;DST_START_HOUR=3 ;automatically calculated by res_phoneprov ;DST_END_MONTH=11 ;automatically calculated by res_phoneprov ;DST_END_MDAY=2 ;automatically calculated by res_phoneprov ;DST_END_HOUR=1 ;automatically calculated by res_phoneprov ;ENDPOINT_ID=1000 ;automatically added by this module ;AUTH_ID=1000-auth ;automatically added by this module ;TRANSPORT_ID=transport-udp-nat ;automatically added by this module ;LABEL=1000 ;added by user Diffs (updated) - branches/12/res/res_pjsip_phoneprov_provider.c
