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
Harald Welte
Philipp Maier
Pau Espin
Sylvain Munaut
Vadim Yanitskiy
Oliver Smith
Daniel Willmann
Eric Wild
Neels Hofmeyr
Alexander Couzens