Project

General

Profile

Bug #2362

rate_ctr with name prefix containing "." not visible on CTRL interface

Added by laforge 5 months ago. Updated 29 days ago.

Status:
Closed
Priority:
High
Assignee:
Target version:
-
Start date:
07/11/2017
Due date:
% Done:

100%

Spec Reference:

Description

As the CTRL interface is usinh "." as a separator between tokens, we cannot use "." inside a rate counter name prefix. Doing so renders the related counter group inaccessible from the CTRL interface.

Unfortunately, we quite frequently use "." inside a counter group prefix :/


Related issues

Related to Cellular Infrastructure - Bug #2361: failure of rate_ctr_group_alloc() not checked in most callers Closed 07/11/2017
Related to Cellular Infrastructure - Support #2360: Read CDR via CTRL interface in SGSN New 07/11/2017

History

#1 Updated by laforge 5 months ago

  • Status changed from New to In Progress

#2 Updated by laforge 5 months ago

  • Related to Bug #2361: failure of rate_ctr_group_alloc() not checked in most callers added

#3 Updated by laforge 5 months ago

so the question is what do we do here. Do we

a) rewrite all counter group prefixes to not contain '.' and refuse the allocation of counter groups with invalid names (those containing '.')?

or

b) silently translate a different symbol (e.g. '/') during lookup, so a look-up by name 'sgsn/foo' will automagically match a counter registered with 'sgsn.foo'?

I personally perfer 'a' as it is obvious and transparent. Particularly as we generate counter related documentation from the source code, it would be easy to create hard to explain inconsistencies.

#4 Updated by laforge 4 months ago

  • Related to Support #2360: Read CDR via CTRL interface in SGSN added

#5 Updated by laforge about 2 months ago

  • % Done changed from 0 to 20

I have a libosmocore patch that verifies the validity of counter names
(to not contain spaces or dots) at registration time. This way we can
catch any application with erroneous behaviour.

However, this causes the following problem: New libosmocore versions
will make old apps crash or at least fail to have propre counters, as
their names are wrong :(

Also, I don't like to replace all '.' in the counter with '_', as in
seen in the example of 'bssgp.bss_ctx' there is a semantic difference.
'bssgp.' is denoted to indicate the code module from which the counter
is, and 'bss_ctx' denotes the specific object whose counters we refer
to. Manually replacing this with 'bssgp_bss_ctx' is ugly.

One alternative would be to replace '.' with ':' or '/', which are not
characters with a special functoin in the CTRL syntax. This could be
even done automatically at counter registration time, simply replace all
occurrences of '.' with ':' (and log a warning as a reminder to fix the
code?).

I prefer that more, but the question is which characters should be
reserved for CTRL syntactic purpoess, and which are free to use by the
applications to name elements of the CTRL string/node.

Any input to this? I personally would go for ':'. As CTRL is very
loosely modelled after sysfs, ':' also occurs frequently in sysfs names
such as '/sys/bus/usb/devices/1-3:2.0'

So we'd keep the '.' as path/node separator, and we use ':' as separator
inside counter (and possibly other) names, which is opaque to CTRL.

Opinions?

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
or special chars like "{|}()~[]\^`'?<>=;/+*&%$#!"? I would support such
a motion. We could even make it more general and use that for all our
identifier strings and have a general validation function that all code
modules call whenever validating a string identifier used, such as e.g.
osmo_fsm names, etc.

Posted also to the mailinglist for further discussion/feedback:
http://lists.osmocom.org/pipermail/openbsc/2017-October/011263.html

#6 Updated by laforge about 2 months ago

  • % Done changed from 20 to 70

#7 Updated by laforge about 2 months ago

https://gerrit.osmocom.org/#/q/topic:ctrl-naming is a gerrit topic listing all related/proposed patches

#8 Updated by laforge 29 days ago

  • Status changed from In Progress to Closed
  • % Done changed from 70 to 100

merged to master now.

Also available in: Atom PDF