Hello all,
For NATed clients, we have the following functions to handle them: - fix_nated_contact() - add_contact_alias() - set_contact_alias()
Both add_contact_alias() and set_contact_alias() are adding the following new param ";alias=ip~port~transport" to the Contact header. There are some subtle differences between the two: set_contact_alias() makes the contact URI is immediately visible to other modules. Recently, Daniel made a fix for set_contact_alias() to work with presence modules (just like fix_nated_contact() does).
Since both add_contact_alias() and set_contact_alias() are doing similar things, but add_contact_alias() has some limitations, I would propose to deprecate add_contact_alias() in favour of set_contact_alias().
If required, we can enhance set_contact_alias() to take extra params just like add_contact_alias() does.
This will make things cleaner and less ambiguity about how to handle NATed clients (especially for newcomers to kamailio). And maybe, we should have a similar discussion about fix_nated_contact().
Regards, Ovidiu Sas
Ovidiu Sas writes:
Both add_contact_alias() and set_contact_alias() are adding the following new param ";alias=ip~port~transport" to the Contact header. There are some subtle differences between the two: set_contact_alias() makes the contact URI is immediately visible to other modules. Recently, Daniel made a fix for set_contact_alias() to work with presence modules (just like fix_nated_contact() does).
Since both add_contact_alias() and set_contact_alias() are doing similar things, but add_contact_alias() has some limitations, I would propose to deprecate add_contact_alias() in favour of set_contact_alias().
normally if a function has limitations, it is enhanced. why is it not possible to enhance add_contact_alias()?
-- juha
On 14/10/14 16:42, Juha Heinanen wrote:
Ovidiu Sas writes:
Both add_contact_alias() and set_contact_alias() are adding the following new param ";alias=ip~port~transport" to the Contact header. There are some subtle differences between the two: set_contact_alias() makes the contact URI is immediately visible to other modules. Recently, Daniel made a fix for set_contact_alias() to work with presence modules (just like fix_nated_contact() does).
Since both add_contact_alias() and set_contact_alias() are doing similar things, but add_contact_alias() has some limitations, I would propose to deprecate add_contact_alias() in favour of set_contact_alias().
normally if a function has limitations, it is enhanced. why is it not possible to enhance add_contact_alias()?
set_contact_alias() is doing what those limitations are in add_contact_alias(). From 'external' SIP singaling point of view, same change is seen in the SIP packets. The difference is how the change is done internally.
- add_contact_alias() is adding/removing pieces of string in existing contact, the old value is seen by any other module needed to store the contact (e.g., dialog, presence) and they won't work if are used in a Kamailio positioned as the first hop after nat router - set_contact_alias() is building the new contact and replaces the old one at once, with some special internal flag that makes it visibile immediately (same flag is used by fixed_nated_contact())
In other words, add_contact_alias() can be pointed directly to set_contact_alias() and no functionality is lost. Ovidiu's suggestion is to deprecate it and remove it in future version.
I don't have any strong opinion on any option: it can be kept and pointed to set_contact_alias() or removed in the future.
Cheers, Daniel
Daniel-Constantin Mierla writes:
In other words, add_contact_alias() can be pointed directly to set_contact_alias() and no functionality is lost. Ovidiu's suggestion is to deprecate it and remove it in future version.
is there any performance difference when change is made immediately to sip message?
if not, i still don't understand why add_contact_alias was not simply modified instead of creating a new function. why don't you rename set_contact_alias to add_contact_alias and deprecate set_contact_alias, since i bet that add_contact_alias is as older function used in more configs than set_contact_alias?
-- juha
On 14/10/14 17:13, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
In other words, add_contact_alias() can be pointed directly to set_contact_alias() and no functionality is lost. Ovidiu's suggestion is to deprecate it and remove it in future version.
is there any performance difference when change is made immediately to sip message?
I don't think it can be any real difference, not measured, though.
if not, i still don't understand why add_contact_alias was not simply modified instead of creating a new function. why don't you rename set_contact_alias to add_contact_alias and deprecate set_contact_alias, since i bet that add_contact_alias is as older function used in more configs than set_contact_alias?
set_contact_alias() replaced the fix_natted_contact() in default config, from this perspective I think that set_contact_alias() is used in more configs than add_contact_alias(), but at the end this is not the point, because you cannot really count all configs and anyhow usage count should not be reason to remove something better. The function was added in the same idea of having alternatives of fix_natted_contact() and add_contact_alias() tackling the same problem from a different point. On other words, as add_contact_alias() would prove to be better for handling nat traversal, we should rename it to fix_natted_contact() because that is older.
If there is a strong desire to keep add_contact_alias(), it should be kept after all, but properly documented that has the limitations on making new contact visible and should not be used in a sip server instance used for presence, dialog, ...
Or, even better in my opinion at this time, just point both to same C function. Overall, as this is discussion about deprecating something, it will happen more recently with the future next major release, 4.3.
Daniel
Daniel-Constantin Mierla writes:
If there is a strong desire to keep add_contact_alias(), it should be kept after all, but properly documented that has the limitations on making new contact visible and should not be used in a sip server instance used for presence, dialog, ...
i have made enhancements to a large number of functions and never created new ones unless there has been backwards incompatible change in parameters or performance penalty.
if there has not been either in case of set_contact_alias, it should not have not created in the first place and therefore i'm against deprecating add_contact_alias.
Or, even better in my opinion at this time, just point both to same C function. Overall, as this is discussion about deprecating something, it will happen more recently with the future next major release, 4.3.
i didn't get answer to performance issue. is there penalty when changes are applied to the message by set_contact_alias function? if yes, then add_contact_alias code should not be changed.
-- juha
On 14/10/14 17:44, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
If there is a strong desire to keep add_contact_alias(), it should be kept after all, but properly documented that has the limitations on making new contact visible and should not be used in a sip server instance used for presence, dialog, ...
i have made enhancements to a large number of functions and never created new ones unless there has been backwards incompatible change in parameters or performance penalty.
Actually there were quite big implications in other modules (like in dialog done initially and then in tm module done recently to get it all work like a replacement for fix_natted_contact()) -- so it was not just about a new (or replacement) function in nathelper module.
if there has not been either in case of set_contact_alias, it should not have not created in the first place and therefore i'm against deprecating add_contact_alias.
It's fine to keep add_contact_alias() from my point of view, if you feel it performs better for your needs (I didn't start this discussion after all), but docs have to list the limitations.
Or, even better in my opinion at this time, just point both to same C function. Overall, as this is discussion about deprecating something, it will happen more recently with the future next major release, 4.3.
i didn't get answer to performance issue. is there penalty when changes are applied to the message by set_contact_alias function? if yes, then add_contact_alias code should not be changed.
I did say something about performances from my point of view in the previous email.
Cheers, Daniel
Well, I started the discussion :)
We can keep both methods, we just need to update the docs that add_contact_alias() doesn't work with presence: NATed subscriptions handled using add_contact_alias() will not work properly.
This is just to avoid same question repeated on the mailing list. I hit the same issue with set_contact_alias() before it was fixed and then I saw the same issue raised by someone else.
Regards, Ovidiu Sas
On Tue, Oct 14, 2014 at 11:53 AM, Daniel-Constantin Mierla miconda@gmail.com wrote:
On 14/10/14 17:44, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
If there is a strong desire to keep add_contact_alias(), it should be kept after all, but properly documented that has the limitations on making new contact visible and should not be used in a sip server instance used for presence, dialog, ...
i have made enhancements to a large number of functions and never created new ones unless there has been backwards incompatible change in parameters or performance penalty.
Actually there were quite big implications in other modules (like in dialog done initially and then in tm module done recently to get it all work like a replacement for fix_natted_contact()) -- so it was not just about a new (or replacement) function in nathelper module.
if there has not been either in case of set_contact_alias, it should not have not created in the first place and therefore i'm against deprecating add_contact_alias.
It's fine to keep add_contact_alias() from my point of view, if you feel it performs better for your needs (I didn't start this discussion after all), but docs have to list the limitations.
Or, even better in my opinion at this time, just point both to same C function. Overall, as this is discussion about deprecating something, it will happen more recently with the future next major release, 4.3.
i didn't get answer to performance issue. is there penalty when changes are applied to the message by set_contact_alias function? if yes, then add_contact_alias code should not be changed.
I did say something about performances from my point of view in the previous email.
Cheers, Daniel
-- Daniel-Constantin Mierla http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Ovidiu Sas writes:
We can keep both methods, we just need to update the docs that add_contact_alias() doesn't work with presence: NATed subscriptions handled using add_contact_alias() will not work properly.
that is fine with me. add_contact_alias was intended to be used by a sip proxy, not one working as sip user agent.
-- juha
On 14/10/14 18:09, Juha Heinanen wrote:
Ovidiu Sas writes:
We can keep both methods, we just need to update the docs that add_contact_alias() doesn't work with presence: NATed subscriptions handled using add_contact_alias() will not work properly.
that is fine with me. add_contact_alias was intended to be used by a sip proxy, not one working as sip user agent.
For clarification, it is still used in server side, affecting the presence modules and dialog (didn't review all to see if any other is) when used on the same instance.
If you refer to sip user agent because presence generates a request (e.g., NOTIFY), then it is ok -- I wanted to be sure will not be misinterpreted by readers that it refers to the case when using kamailio as client (e.g., for remote registrations). In other words, this function should be used if contact is not stored anywhere for later usage inside same kamailio instance (memory or database).
Daniel
Daniel-Constantin Mierla writes:
If you refer to sip user agent because presence generates a request (e.g., NOTIFY), then it is ok -- I wanted to be sure will not be misinterpreted by readers that it refers to the case when using kamailio as client (e.g., for remote registrations). In other words, this function should be used if contact is not stored anywhere for later usage inside same kamailio instance (memory or database).
understood. i have never had any problems with add_contact_alias, because i don't use dialog module (the one that historically has had more problems than any other kamailio module) and my sip proxy frontends my presence server.
-- juha
-
Daniel-Constantin Mierla -
Juha Heinanen -
Ovidiu Sas