| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
Fixes crashes when using vty `show fsm-instances all`
when fsm children are present.
Change-Id: I4ae0bee7f7f57ec6675cfb52ca6cf0d523d15362
|
|
|
|
| |
Change-Id: I7e66432f37e13fd4c31389e3d89593fa0981e58f
|
|
|
|
| |
Change-Id: I396fd1e7548beea31b2b7aa9f764edb765b02941
|
|
|
|
|
|
|
|
|
| |
Callers require to know whether the returned ERROR cmd was received or
generated locally, in order to send it or do something with it locally.
Related: OS#3394
Change-Id: Ide9170e5c31967c353f8fe4e8227e64130b91eae
|
|
|
|
|
|
|
|
|
|
|
| |
Our implementation generates ERROR CTRL messages with ID=error when it
is unable to parse a CMD. However, it doesn't account for them when
trying to receive and parse this kind of message. As a result, it will
return an ERROR message with a different description. This commit fixes
the old behaviour to success at parsing and returning the received
description to the caller.
Change-Id: I564ab1a7e845388f87accda44fbf165e5adc2480
|
|
|
|
| |
Change-Id: Ia890d4b841ef02342cc1cf7f5926866b040dc8ab
|
|
|
|
|
| |
Closes: OS#3293
Change-Id: I8dc2f24d4bf557ff7bb0f2f46881f9f8d9d7f86f
|
|
|
|
|
|
|
|
| |
There are some symbols for use between control_cmd.c and control_if.c,
which are not supposed to be exported publicly. Let's make sure we
keep those symbols local.
Change-Id: Ia85f36a9c4b2ebf4003718e0a230959638370320
|
|
|
|
| |
Change-Id: I24666d0b90a355e9fdefd280d48900b8cac1de64
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Imagine following scenario:
1- client connects to CTRL iface, a new conn is created with POLL_READ
enabled.
2- A non-related event happens which triggers a TRAP to be sent. As a
result, the wqueue for the conn has now enabled POLL_WRITE, and message
will be sent next time we go through osmo_main_select().
3- At the same time, we receive the GET cmd from the CTRL client, which
means POLL_READ event will be also triggered next time we call
osmo_main_select().
4- osmo_main_select triggers osmo_wqueue_bfd_cb with both READ/WRITE
flags set.
5- The read_cb of wqueue is executed first. The handler closes the CTRL
conn for some reason, freeing the osmo_fd struct and returns.
6- osmo_qeueue_bfd_cb keeps using the already freed osmo_fd and calls
write_cb.
So in step 6 we get a heap-use-after-free catched by AddressSanitizer:
[0;m20180424135406115 [1;32mDLCTRL[0;m <0018> control_if.c:506 accept()ed new CTRL connection from (r=10.42.42.1:53910<->l=10.42.42.7:4249)
[0;m20180424135406116 [1;34mDLCTRL[0;m <0018> control_cmd.c:378 Command: GET bts.0.oml-connection-state
[0;m20180424135406117 [1;34mDLINP[0;m <0013> bts_ipaccess_nanobts.c:417 Identified BTS 1/0/0
[0;m[1;36m20180424135406118 [1;34mDNM[0;m[1;36m <0005> abis_nm.c:1628 Get Attr (bts=0)
[0;m[1;36m20180424135406118 [1;34mDNM[0;m[1;36m <0005> abis_nm.c:1628 Get Attr (bts=0)
[0;m20180424135406118 [1;34mDCTRL[0;m <000e> osmo_bsc_ctrl.c:158 BTS connection (re)established, sending TRAP.
[0;m20180424135406119 [1;32mDLCTRL[0;m <0018> control_if.c:173 close()d CTRL connection (r=10.42.42.1:53910<->l=10.42.42.7:4249)
[0;m=================================================================
==12301==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000003e04 at pc 0x7f23091c3a2f bp 0x7ffc0cb73ff0 sp 0x7ffc0cb73fe8
READ of size 4 at 0x611000003e04 thread T0
#0 0x7f23091c3a2e in osmo_wqueue_bfd_cb /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/libosmocore/src/write_queue.c:65
#1 0x7f23091ad5d8 in osmo_fd_disp_fds /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/libosmocore/src/select.c:216
#2 0x7f23091ad5d8 in osmo_select_main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/libosmocore/src/select.c:256
#3 0x56538bdb7a26 in main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bsc/osmo-bsc/src/osmo-bsc/osmo_bsc_main.c:532
#4 0x7f23077532e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#5 0x56538bdb8999 in _start (/home/jenkins/workspace/osmo-gsm-tester_run-prod/trial-896/inst/osmo-bsc/bin/osmo-bsc+0x259999)
Fixes: OS#3206
Change-Id: I84d10caaadcfa6bd46ba8756ca89aa0badcfd2e3
|
|
|
|
|
|
|
| |
Remark: For libosmogb and libosmogsm, LIBVERSION was
already bumped in c4fce1425e19d604c199c895e227dc2519110456.
Change-Id: Ib4fa53a9bb9954ae48d0a610ba9a81dd8e8b4ef6
|
|
|
|
| |
Change-Id: I3a711f5c974b7f56e27b333d390d1a706fb57007
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The CTRL interface has a ctrl_cmd_def_* API that allows deferring a CTRL
command reply until later. However, the command handling currently fails to
acknowledge this and deallocates the struct ctrl_cmd anyway.
Fix: in struct ctrl_cmd, add a defer pointer to be populated by
ctrl_cmd_def_make(). A cmd thus marked as deferred is not deallocated at the
end of command handling. This fix needs no change in calling code.
(Another idea was to return a different code than CTRL_CMD_HANDLED when the
command is to be deferred, but that would require adjusting each user of
ctrl_cmd_def_make(). The implicit marking is safer and easier.)
Show that handling deferred commands is fixed by adjusting the expectations of
ctrl_test.c's test_deferred_cmd() and removing the now obsolete exit_early
label.
One symptom of the breakage is that osmo-bts-sysmo crashes when asked to report
a trx's clock-info, which is aggravated by the fact that the sysmobts-mgr does
ask osmo-bts-sysmo for a clock-info.
The crash appears since Id583b413f8b8bd16e5cf92a8a9e8663903646381 -- it looked
like just fixing an obvious memory leak, which it did as shown by the unit
test, but deferred ctrl commands actually relied on that leak. Both fixed now.
Related: OS#3120
Change-Id: I24232be7dcf7be79f4def91ddc8b8f8005b56318
|
|
|
|
|
|
|
|
|
| |
Now that we use osmo_sock_get_name() to print connection information
at disconnect, let's use the same also at accept() time.
Furthermore, let's call it CTRL connection everywhere for consistency.
Change-Id: I33ee7d0ed853c5b2a4ae4e8ef945f8f27753cdea
|
|
|
|
|
|
|
| |
We are logging new CTRL connections at LOGL_INFO, so we should
also log disconnects for symmetry.
Change-Id: Id30aa76a5a3dab32d6b4121ce6fdf56d71dfc2ba
|
|
|
|
|
|
|
|
|
|
|
| |
When read() or write() system calls return '0' on a stream socket,
it means that the connection has been closed ("EOF"). We must
accordingly close this socket and remove all related state.
Before this patch, every new CTRL connection would introduce a leak
of both some memory/state, as well as a file descriptor :(
Change-Id: I4fb70e5f123b37dece29f156c5f430c875e7cbaf
|
|
|
|
|
|
|
| |
So far, error reporting just says "Trap/Reply", more accurately report 'GET
REPLY', 'SET REPLY' and 'TRAP' as appropriate.
Change-Id: Ic25a251502499aeda4e2952ec4190a1fa0bebb01
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Validate that incoming CTRL commands...
- have decimal IDs,
- return error on trailing characters,
- have invalid characters in variable identifiers,
- send detailed error messages as reply to the requestor.
Adjust ctrl_test.{c,ok}, which best show the change in behavior.
Message handling causes log messages on stderr; previously, stderr was empty.
Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test
failures.
Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1
|
|
|
|
|
|
|
|
|
|
|
| |
The recently added ctrl_cmd_parse2() returns non-NULL cmd with error messages
upon parsing errors. In handle_control_read(), use ctrl_cmd_parse2() and send
those back to the CTRL command sender as reply.
Retain the previous "Command parser error" reply only in case ctrl_cmd_parse2()
should return NULL, which shouldn't actually happen at all.
Change-Id: Ie35a02555b76913bb12734a76fc40fde7ffb244d
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If a control command fails to parse, we so far discard specific error messages
and instead send just "Command parser error".
In ctrl_cmd_parse() we actually compose detailed error replies, but in the end
simply talloc_free() them and return NULL.
A first step to report these errors to the ctrl command issuer is to not return
NULL and instead return the cmd with type = CTRL_TYPE_ERROR. Add
ctrl_cmd_parse2() to return such instead of NULL.
To stay API compatible, provide ctrl_cmd_parse2() to return a cmd on errors.
ctrl_cmd_parse() retains identical behavior but becomes just a simple wrapper
around ctrl_cmd_parse2() which discards the cmd on error.
No need really to deprecate ctrl_cmd_parse() yet; especially as long as
compiler warnings might break jenkins builds.
Change-Id: I5047c9f977d70b03eea77cbcfd2b96d43ea46880
|
|
|
|
|
|
|
|
|
|
|
|
| |
In ctrl_handle_msg() (code recently propagated from handle_control_read()),
talloc_free() the parsed ctrl_cmd in all code paths. In particular, a free was
missing in case ctrl_cmd_handle() returns CTRL_CMD_HANDLED.
CTRL_CMD_HANDLED is triggered by GET_REPLY / SET_REPLY parsing, as show by
ctrl_test.c. With the memleak fixed, adjust expected test output and make a
detected mem leak abort the test immediately.
Change-Id: Id583b413f8b8bd16e5cf92a8a9e8663903646381
|
|
|
|
|
|
|
|
|
|
| |
In order to allow unit testing the ctrl iface msgb handling, have a separate
msgb entry point function from the actual fd read function.
An upcoming patch will prove a memory leak in CTRL msgb handling by a unit test
that needs this separation.
Change-Id: Ie09e39db668b866eeb80399b82e7b04b8f5ad7c3
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously ctrl request for all counters in
group (e. g. 'rate_ctr.abs.msc.0') will result in human-readable
description which is not regular enough and is hard to both parse and
generate. The ctrl interface is intended for m2m, not for human
interaction. Let's simplify things by making response similar to counter
group request ('rate_ctr.*').
Reply now looks as follows:
GET_REPLY 9084354783926137287 rate_ctr.abs.msc.0 loc_update_type:attach 0;loc_update_type:normal 0;
Previously it was:
GET_REPLY 9084354783926137287 rate_ctr.abs.msc.0 All counters in msc.0
loc_update_type:attach 0
loc_update_type:normal 0
Change-Id: I7a24cc307450efdcd28168fffe477320c59fcd36
Related: OS#2550
|
|
|
|
|
|
|
| |
Report back expected interval values.
Change-Id: I05ca7f716342af4f7424b28216ed6c1bf2bd589f
Related: OS#2550
|
|
|
|
|
|
|
|
| |
This should never happen with the current code, but if it ever does, we
should log the error instead of silently returning 0.
Change-Id: I544001d3072e5f12a96a67e4178f9b945c5f6b6c
Related: OS#2550
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before user have to know group name and index in advance to request rate
counter value. Introduce introspection function which allows user to
obtain all the groups and their indexes by requesting 'rate_ctr.*'
variable.
This simplifies KPI dumping over ctrl interface.
Change-Id: Ifad8b4f0360c8bcd123a838676516476e84c246a
Related: OS#2550
|
|
|
|
|
|
|
|
| |
Let's fix some erroneous/accidential references to wrong license,
update copyright information where applicable and introduce a
SPDX-License-Identifier to all files.
Change-Id: I39af26c6aaaf5c926966391f6565fc5936be21af
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
According to
https://www.gnu.org/software/automake/manual/automake.html#Libtool-Flags
the libraries supposed to be added to *_LDADD or *_LIBADD
while *_LDFLAGS should contain additional libtool linking
flags. Previously we used both. Let's unify this and move all the
libraries into proper automake variable. While at it - also add
libosmocore.la for tests to LDADD since all the tests link against it
anyway.
Change-Id: Ia657a66db75df831421af5df1175a992da5ba80f
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It's been way too long since the last release. Almost one year and
468 commits.
A brief summary of the changes below:
* Doxygen for libosmo{coding,gb}
* pseudotalloc for embedded builds, jenkins for arm-none-gnueabi
* --disable-doxygen, --disable-ctrl, --disable-simd
* update debian packaging
* gsm0503 coding routines
* osmo_hton[sl]
* statistics.h -> counter.h
* QCDIAG in gsmtap
* llist_{first,last}_entry()
* llist_count()
* LOGPSRC() macro
* msgb_pull_to_l2()
* msgb_printf()
* prbs
* osmo_sock_init2()
* osmo_sock_mcast_{name,loop_set,ttl_set,all_set,subscribe,ip}()
* OSMO_STRINGIFY()
* OSMO_VALUE_STRING()
* OSMO_BYTES_FOR_BITS()
* osmo_talloc_asprintf()
* osmo_sub_auth_type_name()
* osmo_sub_auth_data support for IND/SQN_MS
* osmo_fsm ctrl interface
* ctrl_handle_alloc2()
* ctrl_interface_setup_dynip2()
* OSMO_CTRL_PORT_HLR
* bssgp_tx_bvc_ptp_reset()
* gprs_ns_inst connect/remote_{ip,port}
* osmo_gprs_{ul,dl}_block_size_{bits,bytes}()
* osmo_gprs_{dl,ul}_cs_by_block_bytes()
* gprs_ns_pdu_strings[]
* more BSSGP cause values
* abis_nm_admin_name()
* AoIP support in gsm0808
* gsm_fn_as_gsmtime_str()
* osmo_dump_gsmtime()
* gsup charging support
* ipa_ccm_make_id_resp()
* ipa_ccm_make_id_resp_from_req()
* struct gsm48_gprs_susp_req
* gsm_04_14.h
* rsl measurement preprocessing related IEs
* abis_nm_event_cause_names[]
* abis_nm_sw_desc and friends
* more SYSINFO_TYPE_ values
* osmo_earfcn_bit_size_ext()
* t16lv_put()
* msgb_t16lv_put()
* tlvp_val16be()
* tlvp_val32be()
* osmo_tlvp_copy()
* osmo_tlvp_merge()
* many additional VTY nodes
* cmd_node.name member
* bitvec_set_u64()
* bitvec_rl_curbit
* ctrl_lookup_register()
* osmo_fsm_find_by_name()
* osmo_fsm_inst_find_by_name()
* osmo_fsm_inst_find_by_id()
Change-Id: Ieb5db2e910a90db780ea058b3280f2facbd68d76
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add ctrl_interface_setup_dynip2() to add a node_count parameter, which can be
used to define more ctrl nodes without having to merge a patch to libosmocore.
In consequence, also add ctrl_handle_alloc2(), since
ctrl_interface_setup_dynip() uses ctrl_handle_alloc() to allocate the node
slots, and add node_count param to static ctrl_init().
Passing zero as node_count indicates to use the default of _LAST_CTRL_NODE as
before, i.e. to not define more ctrl nodes. Assert that we never allocate less
than _LAST_CTRL_NODE slots.
The current ctrl_interface_setup_dynip() and ctrl_handle_alloc() become simple
wrappers that pass zero as node_count. Their use is still valid and they do not
need to be deprecated.
The API comment to ctrl_interface_setup_dynip2() explains how to define more
node IDs.
This patch was verified to work by osmo-hlr.git change
I98ee6a06b3aa6a67adb868e0b63b0e04eb42eb50 which adds two node IDs for use by
osmo-hlr only.
Change-Id: I1bd62ae0d4eefde7e1517db15a2155640a1bab58
|
|
|
|
|
|
|
|
|
| |
With stat_item, stats.c and stats_statsd.c, it is becoming a bit
difficult to understand file naming. Also, the 'statistics.c' file
actually only contained osmo_counter handling, so let's rename it to
counter.c altogether.
Change-Id: I2cfb2310543902b7da46cb15a76e2da317eaed7d
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In many callers of the VTY API, we are lacking the vty_install_default() step
at certain node levels. This creates nodes that lack the 'exit' command, and
hence the only way to exit such a node is to restart the telnet session.
Historically, the VTY looked for missing commands on the immediate parent node,
and hence possibly found the parent's 'exit' command when the local node was
missing it. That is why we so far did not notice the missing default commands.
Furthermore, some callers call install_default() instead of
vty_install_default(). Only vty_install_default() also includes the 'exit' and
'end' commands. There is no reason why there are two sets of default commands.
To end this confusion, to catch all missing 'exit' commands and to prevent this
from re-appearing in the future, simply *always* install all default commands
implicitly when calling install_node().
In cmd_init(), there are some top-level nodes that apparently do not want the
default commands installed. Keep those the way they are, by changing the
invocation to new install_node_bare() ({VIEW,AUTH,AUTH_ENABLE}_NODE).
Make both install_default() and vty_install_default() no-ops so that users of
the API may still call them without harm. Do not yet deprecate yet, which
follows in Icf5d83f641e838cebcccc635a043e94ba352abff.
Drop all invocations to these two functions found in libosmocore.
Change-Id: I5021c64a787b63314e0f2f1cba0b8fc7bff4f09b
|
|
|
|
| |
Change-Id: I72569ed767b6a4d792b2867d02caa65520e27cd3
|
|
|
|
|
|
|
|
| |
Fixes following warning:
control_cmd.c:294:16: warning: comparison of unsigned enum expression < 0 is always false [-Wtautological-compare]
if (cmd->type < 0 || cmd->type == CTRL_TYPE_UNKNOWN) {
Change-Id: I3df8a4f646222337927d9e3cac6d09a8a05cb20c
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Considering the various styles and implications found in the sources, edit
scores of files to follow the same API doc guidelines around the doxygen
grouping and the \file tag.
Many files now show a short description in the generated API doc that was so
far only available as C comment.
The guidelines and reasoning behind it is documented at
https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_API_documentation
In some instances, remove file comments and add to the corresponding group
instead, to be shared among several files (e.g. bitvec).
Change-Id: Ifa70e77e90462b5eb2b0457c70fd25275910c72b
|
|
|
|
|
|
|
|
|
|
| |
Especially for short descriptions, it is annoying to have to type \brief for
every single API doc.
Drop all \brief and enable the AUTOBRIEF feature of doxygen, which always takes
the first sentence of an API doc as the brief description.
Change-Id: I11a8a821b065a128108641a2a63fb5a2b1916e87
|
|
|
|
|
|
|
| |
Replace if-else ladder & gotos with single switch statement & explicit
return to make reading code easier.
Change-Id: Ida1b389b571c60c26813cd29e61b3e4423c5df0f
|
|
|
|
|
|
|
|
|
|
|
| |
Recent changes to libosmoctrl resulted in ctrl comands being broken
because local lookup helper returned incorrect value for ROOT_NODE.
Note: although this commit seems to fix it for me, I'm still not sure
how the logic behind lookup function return values work. Would be nice
to get it documented.
Change-Id: Iddd20602047ebd9be1b668593f5dfa6f1d3e8369
|
|
|
|
|
|
| |
We have a proper constant for this (_LAST_CTRL_NODE), so let's use it.
Change-Id: I46275e644166156cb665da70d2964008f1c6cd88
|
|
|
|
|
|
|
|
|
| |
This allows programmatic access to introspection of FSM instances, which
is quite handy from e.g. external test cases: Send a message to the
code, then use the CTRL interface to check if that message has triggered
the right kind of state transition.
Change-Id: I0f80340ee9c61c88962fdd6764a6098a844d0d1e
|
|
|
|
|
|
|
|
| |
Sometimes (particularly when testing), we may want to parse+execute an
arbitrary control command simply form a string buffer, rather than from
a msgb. Let's add a helper for that.
Change-Id: Iaca748e0d942bb2a1ee7c2776b37485e1439eb0c
|
|
|
|
|
|
|
| |
When executing test cases, we don't want to bind to a local TCP port, as
we cannot make assumptions as to which ports are actually free.
Change-Id: I5717f9dd92d1f143f069cecd4b4c8ba3d03b25f8
|
|
|
|
|
|
|
|
|
|
|
|
| |
The existing code assumes that the main application knows about all
control command nodes and can thus present one lookup function.
As libraries are getting their own control interface handling, this
is too restrictive, and we need a way how library code can dynamically
register more node lookup helpers. We can now do this by means of a
ctrl_lookup_register() function.
Change-Id: Ib69908d1c57f5bb721d5496e3b4a5258fca450e3
|
|
|
|
|
|
| |
sections: ctrl, gb, gsm, vty
Change-Id: Iac211b5cd8504da36b699777b95a2448dd7c3e70
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Don't use CTRL_TYPE_UNKNOWN as value_string[] terminator, use an explicit, more
obvious { 0, NULL } termination. Set an explicit string for CTRL_TYPE_UNKNOWN.
No other value_string[]s to date have such a "hidden" terminator.
BTW, a { 0, "string" } item is not a terminator, only { 0, NULL } is, so we can
set a string for CTRL_TYPE_UNKNOWN == 0.
Also, having a string value for CTRL_TYPE_UNKNOWN is not harmful because all
code paths explicitly check for the CTRL_TYPE_*s that are valid.
Adjust the test expectation.
From the ctrl_type_vals enum, remove the = 0, because it is implicitly 0
anyway.
One motivation to press this fixup: I am trying to add a script that checks
whether all value_string[]s are terminated to our jenkins jobs, and to find
that this one is terminated, it would need to interpret the CTRL_TYPE_UNKNOWN
constant, which would make things far more complex. At this point, all of the
value_string[]s have an explicit termination, and I would like to enforce this
from now on -- for readable code and to not spend more time on the validator.
The patch adding ctrl_type_vals (Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28) was
accepted by another reviewer before I could reconfirm my -1, so this is a fixup
to enable the termination checking script patches.
Related: I2bc93ab4781487e7685cfb63091a489cd126b1a8 (adds script to libosmocore)
I7fe3678b524d602fc6aa14bc0ed06308df809a3e (uses in jenkins.sh)
Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28 (adds ctrl_type_vals)
Change-Id: Ia99f37464c7b36b587da2cc78f52c82725f02cbc
|