Re: [asterisk-dev] [Code Review] 4442: chan_sip: Asterisk fails to re-activate an inactive media session when an offer does not contain a=sendrecv
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/#review14522 --- I don't see the *_no_direction.xml files. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml https://reviewboard.asterisk.org/r/4442/#comment25076 Heh, that was sloppy ;) - wdoekes On Feb. 23, 2015, 9:38 p.m., Ashley Sanders wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/ --- (Updated Feb. 23, 2015, 9:38 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24824 https://issues.asterisk.org/jira/browse/ASTERISK-24824 Repository: testsuite Description --- This test is to ensure that Asterisk correctly applies the direction of the media stream when a=sendonly|recvonly|inactive|sendrecv is missing from the offer's SDP. The expected behavior is for Asterisk to apply sendrecv as the direction of the media stream when no direction attribute is present in an offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): If none of the attributes sendonly, recvonly, inactive, and sendrecv is present, sendrecv SHOULD be assumed as the default for sessions that are not of the conference type broadcast or H332 [...] The test scenario: 1. From Phone A, send an offer to Phone B to establish a call 2. From Phone B, send an offer to Phone A to put the call on hold. 3. Observe that the MOH start event occurs. 4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that the direction attribute from the offer's SDP is omitted) 5. Observe that the MOH stop event occurs. Presently, this test fails for certain versions of Asterisk. From what I can tell, it is present from (at least) 1.8.21 up to the 11 branch. ***Note*** This is the test. It is only the test. The update to the Asterisk source is coming soon to a review board near you (well, this review board). Diffs - ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 Diff: https://reviewboard.asterisk.org/r/4442/diff/ Testing --- Thanks, Ashley Sanders -- _ -- 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] 4442: chan_sip: Asterisk fails to re-activate an inactive media session when an offer does not contain a=sendrecv
On Feb. 24, 2015, 2:29 a.m., wdoekes wrote: ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml, line 213 https://reviewboard.asterisk.org/r/4442/diff/1/?file=71571#file71571line213 Heh, that was sloppy ;) I blame whoever wrote this test initially. /me hides - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/#review14522 --- On Feb. 23, 2015, 3:38 p.m., Ashley Sanders wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/ --- (Updated Feb. 23, 2015, 3:38 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24824 https://issues.asterisk.org/jira/browse/ASTERISK-24824 Repository: testsuite Description --- This test is to ensure that Asterisk correctly applies the direction of the media stream when a=sendonly|recvonly|inactive|sendrecv is missing from the offer's SDP. The expected behavior is for Asterisk to apply sendrecv as the direction of the media stream when no direction attribute is present in an offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): If none of the attributes sendonly, recvonly, inactive, and sendrecv is present, sendrecv SHOULD be assumed as the default for sessions that are not of the conference type broadcast or H332 [...] The test scenario: 1. From Phone A, send an offer to Phone B to establish a call 2. From Phone B, send an offer to Phone A to put the call on hold. 3. Observe that the MOH start event occurs. 4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that the direction attribute from the offer's SDP is omitted) 5. Observe that the MOH stop event occurs. Presently, this test fails for certain versions of Asterisk. From what I can tell, it is present from (at least) 1.8.21 up to the 11 branch. ***Note*** This is the test. It is only the test. The update to the Asterisk source is coming soon to a review board near you (well, this review board). Diffs - ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 Diff: https://reviewboard.asterisk.org/r/4442/diff/ Testing --- Thanks, Ashley Sanders -- _ -- 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] 4379: Example configuration scenario - Super Awesome Company: Phase 1 - Patch 1
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/#review14524 --- Ship it! Ship It! - Matt Jordan On Feb. 24, 2015, 7:20 a.m., rnewton wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/ --- (Updated Feb. 24, 2015, 7:20 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- One of things discussed at the last AstriDevCon was better documentation (for everything!), but in particular, we mentioned needing some example configurations that pertain to a real-world scenario. That is, as opposed to the current sample files which are sort of all over the place at this point. This patch proposes a basic and minimal configuration of Asterisk to satisfy the requirements for the first phase of Super Awesome Company's implementation of Asterisk. I will submit four separate patches for the first phase, so that we don't have to review the entire thing all at once. This review is for the first patch. Who is Super Awesome Company? See https://wiki.asterisk.org/wiki/display/AST/Super+Awesome+Company For the first patch, I am attempting to satisfy the below requirements. The patch does not include a new make target, as I believe Matt Jordan offered to handle that. SAC requires: * PJSIP connectivity for all employee desk phones. * The ability for employees to call one another inside of the office. * Voicemail boxes for each of the employees. Basic configuration We want SAC to have a clean system. That means: * No 'autoload' in modules.conf. Explicitly load a basic configuration. If SAC doesn't need the module, don't load it. * Every module loaded should have a configuration file that is appropriate for it. This includes all the 'core' things that need configuration. pjsip.conf * A PJSIP configuration for their desk phones. Assume every endpoint that is a phone has: * A voicemail mailbox that they can subscribe to * A hint for their device * Note that the PJSIP configuration should adhere to best practices. That means MAC addresses for device names, etc. extensions.conf * A safe dialplan for intra-company communication. This should be templated out so that it is trivial to add additional devices (use pattern matching/pattern matching hints, etc.) * Receiving a Busy/Unavailable should result in going to VoiceMail * A user should be able to dial something and get to their VoiceMailMain without having to enter in their extension number * Note that mapping of MAC address endpoints to extension numbers should be done in some fashion that is easily extensible. voicemail.conf * Set up mailboxes for every person in SAC. Assign 'default' pins. Create reasonable basic settings. * Do not set up e-mail or pager addresses. REVIEW? Please, if possible look at this from a few angles: * Use the configuration, configure a couple phones and call between them. Leave voicemails and retrieve them. * Have I created any security issues? * Is my dialplan easy to understand? * Could anything be done more efficiently without making it over-complicated? * Have I over-complicated anything? * Are there any critical settings I'm missing from any of the files? A couple, more specific questions: * We have sample configs in /configs/samples; what directory do we want these configurations in? (I used /configs/examples for now, but I don't really like it) * We have the make target make samples for the current samples; what do we want for these new configs? Diffs - /branches/13/configs/examples/awesome/voicemail.conf PRE-CREATION /branches/13/configs/examples/awesome/pjsip.conf PRE-CREATION /branches/13/configs/examples/awesome/musiconhold.conf PRE-CREATION /branches/13/configs/examples/awesome/modules.conf PRE-CREATION /branches/13/configs/examples/awesome/logger.conf PRE-CREATION /branches/13/configs/examples/awesome/indications.conf PRE-CREATION /branches/13/configs/examples/awesome/extensions.conf PRE-CREATION /branches/13/configs/examples/awesome/asterisk.conf PRE-CREATION /branches/13/configs/examples/awesome/README PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4379/diff/ Testing --- Setup Asterisk with configuration, connected up three phones using the first three users. Made calls between them all, left voicemails and retrieved them with all users. Verified MWI working with all phones. Thanks, rnewton --
Re: [asterisk-dev] [Code Review] 4379: Example configuration scenario - Super Awesome Company: Phase 1 - Patch 1
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/#review14526 --- Ship it! Ship It! - Joshua Colp On Feb. 24, 2015, 1:20 p.m., rnewton wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/ --- (Updated Feb. 24, 2015, 1:20 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- One of things discussed at the last AstriDevCon was better documentation (for everything!), but in particular, we mentioned needing some example configurations that pertain to a real-world scenario. That is, as opposed to the current sample files which are sort of all over the place at this point. This patch proposes a basic and minimal configuration of Asterisk to satisfy the requirements for the first phase of Super Awesome Company's implementation of Asterisk. I will submit four separate patches for the first phase, so that we don't have to review the entire thing all at once. This review is for the first patch. Who is Super Awesome Company? See https://wiki.asterisk.org/wiki/display/AST/Super+Awesome+Company For the first patch, I am attempting to satisfy the below requirements. The patch does not include a new make target, as I believe Matt Jordan offered to handle that. SAC requires: * PJSIP connectivity for all employee desk phones. * The ability for employees to call one another inside of the office. * Voicemail boxes for each of the employees. Basic configuration We want SAC to have a clean system. That means: * No 'autoload' in modules.conf. Explicitly load a basic configuration. If SAC doesn't need the module, don't load it. * Every module loaded should have a configuration file that is appropriate for it. This includes all the 'core' things that need configuration. pjsip.conf * A PJSIP configuration for their desk phones. Assume every endpoint that is a phone has: * A voicemail mailbox that they can subscribe to * A hint for their device * Note that the PJSIP configuration should adhere to best practices. That means MAC addresses for device names, etc. extensions.conf * A safe dialplan for intra-company communication. This should be templated out so that it is trivial to add additional devices (use pattern matching/pattern matching hints, etc.) * Receiving a Busy/Unavailable should result in going to VoiceMail * A user should be able to dial something and get to their VoiceMailMain without having to enter in their extension number * Note that mapping of MAC address endpoints to extension numbers should be done in some fashion that is easily extensible. voicemail.conf * Set up mailboxes for every person in SAC. Assign 'default' pins. Create reasonable basic settings. * Do not set up e-mail or pager addresses. REVIEW? Please, if possible look at this from a few angles: * Use the configuration, configure a couple phones and call between them. Leave voicemails and retrieve them. * Have I created any security issues? * Is my dialplan easy to understand? * Could anything be done more efficiently without making it over-complicated? * Have I over-complicated anything? * Are there any critical settings I'm missing from any of the files? A couple, more specific questions: * We have sample configs in /configs/samples; what directory do we want these configurations in? (I used /configs/examples for now, but I don't really like it) * We have the make target make samples for the current samples; what do we want for these new configs? Diffs - /branches/13/configs/examples/awesome/voicemail.conf PRE-CREATION /branches/13/configs/examples/awesome/pjsip.conf PRE-CREATION /branches/13/configs/examples/awesome/musiconhold.conf PRE-CREATION /branches/13/configs/examples/awesome/modules.conf PRE-CREATION /branches/13/configs/examples/awesome/logger.conf PRE-CREATION /branches/13/configs/examples/awesome/indications.conf PRE-CREATION /branches/13/configs/examples/awesome/extensions.conf PRE-CREATION /branches/13/configs/examples/awesome/asterisk.conf PRE-CREATION /branches/13/configs/examples/awesome/README PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4379/diff/ Testing --- Setup Asterisk with configuration, connected up three phones using the first three users. Made calls between them all, left voicemails and retrieved them with all users. Verified MWI working with all phones. Thanks, rnewton --
Re: [asterisk-dev] [Code Review] 4443: dial api: add self destruction option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4443/#review14527 --- /branches/13/main/dial.c https://reviewboard.asterisk.org/r/4443/#comment25079 I'm personally a fan of always doing an enum ast_dial_result state = dial-state and only having the one return path. But that's just me. - Joshua Colp On Feb. 23, 2015, 11:26 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4443/ --- (Updated Feb. 23, 2015, 11:26 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds a self-destruction ability to the dial api. The usefulness of this is mostly when using async mode to spawn a separate thread to handle the new call, while the calling thread is allowed to go on about other business. The alternative of this option is that the calling thread must either hang around for the duration or spawn it's own thread in order to create and then later destroy the dial structure after the call completes. Example (minus error checking): struct ast_dial *dial = ast_dial_create(); ast_dial_append(dial, PJSIP, 200, NULL); ast_dial_option_global_enable(dial, AST_DIAL_OPTION_ANSWER_EXEC, Echo); ast_dial_option_global_enable(dial, AST_DIAL_OPTION_SELF_DESTROY, NULL); ast_dial_run(dial, NULL, 1); The dial_run call returns almost immediately after spawning a new thread to complete and monitor the dial. If the call is answered, it is put into echo. When completed, ast_dial_destroy() will be called on the dial structure. Note that any allocations made to pass values to ast_dial_set_user_data() or other dial options will need to be free'd in a state callback function on any of AST_DIAL_RESULT_UNASWERED, AST_DIAL_RESULT_ANSWERED, AST_DIAL_RESULT_HANGUP, or AST_DIAL_RESULT_TIMEOUT. Diffs - /branches/13/main/dial.c 432173 /branches/13/include/asterisk/dial.h 432173 Diff: https://reviewboard.asterisk.org/r/4443/diff/ Testing --- Correct operation confirmed with a temporary test function running under valgrind to insure there are no invalid references or leaks. Thanks, Scott Griepentrog -- _ -- 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] 4379: Example configuration scenario - Super Awesome Company: Phase 1 - Patch 1
On Feb. 20, 2015, 2:37 p.m., Matt Jordan wrote: /branches/13/configs/examples/awesome/extensions.conf, lines 29-31 https://reviewboard.asterisk.org/r/4379/diff/2/?file=71364#file71364line29 Is this comment correct still with this pattern match? Nope. Fixed. I also adjusted the pattern match here and for hints as for the current users we only need '_11XX' . - rnewton --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/#review14511 --- On Feb. 13, 2015, 12:46 a.m., rnewton wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/ --- (Updated Feb. 13, 2015, 12:46 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- One of things discussed at the last AstriDevCon was better documentation (for everything!), but in particular, we mentioned needing some example configurations that pertain to a real-world scenario. That is, as opposed to the current sample files which are sort of all over the place at this point. This patch proposes a basic and minimal configuration of Asterisk to satisfy the requirements for the first phase of Super Awesome Company's implementation of Asterisk. I will submit four separate patches for the first phase, so that we don't have to review the entire thing all at once. This review is for the first patch. Who is Super Awesome Company? See https://wiki.asterisk.org/wiki/display/AST/Super+Awesome+Company For the first patch, I am attempting to satisfy the below requirements. The patch does not include a new make target, as I believe Matt Jordan offered to handle that. SAC requires: * PJSIP connectivity for all employee desk phones. * The ability for employees to call one another inside of the office. * Voicemail boxes for each of the employees. Basic configuration We want SAC to have a clean system. That means: * No 'autoload' in modules.conf. Explicitly load a basic configuration. If SAC doesn't need the module, don't load it. * Every module loaded should have a configuration file that is appropriate for it. This includes all the 'core' things that need configuration. pjsip.conf * A PJSIP configuration for their desk phones. Assume every endpoint that is a phone has: * A voicemail mailbox that they can subscribe to * A hint for their device * Note that the PJSIP configuration should adhere to best practices. That means MAC addresses for device names, etc. extensions.conf * A safe dialplan for intra-company communication. This should be templated out so that it is trivial to add additional devices (use pattern matching/pattern matching hints, etc.) * Receiving a Busy/Unavailable should result in going to VoiceMail * A user should be able to dial something and get to their VoiceMailMain without having to enter in their extension number * Note that mapping of MAC address endpoints to extension numbers should be done in some fashion that is easily extensible. voicemail.conf * Set up mailboxes for every person in SAC. Assign 'default' pins. Create reasonable basic settings. * Do not set up e-mail or pager addresses. REVIEW? Please, if possible look at this from a few angles: * Use the configuration, configure a couple phones and call between them. Leave voicemails and retrieve them. * Have I created any security issues? * Is my dialplan easy to understand? * Could anything be done more efficiently without making it over-complicated? * Have I over-complicated anything? * Are there any critical settings I'm missing from any of the files? A couple, more specific questions: * We have sample configs in /configs/samples; what directory do we want these configurations in? (I used /configs/examples for now, but I don't really like it) * We have the make target make samples for the current samples; what do we want for these new configs? Diffs - /branches/13/configs/examples/awesome/voicemail.conf PRE-CREATION /branches/13/configs/examples/awesome/pjsip.conf PRE-CREATION /branches/13/configs/examples/awesome/musiconhold.conf PRE-CREATION /branches/13/configs/examples/awesome/modules.conf PRE-CREATION /branches/13/configs/examples/awesome/logger.conf PRE-CREATION /branches/13/configs/examples/awesome/indications.conf PRE-CREATION /branches/13/configs/examples/awesome/extensions.conf PRE-CREATION /branches/13/configs/examples/awesome/asterisk.conf PRE-CREATION /branches/13/configs/examples/awesome/README PRE-CREATION Diff:
Re: [asterisk-dev] [Code Review] 4379: Example configuration scenario - Super Awesome Company: Phase 1 - Patch 1
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4379/ --- (Updated Feb. 24, 2015, 1:20 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- One of things discussed at the last AstriDevCon was better documentation (for everything!), but in particular, we mentioned needing some example configurations that pertain to a real-world scenario. That is, as opposed to the current sample files which are sort of all over the place at this point. This patch proposes a basic and minimal configuration of Asterisk to satisfy the requirements for the first phase of Super Awesome Company's implementation of Asterisk. I will submit four separate patches for the first phase, so that we don't have to review the entire thing all at once. This review is for the first patch. Who is Super Awesome Company? See https://wiki.asterisk.org/wiki/display/AST/Super+Awesome+Company For the first patch, I am attempting to satisfy the below requirements. The patch does not include a new make target, as I believe Matt Jordan offered to handle that. SAC requires: * PJSIP connectivity for all employee desk phones. * The ability for employees to call one another inside of the office. * Voicemail boxes for each of the employees. Basic configuration We want SAC to have a clean system. That means: * No 'autoload' in modules.conf. Explicitly load a basic configuration. If SAC doesn't need the module, don't load it. * Every module loaded should have a configuration file that is appropriate for it. This includes all the 'core' things that need configuration. pjsip.conf * A PJSIP configuration for their desk phones. Assume every endpoint that is a phone has: * A voicemail mailbox that they can subscribe to * A hint for their device * Note that the PJSIP configuration should adhere to best practices. That means MAC addresses for device names, etc. extensions.conf * A safe dialplan for intra-company communication. This should be templated out so that it is trivial to add additional devices (use pattern matching/pattern matching hints, etc.) * Receiving a Busy/Unavailable should result in going to VoiceMail * A user should be able to dial something and get to their VoiceMailMain without having to enter in their extension number * Note that mapping of MAC address endpoints to extension numbers should be done in some fashion that is easily extensible. voicemail.conf * Set up mailboxes for every person in SAC. Assign 'default' pins. Create reasonable basic settings. * Do not set up e-mail or pager addresses. REVIEW? Please, if possible look at this from a few angles: * Use the configuration, configure a couple phones and call between them. Leave voicemails and retrieve them. * Have I created any security issues? * Is my dialplan easy to understand? * Could anything be done more efficiently without making it over-complicated? * Have I over-complicated anything? * Are there any critical settings I'm missing from any of the files? A couple, more specific questions: * We have sample configs in /configs/samples; what directory do we want these configurations in? (I used /configs/examples for now, but I don't really like it) * We have the make target make samples for the current samples; what do we want for these new configs? Diffs (updated) - /branches/13/configs/examples/awesome/voicemail.conf PRE-CREATION /branches/13/configs/examples/awesome/pjsip.conf PRE-CREATION /branches/13/configs/examples/awesome/musiconhold.conf PRE-CREATION /branches/13/configs/examples/awesome/modules.conf PRE-CREATION /branches/13/configs/examples/awesome/logger.conf PRE-CREATION /branches/13/configs/examples/awesome/indications.conf PRE-CREATION /branches/13/configs/examples/awesome/extensions.conf PRE-CREATION /branches/13/configs/examples/awesome/asterisk.conf PRE-CREATION /branches/13/configs/examples/awesome/README PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4379/diff/ Testing --- Setup Asterisk with configuration, connected up three phones using the first three users. Made calls between them all, left voicemails and retrieved them with all users. Verified MWI working with all phones. Thanks, rnewton -- _ -- 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] 4443: dial api: add self destruction option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4443/#review14528 --- Ship it! Ship It! - Joshua Colp On Feb. 23, 2015, 11:26 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4443/ --- (Updated Feb. 23, 2015, 11:26 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds a self-destruction ability to the dial api. The usefulness of this is mostly when using async mode to spawn a separate thread to handle the new call, while the calling thread is allowed to go on about other business. The alternative of this option is that the calling thread must either hang around for the duration or spawn it's own thread in order to create and then later destroy the dial structure after the call completes. Example (minus error checking): struct ast_dial *dial = ast_dial_create(); ast_dial_append(dial, PJSIP, 200, NULL); ast_dial_option_global_enable(dial, AST_DIAL_OPTION_ANSWER_EXEC, Echo); ast_dial_option_global_enable(dial, AST_DIAL_OPTION_SELF_DESTROY, NULL); ast_dial_run(dial, NULL, 1); The dial_run call returns almost immediately after spawning a new thread to complete and monitor the dial. If the call is answered, it is put into echo. When completed, ast_dial_destroy() will be called on the dial structure. Note that any allocations made to pass values to ast_dial_set_user_data() or other dial options will need to be free'd in a state callback function on any of AST_DIAL_RESULT_UNASWERED, AST_DIAL_RESULT_ANSWERED, AST_DIAL_RESULT_HANGUP, or AST_DIAL_RESULT_TIMEOUT. Diffs - /branches/13/main/dial.c 432173 /branches/13/include/asterisk/dial.h 432173 Diff: https://reviewboard.asterisk.org/r/4443/diff/ Testing --- Correct operation confirmed with a temporary test function running under valgrind to insure there are no invalid references or leaks. Thanks, Scott Griepentrog -- _ -- 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] 4442: chan_sip: Asterisk fails to re-activate an inactive media session when an offer does not contain a=sendrecv
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/ --- (Updated Feb. 24, 2015, 12:47 p.m.) Review request for Asterisk Developers. Changes --- Added the missing *_no_direction.xml files from the previous diff. Bugs: ASTERISK-24824 https://issues.asterisk.org/jira/browse/ASTERISK-24824 Repository: testsuite Description --- This test is to ensure that Asterisk correctly applies the direction of the media stream when a=sendonly|recvonly|inactive|sendrecv is missing from the offer's SDP. The expected behavior is for Asterisk to apply sendrecv as the direction of the media stream when no direction attribute is present in an offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): If none of the attributes sendonly, recvonly, inactive, and sendrecv is present, sendrecv SHOULD be assumed as the default for sessions that are not of the conference type broadcast or H332 [...] The test scenario: 1. From Phone A, send an offer to Phone B to establish a call 2. From Phone B, send an offer to Phone A to put the call on hold. 3. Observe that the MOH start event occurs. 4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that the direction attribute from the offer's SDP is omitted) 5. Observe that the MOH stop event occurs. Presently, this test fails for certain versions of Asterisk. From what I can tell, it is present from (at least) 1.8.21 up to the 11 branch. ***Note*** This is the test. It is only the test. The update to the Asterisk source is coming soon to a review board near you (well, this review board). Diffs (updated) - ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml PRE-CREATION ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_A_no_direction.xml PRE-CREATION ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 Diff: https://reviewboard.asterisk.org/r/4442/diff/ Testing --- Thanks, Ashley Sanders -- _ -- 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] 4442: chan_sip: Asterisk fails to re-activate an inactive media session when an offer does not contain a=sendrecv
On Feb. 24, 2015, 2:29 a.m., wdoekes wrote: I don't see the *_no_direction.xml files. Sorry about that. The files are included in the update to this review. :) - Ashley --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/#review14522 --- On Feb. 24, 2015, 12:47 p.m., Ashley Sanders wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/ --- (Updated Feb. 24, 2015, 12:47 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24824 https://issues.asterisk.org/jira/browse/ASTERISK-24824 Repository: testsuite Description --- This test is to ensure that Asterisk correctly applies the direction of the media stream when a=sendonly|recvonly|inactive|sendrecv is missing from the offer's SDP. The expected behavior is for Asterisk to apply sendrecv as the direction of the media stream when no direction attribute is present in an offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): If none of the attributes sendonly, recvonly, inactive, and sendrecv is present, sendrecv SHOULD be assumed as the default for sessions that are not of the conference type broadcast or H332 [...] The test scenario: 1. From Phone A, send an offer to Phone B to establish a call 2. From Phone B, send an offer to Phone A to put the call on hold. 3. Observe that the MOH start event occurs. 4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that the direction attribute from the offer's SDP is omitted) 5. Observe that the MOH stop event occurs. Presently, this test fails for certain versions of Asterisk. From what I can tell, it is present from (at least) 1.8.21 up to the 11 branch. ***Note*** This is the test. It is only the test. The update to the Asterisk source is coming soon to a review board near you (well, this review board). Diffs - ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml PRE-CREATION ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_A_no_direction.xml PRE-CREATION ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 Diff: https://reviewboard.asterisk.org/r/4442/diff/ Testing --- Thanks, Ashley Sanders -- _ -- 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] AstriDevCon Follow Up - Asterisk and Kamailio - smoother integration
On Mon, Feb 23, 2015 at 2:07 PM, Olle E. Johansson o...@edvina.net wrote: On 23 Feb 2015, at 20:10, Matthew Jordan mjor...@digium.com wrote: On Mon, Feb 23, 2015 at 11:16 AM, James Cloos cl...@jhcloos.com wrote: MJ == Matthew Jordan mjor...@digium.com writes: MJ What I'm trying to reason out is: given a set of routing constraints - MJ which includes not only load balancing but also application level routing MJ decisions - what's the appropriate place for that information to live? MJ Particularly when you want your entire system - not just Asterisk - to be MJ scalable? You need to use something to keep track of whether each box is reachable and what each is doing. There a lots of ways of doing that, including custom network applications, shared networkable databases, shared nfs, et alia. Then each node needs a bit of logic to use that data to determine what to do with any given event. As one example, if a sip server gets an invite which directs on to an existing conference, it needs to know which asterisk is handling that conference, so that it can send the invite there. A shared database is required, but whether it is a custom application, a networkable db or a local db stored on a networked file systems is something anyone writing the code needs to choose. Completely agree. What I'm driving towards - albeit in something of a roundabout fashion - are two notions: (1) Hard coding logic in application configuration does not lend itself well to scalability. Kamailio lessens the pain in certain ways - you're typically going to have fewer proxies than application servers (although, I suppose that depends on what you are doing). Also, as Olle pointed out, you can replicate information out using an htable. Or use a database. To some extent though, this is not much different than using func_odbc with Asterisk (a concept many people miss often.) At the same time, requiring direct access to a database from my routing engine/media application server does not lend itself well to expressive logic. While I've managed to push the data out of the application, I haven't necessarily done the same with the business rules. If you approach the problem purely from a how horizontally scalable can I make this, then ideally most of your business logic would lie outside of the routing engine (Kamailio) or the media application server (Asterisk). That may mean a web microservice architecture that exposes REST APIs for the real-time component to consume. That may mean something else. As Daniel noted, the more REST APIs you hit using a cURL module (or what have you), the slower things get in Kamailio. The same is true for Asterisk. What I'm trying to fish for is where that dividing line should occur - that is, what properly belongs to Kamailio (routing decisions) and what properly belongs to something else. (2) Domain specific languages require domain specific knowledge. This is not necessarily a bad thing. At the same time, it's far easier to parallelize the problem of application development if you can split tasks into well defined domains that make use of tools that have a wider base of knowledge. If, for example, the logic of who is in a conference on which server can be answered by a REST API written in Python, or JavaScript, or something else - and does not even live on the Kamailio server - then not only does the job of the Kamailio integrator become easier, it is also becomes easier to find multiple people to help write the services that it integrates with. Kamailio really doesn't need to know as long as Asterisk knows... You are trying to solve something that is not really a problem, Matt. I have tried this path before with conferences and ended up with something too complex. Went back to forking, if a server responded with 200 OK I added a temporary state in an autoexpire hashtable so I remembered for next call which did not need to fork. Simple. Fast. No distributed states. The real key to scalability is keeping it simple and keeping states local. As soon as you start trying to synch or distribute states it gets very complex and you loose a lot of scalability. Asterisk is filled with all kinds of states, keep it there as much as possible and be smart with your signalling. You want to be able to restart Kamailio anytime without loosing states and without disrupting any single call. As soon as you start adding call states, server states and conference server states you loose a lot of that and end up with replication in databases and other complex schemes. Really, distributing the state in Asterisk is not much of a problem in this scenario. Sure, if the application in question is ConfBridge, or Queue, or some other dialplan application, then the difficulty of sharing that state across clustered Asterisk instances is non-trivial. However, in a system where the application state lives off of the Asterisk boxes - that is, where ARI is
Re: [asterisk-dev] [Code Review] 4441: Enable TLS Dual-Certificates (ECC+RSA)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4441/#review14529 --- See coding guidelines: https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines trunk/main/tcptls.c https://reviewboard.asterisk.org/r/4441/#comment25080 Please put a blank line between variable declarations and code. ast_strdup() allocates memory from the heap and that allocation can fail. There is no NULL pointer check for allocation failure. If you use ast_strdupa() instead you will allocate from the stack and not have to worry about allocation failure or having to free the memory. CamelCase is not used per Asterisk coding guidelines. The names should be ecc_file and ecc_file_len. trunk/main/tcptls.c https://reviewboard.asterisk.org/r/4441/#comment25085 It doesn't look like there is a standard cert file naming convention to name the files example_rsa.pem, example_ecc.pem, and example_dsa.pem. The patch assumes this naming convention. The patch should verify that cfg-certfile name is in this format before trying. i.e. Check that cfg-certfile ends with _rsa.pem. This should be documented in the sample config files (pjsip.conf.sample and sip.conf.sample at least). For pjsip the online documentation should be updated in res_pjsip.c. trunk/main/tcptls.c https://reviewboard.asterisk.org/r/4441/#comment25084 Probably should check if the file exists first before attempting to load it as a cert file. That way if it doesn't exist the error loading the cert file message won't happen unless it actually exists. The same should be done for the dsa cert file. trunk/main/tcptls.c https://reviewboard.asterisk.org/r/4441/#comment25082 This should be: ast_log(LOG_ERROR, TLS/SSL error... trunk/main/tcptls.c https://reviewboard.asterisk.org/r/4441/#comment25081 Guidelines: Declaring variables in the middle of a block is not allowed. Same here for ast_strdup() allocation failure check. It seems like a waste to allocate the eccFile then free it then allocate the dsaFile then free it. Why not allocate the duplicate file once and modify it for each cert file attempt. trunk/main/tcptls.c https://reviewboard.asterisk.org/r/4441/#comment25083 This should be: ast_log(LOG_ERROR, TLS/SSL error... - rmudgett On Feb. 23, 2015, 10:10 a.m., Alexander Traud wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4441/ --- (Updated Feb. 23, 2015, 10:10 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24815 https://issues.asterisk.org/jira/browse/ASTERISK-24815 Repository: Asterisk Description --- Already works for Asterisk as the client. Enables dual- (or triple-) certificates for Asterisk as the TLS server. When a client connects via SSL/TLS, the server uses a RSA key-pair usually. However, more such algorithms exist like DSA and ECDSA. If you go for one of those, you would loose compatibility to RSA-only clients. This patch allows you to provide up-to one RSA, ECDSA and DSA key each (= one key or two keys or three keys). Copied over from the Apache HTTP server project, added in version 2.4.8. Usage: tlscertfile=/etc/asterisk/example_rsa.pem Then, the code of this patch picks that path, filename, and searches for files called example_ecc.pem and example_dsa.pem automatically. Diffs - trunk/main/tcptls.c 431938 Diff: https://reviewboard.asterisk.org/r/4441/diff/ Testing --- by developer, manually This patch was tested in Ubuntu 14.04 LTS with a certificate from Comodo (ECC; chains-up to AddTrust and UTN) and RapidSSL (RSA; chains-up to GeoTrust and Equifax). TLS clients were CounterPath Bria (BlackBerry) and CSipSimple (Android). The test was done with OpenSSL 1.0.1 and OpenSSL 1.0.2. Both versions work as expected. However, if you use well-known (commercial) certificates, you might use different certificate chains. For this, you need at least OpenSSL 1.0.2. If you use your own certificate authority without a certificate chain, OpenSSL 1.0.1 is sufficient. Because no new symbol of OpenSSL was used, I do not see a reason why this patch should not be compatible with older OpenSSL releases. Therefore, no if/def/version is introduced in this patch. Thanks, Alexander Traud -- _ -- 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] 4429: bridge_softmix: G.729 codec license held
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4429/ --- (Updated Feb. 24, 2015, 12:23 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 432174 Bugs: ASTERISK-24797 https://issues.asterisk.org/jira/browse/ASTERISK-24797 Repository: Asterisk Description --- When more than one call using the same codec type enters into a softmix bridge and no audio is present for a channel the bridge optimizes the out frame by using the same one for all channels with the same codec type. Unfortunately, when that number (channels with same codec type) dropped to = 1 the codec was not dereferenced. At least not until all parties left the bridge. Thus in the case of G.729 the license was not released. This patch ensures that the codec is dereferenced immediately when the optimization no longer applies. Diffs - branches/11/bridges/bridge_softmix.c 431876 Diff: https://reviewboard.asterisk.org/r/4429/diff/ Testing --- Created a 3 party confbridge. Two channels were using g.729 and the other using ulaw. Hung up both the channels using g.729 and noted that the confbridge was still holding a reference to a g.729 license. After applying the patch noted that it no longer held onto the license. Thanks, Kevin Harwell -- _ -- 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] 4442: chan_sip: Asterisk fails to re-activate an inactive media session when an offer does not contain a=sendrecv
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/#review14531 --- Ship it! The only findings I have are very minor ones that don't actually impact test performance or success. I figure these can be corrected when committing the changes. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml https://reviewboard.asterisk.org/r/4442/#comment25086 I suggest removing these three headers. Session timers aren't a requirement for this test, AFAICT. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml https://reviewboard.asterisk.org/r/4442/#comment25087 For the same reason as before, I suggest removing these headers. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml https://reviewboard.asterisk.org/r/4442/#comment25089 I suggest removing these headers. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml https://reviewboard.asterisk.org/r/4442/#comment25088 Pedantic nitpick: The origin and session lines in previous SDPs being sent from this script had LiveOps for the session name and LiveOps for the origin username. Asterisk currently apparently doesn't actually care about this change, but it's not a good thing to do mid-session. - Mark Michelson On Feb. 24, 2015, 6:47 p.m., Ashley Sanders wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/ --- (Updated Feb. 24, 2015, 6:47 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24824 https://issues.asterisk.org/jira/browse/ASTERISK-24824 Repository: testsuite Description --- This test is to ensure that Asterisk correctly applies the direction of the media stream when a=sendonly|recvonly|inactive|sendrecv is missing from the offer's SDP. The expected behavior is for Asterisk to apply sendrecv as the direction of the media stream when no direction attribute is present in an offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): If none of the attributes sendonly, recvonly, inactive, and sendrecv is present, sendrecv SHOULD be assumed as the default for sessions that are not of the conference type broadcast or H332 [...] The test scenario: 1. From Phone A, send an offer to Phone B to establish a call 2. From Phone B, send an offer to Phone A to put the call on hold. 3. Observe that the MOH start event occurs. 4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that the direction attribute from the offer's SDP is omitted) 5. Observe that the MOH stop event occurs. Presently, this test fails for certain versions of Asterisk. From what I can tell, it is present from (at least) 1.8.21 up to the 11 branch. ***Note*** This is the test. It is only the test. The update to the Asterisk source is coming soon to a review board near you (well, this review board). Diffs - ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml PRE-CREATION ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml 6458 ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_A_no_direction.xml PRE-CREATION ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 Diff: https://reviewboard.asterisk.org/r/4442/diff/ Testing --- Thanks, Ashley Sanders -- _ -- 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] 4446: How to do the valgrind test of module?
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4446/#review14543 --- No, this is not an appropriate place to ask questions. Review Board is for peer review of code that is being submitted back to the Asterisk project. It is not used for review of code outside of the project, nor is it a place to ask general development questions. If you have questions regarding Asterisk development, please use the project mailing lists. In particular, for development related questions, use the asterisk-dev list: http://lists.digium.com/mailman/listinfo/asterisk-dev Additionally, there is a lot of information available on common Asterisk development topics on the Asterisk wiki, including on how to run Asterisk under valgrind. See https://wiki.asterisk.org/wiki/display/AST/Development and, more specifically, https://wiki.asterisk.org/wiki/display/AST/Valgrind . As this is not a code review, I'm going to close this out. Please ask any further questions in the appropriate locations. - Matt Jordan On Feb. 24, 2015, 6:07 p.m., sungtae kim wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4446/ --- (Updated Feb. 24, 2015, 6:07 p.m.) Review request for Asterisk Developers and Joshua Colp. Repository: Asterisk-addons Description --- Hello, I don't know this place is right place for asking about this. If not, sorry, just let me know. I will ask to another place. I just release asterisk-zmq module.(https://github.com/pchero/asterisk-zmq) May I know how to run the valgrind with this module? And also, may I know how to release with asterisk distribution? Again, if I make you annoyed, I'm sorry.. Regards, Sungtae. Diffs - Diff: https://reviewboard.asterisk.org/r/4446/diff/ Testing --- Thanks, sungtae kim -- _ -- 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] 4431: Increase WebSocket frame size and improve large read handling
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4431/#review14532 --- Ship it! Ship It! - Mark Michelson On Feb. 20, 2015, 3:39 p.m., David Lee wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4431/ --- (Updated Feb. 20, 2015, 3:39 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Some WebSocket applications, like [chan_respoke][], require a larger frame size than the default 8k; this patch bumps the default to 16k. This patch also fixes some problems exacerbated by large frames. The sanity counter was decremented on every fread attempt in ws_safe_read(), regardless of whether data was read from the socket or not. For large frames, this could result in loss of sanity prior to reading the entire frame. (16k frame / 1448 bytes per segment = 12 segments). This patch changes the sanity counter so that it only decrements when fread() doesn't read any bytes. This more closely matches the original intention of ws_safe_read(), given that the error message is Websocket seems unresponsive. This patch also properly logs EOF conditions, so disconnects are no longer confused with unresponsive connections. [chan_respoke]: https://github.com/respoke/chan_respoke Diffs - /branches/11/res/res_http_websocket.c 431915 Diff: https://reviewboard.asterisk.org/r/4431/diff/ Testing --- Ran a Node app that continuously send large WebSocket frame to Asterisk. https://gist.github.com/leedm777/ba6d86468d7646073286 Without the patch, Asterisk fails in less than 10 frames. With the patch, it runs like a boss. 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] [Code Review] 4418: SDES-SRTP: Handle SRTP keys negotiated with key lifetime/MKI (oej branch lingon-srtp-key-lifetime-1.8) - Asterisk 13
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4418/#review14535 --- The 32-bit system overflow problem needs to be addressed on this review. - Mark Michelson On Feb. 14, 2015, 3:26 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4418/ --- (Updated Feb. 14, 2015, 3:26 a.m.) Review request for Asterisk Developers and Olle E Johansson. Bugs: ASTERISK-17721, ASTERISK-17899 and ASTERISK-22748 https://issues.asterisk.org/jira/browse/ASTERISK-17721 https://issues.asterisk.org/jira/browse/ASTERISK-17899 https://issues.asterisk.org/jira/browse/ASTERISK-22748 Repository: Asterisk Description --- Note that this patch is a forward port of oej's lingon-srtp-key-lifetime-1.8 branch, with a few small modifications to reduce block indentation and a few improvements made to surrounding existing code. This patch is for Asterisk 13, and is essentially identical to https://reviewboard.asterisk.org/r/4419 - save for a few small modifications for the public API written when the code was ported into the core. As such, this patch should only be reviewed for those changes that are not in r4419. Diffs - /branches/13/main/sdp_srtp.c 431750 Diff: https://reviewboard.asterisk.org/r/4418/diff/ Testing --- Tests were added for chan_sip and updated for chan_pjsip - see https://reviewboard.asterisk.org/r/4420 . This includes both nominal and off-nominal offers negotiating SDES-SRTP. Thanks, Matt Jordan -- _ -- 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] 4437: dns: Define a core DNS API with examples.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4437/#review14539 --- First off, good page! It's pretty easy to follow the expected flow for DNS queries based on the API you've provided. From a low-level, there isn't much that's missing: you have ways of performing synchronous and asynchronous DNS lookups, and you have methods for examining the results. I have a couple of critiques of the presented API: * ast_dns_resolve_async_recurring() is not very well defined and there are no examples illustrating its use. * The function ast_dns_query_set_get() creates a leaky abstraction for the query set. I suggest one of the following: * Have ast_dns_query_set_add() return an integer token that can be used to retrieve that record from the query set. (enthusiasm level: meh) * Have an ast_dns_query_set iterator API to iterate over the queries in the set. (enthusiasm level: YES) * While there are ways of querying if a result is secure, there is no current way of requesting only secure results. From a low-level, the one big thing that I feel is not talked about is the threading model. I know that's getting really low level, but there are some questions that came to mind, such as: if I perform multiple asynchronous DNS lookups (without using a query set) can I guarantee the results will be presented to me serially, or can the results be presented in separate threads at the same time? This can have an impact on how I write my async callbacks, especially if I pass a reference to the same user data to both async queries. The low-level API looks fine, and it provides a lot of areas where some higher-level functions could be created. For instance: * NAPTR has a bunch of interesting possibilities: * Have a function to automatically perform the regex replacement and present the result to you as a string. * Or if you want to be even lazier, have a NAPTR function that will take a NAPTR result and convert that into an ast_dns_query that you can then resolve yourself. * Or if you want to be even lazier, have an async NAPTR function not return results until it boils down to an A or record. * NAPTR and SRV have the potential for functions that allow for you to iterate over the different priority records that are returned. * NAPTR and SRV could have fallback functions built into them, too. As far as the examples are concerned, they're good, but I feel like an example that uses NAPTR (which subsequently results in an SRV lookup) could be useful. You may find that when you write the example, manual construction of further queries based on the data that you are retrieving from NAPTR records leads you to want to implement some of the suggested high-level functions for it. Regarding your question about DNS being core or module-based, it kind of depends on a few factors: * What code in Asterisk is going to be updated to use the new DNS API? If older code is not going to be updated to use the new DNS API, then I think that resolvers should live in loadable modules. If someone upgrades to Asterisk 15 and plans to continue using chan_sip and hasn't had issues with DNS, then they probably aren't going to be too thrilled about having to download c-ares or libunbound, only to not actually use it for anything. If old code is going to be updated to use the new DNS API, then... * Are we planning on keeping any of the old DNS code from Asterisk around as a resolver implementation choice? If so, then again, I suggest having resolvers be separate modules. This way we still provide an upgrade path that does not have new dependencies or new underlying implementations. My personal opinion is that all old code should be updated to use the new DNS API (without actually changing behavior, even if the old behavior is incorrect) and that the old implementation should be thrown away. If we go that route, then resolvers should be part of the core since DNS resolution is a fundamental thing in most Asterisk installations. There is one further thing I can think of here, and that is the current dnsmgr code in Asterisk. Is that simply going to be updated to use the new DNS API, or is dnsmgr going to get some sort of overhaul to provide new functionality that the underlying DNS API makes it capable of? - Mark Michelson On Feb. 23, 2015, 12:25 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4437/ --- (Updated Feb. 23, 2015, 12:25 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- A wiki page is present at: https://wiki.asterisk.org/wiki/display/~jcolp/Asterisk+DNS+API Which
[asterisk-dev] [Code Review] 4447: ARI: Fix crash if integer values used in JSON payload 'variables' object.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4447/
---
Review request for Asterisk Developers.
Bugs: ASTERISK-24751
https://issues.asterisk.org/jira/browse/ASTERISK-24751
Repository: Asterisk
Description
---
Sending the following ARI commands caused Asterisk to crash if the JSON
body 'variables' object passes values of types other than strings.
POST /ari/channels
POST /ari/channels/{channelid}
PUT /ari/endpoints/sendMessage
PUT /ari/endpoints/{tech}/{resource}/sendMessage
* Eliminated RAII_VAR usage in ast_ari_channels_originate_with_id(),
ast_ari_channels_originate(), ast_ari_endpoints_send_message(), and
ast_ari_endpoints_send_message_to_endpoint().
Diffs
-
/branches/13/rest-api/api-docs/endpoints.json 432194
/branches/13/res/res_ari_endpoints.c 432194
/branches/13/res/ari/resource_endpoints.c 432194
/branches/13/res/ari/resource_channels.c 432194
/branches/13/main/json.c 432194
/branches/13/include/asterisk/json.h 432194
Diff: https://reviewboard.asterisk.org/r/4447/diff/
Testing
---
The four commands no longer crash and now report 400 Bad Request with a
message that the 'variables' object only accepts string values when I
pass an integer value.
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] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14533 --- Tested with a looped tdm board generating various DR patterns and confirmed to resolve CID detection problem. Ran out of time to validate DR detection, will attempt later. - Scott Griepentrog On Feb. 23, 2015, 6:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated Feb. 23, 2015, 6:51 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432173 /branches/11/channels/sig_analog.h 432173 /branches/11/channels/chan_dahdi.c 432173 /branches/11/UPGRADE.txt 432173 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences: 1) usedistinctiveringdetection=no 2) usedistinctiveringdetection=yes with distinctiveringaftercid=no 3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes * Caller-ID was detected for each call * The configured distinctive ring dringX pattern was detected and the specified context set. 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] 4434: ARI/PJSIP: Apply requesting channel's capabilities to originated channels in ARI; clean up a bit of PJSIP's usage of format capabalities
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4434/ --- (Updated Feb. 24, 2015, 3:58 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 432195 Bugs: ASTERISK-24812 https://issues.asterisk.org/jira/browse/ASTERISK-24812 Repository: Asterisk Description --- This patch addresses the following problems: * ari/resource_channels: In ARI, we currently create a format capability structure of SLIN and apply it to the new channel being created. This was originally done when the PBX core was used to create the channel, as there was a condition where a newly created channel could be created without any formats. Unfortunately, now that the Dial API is being used, this has two drawbacks: (a) SLIN, while it will ensure audio will flows, can cause a lot of needless transcodings to occur, particularly when a Local channel is created to the dialplan. When no format capabilities are available, the Dial API handles this better by handing all audio formats to the requsted channels. As such, we defer to that API to provide the format capabilities. (b) If a channel (requester) is causing this channel to be created, we currently don't use its format capabilities as we are passing in our own. However, the Dial API will use the requester channel's formats if none are passed into it, and the requester channel exists and has format capabilities. This is the best scenario, as it is the most likely to create a media path that minimizes transcoding. Fixing this simply entails removing the providing of the format capabilities structure to the Dial API. * chan_pjsip: Rather than blindly picking the first format in the format capability structure - which actually *can* be a video or text format - we select an audio format, and only pick the first format if that fails. That minimizes the weird scenario where we attempt to transcode between video/audio. * channel: Fixed a comment. The reason we have to minimize our requested format capabilities down to a single format is due to Asterisk's inability to convey the format to be used back up a channel chain. Consider the following: PJSIP/A = L;1 = L;2 = PJSIP/B g,u,a g,u,ag,u,a u That is, we have PJSIP/A dialing a Local channel, where the Local;2 dials PJSIP/B. PJSIP/A has native format capabilities g722,ulaw,alaw; the Local channel has inherited those format capabilities down the line; PJSIP/B supports only ulaw. According to these format capabilities, ulaw is acceptable and should be selected across all the channels, and no transcoding should occur. However, there is no way to convey this: when L;2 and PJSIP/B are put into a bridge, we will select ulaw, but that is not conveyed to PJSIP/A and L;1. Thus, we end up with: PJSIP/A = L;1 = L;2 = PJSIP/B g g X uu Which causes g722 to be written to PJSIP/B. Even if we can convey the 'ulaw' choice back up the chain (which through some severe hacking in Local channels was accomplished), such that the chain looks like: PJSIP/A = L;1 = L;2 = PJSIP/B u u u u We have no way to tell PJSIP/A's *channel driver* to Answer in the SDP back with only 'ulaw'. This results in all the channel structures being set up correctly, but PJSIP/A *still* sending g722 and causing the chain to fall apart. There's a lot of difficulty just in setting this up, as there are numerous race conditions in the act of bridging, and no clean mechanism to pass the selected format backwards down an established channel chain. As such, I punted on improving this and simply updated the comment. * res_pjsip_sdp_rtp: Applied the joint capapbilites to the format structure. Since ast_request already limits us down to one format capability once the format capabilities are passed along, there's no reason to squelch it here. The comment about bridge_softmix is a purely theoretical concern that should not limit what the PJSIP stack does. Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 431991 /branches/13/res/ari/resource_channels.c 431991 /branches/13/main/channel.c 431991 /branches/13/channels/chan_pjsip.c 431991 Diff: https://reviewboard.asterisk.org/r/4434/diff/ Testing --- The PJSIP SDP Offer/Answer tests all pass. Prior to this patch, we could set up to 8 transcoding paths on a channel chain created by ARI; now, we have none (if both far ends support the same formats). Thanks, Matt Jordan -- _ -- 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] 4419: SDES-SRTP: Handle SRTP keys negotiated with key lifetime/MKI (oej branch lingon-srtp-key-lifetime-1.8) - Asterisk 11
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4419/#review14534
---
Ship it!
Ship It!
- Mark Michelson
On Feb. 20, 2015, 2:41 a.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4419/
---
(Updated Feb. 20, 2015, 2:41 a.m.)
Review request for Asterisk Developers and Olle E Johansson.
Bugs: ASTERISK-17721, ASTERISK-17899 and ASTERISK-22748
https://issues.asterisk.org/jira/browse/ASTERISK-17721
https://issues.asterisk.org/jira/browse/ASTERISK-17899
https://issues.asterisk.org/jira/browse/ASTERISK-22748
Repository: Asterisk
Description
---
Note that this patch is a forward port of oej's lingon-srtp-key-lifetime-1.8
branch, with a few small modifications to reduce block indentation and a few
improvements made to surrounding existing code.
To quote Olle from ASTERISK-17721:
{quote}
The Lingon branch now handle lifetime and MKI parameters.
We only accept lifetimes up to max for the crypto and higher than 10 hours
for packetization of 20 ms (50 pps).
We only handle MKI with index 1.
We do not really bother with counting packets and reinviting at end of
lifetime, so the min of 10 hours kind of takes care of most calls. If there
are longer ones, we rely on the other side for re-invites.
It's still not perfect, but I personally think this is an improvement. A
configuration option for minimum lifetime accepted could be added.
{quote}
I personally don't think the minimum lifetime option is needed, as in
practice, every instance of an SDP offer for SDES-SRTP with lifetime I've
seen offers a lifetime of 2^32 - but it could be added in the future if
necessary.
Note that since the sdp_crypto code was moved to the core in 12+, a separate
review (r4418) has been made for the Asterisk 13 variant. It is essentially
identical to this review.
Diffs
-
/branches/11/channels/sip/sdp_crypto.c 432011
Diff: https://reviewboard.asterisk.org/r/4419/diff/
Testing
---
Tests were added for chan_sip and updated for chan_pjsip - see
https://reviewboard.asterisk.org/r/4420 . This includes both nominal and
off-nominal offers negotiating SDES-SRTP.
Thanks,
Matt Jordan
--
_
-- 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] 4418: SDES-SRTP: Handle SRTP keys negotiated with key lifetime/MKI (oej branch lingon-srtp-key-lifetime-1.8) - Asterisk 13
On Feb. 24, 2015, 4:29 p.m., Mark Michelson wrote: The 32-bit system overflow problem needs to be addressed on this review. I'll catch it when I do the merge over. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4418/#review14535 --- On Feb. 13, 2015, 9:26 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4418/ --- (Updated Feb. 13, 2015, 9:26 p.m.) Review request for Asterisk Developers and Olle E Johansson. Bugs: ASTERISK-17721, ASTERISK-17899 and ASTERISK-22748 https://issues.asterisk.org/jira/browse/ASTERISK-17721 https://issues.asterisk.org/jira/browse/ASTERISK-17899 https://issues.asterisk.org/jira/browse/ASTERISK-22748 Repository: Asterisk Description --- Note that this patch is a forward port of oej's lingon-srtp-key-lifetime-1.8 branch, with a few small modifications to reduce block indentation and a few improvements made to surrounding existing code. This patch is for Asterisk 13, and is essentially identical to https://reviewboard.asterisk.org/r/4419 - save for a few small modifications for the public API written when the code was ported into the core. As such, this patch should only be reviewed for those changes that are not in r4419. Diffs - /branches/13/main/sdp_srtp.c 431750 Diff: https://reviewboard.asterisk.org/r/4418/diff/ Testing --- Tests were added for chan_sip and updated for chan_pjsip - see https://reviewboard.asterisk.org/r/4420 . This includes both nominal and off-nominal offers negotiating SDES-SRTP. Thanks, Matt Jordan -- _ -- 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] 4446: How to do the valgrind test of module?
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4446/ --- Review request for Asterisk Developers and Joshua Colp. Summary (updated) - How to do the valgrind test of module? Repository: Asterisk-addons Description (updated) --- Hello, I don't know this place is right place for asking about this. If not, sorry, just let me know. I will ask to another place. I just release asterisk-zmq module.(https://github.com/pchero/asterisk-zmq) May I know how to run the valgrind with this module? And also, may I know how to release with asterisk distribution? Again, if I make you annoyed, I'm sorry.. Regards, Sungtae. Diffs - Diff: https://reviewboard.asterisk.org/r/4446/diff/ Testing --- Thanks, sungtae kim -- _ -- 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] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
On Feb. 24, 2015, 5:35 p.m., Kevin Harwell wrote: /branches/11/channels/chan_dahdi.c, lines 1965-1967 https://reviewboard.asterisk.org/r//diff/2/?file=71580#file71580line1965 Since you rearranged the 'ring - range' check should it be '' instead of '='? Otherwise it is different than before. rmudgett wrote: It is not different than before. Before it was: if (x = ring + range x = ring - range) Now it is: if (ring - range = x x = ring + range) The code now better expreses that x is between two points on a number line. A = x = B ooops yeah I misread it. You are right! - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14538 --- On Feb. 23, 2015, 6:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated Feb. 23, 2015, 6:51 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432173 /branches/11/channels/sig_analog.h 432173 /branches/11/channels/chan_dahdi.c 432173 /branches/11/UPGRADE.txt 432173 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences: 1) usedistinctiveringdetection=no 2) usedistinctiveringdetection=yes with distinctiveringaftercid=no 3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes * Caller-ID was detected for each call * The configured distinctive ring dringX pattern was detected and the specified context set. 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
