ims_rdn: new experimental module for IMS RDNs - #4786
luca-nardin-kontron wants to merge 2 commits into
Conversation
|
Hi Luca, Two comments from my side.
Thank you for doing this important PR |
|
Thanks for the PR. I only had a brief look to the code so far. Several things should be addressed:
Regarding the note from @christoph-v-kontron, there seems to be pseudo-variables exported, which is fine and actually the preferred way since some time. But please also review of course. |
|
@luca-nardin-kontron Any comments from your side to the earlier remarks? |
Hi Henning, regarding clang, seems we used a more recent version of the tool that generates a format that is not compatible with the version you used. Regarding the other topics, we did already some correction and plan to update our pull request soon. Kind Regards, |
|
@luca-nardin-kontron Thanks for the previous update, are there any news? |
Hi Henning, all the requested changes are done and recompiled successfully against the community master, I just need to re-create the README, so I guess tomorrow or on friday I can update the pull request (if our machines survive the record heatwave of Vienna!) Kind regards, |
Added update_contacts(..) and parse_user_data(..) in the module's API.
We (Kontron Transportation) are issuing this pull request with a new experimental module, which is called ims_rdn, hopeful that this module might be useful for at least some users of Kamailio IMS (I-CSCF and S-CSCF). This module is most useful for terminating unregistered services that are triggered by the I/S-CSCF and that are identified by hierarchical address ranges of E.164 addresses and of EIRENE addresses (in the Request-URI), not unlike wildcarded PSIs. By configuring these address ranges at the I/S-CSCF, one can save one SAR/SAA exchange with the HSS per call. EIRENE addresses are railway specific telephone numbers, which have been defined by the UIC. This explains the name of the module RDN = Railway Dedicated Networks. Additionally, this module defines kamailio transformations, which are tailored for the use in RDNs, e.g. for the transformation of functional numbers and of MCData SDS into instant messages and vice versa. This module is even more useful, when implementing an IWF according to ETSI TS 103 792, in which case some specific routing scripts will be needed. We will be glad to help with those.
f5ed968 to
78d84b1
Compare
|
We had to include in this PR small additions to ims_registrar_scscf as well, otherwise ims_rdn would not compile against community master. Please let us know if you prefer a separate PR for that change. |
|
@henningw it seems the failed check is due to a bug in github? Should we do something on our side to solve the issue or just wait that it gets fixed? |
You can ignore this check, its a known issue. Regarding a second review, I planned it for the next days. Some delay during the holiday period. |
henningw
left a comment
There was a problem hiding this comment.
Thanks for the update, I did another review. Mostly standard topics I added as comments:
- missing logging and error handling for internal and external function calls
- inconsistent logging, also inconsistent log messages, redundant prefixes etc.
- dead and commented out code that should be removed.
Regarding the naming - the functions have somehow generic names (e.g. analyze_..._ruri). But this module is quite special so its probably not an issue for eventual overlapping to other modules.
|
|
||
| e = (struct rdn_hash_entry *)shm_malloc(sizeof(struct rdn_hash_entry)); | ||
| if(!e) { | ||
| ERR("Not enough memory left\n"); |
There was a problem hiding this comment.
Use LM_ERR as elsewhere, or SHM_MEM_ERROR
| sizeof(struct rdn_hash_entry *) * RDN_HASH_SIZE); | ||
|
|
||
| if(!dpsi_table) { | ||
| ERR("No memory left\n"); |
There was a problem hiding this comment.
Use LM_ERR as elsewhere, or SHM_MEM_ERROR
| int err = 0; | ||
| rdn_special_handling_t *new_sh = 0; | ||
| if(temp) { | ||
| new_sh = (rdn_special_handling_t *)shm_malloc( |
There was a problem hiding this comment.
Error handling and logging
| strncpy(new_sh->iwf_domain.s, temp->iwf_domain.s, | ||
| temp->iwf_domain.len); | ||
| } else | ||
| err = 1; |
| err = 1; | ||
| } | ||
| if(temp->hardcoded_ruri.s && temp->hardcoded_ruri.len) { | ||
| new_sh->hardcoded_ruri.s = (char *)shm_malloc( |
There was a problem hiding this comment.
Error handling and logging
| } | ||
| // Print timevalue as HEX number. | ||
| // The string is ASCII encoded (not binary) because "append_message_body_hex()" transforms it into binary bytes anyway. | ||
| return sprintf(dst, "%010llx", timevalue); |
There was a problem hiding this comment.
Error handling and logging
|
|
||
| switch(subtype) { | ||
| case TR_RDN_UUI2FN: | ||
| length = base16_decode_update(_tr_buffer, &val->rs); |
There was a problem hiding this comment.
Error handling and logging for this and following function calls
| case TR_RDN_BIN2INT: | ||
| // read any character from binary message body | ||
| // and cast it as integer. | ||
| val->ri = (unsigned char)val->rs.s[0]; |
There was a problem hiding this comment.
Error handling and logging..
| val->ri = get_sds_disposition_type(val->rs.len, val->rs.s); | ||
| // Check for Error | ||
| if(!val->ri) | ||
| return -1; |
There was a problem hiding this comment.
Logging here and the following function calls
| break; | ||
| case TR_RDN_TXT_2_MSG_BODY: | ||
| // build SDS header for SDS DATA-PAYLOAD | ||
| length = build_sds_header_for_txt_body( |
There was a problem hiding this comment.
Error handling and logging..
|
@luca-nardin-kontron Any update from your side? Were you able to have a look to the comments? |
Pre-Submission Checklist
clang-formatusing the config file.clang-formatfrom source code folder
in
doc/subfolder, the README file is autogenerated)Type Of Change
Checklist:
Description
We (Kontron Transportation) are issuing this pull request
with a new experimental module, which is called ims_rdn,
hopeful that this module might be useful for at least
some users of Kamailio IMS (I-CSCF and S-CSCF).
This module is most useful for terminating unregistered services
that are triggered by the I/S-CSCF and that are identified
by hierarchical address ranges of E.164 addresses and of
EIRENE addresses (in the Request-URI), not unlike wildcarded PSIs.
By configuring these address ranges at the I/S-CSCF,
one can save one SAR/SAA exchange with the HSS per call.
EIRENE addresses are railway specific telephone numbers,
which have been defined by the UIC.
This explains the name of the module RDN = Railway Dedicated Networks.
Additionally, this module defines kamailio transformations,
which are tailored for the use in RDNs,
e.g. for the transformation of functional numbers and
of MCData SDS into instant messages and vice versa.
This module is even more useful, when implementing
an IWF according to ETSI TS 103 792,
in which case some specific routing scripts will be needed.
We will be glad to help with those.