Skip to content

Commit

Permalink
acc: Fix drop_accounting() regression (211a63c)
Browse files Browse the repository at this point in the history
Commit 211a63c changed drop_accounting() from doing too little
dropping (the "backend bit" would never get reset) into doing too much
dropping (the "backend bit" would always be reset, despite some of the
flags still remaining enabled after the drop operation).

This commit fixes the code and also adds some unit tests for the
set/reset bitmask operations, to lock in the correct behavior.

(cherry picked from commit 0f0de89)
(cherry picked from commit 9608d8a)
  • Loading branch information
liviuchircu committed Dec 4, 2023
1 parent 218d991 commit e771060
Show file tree
Hide file tree
Showing 4 changed files with 269 additions and 43 deletions.
42 changes: 4 additions & 38 deletions modules/acc/acc_logic.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,33 +72,6 @@ static query_list_t *mc_ins_list = NULL;

int is_cdr_enabled=0;

#define is_acc_flag_set(_mask, _type, _flag) ( _mask & ((_type * _flag)))

#define is_log_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_LOG, _flag)
#define is_log_acc_on(_mask) is_log_flag_on(_mask, DO_ACC)
#define is_log_cdr_on(_mask) is_log_flag_on(_mask, DO_ACC_CDR)
#define is_log_mc_on(_mask) is_log_flag_on(_mask, DO_ACC_MISSED)
#define is_log_failed_on(_mask) is_log_flag_on(_mask, DO_ACC_FAILED)

#define is_aaa_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_AAA, _flag)
#define is_aaa_acc_on(_mask) is_aaa_flag_on(_mask, DO_ACC)
#define is_aaa_cdr_on(_mask) is_aaa_flag_on(_mask, DO_ACC_CDR)
#define is_aaa_mc_on(_mask) is_aaa_flag_on(_mask, DO_ACC_MISSED)
#define is_aaa_failed_on(_mask) is_aaa_flag_on(_mask, DO_ACC_FAILED)

#define is_db_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_DB, _flag)
#define is_db_acc_on(_mask) is_db_flag_on(_mask, DO_ACC)
#define is_db_cdr_on(_mask) is_db_flag_on(_mask, DO_ACC_CDR)
#define is_db_mc_on(_mask) is_db_flag_on(_mask, DO_ACC_MISSED)
#define is_db_failed_on(_mask) is_db_flag_on(_mask, DO_ACC_FAILED)

#define is_evi_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_EVI, _flag)
#define is_evi_acc_on(_mask) is_evi_flag_on(_mask, DO_ACC)
#define is_evi_cdr_on(_mask) is_evi_flag_on(_mask, DO_ACC_CDR)
#define is_evi_mc_on(_mask) is_evi_flag_on(_mask, DO_ACC_MISSED)
#define is_evi_failed_on(_mask) is_evi_flag_on(_mask, DO_ACC_FAILED)


#define is_acc_on(_mask) \
( (is_log_acc_on(_mask)) || (is_db_acc_on(_mask)) \
|| (is_aaa_acc_on(_mask)) || (is_evi_acc_on(_mask)) )
Expand Down Expand Up @@ -1254,7 +1227,7 @@ int w_do_acc(struct sip_msg* msg, unsigned long long *type,
return 1;
}

