On Tue, Oct 03, 2017 at 08:49:04AM +0800, Harald Welte wrote:
* CTRL uses '.' to separate individual
elements/nodes of the command
* a lot of rate_ctr groups we use so far use '.' in their group names
(e.g. 'bssgp.bss_ctx') and counter names (e.g. 'tx.bytes')
* This makes the counters un-exportable via CTRL
I'm thinking, we could make the current rate_ctr_group_alloc() replace '.'
with
':' automatically to fix all older and current builds' CTRL for rate_ctr. At
the same time we can deprecate rarate_ctr_group_alloc() and provide
rate_ctr_group_alloc2(), which then aborts the program when rate_ctr names
don't adhere to strict identifier rules. Thus we get a compile-time warning for
applications that haven't reviewed correctness of rate_ctr naming
("rate_ctr_group_alloc() is deprecated...") and once an application has moved
on we make sure no new non-compliant names are introduced. However, newly
introduced names will get checked only during run time.
Another addition could be a script like
verify_value_string_arrays_are_terminated.py but for struct rate_ctr_group_desc
arrays' identifier adherence, to verify even before compilation.
Should we further restrict the CTRL interface strings
(and those of
systems exporting to CTRL) to standard US 7-bit ASCII with a limited set
of special characters such as ":-_@" but prevent any non-printable chars
For identifier validation, we should probably include that identifiers must not
start with a number.
I think we could also include ",~+^"
as pseudocode:
valid = isalpha(str[0]) && each(str[1..len] as c){ isalnum(c) || c in
":-_@,~+^" }
Losely related is the OsmoHLR idea for CTRL interface to use names like
SET subscriber.by-imsi.1234567898765.ps-enabled 1
where a CTRL identifier name is used for inputting an IMSI string. If we do the
same identifier checking during incoming CTRL interface commands, we would
forbid the IMSI name because it starts with a number.
Thinking now that we could go for something like
subscriber.by-imsi=1234567898765.ps-enabled 1
or maybe rather
subscriber.by-imsi-1234567898765.ps-enabled 1
I would have liked to use existing CTRL parsing to separate out the IMSI number
without added char parsing... but it's not too hard to add a ctrl_cmd_lookup()
that checks starts_with('by-imsi-') and then takes &str[8].
~N