[kamailio/kamailio] ims_qos, ima_registrar_pcscf: fixes for de-registration and adding a flow to registration (#721)
ims_qos: create a proper flow description for the SIP signaling - before there was just an empty one in the AAR - re-using the function typically used for the RTP media - fixing then that function to not just hard-code protocol 17 (UDP) for everything - now supporting also 6 (TCP) and IP (IP has no protocol number, we use just the magic word "ip" (see RFC 3588 IPFilterRule for more details) - adding the IP of the P-CSCF as a parameter to the module, as this is required in the flow (UE IP <-> P-CSCF IP) - also some inline/static fixes to function for avoiding warnings on gcc >=5
ims_registrar_pcscf: fixed removal of contacts on de-registration - the code was there, but I guess some API changes resulted in temporary commented-out code; this uses the new parameters and seems to fix the issue. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/721
-- Commit Summary --
* ims_qos: create a proper flow description for the SIP signaling * ims_registrar_pcscf: fixed removal of contacts on de-registration
-- File Changes --
M modules/ims_qos/mod.c (4) M modules/ims_qos/rx_aar.c (32) M modules/ims_qos/rx_avp.c (47) M modules/ims_qos/rx_avp.h (12) M modules/ims_registrar_pcscf/save.c (6)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/721.patch https://github.com/kamailio/kamailio/pull/721.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721
Hi Dragos,
looks okay to me, waiting for @jaybeepee or @richardgood to approve, too. Can you add the new parameter to the docs, too?
Thanks, Carsten
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234038459
@vingarzan pushed 1 commit.
d99753e ims_qos: better name for the rx_pcscf_ip parameter and added documentation
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/721/files/e102306fe779b8d11ce2151e...
Hi Carsten,
fixed the missing doc. I also changed the name to something hopefully more relevant. "rx_" was for parameters related to the Diameter interface.
Cheers, -Dragos
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234201182
Thanks @vingarzan
All looks good - just want to check one thing with @jaybeepee on ims_registrar_pcscf: fixed removal of contacts on de-registration
I have a sneaking suspicion this was removed on purpose and the contact removal is done through NOTIFY from S-CSCF.
Though I don't think this would cause any issues having it here as well (and could be useful in the event of P-CSCF not implementing subscribe to reg event)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234253888
That's what I thought also, that reginfo would save the day. Then spent a long time tracking down structure of pua, etc, just to find out that the NOTIFY is not doing it unfortunately. Or maybe we missed something...
Anyway: my recommendation is to do both really, to make sure that you catch it even if the configuration has the SUBSCRIBE disabled or some strange S-CSCFs would reject the SUBSCRIBE.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234262630
Hi
Have a look at ims_registrar_pcscf/notify.c line 205.
If the contact is in usrloc and a NOTIFY is received for terminate it should remove it.
But agreed we should do both - just in case. Give me a day or two I just want to double confirm with @jaybeepee why it was removed in the first place to ensure it doesn't have any unintended consequences.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234266062
Ahh I see the confusion - we call reginfo_handle_notify from cfg file - which might not be the case for all P-CSCF cfg files. I think we still need to do quite a bit of polishing on this code and config.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234267456
Merged #721.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#event-730808203
Can i haz the pcscf example file plz :) ?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234350009
I see the reginfo_handle_notify is actually in the example pcscf config file in git:
examples/pcscf/kamailio.cfg line 1184.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/721#issuecomment-234466496
-
Dragos Vingarzan -
ng-voice GmbH
-
richardgood