Avoid to force vendor_id variable to zero if AVP AVP_Auth_Application_Id or AVP_Acct_Application_Id does not exist on AAR message. In this case, if Vendor_id AVP exists but AVP_Auth_Application_Id or AVP_Acct_Application_Id does not exist vendor_id is forced to zero, thus cdp is not able to route the requests correctly anymore. vendor_id and app_id variables are updated only if both AVPs (vendor_id and Acct or vendor_id and Auth exist). You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/238
-- Commit Summary --
* Fix cdp routing for Rx interface
-- File Changes --
M modules/cdp/routing.c (10)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/238.patch https://github.com/kamailio/kamailio/pull/238.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/238
Dear all,
With this mail i would like to explain better my modifications to CDP module for Rx diameter message routing. (pull request #238). As you know, the ims_qos module create AAR Diameter message sent from P-CSCF to PCRF on REGISTER or on the first response received by the P-CSCF with a valid SDP during INVITE and STR message on BYE
After the creation of AAR message, the routing decision is made in the cdp module.
On get_routing_peer function (routing.c file), fist of all, AVP_Vendor_Specific_Application_Id is checked. If this AVP is present, AVP_Vendor_Id and AVP_Auth_Application_Id is checked together. If they are present on the message, vendor_id and app_id are assigned with the respective values retrieved from the message. After that, AVP_Auth_Application_Id is checked. If this AVP is present, AVP_Vendor_Id is checked. If this is not present, vendord_id is assigned to zero. The same behavior is implemented for AVP_Acct_Application_Id.
This way, if Auth_Application_Id is present but not AVP_Vendor_Id is not, vendor_id variable is zero and peer_handles_application function will return always zero and the module will not be able to route correctly the request (unless you by-pass the routing forcing explicitely the peer on the script).
Taking a look to the specs (TS 29.214) AVP_Auth_Application_id is mandatory, meanwhile AVP_Vendor_Id and AVP_Acct_Application_Id is not present. For this reason, IMHO vendor_id variable cannot be forced to zero if one of them is not present. In my patch, vendor_id variable is updated if only both AVP are present (Auth_Application_Id and Vendor_Id or AVP_Acct_Application_Id and Vendor_Id).
In my tests, everything is working with my patch and also the other Diameter messages (e.g. from I-CSCF or S-CSCF to HSS and vice-versa). However, i would like to have your point of view on the matter.
Best regards,
Federico Favaro R&D Department Athonet s.r.l.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/238#issuecomment-123653011
Hi Federico,
I agree with you re. the specs. I will have a look this evening on our test setup and commit your patch if I am happy. Thanks for your feedback!
Cheers Jason
On Wed, 22 Jul 2015 at 12:23 Federico Favaro notifications@github.com wrote:
Dear all,
With this mail i would like to explain better my modifications to CDP module for Rx diameter message routing. (pull request #238 https://github.com/kamailio/kamailio/pull/238). As you know, the ims_qos module create AAR Diameter message sent from P-CSCF to PCRF on REGISTER or on the first response received by the P-CSCF with a valid SDP during INVITE and STR message on BYE
After the creation of AAR message, the routing decision is made in the cdp module.
On get_routing_peer function (routing.c file), fist of all, AVP_Vendor_Specific_Application_Id is checked. If this AVP is present, AVP_Vendor_Id and AVP_Auth_Application_Id is checked together. If they are present on the message, vendor_id and app_id are assigned with the respective values retrieved from the message. After that, AVP_Auth_Application_Id is checked. If this AVP is present, AVP_Vendor_Id is checked. If this is not present, vendord_id is assigned to zero. The same behavior is implemented for AVP_Acct_Application_Id.
This way, if Auth_Application_Id is present but not AVP_Vendor_Id is not, vendor_id variable is zero and peer_handles_application function will return always zero and the module will not be able to route correctly the request (unless you by-pass the routing forcing explicitely the peer on the script).
Taking a look to the specs (TS 29.214) AVP_Auth_Application_id is mandatory, meanwhile AVP_Vendor_Id and AVP_Acct_Application_Id is not present. For this reason, IMHO vendor_id variable cannot be forced to zero if one of them is not present. In my patch, vendor_id variable is updated if only both AVP are present (Auth_Application_Id and Vendor_Id or AVP_Acct_Application_Id and Vendor_Id).
In my tests, everything is working with my patch and also the other Diameter messages (e.g. from I-CSCF or S-CSCF to HSS and vice-versa). However, i would like to have your point of view on the matter.
Best regards,
Federico Favaro R&D Department Athonet s.r.l.
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/238#issuecomment-123653011. _______________________________________________ sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
@jaybeepee - any resolution to this one, can it be merged?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/238#issuecomment-126044780
Hey Daniel,
yes it can be merged. Thanks Frederico
On Wed, 29 Jul 2015 at 20:09 Daniel-Constantin Mierla < notifications@github.com> wrote:
@jaybeepee https://github.com/jaybeepee - any resolution to this one, can it be merged?
—
Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/238#issuecomment-126044780.
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Thanks to you for the feedback!
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/238#issuecomment-126268959
Merged #238.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/238#event-369302651
-
Daniel-Constantin Mierla -
Federico Favaro
-
Jason Penton