cnxcc: remove duplicated entries with a same CID value

[Pepelux]

Pre-Submission Checklist

• [ x] Commit message has the format required by CONTRIBUTING guide
• [ x] Commits are split per component (core, individual modules, libs, utils, ...)
• [ x] Each component has a single commit (if not, squash them into one commit)
• [ x] No commits to README files for modules (changes must be done to docbook files
  in doc/ subfolder, the README file is autogenerated)

Type Of Change

• [x ] Small bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds new functionality)
• Breaking change (fix or feature that would change existing functionality)

Checklist:

• [x ] PR should be backported to stable branches
• [x ] Tested changes locally
• Related to issue #XXXX (replace XXXX with an open issue number)

Description

When cnxcc receives a message wih a registered CID (for example in a re-INVITE) it saves as a new call and this is a problem:

• cnxcc store 2 times the same call and it discounts credit for both
• when the call is finished, cnxcc only remove one record and the other continues discounting credit.
• when credit is finished (of an non active call), all active calls are killed

An example:

kamcmd> cnxcc.check_client test
id:0,confirmed:no,local_consumed_amount:0.004800,global_consumed_amount:3.126200,local_max_amount:30.000000,global_max_amount:30.000000,call_id:41211f7e0a65b41c4ca7181b18db0f1e@185.47.15.56:5080,start_timestamp:0,inip:1,finp:1,connect:0.000000,cps:0.004800;
id:1,confirmed:no,local_consumed_amount:0.004800,global_consumed_amount:3.126200,local_max_amount:30.000000,global_max_amount:30.000000,call_id:41211f7e0a65b41c4ca7181b18db0f1e@185.47.15.56:5080,start_timestamp:0,inip:1,finp:1,connect:0.000000,cps:0.004800;
id:2,confirmed:no,local_consumed_amount:0.000200,global_consumed_amount:3.126200,local_max_amount:30.000000,global_max_amount:30.000000,call_id:5e911ab1452cf1a3165af5262b519977@185.47.15.56:5080,start_timestamp:0,inip:1,finp:1,connect:0.000000,cps:0.000200;
id:3,confirmed:no,local_consumed_amount:0.000200,global_consumed_amount:3.126200,local_max_amount:30.000000,global_max_amount:30.000000,call_id:5e911ab1452cf1a3165af5262b519977@185.47.15.56:5080,start_timestamp:0,inip:1,finp:1,connect:0.000000,cps:0.000200;
id:4,confirmed:no,local_consumed_amount:0.000800,global_consumed_amount:3.126200,local_max_amount:30.000000,global_max_amount:30.000000,call_id:0f44fbc63883e52d72c9f52f49253476@185.47.15.56:5080,start_timestamp:0,inip:1,finp:1,connect:0.000000,cps:0.000800;

• id 0 and id 1 are the same call that has been inserted twice.
• id 2 and id 3 are the same call that has been inserted twice.
• id 4 is a finished call that has not been removed from the stack and continues discounting credit.

[miconda]

I guess the function was supposed to be called only for the initial INVITE, the one that creates the call.

Overall, I do not use that much the module to have an opinion of this is good to have or someone also wants to charge a call twice (e.g., incoming leg and outgoing leg). The initial developer of the module is not active lately in the project, maybe @linuxmaniac can give his opinion as I saw some commits to this module from him rather recently.

As a further remark, the protection could be good, but if there are cases to charge a call many times, then it should be made a modparam to control the behaviour.

[Pepelux]

Sure. It supposed to be called only for the initial INVITE but there are a check in the code for that (which log message was fixed yesterday by @linuxmaniac). The problem is that this check only print a message and has no control the insertion of the new record.

I've found the problem in some calls where '100 Trying' takes 1 second and origin sends another INVITE message

[miconda]

The t_precheck_trans() and t_check_trans() should help filtering out such retransmissions.

Anyhow, again, the only concern is about the case when one wants to charge a call twice (or even more), either from same billing account or from different accounts... not sure how the PR affects such cases.

[Pepelux]

You are right. If you want I can prepare a modparam to control the behaviour. Only one CID o multiples CIDs

[linuxmaniac]

AFAICT module just supports one record per call-id since __stop_billing(), that is called via dialog.callback when call is DLGCB_TERMINATED | DLGCB_FAILED | DLGCB_EXPIRED, is just getting one record via try_get_call_entry() and even more it does not support mixing limits ( money, time or channel )

[Pepelux]

I've been reviewing the code another time.

There are 3 different functions to control credit, time and channels: __set_max_credit(), __set_max_time() and __set_max_channels(). All of them do the same:

function __set_max_money():
credit_data = __get_or_create_credit_data_entry(client_id, CREDIT_MONEY)
call = __alloc_new_call_by_time(client_id) // Allocate new call for client
__add_call_by_cid(cid, call, CREDIT_MONEY) // Allocate new CID_BY_CLIENT for client

function __set_max_time():
credit_data = __get_or_create_credit_data_entry(client_id, CREDIT_TIME)
call = __alloc_new_call_by_time(client_id) // Allocate new call for client
__add_call_by_cid(cid, call, CREDIT_TIME) // Allocate new CID_BY_CLIENT for client

function __set_max_channel():
credit_data = __get_or_create_credit_data_entry(client_id, CREDIT_CHANNEL)
call = __alloc_new_call_by_time(client_id) // Allocate new call for client
__add_call_by_cid(cid, call, CREDIT_CHANNEL) // Allocate new CID_BY_CLIENT for client

If credit is exhausted, the function __terminate_all() is called to kill all active calls for this client. This works fine:

__terminate_all(pointer_client_id) -> ki_terminate_all(client_id) -> terminate_all_calls(credit_data):
foreach(call_list) {
terminate_call(call)
}

On the other hand, when a dialog is finished, the function called is __stop_billing():

__dialog_terminated_callback() -> __stop_billing(callid) -> __delete_call(call, credit_data) -> __free_call(call)

In this case, there is no loop to delete and free all calls for this client with a same CID. Only remove the first match found.
If the module has prepared to handle several times the same CID (by client), when dialog is finished, it should check and remove all calls with of this client with a same CID. Currently the module is not working in this way and I'm sure that nobody is using it to charge more than 1 times each CID, because it doesn't work. If anyone is trying to charge a call twice, is having problems because only one call by cid is terminated.

However, if you think that this feature could be important (being able to charge the same call more than once), it will be necessary to fix that, because currently it doesn't work.

[miconda]

My comments were to be sure it doesn't break scenarios expected to work. If not, then it can be merged from my point of view.

[linuxmaniac]

I've squashed the commits and reworked the commit messages