Skip to content
Snippets Groups Projects
  1. Apr 05, 2016
    • George Joseph's avatar
      stringfields: Refactor to allow fields to be added to the end of structures · 4d40b161
      George Joseph authored
      String fields are great, except that you can't add new ones without breaking
      ABI compatibility because it shifts down everything else in the structure.
      The only alternative is to add your own char * field to the end of the
      structure and manage the memory yourself which isn't ideal, especially since
      you then can't use the OPT_STRINGFIELD_T type.
      
      Background:
      
      The reason string fields had to be declared inside the
      AST_DECLARE_STRING_FIELDS block was to facilitate iteration over all declared
      fields for initialization, compare and copy.  Since AST_DECLARE_STRING_FIELDS
      declared the pool, then the fields, then the manager, you could use the offsets
      of the pool and manager and iterate over the sequential addresses in between to
      access the fields. The actual pool, field allocation and field set operations
      don't actually care where the field is.  It's just iteration over the fields
      that was the problem.
      
      Solution: Extended String Fields
      
      An extended string field is one that is declared outside the
      AST_DECLARE_STRING_FIELDS block but still (anywhere) inside the parent
      structure.  Other than using AST_STRING_FIELD_EXTENDED instead of
      AST_STRING_FIELD, it looks the same as other string fields.  It's storage comes
      from the pool and it participates in string field compare and copy operations
      peformed on the parent structure. It's also a valid target for the
      OPT_STRINGFIELD_T aco option type.
      
      Implementation:
      
      To keep track of the extended fields and make sure that ABI isn't broken, the
      existing embedded_pool pointer in the manager structure was repurposed to be a
      pointer to a separate header structure that contains the embedded_pool pointer
      plus a vector of fields.  The length of the manager structure didn't change and
      the embedded_pool pointer isn't used in the macros, only the stringfields C
      code.  A side benefit of this is that changing the header structure in the
      future won't break ABI.
      
      ast_string_fields_init initializes the normal string fields and appends them to
      the vector, and subsequent calls to ast_string_field_init_extended initialize
      and append the extended fields. Cleanup, ast_string_fields_cmp, and
      ast_string_fields_copy can now work on the vector instead of sequentially
      traversing the addresses between the pool and manager.
      
      The total size of a structure using string fields didn't change, whether using
      extended fields or not, nor have the offsets of any structure members, either
      inside the original block or outside.  Adding an extended field to the end of a
      structure is the same as adding a char *.
      
      Details:
      
      The stringfield C code was pulled out from utils.c and into stringfields.c.
      It just made sense.
      
      Additional work was done in ast_string_field_init and
      ast_calloc_with_stringfields to handle the allocation of the new header
      structure and the vector, and the associated cleanup.  In the process some
      additional NULL pointer checking was added.
      
      A lot of work was done in stringfields.h since the logic for compare and copy
      is there.  Documentation was added as well as somne additional NULL checking.
      
      The ability to call ast_calloc_with_stringfields with a number of structures
      greater than 1 never really worked.  Well, the calloc worked but there was no
      way to access the additional structures or clean them up.  It was agreed that
      there was no use case for requesting more than 1 structure so an ast_assert
      was added to prevent it and the iteration code removed.
      
      Testing:
      
      The stringfield unit tests were updated to test both normal and extended
      fields.  Tests for ast_string_field_ptr_set_by_fields and
      ast_calloc_with_stringfields were also added.
      
      As an ABI test, 13 was compiled from git and the res_pjsip_* modules, except
      res_pjsip itself, saved off.  The patch was then added and a full compile and
      install was performed.  Then the older res_pjsip_* moduled were copied over the
      installed versions so res_pjsip was new and the rest were old.  No issues.
      
      contact->aor, which is a char * at the end of contact, was then changed to an
      extended string field and a recompile and reinstall was performed, again
      leaving stock versions of the the res_pjsip_* modules.  Again, no issues with
      the res_pjsip_* modules using the old stringfield implementation and with
      contact->aor as a char *, and res_pjsip itself using the new stringfield
      implementation and contact->aor being an extended string field.
      
      Finally, several existing string fields were converted to extended string
      fields to test OPT_STRINGFIELD_T.  Again, no issues.
      
      Change-Id: I235db338c5b178f5a13b7946afbaa5d4a0f91d61
      4d40b161
  2. Apr 04, 2016
  3. Apr 01, 2016
  4. Mar 31, 2016
  5. Mar 30, 2016
    • George Joseph's avatar
      pjproject_bundled: Fix use of LDCONFIG for shared library link creation · 304f8178
      George Joseph authored
      LDCONFIG apparently isn't set to something sane on all systems so the creation
      of the shared library links fails.  Instead of just testing for non-blank,
      main/Makefile now checks that LDCONFIG is actually executable and reverts to
      LN if it isn't.
      
      This applies to both libasteriskpj and libasteriskssl.
      
      Thanks to 'abelbeck' for pointing out that the issue was LDCONFIG.
      
      ASTERISK-25873 #close
      Reported-by: Hans van Eijsden
      
      Change-Id: I25b76379bc637726ec044b2c0e709b56b3701729
      304f8178
    • Richard Mudgett's avatar
      res_stasis: Add control ref to playback and recording structs. · 0ea742d3
      Richard Mudgett authored
      The stasis_app_playback and stasis_app_recording structs need to have a
      struct stasis_app_control ref.  Other threads can get a reference to the
      playback and recording structs from their respective global container.
      These other threads can then use the control pointer they contain after
      the control struct has gone.
      
      * Add control ref to stasis_app_playback and stasis_app_recording structs.
      
      With the refs added, the control command queue can now have a circular
      control reference which will cause the control struct to never get
      released if the control's command queue is not flushed when the channel
      leaves the Stasis application.  Also the command queue needs better
      protection from adding commands if the control->is_done flag is set.
      
      * Flush the control command queue on exit.
      
      ASTERISK-25882 #close
      
      Change-Id: I3cf1fb59cbe6f50f20d9e35a2c07ac07d7f4320d
      0ea742d3
    • Richard Mudgett's avatar
      res_stasis: Fix crash on a hanging up channel. · 53f63ad7
      Richard Mudgett authored
      * Give the struct stasis_app_control ao2 object a ref to the channel held
      in the object.  Now the channel will still be around if a thread needs to
      post a stasis message instead of crash because the topic was destroyed.
      
      * Moved stopping any lingering silence generator out of the struct
      stasis_app_control destructor and made it a part of exiting the Stasis
      application.  Who knows which thread the destructor will be called under
      so it cannot affect the channel's silence generator.  Not only was the
      channel unprotected when the silence generator was stopped, stasis may no
      longer even control the channel.
      
      ASTERISK-25882
      
      Change-Id: I21728161b5fe638cef7976fa36a605043a7497e4
      53f63ad7
    • Richard Mudgett's avatar
      res_stasis.c: Protect channel datastore list from stasis end. · 2fab4d7d
      Richard Mudgett authored
      Change-Id: Ifadc469590bd4d5368e19d3763db3bd1f80fdb95
      2fab4d7d
    • Richard Mudgett's avatar
      res_ari: Cannot get control also means channel is unavailable. · ece2edaa
      Richard Mudgett authored
      The only caller of ari_bridges_play_found() has this note:
      
      If ari_bridges_play_found fails because the channel is unavailable for
      playback, The channel will be removed from the playback list soon.  We can
      keep trying to get channels from the list until we either get one that
      will work or else there isn't a channel for this bridge anymore, in which
      case we'll revert to ari_bridges_play_new.
      
      Change-Id: Ib068141b367ccaa17be0dab4181c98e26c5127d6
      ece2edaa
    • Richard Mudgett's avatar
      res_stasis_recording.c: Cleanup stasis_app_recording_find_by_name(). · 2f36cba4
      Richard Mudgett authored
      Change-Id: Ic7d93c402c498677a122505558859c853d4e5ac7
      2f36cba4
    • Richard Mudgett's avatar
      core_unreal.c: Add clarification comment about channel ref. · 34457dd9
      Richard Mudgett authored
      Change-Id: I0be0627260cd8d6b6c3cc345949dcfdf32eff1f3
      34457dd9
    • George Joseph's avatar
      res_pjsip_mwi: Allow subscribe to vm access extension as an alias · 2b3261cd
      George Joseph authored
      Background:
      
      If your extension is 1000 and the voicemail access extension is 1571 and you
      dial 1571, usually a dialplan rule calls voicemailmain with your extension and
      you are placed directly in your mailbox.  Therefore most admins program the
      voicemail (or other speed dial) button on their phones to the access extension.
      Some phones (Snom at least) use whatever is programmed there to also subscribe
      for MWI and so can't dial one number and subscribe to another.  This works fine
      in chan_sip because chan_sip completely ignores the user portion of the
      SUBSCRIBE message request URI.  If it can match the peer, is subscribes to the
      peer's mailbox.  The user could be set to anything or nothing and you'd still
      get subscribed to your mailbox.
      
      Issue:
      
      chan_pjsip actually uses the user portion of the URI to find an aor and its
      mailboxes.  Therefore a subscribe to 1571 results in a 404.  Sure, you can
      create an aor for 1571 but you certainly can't add your entire voicemail
      system's mailboxes to it and everyone would get notified of every MWI.
      
      Solution:
      
      When an MWI subscribe comes in and an aor can't be found that matches the
      resource directly, check the resource against the endpoint's aors.  If an aor
      is found that has a voicemail_extension that matches the resource, use it.
      
      ASTERISK-25865
      Reported-by: Ross Beer
      
      Change-Id: I770ea185f751f1ada888fafb4b452115f1c06e9e
      2b3261cd
    • George Joseph's avatar
      res_pjsip_mwi: Add voicemail extension and mwi_subscribe_replaces_unsolicited · e2524fce
      George Joseph authored
      res_pjsip_mwi was missing the chan_sip "vmexten" functionality which adds
      the Message-Account header to the MWI NOTIFY.  Also, specifying mailboxes
      on endpoints for unsolicited mwi and on aors for subscriptions required
      that the admin know in advance which the client wanted.  If you specified
      mailboxes on the endpoint, subscriptions were rejected even if you also
      specified mailboxes on the aor.
      
      Voicemail extension:
      * Added a global default_voicemail_extension which defaults to "".
      * Added voicemail_extension to both endpoint and aor.
      * Added ast_sip_subscription_get_dialog for support.
      * Added ast_sip_subscription_get_sip_uri for support.
      
      When an unsolicited NOTIFY is constructed, the From header is parsed, the
      voicemail extension from the endpoint is substituted for the user, and the
      result placed in the Message-Account field in the body.
      
      When a subscribed NOTIFY is constructed, the subscription dialog local uri
      is parsed, the voicemail_extension from the aor (looked up from the
      subscription resource name) is substituted for the user, and the result
      placed in the Message-Account field in the body.
      
      If no voicemail extension was defined, the Message-Account field is not added
      to the NOTIFY body.
      
      mwi_subscribe_replaces_unsolicited:
      * Added mwi_subscribe_replaces_unsolicited to endpoint.
      
      The previous behavior was to reject a subscribe if a previous internal
      subscription for unsolicited MWI was found for the mailbox.  That remains the
      default.  However, if there are mailboxes also set on the aor and the client
      subscribes and mwi_subscribe_replaces_unsolicited is set, the existing internal
      subscription is removed and replaced with the external subscription.  This
      allows an admin to configure mailboxes on both the endpoint and aor and allows
      the client to select which to use.
      
      ASTERISK-25865 #close
      Reported-by: Ross Beer
      
      Change-Id: Ic15a9415091760539c7134a5ba3dc4a6a1217cea
      e2524fce
    • zuul's avatar
    • George Joseph's avatar
      res_rtp_asterisk: Fix placement of txcount increment · 724b9ab2
      George Joseph authored
      Commit 1bce690c was incrementing txcount
      for rtcp packets as well as rtp packets and that was causing sender reports
      to be generated instead of receiver reports in cases where no rtp was actually
      being sent.
      
      Moved the txcount increment from __rtp_sento, which handles both rtp and rtcp,
      to rtp_sento which only handles rtp packets.
      
      Discovered by the hep/rtcp-receiver test.
      
      Change-Id: Ie442e4bb947a68847a676497021ba10ffaf376d5
      724b9ab2
  6. Mar 29, 2016
  7. Mar 28, 2016
    • George Joseph's avatar
      res_rtp_asterisk: Fix packet stats on bridged connection · 44ffb510
      George Joseph authored
      rxcount, txcount, rxoctetcount and txoctetcount weren't being calculated
      for bridged streams because the calulations were being done after the
      bridged short-circuit.  Actually, rxoctetcount wasn't ever being calculated.
      
      Moved the calculations so they occur for all valid received packets and
      all transmitted packets.  Also added rxoctetcount and txoctetcount to
      ast_rtp_instance_stat.
      
      Change-Id: I08fb06011a82d38c3b4068867a615068fbe59cbb
      44ffb510
    • George Joseph's avatar
      res_pjsip/pjsip_options: Fix From generation on outgoing OPTIONS · c971a643
      George Joseph authored
      No one seemed to notice but every time an OPTIONS goes out, it goes
      out with a From of "asterisk" (or whatever the default from_user is set to),
      even if you specify an endpoint.
      
      The issue had several causes...
      qualify_contact is only called with an endpoint if called from the CLI.
      If the endpoint is NULL, qualify_contact only looks up the endpoint if
      authenticate_qualify=yes. Even then, it never passes it on to
      ast_sip_create_request where the From header is set.  Therefore From
      is always "asterisk" (or whatever the default from_user is set to).
      Even if ast_sip_create_request were to get an endpoint, it only sets
      the From if endpoint->from_user is set.
      
      The fix is 4 parts...
      
      First, create_out_of_dialog_request was modified to use the endpoint id
      if endpoint was specified and from_user is not set.
      
      Second, qualify_contact was modified to always look up an endpoint if
      one wasn't specified regardless of authenticate_qualify.  It then passes
      the endpoint on to create_out_of_dialog_request.
      
      Third (and most importantly), find_an_endpoint was modified to find
      an endpoint by using an "aors LIKE %contact->aor%" predicate with
      ast_sorcery_retrieve_by_fields.  As such, this patch will only work
      if the sorcery realtime optimizations patch goes in.  Otherwise we'd
      be pulling the entire endpoints database every time we send an OPTIONS.
      Since we already know the contact's aor, the on_endpoint callback was also
      modified to just check if the contact->aor is an exact match to one of
      the endpoint's.
      
      Finally, since we now have an endpoint for every OPTIONS request,
      res_pjsip/endpt_send_request (which handles out-of-dialog reqests) was
      updated to get the transport from the endpoint and set it on tdata.
      Now the correct transport is used.
      
      Change-Id: I2207e12bb435e373bd1e03ad091d82e5aba011af
      c971a643
    • George Joseph's avatar
      sorcery/res_pjsip: Refactor for realtime performance · c948ce96
      George Joseph authored
      There were a number of places in the res_pjsip stack that were getting
      all endpoints or all aors, and then filtering them locally.
      
      A good example is pjsip_options which, on startup, retrieves all
      endpoints, then the aors for those endpoints, then tests the aors to see
      if the qualify_frequency is > 0.  One issue was that it never did
      anything with the endpoints other than retrieve the aors so we probably
      could have skipped a step and just retrieved all aors. But nevermind.
      
      This worked reasonably well with local config files but with a realtime
      backend and thousands of objects, this was a nightmare.  The issue
      really boiled down to the fact that while realtime supports predicates
      that are passed to the database engine, the non-realtime sorcery
      backends didn't.
      
      They do now.
      
      The realtime engines have a scheme for doing simple comparisons. They
      take in an ast_variable (or list) for matching, and the name of each
      variable can contain an operator.  For instance, a name of
      "qualify_frequency >" and a value of "0" would create a SQL predicate
      that looks like "where qualify_frequency > '0'".  If there's no operator
      after the name, the engines add an '=' so a simple name of
      "qualify_frequency" and a value of "10" would return exact matches.
      
      The non-realtime backends decide whether to include an object in a
      result set by calling ast_sorcery_changeset_create on every object in
      the internal container.  However, ast_sorcery_changeset_create only does
      exact string matches though so a name of "qualify_frequency >" and a
      value of "0" returns nothing because the literal "qualify_frequency >"
      doesn't match any name in the objset set.
      
      So, the real task was to create a generic string matcher that can take a
      left value, operator and a right value and perform the match. To that
      end, strings.c has a new ast_strings_match(left, operator, right)
      function.  Left and right are the strings to operate on and the operator
      can be a string containing any of the following: = (or NULL or ""), !=,
      >, >=, <, <=, like or regex.  If the operator is like or regex, the
      right string should be a %-pattern or a regex expression.  If both left
      and right can be converted to float, then a numeric comparison is
      performed, otherwise a string comparison is performed.
      
      To use this new function on ast_variables, 2 new functions were added to
      config.c.  One that compares 2 ast_variables, and one that compares 2
      ast_variable lists.  The former is useful when you want to compare 2
      ast_variables that happen to be in a list but don't want to traverse the
      list.  The latter will traverse the right list and return true if all
      the variables in it match the left list.
      
      Now, the backends' fields_cmp functions call ast_variable_lists_match
      instead of ast_sorcery_changeset_create and they can now process the
      same syntax as the realtime engines.  The realtime backend just passes
      the variable list unaltered to the engine.  The only gotcha is that
      there's no common realtime engine support for regex so that's been noted
      in the api docs for ast_sorcery_retrieve_by_fields.
      
      Only one more change to sorcery was done...  A new config flag
      "allow_unqualified_fetch" was added to reg_sorcery_realtime.
      "no": ignore fetches if no predicate fields were supplied.
      "error": same as no but emit an error. (good for testing)
      "yes": allow (the default);
      "warn": allow but emit a warning. (good for testing)
      
      Now on to res_pjsip...
      
      pjsip_options was modified to retrieve aors with qualify_frequency > 0
      rather than all endpoints then all aors.  Not only was this a big
      improvement in realtime retrieval but even for config files there's an
      improvement because we're not going through endpoints anymore.
      
      res_pjsip_mwi was modified to retieve only endpoints with something in
      the mailboxes field instead of all endpoints then testing mailboxes.
      
      res_pjsip_registrar_expire was completely refactored.  It was retrieving
      all contacts then setting up scheduler entries to check for expiration.
      Now, it's a single thread (like keepalive) that periodically retrieves
      only contacts whose expiration time is < now and deletes them.  A new
      contact_expiration_check_interval was added to global with a default of
      30 seconds.
      
      Ross Beer reports that with this patch, his Asterisk startup time dropped
      from around an hour to under 30 seconds.
      
      There are still objects that can't be filtered at the database like
      identifies, transports, and registrations.  These are not going to be
      anywhere near as numerous as endpoints, aors, auths, contacts however.
      
      Back to allow_unqualified_fetch.  If this is set to yes and you have a
      very large number of objects in the database, the pjsip CLI commands
      will attempt to retrive ALL of them if not qualified with a LIKE.
      Worse, if you type "pjsip show endpoint <tab>" guess what's going to
      happen? :)  Having a cache helps but all the objects will have to be
      retrieved at least once to fill the cache.  Setting
      allow_unqualified_fetch=no prevents the mass retrieve and should be used
      on endpoints, auths, aors, and contacts.  It should NOT be used for
      identifies, registrations and transports since these MUST be
      retrieved in bulk.
      
      Example sorcery.conf:
      
      [res_pjsip]
      endpoint=config,pjsip.conf,criteria=type=endpoint
      endpoint=realtime,ps_endpoints,allow_unqualified_fetch=error
      
      ASTERISK-25826 #close
      Reported-by: Ross Beer
      Tested-by: Ross Beer
      
      Change-Id: Id2691e447db90892890036e663aaf907b2dc1c67
      c948ce96
  8. Mar 26, 2016
    • Joshua Colp's avatar
    • zuul's avatar
    • Richard Mudgett's avatar
      res_parking: Fix blind transfer dynamic lots creation. · 8e8cf80c
      Richard Mudgett authored
      Blind transfers to a recognized parking extension need to use the parker's
      channel variable values to create the dynamic parking lot.  This is
      because there is always only one parker while the parkee may actually be a
      multi-party bridge.  A multi-party bridge can never supply the needed
      channel variables to create the dynamic parking lot.  In the multi-party
      bridge blind transfer scenario, the parker's CHANNEL(parkinglot) value and
      channel variables are inherited by the local channel used to park the
      bridge.
      
      * In park_common_setup(), make use the parker instead of the parkee to
      supply the dynamic parking lot channel variable values.  In all but one
      case, the parkee is the same as the parker.  However, in the recognized
      parking extension blind transfer scenario for a two party bridge they are
      different channels.  For consistency, we need to use the parker channel.
      
      * In park_local_transfer(), pass the CHANNEL(parkinglot) value to the
      local channel when blind transferring a multi-party bridge to a recognized
      parking extension.
      
      * When a local channel starts a call, the Local;2 side needs to inherit
      the CHANNEL(parkinglot) value from Local;1.
      
      The DTMF one-touch parking case wasn't even trying to create dynamic
      parking lots before it aborted the attempt.
      
      * In parking_park_call(), add missing code to create a dynamic parking
      lot.
      
      A DTMF bridge hook is documented as returning -1 to remove the hook.
      Though the hook caller is really coded to accept non-zero.  See the
      ast_bridge_hook_callback typedef.
      
      * In feature_park_call(), don't remove the DTMF one-touch parking hook
      because of an error.
      
      ASTERISK-24605 #close
      Reported by:  Philip Correia
      Patches:
            call_park.patch (license #6672) patch uploaded by Philip Correia
      
      Change-Id: I221d3a8fcc181877a1158d17004474d35d8016c9
      8e8cf80c
  9. Mar 25, 2016
    • Richard Mudgett's avatar
      res_parking: Cleanup find_channel_parking_lot_name() usage. · 3cf71403
      Richard Mudgett authored
      Change-Id: I8f7a8890aef27824301c642d4d15407ac83e6f02
      3cf71403
    • Richard Mudgett's avatar
      res_parking: Misc fixes. · 13e75ee0
      Richard Mudgett authored
      res/parking/parking_applications.c:
      
      * Add malloc fail checks in setup_park_common_datastore().
      
      * Fix playing parking failed announcement to only happen on non-blind
      transfers in park_app_exec().  It could never go out before because a test
      was provedly always false.
      
      res/parking/parking_bridge.c:
      
      * Fix NULL tolerance in generate_parked_user() because
      bridge_parking_push() can theoretically pass a NULL parker channel if the
      parker channel went away for some reason.
      
      * Clarify some weird code dealing with blind_transfer in
      bridge_parking_push().
      
      res/parking/parking_bridge_features.c:
      
      * Made park_local_transfer() set BLINDTRANSFER on the Local;1 channel
      which will be bulk copied to the Local;2 channel on the subsequent
      ast_call().  The additional advantage is if the parker channel has the
      BLINDTRANSFER and ATTENDEDTRANSFER variables set they are now guaranteed
      to be overridden.
      
      res/parking/parking_manager.c:
      
      * Fix AMI Park action input range checking of the Timeout header in
      manager_park().
      
      * Reduced locking scope to where needed in manager_park().
      
      res/res_parking.c:
      
      * Fix some off nominal missing unlocks by eliminating the returns.
      
      Change-Id: Ib64945bc285acb05a306dc12e6f16854898915ca
      13e75ee0
    • Philip Correia's avatar
      res_parking: Update parking documentation for dynamic parking lots. · e2853ae3
      Philip Correia authored
      * Remove duplicate res_parking.conf courtesytone config option
      documentation.
      
      ASTERISK-24596 #close
      Reported by:  Philip Correia
      
      ASTERISK-24605
      Reported by:  Philip Correia
      Patches:
            call_park_app_doc.patch (license #6672) patch uploaded by Philip Correia
      
      Change-Id: I90a92a891c6494dc08173e675856afcc4764c5b5
      e2853ae3
Loading