30 Jun
2020
30 Jun
'20
5:57 p.m.
<!-- Kamailio Pull Request Template -->
<!--
IMPORTANT:
- for detailed contributing guidelines, read:
https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md
- pull requests must be done to master branch, unless they are backports
of fixes from master branch to a stable branch
- backports to stable branches must be done with 'git cherry-pick -x
...'
- code is contributed under BSD for core and main components (tm, sl, auth, tls)
- code is contributed GPLv2 or a compatible license for the other components
- GPL code is contributed with OpenSSL licensing exception
-->
#### Pre-Submission Checklist
<!-- Go over all points below, and after creating the PR, tick all the checkboxes
that apply -->
<!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines
from above-->
<!-- If you're unsure about any of these, don't hesitate to ask on
sr-dev mailing list -->
- [ 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:
<!-- Go over all points below, and after creating the PR, tick the checkboxes that
apply -->
- [x ] PR should be backported to stable branches
- [x ] Tested changes locally
- [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description
<!-- Describe your changes in detail -->
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.
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2383
-- Commit Summary --
* cnxcc: remove duplicated entries with a same CID value
-- File Changes --
M src/modules/cnxcc/cnxcc_mod.c (50)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2383.patch
https://github.com/kamailio/kamailio/pull/2383.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/2383
30 Jun
30 Jun
6:24 p.m.
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.
--
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/2383#issuecomment-651865050
6:35 p.m.
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
--
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/2383#issuecomment-651872250
11:47 p.m.
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.
--
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/2383#issuecomment-652034275
1 Jul
1 Jul
11:58 a.m.
You are right. If you want I can prepare a modparam to control the behaviour. Only one CID
o multiples CIDs
--
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/2383#issuecomment-652290150
2:01 p.m.
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 )
--
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/2383#issuecomment-652351386
2:03 p.m.
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.
--
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/2383#issuecomment-652352229
6:55 p.m.
@Pepelux pushed 1 commit.
77aec0e628d273629075b300d5a251f4d13a9ea8 cnxcc: readme example fixed [skip ci]
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2383/files/c4f075c6b3cff4db9b75c8…
2 Jul
2 Jul
11:10 a.m.
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.
--
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/2383#issuecomment-652856977
11:45 a.m.
@Pepelux pushed 1 commit.
f16ea4705b9ed06184863ff63e9ad7505b1bc177 cnxcc: fix readme example [skip ci]
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2383/files/77aec0e628d273629075b3…
1:07 p.m.
@Pepelux pushed 2 commits.
5b94195e3107aa4273205c8c14577d4d0f2ba5e6 cnxcc: some return values do not match in readme
and code [skip ci]
2c568178154fcd174e61ea3a5168b15eb3871552 cnxcc: Changing return code since -2 is in use
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/2383/files/f16ea4705b9ed06184863f…
5:32 p.m.
I've squashed the commits and reworked the commit messages
--
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/2383#issuecomment-653041422
5:32 p.m.
Closed #2383.
--
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/2383#event-3507395813
1607
days inactive
1609
days old
12 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Jose Luis Verdeguer
-
Victor Seva