The problem is that we don't zero-initialize the struct pcap_rechdr +
pcap_lapdhdr before memcpy'ing them to buf, before we call write:
==20097== Syscall param write(buf) points to uninitialised byte(s)
==20097== at 0x4E48471: write (write.c:26)
==20097== by 0x4DA8DE9: osmo_pcap_lapd_write (lapd_pcap.c:168)
==20097== by 0x4DA8433: send_ph_data_req (lapd.c:628)
==20097== by 0x4C94F5C: lapd_send_rej (lapd_core.c:536)
==20097== by 0x4C9A08A: lapd_rx_i (lapd_core.c:1574)
==20097== by 0x4C9AA8F: lapd_ph_data_ind (lapd_core.c:1708)
==20097== by 0x4DA7C55: lapd_receive (lapd.c:496)
==20097== by 0x4D96B2C: e1inp_rx_ts_lapd (e1_input.c:778)
==20097== by 0x4D9C97C: handle_ts_sign_read (e1d.c:78)
==20097== by 0x4D9D908: e1d_fd_cb (e1d.c:281)
==20097== by 0x4D1281B: poll_disp_fds (select.c:361)
==20097== by 0x4D12928: _osmo_select_main (select.c:399)
==20097== Address 0x1ffefffed7 is on thread 1's stack
==20097== in frame #1, created by osmo_pcap_lapd_write (lapd_pcap.c:129)
The whole idea of first filling the two structs on the stack, and then
copying them to another buffer on the stack is somehow weird. Let's
just create a combined struct on the stack and then fill that one
directly.
Change-Id: I358c71354cc6ddad1964cc4a988ad29b7ba617f1
Closes: OS#5592
Before this patch, the logic (both for delayed tx and immediate tx)
always left the WRITE flag set, and relied on an extra call back from
the main loop (poll()) to disable the flag until it found out there was
nothing else to send.
Instead, let's disable it immediatelly at the time we submit the last
message in the queue.
Change-Id: I0e5da5d1342f352d0e2bca9ee39c768bccb2c8d5
Recent commit optimize the same function by avoiding an extra poll loop
when e1i_ts->sign.delay was zero. Upon doing so, the
osmo_fd_write_disable() was moved to some conditional paths. Hence, the
WRITE flag is left set and we don't need to set it again in the code
path modified in this commit.
Fixes: 28fea7746b
Change-Id: I84787b6de2a5ccc82bd8f19ce874e73708bc287f
Historically, before November 15, 2010 when commit
d49fc5ae24fc9d44d2b284392ab619cc7a69a876 was merged to [back then]
OpenBSC, before libosmo-abis became a separate library, we used to
have a 10us delay timer for subsequent writes to ip.access nanoBTS 900.
ts: Reduce the delay to 0 for OML and RSL
This is possible after not sending more than one OML command that
requires an extra ACK. For the RSL line we do not need any speed
limitation.
Ever since the above-mentioned commit, the BSC always sets that timeout
to zero, which makes libosmo-abis start a zero-microsecond libosmocore timer,
which in turn will make libosmocore call select/poll with zero timeout, which
makes the kernel return immediately.
Why not remove the timer completely? Because ipaccess-config.c still specifies
a non-zero signaling delay, and we cannot be sure that this is really not
needed.
So let's alter the code to only start the timer if it's non-zero
Change-Id: I9c379364e7e6afce35fc6316392b5b33748980f7
Multiple IPA units can have the same bts_id but scoping by their
site_ids will make them unique. This also clarifies the "bts"
number being communicated. It is not the bts configuration index
in osmo-bsc.cfg, it is the bts id specified in the vty line:
bts X
ipa unit-id SITE BTS
Change-Id: I3b44319fb4bc6a812800001c58dfe1a664645b43
The osmo_fds used for ipaccess based E1 traffic should be registered for
monitoring to get tcp_stats for all RSL/OML links.
Change-Id: I42ca4e9257eee2857784dd4f5b17a83220e2be9e
depends: libosmocore I64b288174ae3dd22d50a701ce7cbf9e623246479
Related: SYS#5701
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: I0ad20cdf1e571c1a15f6e67d46e07155a6a02520
The name [printing] works like this:
* libosmo-abis:lapd tells libosmogsm:lapd_core a name for each data link
* libosmgsm:lapd_core assumes this name has no paranthesis and puts additional
parenthesis around it in macros like LOGDL
However, current libosmo-abis:lapd [before this patch] adds its own set
of parenthesis, causing all ISDN-LAPD related log messages generated by
libosmgsm:lapd_core to have double-parenthesis.
So we have to remove the parenthesis from lapd_datalink.name and
lapd_instance.name to fix the log lines printed in libosmocore.
This in turn means we have to add parenthesis to some log statements
here in libosmo-abis:lapd.
Why can't we instead modify libosmogsm:lapd_core to drop parenthesis
there? Becaus it is also used by LAPDm code in osmo-bts, and those two
get it right :)
Change-Id: I688cf2a73a8c31cb3cf66ec005d02a14de197008
Related: Ie6742843fff809edffcac24c4dce4edf66bc71be
Related: OS#1938
Since a recent commit (see hash below), e1_input.c code was fixed to
release the line from the global line list (e1inp_line_list) before
freeing the object. However, the change didn't have into account that
there were cloned lines from the original one which had a duplicate of
the original line->list. Hence, whenver a clone was freed, it actually
removed the original line from the global list.
Since the original line is the one actually holding the permanent
reference due to being configured over VTY, let's fix e1inp_line_clone
to reset the clone->list so that whenever it is freed it doesn't affect
the original line.
Fixes: 49924f0a79
Related: SYS#5642
Change-Id: I0627da0d28057649b494e2bd83b283b09fb9a2d7
If BTS, using this app, tried to use it in order to re-create the
connection, it would leak the previous strut ipa_client_conn. A similar
fix was already put in place recently for OML, but it was not applied
for RSL.
The leak was spotted by having a BTS connecting fine over OMl but then
failing each time to connect on RSL.
Related: OS#5248
Change-Id: I4ee1ae318b446490783c8b910fca10ba5f72dd5a
The e1inp_line_update() API is precisely aimed at that: tell that we
want to reconfigure some aspects of the e1 line. Hence, it makes no
sense to apply a no-op with the flag "line_already_initialized".
That flag was added a long time ago in order to avoid problems when
several BTS are attached to the same ipa server link. That case doesn't
apply to the BTS role, so we can simply recreate the ipa_cli_conn.
Fixes: d6216405b7
Change-Id: Iff8092f88726cf238dac2abb99e135ac5864272d
Specially important regarding the pre-allocated reference count.
This was done when moving the API to use osmo_use_count internally,
because previous code already came with refcount=1.
Change-Id: I819a13732c5d0ff150bc11d1cfd64e96ee81bad2
This was not an observable issue so far because both osmo-bts and
osmo-bsc used to keep the same line created once, keeping at least 1
reference count to it.
Change-Id: I1314d6b917ecb622994507475eb894e649a1a2ad
It was spotted in an osmo-mgw instance receiving RTP from osmo-bts 1.2.3
that sometimes the seqno was reset to 0 (while tx_timestamp was kept
properly). This seems to be the only possible way osmo-bts can reset the
seqno to 0 (calling rtp_session_reset()), since osmo_rtp_socket_create
already sets it to other values and inside ortp rtp_session_reset() is
the only one setting it to 0.
If SSRC changes, we are fine resetting some Rx state, which is done by
calling rtp_session_resync. However, calling rtp_session_reset() is too
much, since it erases other stuff like Tx state, which sholdn't be
affected in this case, since we are still transmitting the same stream.
Related: OS#3104
Related: OS#3299
Related: SYS#5498
Fixes: 28eeb6bc93
Change-Id: Ic6006b4991c85413b009b70f8f3a6f97c41990ec
The function ipaccess_line_update (called when the API user calls
e1inp_line_update and the line is of type ipaccess) internally sets
the line_already_initialized regardless whether the initalization is
successful or not.
This also means that if the initalization fails the API user is unable
to retry the initalition with corrected line options.
Change-Id: I90d330e2864f85b964628502afdb0c38eb54f15d
Related: SYS#4971
During normal operations, osmo-mgw floods the log/vty at level NOTICE with:
Received Event RX_BITS (trau_sync.c:525)
Change-Id: Ib4c5ac130e766906361c63090b30111970343075
For consistency reasons with osmo_rtp_socket_set_dscp(), let's also
introduce a osmo_rtp_socket_set_priority() function to wrap the two
calls to osmo_sock_set_priority().
Change-Id: I52c08f4b2a46981d002ef0c21e6549445d845a6e
This adds new members to struct ipa_server_link and ipa_client_conn,
which can be set by the user before ipa_client_conn_open() or
ipa_server_link_open() to ensure DSCP / priority values are applied
from the first outgoing packet onwards.
Change-Id: I5ab22d35d9b11eca47d9ea12208e5589a1a5713c
Depends: libosmocore.git If22988735fe05e51226c6b091a5348dcf1208cdf
In order to allow configuration of pcap files per e1_line
the vty command is now (for example line 0):
e1_line 0 pcap /tmp/e1cap.pcap
in place of:
pcap /tmp/e1cap.pcap
Also ensures that a configured pcap appears in 'show running-config'
and is written to the config file on issuing 'write'
This commit deprecates e1_set_pcap_fd()
Change-Id: I316c3d6a839e84c2f52a148c6b8dd6f5933cf4bf
In Ic66eb67254d35f02ec8cdea4d79fa4b1899e8230 the soversion was
chhanged (e.g. libosmoabis.so.9 to libosmoabis.so.10) but the
spec.in file was not adjusted accordingly.
Change-Id: I0ca9eb5a60557a1bc8e3611fdd2668469ac19031
Let's disable hex category here since we don't care about its formatting here.
In any case, every test relying on logging output validation should
always explicitly state the config to avoid issues in the future if
default values change.
Related: OS#5034
Change-Id: I332d1215fc7784a6647b90e2eb8b1efcd644002e
* it's inconsistent to print ERROR in a LOGL_INFO message
* loosing RTP packets is not that unusual, especially when the channel
is just starting up
Change-Id: Ide0017987b4cea7fd24795941494f1da913ea673
Closes: OS#4464
Unlike the legacy TRAU mode, this only adds I460 mux/demux,
without any TRAU frame synchronization. The user must still be
adding the actual sub-channels using osmo_i460_subchan_add()
depending on his requirements.
Change-Id: I44da6dfec77ef475adb35001a0e4fa11d549aa02
The code wrongly assumed that ipaccess_drop was only called by BSC code,
which is wrong. ipaccess_drop is called by BTS run code path in __handle_ts1_write(),
if send() syscall fails (for instance because BSC becomes unreachable).
In that case, we need to account for the BTS role case which doesn't
store the line pointer into the ofd->data. In BTS case, it's a pointer
to the struct ipa_client_conn, which we leave up to sign_link_down() cb
to do whatever they please with.
Fixes: OS#4864
Change-Id: If763e5f7736921a4360ad9027ba075ef8e118934
The signal link is set if fd is established (see osmo_fd_setup in
ipaccess.c).
This log message was introduced in 466c5467e2,
where the lifecycle worked a bit different than nowadays:
line->ops->sign_link_down() was called before the log line, so the code
expected by that time that the socket should have been freed by
sign_link_down(). That's no longer the case. In ipaccess_drop, we force
dropping so we release lower layers and then signal upper layers. Hence,
the log lines are misleading nowadays.
Change-Id: Ibc6554e6cacc9c71232238b4e6a17d749dfdd30a
Pau Espin
Harald Welte
Michael Iedema
Vadim Yanitskiy
Philipp Maier
Oliver Smith
Keith Whyte