- add xavp when registered returns success - add reg_received_avp param for strict matching the contact - add reg_contact_avp param for strict matching the contact - add xavp to reg_fetch_contacts
modparam("usrloc", "xavp_contact", "ulattrs") modparam("registrar", "reg_received_avp", "$avp(chk_origin)") modparam("registrar", "reg_contact_avp", "$avp(chk_contact)")
using xavp with registered $avp(chk_origin) = $su; $avp(chk_contact) = $(ct{nameaddr.uri}); if (registered("location","$fu")) { xlog("L_INFO", "$ci|log| checking $fu - $xavp(ulattrs=>account_id)"); }
using xavp with reg_fetch_contacts if(reg_fetch_contacts("location", "$fu", "caller")) { xlog("L_INFO", "$ci|log| checking $fu - $(ulc(caller=>ulattrs=>account_id))"); }
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/91
-- Commit Summary --
* registrar - add user xavp on successful registered call * registrar - add optional avp params for usrloc comparison * registrar - add xavp to reg_fetch_contacts
-- File Changes --
M modules/registrar/lookup.c (35) M modules/registrar/reg_mod.c (50) M modules/registrar/reg_mod.h (4) M modules/registrar/regpv.c (307)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/91.patch https://github.com/kamailio/kamailio/pull/91.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91
Some of the patches are quite big and there is little description of the purpose and benefits. But they affect a really important module, where performances really matter. Just reading the code is not easy to guess your needs and impact on existing behavior.
Send an email to sr-dev providing more details for each of the patches.
As a recommendation, before writing a lot of code changing an important module, it is better to discuss first on sr-dev, to come up with a proper solution fitting the best of common use cases. Otherwise, you may lose time writing code that will not be accepted because it affects performances or the existing behavior that is commonly needed.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-74761779
You misunderstood my proposal. I wanted a new parameter to the function: registered(talble, uri, flags), because I want to avoid searching through xavps if not needed. The parameter is read only if provided.
If I got right the patches, you search for the flags in xavp list.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75125239
i don't think i did, the flags are in params.
i added the extra search for "override" to give more flexibility. i can put that flexibility param dependent.
On Thu, Feb 19, 2015 at 8:03 PM, Daniel-Constantin Mierla < notifications@github.com> wrote:
You misunderstood my proposal. I wanted a new parameter to the function: registered(talble, uri, flags), because I want to avoid searching through xavps if not needed. The parameter is read only if provided.
If I got right the patches, you search for the flags in xavp list.
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/91#issuecomment-75125239.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75125878
I don't see any new parameter to registered(...) function. Do I miss something?
Perhaps I can change (sometime during next days) the current call-id matching to using a new flags param to registered() function and xavp. Then you can add from there what you need extra.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75126660
there are two params match_flags as you proposed 1 - callid 2 - received 3 - contact
the registered() function check that flag, checks the values in xavp_cfg and assigns the value to a temp vars, that are then compared against the value in the record.
match_return_flags controls if the record xavp is added to the transaction.
On Thu, Feb 19, 2015 at 8:11 PM, Daniel-Constantin Mierla < notifications@github.com> wrote:
I don't see any new parameter to registered(...) function. Do I miss something?
Perhaps I can change (sometime during next days) the current call-id matching to using a new flags param to registered() function and xavp. Then you can add from there what you need extra.
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/91#issuecomment-75126660.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75127402
i added a 3rd param to control if we search for "override" values for flags. default is 0, don't search
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75129897
We are not on the same meaning of terms here. I refer to function parameter, not module parameter. I want to do: ``` registered("location", "$fu", "1") ``` in order to do registered() with call-id match. The new parameter can be int or pv (see fixup_igp_null() and fixup_get_ivalue() helper functions)
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75130582
@miconda i believe this is it ?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75176709
@miconda can you check if this is acceptable ? thanks.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75547064
A bit hard to follow, because now here are patches to patches to patches. Would be good to have a single patch against the master in kamailio for review.
One thing I noticed, that some flags are still a module parameter. Wouldn't it be more flexible to be part of the function parameter, different flag values (eventually starting at 8 or 16)?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75637132
@miconda hi, the remaining flag in the module is to control the setting of the contact xavp on return from a match. i left it here because we also want the behavior to apply to reg_fetch_contacts, is that ok ? i can make a simple patch against master.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75640886
But reg_fetch_contacts() is already returning the contacts in an variable structure - $ulc. I don't see a reason to add to $ulc as well as to an xavp.
Could be better to have a new function reg_fetch_contact_to_xavp() that will get the attributes in xavp instead of $ulc.
I may miss some point, in that case, provide some details when you find useful for reg_fetch_contacts() to return some data twice.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75643253
@miconda the $ulc structure doesn't have the contact xavp (it copies all elements but the xavp), the purpose of the flag is to add it to the $ulc structure.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75647789
But that was a patch adding to $ulc(...), not to an $xavp(), right? Maybe seeing all the changes as a single patch will shed light on these aspects.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75648346
right, it doesn't add it to xavp it adds it to $ulc so if we have modparam("usrloc", "xavp_contact", "ulattrs") we can use the $qlc returned like this (if flag is set). if(!reg_fetch_contacts("location", "$fu", "caller")) { xlog("L_INFO", "$ci|log| checking $fu - $ulc(caller=>ulattrs=>account_id)"); }
i'll put everything in one commit and create a new PR
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75649154
https://github.com/kamailio/kamailio/pull/101
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#issuecomment-75935336
Closed #91.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/91#event-241590098
-
Daniel-Constantin Mierla -
lazedo