flag_mask = *type + *type * (flags ? *flags : 0);
flag_mask = acc_bitmask_set(*type, flags);
if (is_cdr_acc_on(flag_mask)) {
/* setting this flag will allow us to register everything
* that is needed for CDR accounting only once */
Expand Down Expand Up @@ -1381,22 +1354,15 @@ int w_do_acc(struct sip_msg* msg, unsigned long long *type,
int w_drop_acc(struct sip_msg* msg, unsigned long long *types,
unsigned long long *flags)
{
unsigned long long flag_mask, _types, _flags;

acc_ctx_t* acc_ctx=try_fetch_ctx();
acc_ctx_t *acc_ctx = try_fetch_ctx();

if (acc_ctx == NULL) {
if (!acc_ctx) {
LM_ERR("do_accounting() not used! This function resets flags in "
"do_accounting()!\n");
return -1;
}

_types = (types ? *types : DO_ACC_LOG|DO_ACC_AAA|DO_ACC_DB|DO_ACC_EVI);
_flags = (flags ? *flags : ALL_ACC_FLAGS);

flag_mask = _types + _types * _flags;
reset_flags(acc_ctx->flags, flag_mask);

acc_ctx->flags = acc_bitmask_reset(types, flags, acc_ctx->flags);
return 1;
}

Expand Down
67 changes: 62 additions & 5 deletions modules/acc/acc_logic.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

#include "../../str.h"
#include "../../context.h"
#include "../../pvar.h"
#include "../../evi/event_interface.h"
#include "../tm/t_hooks.h"
#include "../dialog/dlg_cb.h"

Expand All @@ -40,11 +42,12 @@
#define DO_ACC_EVI (1ULL<<(4*8))
#define DO_ACC_ERR ((unsigned long long)-1)

#define DO_ACC (1<<0) /* generic accouting flag - internal only */
#define DO_ACC_CDR (1<<1)
#define DO_ACC_MISSED (1<<2)
#define DO_ACC_FAILED (1<<3)
#define ALL_ACC_FLAGS (DO_ACC|DO_ACC_CDR|DO_ACC_MISSED|DO_ACC_FAILED)
#define DO_ACC (1ULL<<0) /* generic accounting flag - internal only */
#define DO_ACC_CDR (1ULL<<1)
#define DO_ACC_MISSED (1ULL<<2)
#define DO_ACC_FAILED (1ULL<<3)
#define DO_ACC_FLAGS (DO_ACC_CDR|DO_ACC_MISSED|DO_ACC_FAILED)
#define ALL_ACC_FLAGS (DO_ACC|DO_ACC_FLAGS)

#define DO_ACC_PARAM_TYPE_PV (1<<0)
#define DO_ACC_PARAM_TYPE_VALUE (1<<1)
Expand Down Expand Up @@ -76,7 +79,31 @@

#define ACC_MASK_REF_BYTE (((unsigned long long)(0xFF)<<(8*7))

#define is_acc_flag_set(_mask, _type, _flag) ( _mask & ((_type * _flag)))

#define is_log_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_LOG, _flag)
#define is_log_acc_on(_mask) is_log_flag_on(_mask, DO_ACC)
#define is_log_cdr_on(_mask) is_log_flag_on(_mask, DO_ACC_CDR)
#define is_log_mc_on(_mask) is_log_flag_on(_mask, DO_ACC_MISSED)
#define is_log_failed_on(_mask) is_log_flag_on(_mask, DO_ACC_FAILED)

#define is_aaa_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_AAA, _flag)
#define is_aaa_acc_on(_mask) is_aaa_flag_on(_mask, DO_ACC)
#define is_aaa_cdr_on(_mask) is_aaa_flag_on(_mask, DO_ACC_CDR)
#define is_aaa_mc_on(_mask) is_aaa_flag_on(_mask, DO_ACC_MISSED)
#define is_aaa_failed_on(_mask) is_aaa_flag_on(_mask, DO_ACC_FAILED)

#define is_db_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_DB, _flag)
#define is_db_acc_on(_mask) is_db_flag_on(_mask, DO_ACC)
#define is_db_cdr_on(_mask) is_db_flag_on(_mask, DO_ACC_CDR)
#define is_db_mc_on(_mask) is_db_flag_on(_mask, DO_ACC_MISSED)
#define is_db_failed_on(_mask) is_db_flag_on(_mask, DO_ACC_FAILED)

#define is_evi_flag_on(_mask, _flag) is_acc_flag_set(_mask, DO_ACC_EVI, _flag)
#define is_evi_acc_on(_mask) is_evi_flag_on(_mask, DO_ACC)
#define is_evi_cdr_on(_mask) is_evi_flag_on(_mask, DO_ACC_CDR)
#define is_evi_mc_on(_mask) is_evi_flag_on(_mask, DO_ACC_MISSED)
#define is_evi_failed_on(_mask) is_evi_flag_on(_mask, DO_ACC_FAILED)

#define DO_ACC_PARAM_DELIMITER '|'

Expand Down Expand Up @@ -210,4 +237,34 @@ void free_global_acc_ctx(acc_ctx_t* ctx);
void free_processing_acc_ctx(void* param);
void free_extra_array(extra_value_t* array, int array_len);

static inline unsigned long long acc_bitmask_set(
unsigned long long types, unsigned long long *flags)
{
return types + types * (flags ? *flags : 0);
}

static inline unsigned long long acc_bitmask_reset(
unsigned long long *types, unsigned long long *flags,
unsigned long long crt_mask)
{
unsigned long long rflag_mask, _types, _flags;

_types = (types ? *types : DO_ACC_LOG|DO_ACC_AAA|DO_ACC_DB|DO_ACC_EVI);
_flags = (flags ? *flags : ALL_ACC_FLAGS);

rflag_mask = _types * _flags;
crt_mask &= ~rflag_mask;

/* if there are no more flags for a backend,
* drop the "backend bit" as well */
rflag_mask =
DO_ACC_LOG * !(DO_ACC_LOG*DO_ACC_FLAGS & crt_mask) |
DO_ACC_AAA * !(DO_ACC_AAA*DO_ACC_FLAGS & crt_mask) |
DO_ACC_DB * !(DO_ACC_DB*DO_ACC_FLAGS & crt_mask) |
DO_ACC_EVI * !(DO_ACC_EVI*DO_ACC_FLAGS & crt_mask);
crt_mask &= ~rflag_mask;

return crt_mask;
}

#endif
34 changes: 34 additions & 0 deletions modules/acc/test/opensips.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
log_level = 2
stderror_enabled=yes
syslog_enabled=no

#memdump = 2
udp_workers = 4

auto_aliases = no
enable_asserts = true
abort_on_assert = true
max_while_loops = 10000

socket = udp:localhost:5059

####### Modules Section ########

mpath = "modules/"

loadmodule "proto_udp.so"
loadmodule "mi_fifo.so"

################################

loadmodule "tm.so"
modparam("tm", "auto_100trying", 0)

loadmodule "signaling.so"
loadmodule "acc.so"

loadmodule "usrloc.so"

route {
exit;
}
169 changes: 169 additions & 0 deletions modules/acc/test/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/*
* Copyright (C) 2023 OpenSIPS Solutions
*
* This file is part of opensips, a free SIP server.
*
* opensips is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version
*
* opensips is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/

#include <tap.h>

#include "../../../dprint.h"

#include "../acc_logic.h"


static void test_acc_flags(void)
{
unsigned long long types, flags, mask;
int t = 1;

/* 1. Single backend, no flags */
mask = acc_bitmask_set(DO_ACC_LOG, NULL);
ok(is_log_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_aaa_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_db_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_evi_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_failed_on(mask), "test-acc-flags-%d", t++);


/* 2. ... which we reset: */
types = DO_ACC_LOG;
mask = acc_bitmask_reset(&types, NULL, mask);
ok(!is_log_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_failed_on(mask), "test-acc-flags-%d", t++);


/* 3. Multi-backends, multi-flags */
flags = DO_ACC_CDR|DO_ACC_MISSED;
mask = acc_bitmask_set(DO_ACC_LOG|DO_ACC_DB, &flags);
ok(is_log_acc_on(mask), "test-acc-flags-%d", t++);
ok(is_log_cdr_on(mask), "test-acc-flags-%d", t++);
ok(is_log_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_aaa_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_failed_on(mask), "test-acc-flags-%d", t++);

ok(is_db_acc_on(mask), "test-acc-flags-%d", t++);
ok(is_db_cdr_on(mask), "test-acc-flags-%d", t++);
ok(is_db_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_evi_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_failed_on(mask), "test-acc-flags-%d", t++);


/* 4. reset 1 x flag on 1 x backend */
types = DO_ACC_DB;
flags = DO_ACC_MISSED;
mask = acc_bitmask_reset(&types, &flags, mask);
ok(is_log_acc_on(mask), "test-acc-flags-%d", t++);
ok(is_log_cdr_on(mask), "test-acc-flags-%d", t++);
ok(is_log_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_aaa_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_failed_on(mask), "test-acc-flags-%d", t++);

ok(is_db_acc_on(mask), "test-acc-flags-%d", t++);
ok(is_db_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_evi_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_failed_on(mask), "test-acc-flags-%d", t++);


/* 5. resetting the last flag on a backend resets acc too */
types = DO_ACC_DB;
flags = DO_ACC_CDR;
mask = acc_bitmask_reset(&types, &flags, mask);
ok(is_log_acc_on(mask), "test-acc-flags-%d", t++);
ok(is_log_cdr_on(mask), "test-acc-flags-%d", t++);
ok(is_log_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_aaa_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_db_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_evi_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_failed_on(mask), "test-acc-flags-%d", t++);


/* 6. similar reset, except with multiple flags */
types = DO_ACC_LOG;
flags = DO_ACC_CDR|DO_ACC_MISSED;
mask = acc_bitmask_reset(&types, &flags, mask);
ok(!is_log_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_log_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_aaa_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_aaa_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_db_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_db_failed_on(mask), "test-acc-flags-%d", t++);

ok(!is_evi_acc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_cdr_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_mc_on(mask), "test-acc-flags-%d", t++);
ok(!is_evi_failed_on(mask), "test-acc-flags-%d", t++);

}


void mod_tests(void)
{
test_acc_flags();
}

0 comments on commit e771060

Please sign in to comment.