----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4190/#review13865 -----------------------------------------------------------
branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24350> This may be a bit nitpicky, but the name "char_vector" implies to me a vector whose elements are of type char, not a vector whoe elements are of type char *. Perhaps "string_vector" would be a more apt name for the container. branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24349> I'm curious why this line is here. It seems like this could end up overriding preferences made by the user configuration, leading to unexpected behavior. branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24351> In the examples you've given, remote_host is something like "sip.example.com:5060". The problem is that identify sections do not accept hostnames in "match=" lines. I think your best bet here is not to try to use remote_host as a means of constructing identify sections. Instead, "identify/match" lines in the configuration can be used. branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24352> I don't like the idea of having an empty USERNAME string since the default if no client_uri_pattern is provided is to use sip:${USERNAME}@${REMOTE_HOST}. This could result in a client URI of sip:@sip.example.com, which is almost certainly not what's wanted. I think a reasonable default would be to use the name of the wizard as a fallback if outbound_auth/username is not provided. Also, you're currently getting the value of outbound_auth/username on each iteration of this loop. Since it's the same value every time, you could optimize this a bit by getting the variable value outside the loop. branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24353> The hosts variable should use ast_strdupa() instead of ast_strdup(). hosts gets altered by the calls to ast_strsep(), so you're likely not passing the same pointer to ast_free that you got back from ast_strdup(). branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24354> Why are identifies only loaded if accepts_registrations is not true? To me, the two are not related. Identify sections allow for an incoming SIP request to be recognized based on the source IP address/network of the request. Registrations allow for Asterisk to know where to send SIP requests that are directed to a particular endpoint or AoR. So even if registrations are accepted, it seems fair to me to allow for identifies to be loaded since they serve distinct purposes. branches/12/res/res_pjsip_config_wizard.c <https://reviewboard.asterisk.org/r/4190/#comment24355> It's probably a good idea to print something here if phoneprov/MAC is not specified so that it's clear that phoneprov is not being loaded. - Mark Michelson On Dec. 2, 2014, 12:40 a.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4190/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2014, 12:40 a.m.) > > > Review request for Asterisk Developers, Joshua Colp and Mark Michelson. > > > Repository: Asterisk > > > Description > ------- > > The PJSIP Configuration Wizard allows for creation of simple pjsip scenarios > like phone or trunk without having to directly specify individual endpoints, > aors, auths, identifies or registrations. The easiest way to demonstrate > this is with an example or two from pjsip_wizard.conf.sample... > > ;============EXAMPLE WIZARD CONFIGURATION FOR A PHONE======================= > > ; This config would create an endpoint, aor with dynamic contact, inbound > ; auth and a phoneprov object. > > [myphone] > type = wizard > accepts_auth = yes > accepts_registrations = yes > transport = ipv4 > aor/max_contacts = 1 > inbound_auth/username = testname > inbound_auth/password = test password > endpoint/allow = ulaw > endpoint/context = default > phoneprov/MAC = 001122aa4455 > phoneprov/PROFILE = profile1 > > ;============EXAMPLE WIZARD CONFIGURATION FOR AN ITSP TRUNK================= > > ; This ITSP has 2 servers available and requires registration. > > ; This config would create an endpoint, an aor with 2 static contacts, an > outbound > ; auth, an identify with 2 matches, and 2 registrations. > > [mytrunk] > type = wizard > sends_auth = yes > sends_registrations = yes > transport = ipv4 > ; The number of remote_hosts drives the number of contacts, matches and > registrations. > remote_hosts = sip1.myitsp.com:5060,sip2.myitsp.com:5060 > outbound_auth/username = testname > outbound_auth/password = test password > endpoint/allow = ulaw > endpoint/context = default > > pjsip_wizard.conf.sample has more details. > > The history of the wizard approach can be found in the following 2 threads... > > http://lists.digium.com/pipermail/asterisk-dev/2014-September/070426.html > http://lists.digium.com/pipermail/asterisk-dev/2014-October/070616.html > > THEORY OF OPERATION: > > N.B.: The term 'wizard' is used in 2 contexts here. This module implements > a sorcery wizard similar to res_sorcery_config and provides the functionality > for the PJSIP Configuration Wizard. > > The wizard is implemented in a single module but did require a few tweaks to > other res_pjsip modules and sorcery itself. There are 2 parts to this > module, the config wizard and the sorcery wizard. When the module loads, it > registers itself as a sorcery wizard which is implemented in the bottom half > of res_pjsip_config_wizard.c. When sorcery calls the wizard's load and > reload functions for a specific pjsip object type, the wizard parses through > the pjsip_wizard.conf file and creates the appropriate object for each > 'wizard' type. For example, if asked to load endpoints, the wizard will > parse pjsip_wizard.conf and create an endpoint for each 'wizard' type in the > file. This process happens AFTER objects are read from sources defined in > sorcery.conf and pjsip.conf. In the end, the pjsip stack is none the wiser > about where the objects came from and all AMI, ARI, CLI etc. operate as > normal. The only way to differentiate between objects created discretely and > those created by the wi zard is that the wizard-created ones will all have an extended attribute named '@pjsip_wizard' with a value of the wizard id. > > SUMMARY OF CHANGES MADE: > > * The new res_pjsip_config_wizard module was created. > * An existing internal sorcery api was exposed as > ast_sorcery_apply_wizard_mapping to allow the addition of a new wizard to an > object type. The underlying plumbing was already there. > * config_auth, location, pjsip_configuration, > res_pjsip_endpoint_identifier_ip, res_pjsip_outbound_registration and > res_pjsip_phoneprov_provider were all modified to call > ast_sorcery_apply_wizard_mapping after calling ast_sorcery_apply_default. > * res_pjsip_phoneprov_provider needed a little more work to be compatible. > * During troubleshooting I realized that there were no 'pjsip show identify' > commands so I added them to res_pjsip_endpoint_identifier. I also plugged an > existing CLI reference leak. > > RELOADABILITY: > > The new module itself cannot be reloaded or unloaded but there's no point to > that anyway. It would just unregister as a sorcery wizard and re-register. > 'core reload' and 'module reload res_pjsip' work quite well though which is > much more important. 'core reload' is the preferred reload mechanism over > reloading specific pjsip modules because it reloads all modules so modules > such as res_pjsip_outbound_registration know to start registration for newly > discovered objects. > > BACKWARDS COMPATIBILITY: > > This module does not change any existing functionality. Once created by the > wizard, pjsip objects are indistinguishable from ones created discretely > other than the addition of the '@pjsip_wizard' attribute. > > OTHER: > > This module does not use sorcery to read its pjsip_wizard.conf file. Since > this module implements a sorcery wizard, doing so would have created 'chicken > and egg' scenarios which would have been complex to solve. It does use the > standard config mechanism though so you can use extconfig.conf to get the > config from an external source. > > > Diffs > ----- > > branches/12/res/res_pjsip_phoneprov_provider.c 428167 > branches/12/res/res_pjsip_outbound_registration.c 428167 > branches/12/res/res_pjsip_endpoint_identifier_ip.c 428167 > branches/12/res/res_pjsip_config_wizard.c PRE-CREATION > branches/12/res/res_pjsip/pjsip_configuration.c 428167 > branches/12/res/res_pjsip/pjsip_cli.c 428167 > branches/12/res/res_pjsip/location.c 428167 > branches/12/res/res_pjsip/config_auth.c 428167 > branches/12/main/sorcery.c 428167 > branches/12/include/asterisk/sorcery.h 428167 > branches/12/configs/pjsip_wizard.conf.sample PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/4190/diff/ > > > Testing > ------- > > Test suite tests are available that use the wizard to create objects and AMI > to read the results. The results are indistinguishable except the for > '@pjsip_wizard' attribute. > > I've converted my own PBX to use the wizard approach and phones and trunks > operate normally. > > > 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
