Both gsm0503_tch_a[fh]s_decode_dtx() functions accept an optional
'dtx' pointer, which is used to indicate type of a received AMR
block to the caller in DTX mode of operation. If not NULL, it's
expected to be updated by gsm0503_detect_a[fh]s_dtx_frame() every
time one of the mentioned functions is called.
However, in case of FACCH both functions return early, so the value
of dtx remains unchanged and thus FACCH frames may be misinterpreted
as AMR's special DTX frames. This is rather critical during the DTX
silence periods, when all special DTX frames (e.g. SID_UPDATE) are
being treated as SUB frames. Each unsuccessful FACCH decoding
attempt will 'poison' SUB measurements, causing unexpected RxQual-
SUB values in the Uplink measurement reports.
Fix this by resetting *dtx to AMR_OTHER in the FACCH specific path.
Change-Id: I2e6f4b748c6445725211e264ab5f3f5a2712087a
Related: SYS#5853
There are two similar values in enum gsm0503_amr_dtx_frames:
* AFS_SID_UPDATE - precursor of SID UPDATE,
* AFS_SID_UPDATE_CN - the actual SID UPDATE.
The former is internally used by libosmocoding to mark the current
frame as a precursor of the actual SID UPDATE frame - the later.
+---+---+---+---+---+---+---+---+
| _ | _ | _ | _ | a | b | c | d | AFS_SID_UPDATE
+---+---+---+---+---+---+---+---+
| a | b | c | d | _ | _ | _ | _ | AFS_SID_UPDATE_CN
+---+---+---+---+---+---+---+---+
This is required because function gsm0503_tch_afs_decode_dtx() is
invoked by TDMA scheduler on every 4th received burst, while the
burst buffer is 8 bursts wide.
Currently, whenever gsm0503_detect_afs_dtx_frame() detects an
AFS_SID_UPDATE frame, we still attempt to decode it as a speech
or data below in gsm0503_tch_afs_decode_dtx(). This is indeed
a bug, which results in unexpected BER values:
* expected BER 0/212,
* actual BER 252/448.
We should return immediately once we have detected an AFS_SID_UPDATE.
This patch fixes unexpected BER-SUB values during DTXu silence periods.
Change-Id: I813081a4c0865958eee2496fe251ae17235ac842
Related: SYS#5853
"""
/libosmocore/src/coding/gsm0503_coding.c: In function 'osmo_conv_decode_ber_punctured':
/libosmocore/src/coding/gsm0503_coding.c:563:31: error: 'coded_len' may be used uninitialized [-Werror=maybe-uninitialized]
563 | *n_bits_total = coded_len;
| ~~~~~~~~~~~~~~^~~~~~~~~~~
/libosmocore/src/coding/gsm0503_coding.c:541:21: note: 'coded_len' was declared here
541 | int res, i, coded_len;
| ^~~~~~~~~
"""
This error is really a false positive. However, it is true that the code
used to be a bit more complex than required, since the 2 later conditions
could be inside the first one.
Let's simply do early termination to simplify the function, and get rid
of the gcc warning.
Change-Id: I31ebf0c4be61daf6395d9a9fac05c7fdceb8bcb9
The point of having a public API to register further stats reporters
is to enable applications or other libraries to do so. As we in
libosmocore don't know anything about the parameters of such a stats
reporter, don't try to do a partial save of them when saving the config
file.
Change-Id: I2986313375daec1c4959a6a914e3fb2980a5d7ca
We should either handle talloc returning NULL, or we should
OSMO_ASSERT(). Doing neither of the two is a bad idea.
Change-Id: I5e8d1cc22cf597f7f50c0f92bf86cb1f1413434c
In many cases, a lot of the counters are zero, and we're likely
not interested in those, but only the non-zero counters. Add a version
of the 'show stats' command which dumps only those items with a non-zero
total value.
Change-Id: Ie4df1c139e3c82deca1dd3cdab5d3909e0513684
It was recently found that several IEs which were added in the header
file were not actually added to the tlv_definition, and hence the tlv
parser failed to decode them. Let's make sure we don't foget to add new
IEs in the future.
Related: SYS#5915
Change-Id: Id8a679ca43eb0fcc4882780e9a95ec21c7f51972
There are cases where we want to be notified of a successful BVC reset,
e.g. for a signalling because we can then start resetting the PtP-BVCs.
With this hook it's now possible to do that.
Change-Id: If240dd13f0f674693018c93390386b2c8afb97af
Related: SYS#5908
pthread_getname_np() is a non-portable extension of pthreads. While
it exists in glibc, for example musl didn't have it until rather
recently (April 2021) and there still hasn't yet been a musl release
with this change, resulting even current OpenWRT not yet supporting
pthread_getname_np.
So let's check if pthread_getname_np is supported, and only use it
in that case.
Change-Id: Ibd01485af24e2fe574006f8d049bf37226dda966
Ever since Change-Id If76a4bd2cc7b3c7adf5d84790a944d78be70e10a in 2020
(part of libosmocore >= 1.4.0) we have introduced cpu_sched_vty.c, which
directly uses libpthread. As a result, libosmovty should be using
pthread compiler flags and link against libpthread.
This missing dependency is causing osmocom applications to
fail to link on OpenWRT (at leats for ath79-generic).
Change-Id: I7febbf88cbe61eacd05f46a9316e773b5c148e77
Related: SYS#4986
According to TS 101 318, section 5.2.2, a SID frame is identified
by a SID codeword consisting of 79 bits (r34..r112) which are all 1.
Given that there are no gaps in the codeword, we don't really need
to use bitvec_get_bit_pos() and check each field individually.
This brings additional complexity and negatively affects performance.
Instead, let's use bitvec_get_bit_pos() to check all bits in a row.
Change-Id: I678f8ff92317fe87f1aca511a3a062ad898e55cc
Related: SYS#5853
Make sure that osmo_time_cc counts exactly the same, regardless of a
rate_ctr being present or not. Only skip sending counter increments when
there is no rate counter, do not affect anything else.
In osmo-bsc, we are discussing a patch where an lchan redirects
osmo_time_cc counter increments to different rate counters depending on
its current type. In my comments I am also claiming that osmo_time_cc
works the same regardless of a rate_ctr being present or not. Looking at
the code, this is not actually the case.
Before this patch, when there is no rate_ctr, the reported_sum would
freeze, and as soon as a rate_ctr shows up, all counter increments from
the previous reported_sum would be sent to the newly set rate_ctr.
Instead, we want counter increments to simply be lost while there is no
rate_ctr set. IOW, rate_ctr == NULL should mean >/dev/null.
Related: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347 (osmo-bsc)
Change-Id: I380b28d7ab0a6c8aab6c7e2a64bc73cab14863d2
If there's only a single device with matching VID/PID attached,
we don't need to insist that either the path or the address of the
device matches. Those are only needed to disambiguate multiple
devices with identical VID/PID.
Change-Id: I2ef245a56dfcf22758a0216b86d2a5c602ee5588
If an API user has defined a name for this particular
stat, we should consider it unique and not append ip and
port information from the connection.
By appending ip and port information to all tcp stat
names, we end up creating unique stat names every
time a reconnection occurs and the source port changes.
This makes the statistic impossible to track over time
as it is continually using a different name.
A quick example from the field over the course of a
day:
tcp.ipa-rsl-0,r=192.168.55.88.33056<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.33311<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.35510<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.35958<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.36110<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.39269<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.40394<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.40397<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.42920<->l=192.168.0.1.3003.tcp.rtt
tcp.ipa-rsl-0,r=192.168.55.88.43839<->l=192.168.0.1.3003.tcp.rtt
This change would treat tcp stats like other stats
around the system. A unique name must be set by the
API user. This would let us set a unique name like
the following to avoid the situation above:
bts.0.rsl.0.tcp.rtt
Matching the existing rsl related stats:
bts.0.rsl.delete_ind
bts.0.rsl.ipa_nack
bts.0.rsl.unknown
...they retain a constant name regardless of the underlying
connectivity situation.
Change-Id: Ib04c2f5bfcbd6c19dd87debf1fc053abf0b9bef2
Those are available in 3GPP TS 48.008 version 16.0.0 Release 16, section
3.2.2.17 Cell Identifier. It can be seen that we have a collision
between the osmocom non-standard format and the SAI standard one.
This is because CGI-PS is not really a TS 48.008 Cell Identifier, but only
specified in TS 48.018 and has no ID number assigned. The CGI-PS was
added there because the whole osmo-bsc neighbour configuration works
with CellIds to manage neighbours, so it felt natural to extend the APIs
to also provide means to use CGI-PS format (TS 48.018 even refers 48.008
existance and mentions there's no explicit ID).
At the time this Cell Identifier was added, the firstly available number
(11) was taken, which was of course a really bad idea since newer
versions of the spec can at some point use it, which is the case if one
checks for instance TS 48.008 Release 16 SAI Cell Id.
There no perfect way to fix this bad decision at the time, but the
CGI-PS is only used in osmo-bsc and only for RIM related purposes, so by
changing the ID of CELL_IDENT_WHOLE_GLOBAL_PS, we only break RIM under
some specific CIs being used, and when an osmo-bsc is built against
older libosmocore and then used at runtime against a newer libosmocore
(which should be rare).
Hence, the downside is acceptable, and by moving the new ID number to be
ouside of the spec proto TS 48.008 range (4 bits), we make sure we don't
have the same problem again in the future.
Related: SYS#5838
Fixes: ca33a71ca8
Change-Id: Id25e563febdb7640174540136225f399515a0089
Shorthand for the INET/INET6 switch() to get/put the addr part, useful
for encoding and decoding message buffers.
Related: OS#5599
Change-Id: Ie9e33bfac525c59c30714663d2bfcc62ec9eeb81
There already are osmo_quote_str_buf() and osmo_quote_str_buf2(), same
for _escape_, but none of them return the snprintf() like string length.
A private function does, publish this in the API.
The returned chars_needed is required to accurately allocate sufficient
size in string functions that call osmo_quote_str/osmo_escape_str. I am
adding such in osmo-upf.git.
Related: SYS#5599
Change-Id: I05d75a40599e3133da099a11e8babaaad0e9493a
The OSMO_SOCKADDR_STR_FMT() and _ARGS() macros properly place square
braces around IPv6 addresses, so that the port nr is clearly
distinguishable.
before: 1:2::3:4:5
after: [1:2::3:4]:5
When using a struct reference, the macro resolves to '(&sastr) ? .. : ..',
which the compiler complains about as "condition is always true". Shim
around that error with a pointer variable.
I considered using osmo_sockaddr_to_str_c() instead, but here in
socket.c we cannot assume that osmo_select_main_ctx() is being used and
hence can't just use OTC_SELECT for log string composition. The
struct osmo_sockaddr_str is a string representation in a local variable,
and hence doesn't need talloc for log strings.
I considered adding log_check_level() around the log string conversion,
but since all of these instances are on LOGL_ERROR, I didn't bother.
Related: SYS#5599
Change-Id: Idbe7582b2b7f14540919e911dad08af6d491f68f
It seems it is insufficient to register file-descriptor call-backs
with libusb_set_pollfd_notifiers(), but we also need to call
libusb_get_pollfds() once to get the initial set of file descriptors
which may have been created already during libusb_init().
As we don't have a libusb context before libusb_init() returns,
we cannot call libusb_set_pollfd_notifiers() early enough to be
active already during libusb_init().
Change-Id: Icf81014d689ffa738719af68120fa2dedbeec689
As was demonstrated in I54bf5e5c036efb1908232fe3d8e8e2989715fbb3,
when the logging is configured to print the filename *after* the
logging message, each logging line contains an artifact - '\0'.
The problem is that the 'len' variable is not updated. Fix this.
Change-Id: I5c920a0d5c1cf45bcdd327b39e33d63346b4f51c
Fixes: I393907b3c9e0cc1145e102328adad0a83ee13a9f
To easily log and print a sockaddr using OTC_SELECT, add
osmo_sockaddr_to_str_c().
Implement osmo_sockaddr_to_str_buf2() using osmo_strbuf, so that we can
return the chars_needed which osmo_sockaddr_to_str_c() uses.
From previous osmo_sockaddr_to_str_buf(), call
osmo_sockaddr_to_str_buf2() and return NULL if the buf_len was
insufficient, to mimick previous behavior. This makes it more
consistently returning NULL for insufficient buf_len, as shown in the
tweak that is needed in socket_test.c. Before osmo_sockaddr_to_str_buf()
would return a truncated port number, now it's all or NULL.
I will use osmo_sockaddr_to_str_c() in the new osmo-upf implementation.
Related: SYS#5599
Change-Id: I12771bf8a021e6785217b1faad03c09ec1cfef0e
In general, it's safe not to use talloc API here because those are
internal allocations, and there are no 'return' statements between
calloc() and free(). However, we don't really need to initialize
the heap memory with 0, so let's use the 'normal' malloc().
Change-Id: I6956cbd83b2999dbcf8e2d210134b0a166c33efb
When the logging framework is not initialized we get an error:
"ERROR: osmo_log_info == NULL! You must call log_init() before
using logging in ..."
There are sometimes situations where some code tries to log before
logging was initialied. This is a problem because the actual log
line with the debug info we need is covered up by the error message
from the logging framework.
Lets introduce a fallback logging function that is called when the
the logging framework is not available. This function can just use
fprintf to output to stderr.
Change-Id: I9b1b0988e02322e3e44fd4ceea3e1bc2d4df3c45
The event names contain '.', and there are spaces ' ' in the state
names. This is a problem since states and events can also be monitored
via the CTRL interface. Unfortunately the CTRL interface does not allow
certain reserved characters. So lets rename the states and event names
to make them compatible with the CTRL interface.
Change-Id: Id19973b56f9d7b1e3d0b0d7c7d0be7beba5428fc
Related OS#4149
Change-Id: I5ebc9ab5b1456fee29aa4e254fae862dc053f0aa
The parameters described in the docstrings for osmo_plmn_from_bcd() do not match the actual parameter list.
Change-Id: Ic0999dbe096a98418db7482bd110e20497d8e4a5
The identifier of stats item STATS_TCP_REORD_SEEN is tcp:sndbuf_limited,
it should be tcp:reord_seen.
Change-Id: If3539ceb570ae784cc9b6567c59da7afd11acf82
Related: OS#5701
This allows init-passive users to get the configured sizes for the RFCIs
and other similar information once engotiated with the peer.
Realted: OS#1937
Change-Id: I63ee780b4aa162ea097410b234e73984000c0965
When picking the end state, looking only at the path metric
is highly suboptimal because in a tail biting code, we _know_ that
whatever treillis path is correct, it must start and end at the same
state. So we only consider path meeting that condition. We know any
path that doesn't isn't the right one. We only fallback to only
path metric if no path met that condition.
Fixes OS#4508
Signed-off-by: Sylvain Munaut <tnt@246tNt.com>
Change-Id: I87e51d3880c0fe7bf3d6cd08fd46517a424a230c
The function clearly specified in its documentation that the number of
bytes written to the out buffer were being returned. However, the value
returned was "the number of characters (excluding the terminating null
byte) which would have been written to the final string if enough space
had been available.", aka snprintf-style.
The 2 callers of that function were not expecting it, so if a long
enough buffer was passed, the program asserted.
Closes: OS#5383
Change-Id: I8d71bd1a0dad37606acb8302b05c2ae338112e57
The use of an unsinged integer as for loop counter variable doesn't
work when counting down and comparing with >= 0. The existing code
would be an infinite loop if it wasn't for the (data dependent) break
condition:
>>> CID 243259: Control flow issues (NO_EFFECT)
>>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "i >= 0U".
572 for (i = 15; i >= 0; i--) {
573 if (match_mask & (1<<i)) {
574 iui->mode_version = i;
575 break;
576 }
Change-Id: I019d0f0d8f2b167575a2883a13cca692c96961cf
Closes: CID#243259
This is to fix the following compile error on CentOS 7:
[ 74s] stats_tcp.c: In function 'fill_stats':
[ 74s] stats_tcp.c:138:15: error: 'struct tcp_info' has no member named 'tcpi_notsent_bytes'
[ 74s] tcp_info.tcpi_notsent_bytes);
[ 74s] ^
Closes: OS#5374
Change-Id: Icde6651baeb0828477dbf540a02b16a1a5f91797
osmocom applications are deployed in a variety of different situations.
Dependung on the medium that interconnects the network components
unexpected behaviour may occur. To debug problems with the
interconnection between network components it might help to monitor the
health of the related TCP connections.
Change-Id: I1416f95aff2adcf13689646b7574845de169fa3d
Related: SYS#5701
Only support for SMpSDU mode is introduced in this commit.
Not supported explicit list:
- Transparent mode
- ATM/AAL2 based Transport layer
- GTP-U based Transport Layer
- Iu Rate Control procedure
- Time Alignment procedure
APIs are provided to allocate the primitives properly inside the related
msgb. This way primitives can be placed in the headroom, leaving the
data part of the msgb for the IuUP payload, hence allowing re-use of the
msgb and 0 copy of IuUP payload when forwarding data over RNL<->TNL.
Since RNL and TNL primitives relu struct osmo_prim_header, which is not
packed, they cannot be set to packed, and hence proper memory alignment
in the msgb must be done to avoid misaligned accesses (Asan errors about
it otherwise).
Related: SYS#5516
Change-Id: Ibe356fa7b1abaca0091e368db8478e79c09c6cb0
Just like rate_ctr_group_free, osmo_stat_item_group_free should tolerate
it when the argument is NULL
Change-Id: I23323833e7268356a50c4fc6a19639c4ecd2a101
Remove the paragraph about writing to the Free Software Foundation's
mailing address. The FSF has changed addresses in the past, and may do
so again. In 2021 this is not useful, let's rather have a bit less
boilerplate at the start of source files.
Change-Id: I5050285e75cf120407a1d883e99b3c4bcae8ffd7
gsmtap_sendmsg() places the burden of freeing the msgb in case of
erroneous return codes on the caller. A review of existing users
shows that this is overly optimistic and many calls get it wrong,
opening up memory leaks.
Let's add a new function gsmtap_sendmsg_free() which behaves like
gsmtap_sendmsg() but always takes ownership: Either it is sent + freed,
or it is just freed.
Change-Id: I106b09f2a49bf24ce0e8d11fd4d4ee93e9cafdf5
Related: OS#5329
This kind of API will likely cause memory leaks in case the caller fails
to check the return value of the function and knows he must free the
message.
Change-Id: I7e61c19d32a75e28f08b74a8e3d9d63a2d8bf3d7
Related: OS#5329
In case osmo_wqueue_enqueue_quiet() fails, msgb ownership is not
transferred to the queue, but the caller is responsible for freeing
the message buffer that we just failed to enqueue.
Change-Id: I6306e34dc7289864c889e72adf31d74d4581a810
Closes: OS#5328
Related: OS#5329
The log message is very confusing if printed for PtP-BVCI as well. Move
it into the correct if branch.
Change-Id: I0359443ddc52108b492f741005c4699e06b40183
gcc complains because our char might or might not be signed depending on
arch and phase of the moon:
error: array subscript has type 'char' [-Werror=charsubscripts]
Change-Id: I7c76f9a2318c4f0e5eedeea00ec380824b86567e
The bitvec_read_field() is used in performance critical places,
such as the CSN.1 decoder in osmo-pcu. Thus the less conditional
statements we have in the parsing loop, the better.
The bitvec_get_bit_pos() alone is quite a complex function, which
does check the boundaries and even supports the L/H syntax. Even
if it gets inlined by the compiler, we don't really want to run
redundant checks and run bitval2mask() on each iteration.
Change-Id: I438fc82d33ab2edbabd4215ec7bc46afb07d50ab
While running a sanitized version of the bitvec_test I get:
bitvec.c:492:24: runtime error: shift exponent 64 is too large
for 64-bit type 'long unsigned int'
This error is triggered by the following line in the bitvec_test:
_bitvec_read_field(0, 8 * 8 + 1); /* too many bits */
which basically tries to parse more bits (65) than the test vector
actually has (64). The problem is that we don't check if the
given vector has enough data *before* entering the parsing loop,
so we end up doing weird bit-shifts and getting weird values:
bitvec_read_field(idx=0, len=65) => bd5b7ddffdd7b5db (error)
Unfortunately, this problem remained unnoticed so far because in
'tests/testsuite.at' we don't check if stderr is empty. This is
fixed in a follow up change [1].
Rather than checking for errors in every loop iteration, do this
once and return early if the overrun is possible with the given
offset and length arguments.
Change-Id: I4deeabba7ebb720cdbe7c85b37bc011d05bdfa65
Related: [1] Ia82b92eddb18dc596881abcef2f098dc7385538b
This function returns an *unsigned* integer (uint64_t), so returning
a negative value on error is a bad idea. A negative value turns into
a huge positive value, what was demonstrated in the bitvec_test:
bitvec_read_field(idx=512, len=16) => ffffffffffffffea
bitvec_read_field(idx=0, len=65) => ffffffffffffffea
bitvec_read_field(idx=64, len=16) => ffffffffffffffea
The 0xffffffffffffffea above is basically:
(uint64_t) -EINVAL, or
(uint64_t) -22 + 1, or
0xffffffffffffffff - 0x16 + 1.
Let's make use of the errno in order to indicate an error to the caller.
Change-Id: I2cc734caa3365d03c2ae2b3f2cd9544933c25e9e
Related: OS#4388
It's the usual naming for unit test binaries. Without the '_test' endig,
the tdef_vty_test_{config_root,config_subnode,dynamic} binaries do not
match the 'tests/*/*_test' pattern and appear as untracked files in git.
Change-Id: I828fa45132e11a41c527d4b25df850c19871cb75
There might be library code that has rate counters, and if the main
program calls rate_ctr_init() a second time, we can skip the second
initialization.
Change-Id: I6f5342a77518599eb5ac9a0f0605917a78fcc387
alive_timeout_handler() changes the state to RECOVERING which calls
ns2_st_alive_onenter()->ns2_nse_notify_unblocked(unblocked=false)->
ns2_sns_notify_alive(unblocked=false)
When all (signalling) NSVCs have failed and gss->role is SGSN and not
persistent sns_failed() calls gprs_ns2_free_nse() which talloc_free()s
the nse before returning.
The next line in ns2_nse_notify_unblocked() tries to read nse->alive which then causes the
use-after-free.
Change-Id: I0486a77fd3e21fd3904bd19e4e0225ffbf654935
Related: OS#5302
Recent commit introduced the "blocking-io" param to "log stderr" VTY
command, which calls log_target_file_switch_to_{stream,wqueue}.
The VTY command already locks the log_tgt_mutex mutex, since it has to
access the tgt list. However, the functions mention above also want to
lock the same mutex in order to log information. Let's drop the logging
to avoid the double lock, and update its documentation to mention it
must be called with the lock already held, as documented on other
similar functions.
The issue can be spotted when running osmo-trx-uhd:
"""
(gdb) bt
#0 0x00007ffff75d7600 in __lll_lock_wait () from /usr/lib/libpthread.so.0
#1 0x00007ffff75d0503 in pthread_mutex_lock () from /usr/lib/libpthread.so.0
#2 0x00007ffff66314fb in log_tgt_mutex_lock_impl () at /git/libosmocore/src/logging.c:130
#3 0x00007ffff6638e74 in log_check_level (subsys=8, subsys@entry=-1, level=level@entry=3) at /git/libosmocore/src/logging.c:1510
#4 0x00007ffff6639c91 in log_target_file_switch_to_wqueue (target=target@entry=0x611000000320) at /git/libosmocore/src/logging.c:1186
#5 0x00007ffff68565d3 in cfg_log_stderr (self=<optimized out>, vty=0x6140000018a0, argc=0, argv=<optimized out>) at /git/libosmocore/src/vty/logging_vty.c:859
#6 0x00007ffff683db3d in cmd_execute_command_strict (vline=0x60b0000dfe80, vty=vty@entry=0x6140000018a0, cmd=cmd@entry=0x0) at /git/libosmocore/src/vty/command.c:2768
7 0x00007ffff683e396 in config_from_file (vty=vty@entry=0x6140000018a0, fp=fp@entry=0x615000036400) at /git/libosmocore/src/vty/command.c:2880
8 0x00007ffff684cedb in vty_read_config_filep (confp=confp@entry=0x615000036400, priv=priv@entry=0x0) at /git/libosmocore/src/vty/vty.c:1529
9 0x00007ffff684ebfc in vty_read_config_file (file_name=0x7fffffffe7d8 "/build/new/conf/osmo-trx-uhd.cfg", priv=0x0) at /git/libosmocore/src/vty/vty.c:1920
10 0x0000555555565270 in main (argc=3, argv=0x7fffffffe3c8) at /git/osmo-trx/Transceiver52M/osmo-trx.cpp:652
"""
Debugged by rebuilding libosmocore with "LOG_MTX_DEBUG 1":
"""
/libosmocore/src/logging.c:1510 [log_check_level] lock
/libosmocore/src/logging.c:1522 [log_check_level] unlock
/libosmocore/src/vty/logging_vty.c:844 [cfg_log_stderr] lock
/libosmocore/src/logging.c:1510 [log_check_level] lock
"""
Fixes: b72867f0e6
Related: OS#4311
Change-Id: Idb4215fa2f364e28c0bb73fb9975b6c9f50a46f6
In the old days, we performed synchronous, blocking writes to the log
file or stderr. This was replaced by code that turned all log
file/stderr writes into non-blocking writes behind a write_queue.
This patch now introduces a further optimization: If we currently
don't have any log messages pending in the write queue, we are not
back-logged and assume we have a fair chance of writing the log message
right now, synchronously. So we try that first, and only enqueue
the log message if the write fails (no bytes or insufficient number
of bytes written).
This way we should get the best of both worlds: No delay/re-ordering
(and lower select syscall load) for the "normal" case (benefits of
the old synchronous writes) while at the same time never risking to
block on log output.
Related: OS#4311
Change-Id: I08469a7e4be9bc5bbd39140457bb582f4a0b1703
For file and stderr output, the existing code always generates
the log string on a stack buffer, and then (in case of non-blocking
write via write_queue) copies it over to a msgb.
Let's optimize this by turning _file_output() into a raw_output
callback which first allocates the msgb and then format-prints
directly to that msgb instaed of stack + memcpy.
This has the disadvantage that we don't know how long the buffer
has to be in order to print the entire string to it. As a result
we always have to allocate a 4k-sized buffer (plus msgb overhead).
The write_queue length for log file output has been decreased from
1024 entries to 156 entries in order to stay within the same
memory requirements for each log target memory pool (about 648 kBytes).
Related: OS#4311
Change-Id: I0d10b0199576d2e7ff6421a6dba19ae5ffafd946
So far, we used blocking, buffered fwrite() to write to stderr
and file targets. This causes problems if there are [slow] consumers
causing delays, such as gnome-terminal (when the program is started
interactively) or systemd/journald (where we observe 64..128ms blocks on
stderr).
This patch introduces stderr/file based logging via write_queue
and osmo_select_main(), i.e. switch from glibc-buffered, blocking
to internally buffered, non-blocking writes.
* when osmo_stderr_target is created via application.c, we create it
in blocking stream mode for backwards compatibility, particularly
for [smaller] programs that don't use osmo_select_main()
* when the VTY code encounters 'log stderr' or 'log file FILENAME',
we switch that respective target to non-blocking write-queue mode,
as this means the application is in fact using osmo_select_main()
* The config file can now state 'log stderr blocking-io' or
'log file FILENAME blocking-io' to explicitly enforce using blocking
stream based I/O
* The application can at any time use API functions to switch either way
Closes: OS#4311
Change-Id: Ia58fd78535c41b3da3aeb7733aadc785ace610da
Previous dark shiny blue one is really difficult to read on the
terminal. Let's change it for some purpleish color which is far easier
to read.
Change-Id: Ia5c0860dd8d756bb24eb8972f94590bfba5bc865
BLOCK PDU can be send over a different NSVC than the NSVC.
E.g. informing a NSVC got blocked in case of a lower-layer failure.
Change-Id: I483e3a1d3b8c43bbb0cc6185b7f7f772bcb264bf
When receiving an invalid RESET (e.g. wrong NSEI or NSVCI) do not
forward the PDU to the NSVC fsm. Answer it with correct NSEI & NSVCI,
log the PDU, then ignore it.
Fixes: OS#5258
Change-Id: I6e562def9c5a1e4534d42884215272b1e66d26c2
STATUS PDU can be send over a different NSVC than the NSVC which
generated the STATUS PDU. E.g. informing a NSVC got blocked in case of a lower-layer failure.
Change-Id: I5c9e9de10c669c1226da67bb9e2663c5cfe828a8
To answer correct on a BLOCK PDU with a different NSVCI, the
STATUS PDU needs also a NSVCI parameter.
Change-Id: I373eb48697097cdfa45748a091c11f7b3f0345fa
The BLOCK and BLOCK ACK PDUs can be send over a working NSVC to inform
the NSE that a NSVC is blocked.
Change-Id: I6189229fdc1f054e86811bc60cb7646e1f758a78
Even if not bound to a IF they just exist and work as expected, and make
distinguishing traffic for local setups easy.
Change-Id: I1043dfd8075f14481011f43db45c943e9320413c
The buffer is allocated dynamically on heap, so there is no such
limitation of 4096 bytes / 1365 characters.
Change-Id: I960dd6a53123fd4209ef6e61dcd0d22e4005e397
Replace some with atoi(), where the VTY has already validated correct
range of the argument.
Replace others with the new osmo_str_to_int() or osmo_str_to_int64()
functions, possibly covering more detection of invalid number strings.
Leave those strtol() callers that depend on endptr to provide the next
string token.
Related: SYS#5542
Change-Id: I0ebb06e751c28f7d1cdf328de29cd227a2449391
20 bytes is not enough for some VAMOS specific channel number values,
so the resulting string representation gets truncated by snprintf():
expected: "VAMOS TCH/H(0) on TS4\0"
actual: "VAMOS TCH/H(0) on T\0"
Let's enlarge the buffers to 32 bytes.
Change-Id: I68d839f4ab742cf56de34e7e22572a1163aec2da
Change the functionality of skipping unchanged values: instead of
looking up whether new values have been set on a stat item, rather
remember the last reported value and skip reporting identical values.
stats_test.c shows that previously, a stat item reported a value of 10
again, even though the previous report had already sent a value of 10.
That's just because the value 10 was explicitly set again, internally.
From a perspective of preserving all data points, it could make sense to
send consecutive identical values. But since we already collapse all
data points per reporting period into a max, that is pointless.
Related: SYS#5542
Change-Id: I8f4cf34dfed17e0879716fa2cbeee137c158978b
Intead of attempting to store all distinct values of a reporting period,
just store min, max, last as well as a sum and N of each reporting
period.
This gets rid of error messages like
DLSTATS ERROR stat_item.c:285 num_bts:oml_connected: 44 stats values skipped
while at the same time more accurately reporting the max value for each
reporting period. (So far stats_item only reports the max value; keep
that part unchanged, as shown in stats_test.c.)
With the other so far unused values (min, sum), we are ready to also
report the minimum value as well as an average value per reporting
period in the future, if/when our stats reporter allows for it.
Store the complete record of the previous reporting period. So far we
only compare the 'max' value, but like this we are ready to also see
changes in min, last and average value between reporting periods.
This patch breaks API by removing:
- struct members osmo_stats_item.stats_next_id, .last_offs and .values[]
- struct osmo_stats_item_value
- osmo_stat_item_get_next()
- osmo_stat_item_discard()
- osmo_stat_item_discard_all()
and by making struct osmo_stats_item opaque.
In libosmocore, we do have a policy of never breaking API. But since the
above should never be accessed by users of the osmo_stats_item API -- or
if they are, would no longer yield useful results, we decided to make an
exception in this case. The alternative would be to introduce a new
osmo_stats_item2 API and maintaining an unused legacy osmo_stats_item
forever, but we decided that the effort is not worth it. There are no
known users of the removed items.
Related: SYS#5542
Change-Id: I137992a5479fc39bbceb6c6c2af9c227bd33b39b
To show adminstrator the last state change of a nsvc add a timestamp and
show it on the vty
> show ns nsei 1234
NSEI 01234: UDP, DEAD since 0d 0h 1m 42s
[...]
4 NS-VC:
UNBLOCKED DYNAMIC sig_weight=1 data_weight=1 udp)[127.0.0.1]:22000<>[127.0.0.1]:23001 ALIVE since 0d 0h 0m 1s
UNBLOCKED DYNAMIC sig_weight=2 data_weight=2 udp)[127.0.0.1]:22000<>[127.0.0.1]:23000 ALIVE since 0d 0h 0m 1s
UNBLOCKED DYNAMIC sig_weight=2 data_weight=2 udp)[127.0.0.1]:22001<>[127.0.0.1]:23000 ALIVE since 0d 0h 0m 1s
UNBLOCKED DYNAMIC sig_weight=1 data_weight=1 udp)[127.0.0.1]:22001<>[127.0.0.1]:23001 ALIVE since 0d 0h 0m 1s
Related: OS#5028
Change-Id: Ie3a039a209869295afa5feda39297cee81fedf22
To show adminstrator the last state change of a nse add a timestamp and
show it on the vty
> show ns nse 1234
NSEI 01234: UDP, ALIVE since 0d 0h 0m 16s
FSM Instance Name: 'GPRS-NS2-SNS-SGSN(NSE01234-SNS)[0x6120000012a0]', ID: 'NSE01234-SNS'
Log-Level: 'DEBUG', State: 'CONFIGURED'
Timer: 4
Maximum number of remote NS-VCs: 8, IPv4 Endpoints: 2, IPv6 Endpoints: 0
[...]
Related: OS#5028
Change-Id: I8143080a3c5c9a55d37dfad44ba2ac6561daa216
vty_out_uptime() calculates the time difference to a given timespec
and print it in a human readable format (days, hours,
minutes, seconds) to the vty.
Related: OS#5028
Change-Id: I264a3f49096b96646e0a1f5366623ac20d860793
Using mbedtls commit f9c599cd8ac9d00c484d4f5b027e18c6af4f9fdf before
they re-licensed to Apache 2.0, so we have a GPL-v2-or-later bsae64
implementation and avoid having code under a different license in the
tree.
This code is the unmodified import, so we can record any local changes
compared to the original version.
Change-Id: I39a9d3ab98257d21b9439b00528c744efa372c14
There also is an osmo_stat_item_desc, so the name 'desc' makes it hard
to read the code / the upcoming refactoring patches. It is an
osmo_stat_item_group_desc, so call it group_desc.
Related: SYS#5542
Change-Id: I07bc011450549a44ebf043e7d8a70718ddfd900e
Expose all stat items as RO variables of the form
stat_item.last.group_name.N.item_name
stat_item.last.group_name.by_name.idx_name.item_name
For (possibly contrived) example:
stat_item.last.trunk.0.endpoints:used
stat_item.last.trunk.by_name.virtual-0.endpoints:used
Include the 'last' token to ease future extension, like 'max'.
Put this token in the beginning, similarly to rate_ctr variables, which
begin with 'per_sec', 'per_hour', ...
Related: SYS#5542
Related: I178dcf4516606aa561d47b06061b8a416d3c40cf (osmo-ttcn3-hacks)
Related: Ic1b35b7406547f92818afe399a2383d154576409 (osmo-ttcn3-hacks)
Change-Id: Idace66b37492fe96b2f2e133a69cac7960ca279c
Add "missing" API for looking up a stat_item_group by its index-name.
A subsequent patch, which adds stat_items to the CTRL interface, will
use this to look up stat item groups by object name.
In stat item groups, there are group names, having a number of indexes
denoting different objects. An object can have, besides the index, also
a name that is equivalent to the index.
Apologies for the weird function name, it's still the best one I
could come up with: "group_by_name" refers to the group name, and
"idxname" refers to the name that the object index is associated with.
We already have osmo_stat_item_get_group_by_name_idx().
Other contestants for name of this new function were:
- osmo_stat_item_get_group_by_name_name()
because there is a "name" instead of "idx", but I find it confusing.
- osmo_stat_item_get_group_by_name_idx_name()
but I find that the last "name" should be closer to the "idx".
Related: SYS#5542
Change-Id: Ia1a77a1e4657ba624dd4f4bf7ad274e7751d0141
Properly converting a string to an integer while validating against all
possible errors is not trivial. It is a recurring theme in code review,
and there are places in osmo code that do it wrong.
End this by providing a simple API, if for nothing else then as an
example of how to use strol() / strtoul() / strtoll() / strtoull()
in an airtight way.
A subsequent patch, adding stat items to the CTRL interface, uses this
to properly validate indexes in CTRL variables and convert them to int.
Related: SYS#5542
Change-Id: I4dac826aab00bc1780a5258b6b55d34ce7d50c60
It was so far sufficient to wait for the buffers to drain at some
random point in time, but this is not always the case, sometimes it is
important that the output is flushed immediately.
Change-Id: If984b9ad2eba9f400bc29a7aa8825e241fd1d2a9
A STATUS PDU with cause code NSVC UNKNOWN/NSVC BLOCKED informs the other
side about a state mismatch between the side.
Change-Id: Ib6a2424f3027a30f14ef0a9fc2230e6aae9a2a04
The ns2_vc_force_unconfigured() needs to be protected otherwise it would free
gss->nsvc which will be used later. It further would run into another
SNS failure which is wrong too.
Change-Id: If14b9e3fcd5d139457b10d06517302168091d8d8
Previous the SNS NSVC (the NSVC used for all SNS traffic) was never changed except
when the choosen NSVC went dead or got freed.
When receiving a SNS SIZE PDU over a different NSVC than the current SNS
NSVC the answer would be transmitted to a different port.
Change-Id: I36cd9488b8bca5cb99dae5cf50a55ee282e0557b
When no remaining signalling NSVCs are available the SNS must be
restarted (BSS) or go into unconfigured state (SGSN).
Change-Id: I95e6bbb7a418d647a8426804879571597ae06ff8
When cleaning up the SGSN side (e.g. receiving a SNS SIZE PDU) the
clean up will result in a use-after-free bug when the SGSN side is still
alive.
Change-Id: I0f57dd0577d1fc7bd270f58e15f6f22eb130ef59
When removing a bind the remote side needs to be
informed via the SNS DELETE procedure.
Related: OS#5036
Change-Id: I53cd54dfd262c70c425c3f13dad3b29526daa523
When adding a bind, the remote side needs to be
informed via the SNS ADD procedure.
Related: OS#5036
Change-Id: I71c33200bd1f0307ceb943ee958db5ebe3623d36
When changing the bind ip-sns weight, initiate a
SNS CHANGE WEIGHT procedure to inform the other side.
Related: OS#5036
Change-Id: Icec4dabb46bc198f68f91bfe09ba279fbe68d454
The problem are recursive execution because a free generates an event which could
allow the use to free a nsvcs while the llist_for_each() is still running.
Change-Id: I902557fb6e56e6588728a46e43a9cbe3215d5c68
When removing NSVCs before removing the bind from the SNS list, the removing NSVCs could
trigger a creation of a new NSVC on the same bind ending in a
while(true) loop.
Change-Id: I6f497348f75fb479427d8a4c23313e33fbc62036
Otherwise there could be recursive loop when free'ing NSVCs which
in the end create an event which the SNS want to free the NSVCs a
second time
Change-Id: Ie99ba5fe8a84519fe8a8c0abdf875606715ab7f6
Move the cleanup into it's own state. Also changing the
SGSN unconfigured state which won't be triggered when a
SIZE is received.
Change-Id: I2639345fdf3cd300a934238d676c543065ceaa8b
When other parts of ns2 requires to emit an event to the SNS fsm it would
need a proxy function because the events are private to the
SNS file. To circumvent creating multiple proxy function make the events
available via a header file.
Change-Id: I8e3fae4367c112b5a71bffb33c302d903855cddc
This commit adds new Osmocom specific IEs required to pass C/I related
Power Control Parameters osmo-bsc => osmo-bts to be used by the MS Power
Control Loop being implemented.
Related: SYS#4917
Change-Id: Iffef0611430ad6c90606149c398d80158633bbca
To indicate to the BSC that a BTS supports temporary overpower of
SACCH/FACCH channels a new feature BTS_FEAT_ACCH_TOP is added.
Change-Id: I62fbfc30acd5d67b20727b75a8f256e6b5d31e06
Related: SYS#5319
To transfer the temporary overpower value from the BSC to the BTS, a new
RSL IE (RSL_IE_OSMO_TOP_ACCH_CAP) is added.
Change-Id: I31c5be4bceb9140d63ab8e2f197f0acc68699426
Related: SYS#5319
The encoder function gsm0503_tch_ahs_encode uses gsm0503_afs_ic_ubit
when encoding the CMR or FT (depends on the frame number). This is not
correct. It should use gsm0503_ahs_ic_ubit instead.
Change-Id: Id250b2102ac79ff222bd3ad9d1abc4b60abdd12b
Related: SYS#5549
Exempt all stat_item statistics from 'stats reset'. Only reset rate_ctr
statistics to zero.
The rate_ctr statistics have an implicit time scale, counting occurences
per time unit. For them it makes sense to reset all ratings and start
from zero, for example in a test suite (e.g. our TTCN3 BSC_Tests).
In contrast, stat_item statistics count number of objects or nr of
specific object stati at any given time, and they do not deteriorate
over time. Many stat items depend on increment/decrement to be sane.
For example, in osmo-bsc, if the nr of connected BTS is 3, that does not
make sense to be reset to zero. There are still 3 BTS connected, only
the stat_item would suddenly reflect zero. From then on, it'd be wrong.
All stat_items are by definition wrong after a 'stats reset'.
- Those that depend on increment/decrement will be wrong until the
program exits, and
- those that are set to absolute values will be wrong up until the next
value is set. That could be seconds or hours later, depending.
Related: SYS#5542
Change-Id: If2134768b1076e7af189276c45f2a09a4944303e
Background:
* Individual values can be added to osmo_stat_item.values at any time.
* Stats are reported at a fixed interval (see vty 'stats interval'),
e.g. every 10 seconds.
* In order to report a new stat value, we use the maximum of all
osmo_stat_item.values added since the last report.
* By default, we do not send new stat values if they did not change
(see vty 'config-stats' -> 'flush-period' default of 0).
Fix the following bug:
* If 'flush-period' is 0, and no new osmo_stat_item.values are coming
in, the last value that gets reported is not necessarily the last
entry in osmo_stat_item.values.
* For attached reporters (statsd), it could then be that the given stat
stays at the wrong value for a long stretch of time (think of several
hours/days/forever).
Explanation of how the test shows that it is fixed:
* stats get reported (value is irrelevant)
* osmo_stat_item gets a new value: 20
* osmo_stat_item gets a new value: 10
* stats get reported (value: 20, the maximum of both new values)
* osmo_stat_item gets no new values
* stats get reported (value: 10, this is new because of the bug fix,
the real last value in osmo_stat_item, different from the 20 sent
earlier, without the fix it would not send anything here and the last
sent value would be 20)
* osmo_stat_item gets no new values
* stats get reported (nothing gets sent, since the real last value was
already sent and 'flush-period' is 0)
Fixes: OS#5215
Change-Id: Ibeefd0e3d1dbe4be454ff05a21df4848b2abfabe
When free'ing a NSE/NSVC/BIND ensure there can't be a double
free by using a free anchor in the struct.
Recursive free's can happen when the NS user reacts on an event
(e.g. GPRS_NS2_AFF_CAUSE_VC_FAILURE) and calls the free().
Or when the user free's a NSVC when the NSE uses SNS as configuration,
the fsm tries to free it again.
Change-Id: If9823aadaa936e136aa43e88cee925ddd5974841
The SGSN fsm should be freed when becoming invalid instead of going
into the unconfigured state. The unconfigured states should be only used
when creating the NSE (on the SGSN side).
Change-Id: Ife889091ecba4180a90743deb786767008fe863d
The SNS code will always create NSVC on it's own. The only case
when the SNS dialect allows dynamic NSE/NSVC is on the SGSN side when accepting
dynamic NSE and receiving the first SNS SIZE. In this case the NSVC FSM must not be started yet.
Prevents sending NS_ALIVE before the SNS configuration has been
finished.
Change-Id: I86275c99432262b3c19c1ded9a77090b74303bc8
Use ANSI escape characters to clear the screen with ^L, like it works
in typical Linux shells. I always found it slightly inconvenient that
this didn't work in the VTY.
Change-Id: Ie2356cd92f39b4dc28b5c20bbe4557fb0d972747
Some tests under osmo-pcu (TbfTest) were caught accessning NULL pointer
bssgp_nsi in bssgp_tx_llc_discarded triggered by timeout while stepping
slowly with the debugger.
It seems that test is not properly using neither the old nor the new
API. Let's catch such cases easily.
Change-Id: I3ea42755c4bfd29e4a01ad57f186f28d58ab466a
Vadim Yanitskiy
Pau Espin
Harald Welte
Daniel Willmann
Neels Hofmeyr
Michael Iedema
Philipp Maier
Sylvain Munaut
Oliver Smith
Eric Wild