|Age||Commit message (Collapse)||Author||Files||Lines|
In commit 65c62e50335b500ac9e4658530ca5a1b4f5328d8 a call
to unlink() was erroneously moved up. Since then unlink()
has been called with an uninitialized path variable. The
problem went unnoticed because the return value of unlink()
was never checked.
Ensure that unlink() is called with an initialized argument and
verify success of the unlink() operation if the socket exists.
Display LCLS config and control state as text in "show conns" command.
Before closing or breaking the loop in LCLS we do preliminary
checks. To facilitate adding new LCLS modes it's restructured as
* move check into dedicated static function
* explicitly check for MGW mode in endpoint check
* check for mode mismatch
Coverity points out that abis_nm_rcvmsg_sw() contains a switch
statement with suspicious looking missing break statements.
It is unclear to me if the code intends to process some
types of messages in more than one state, or of all messages
which affect a particular state already appear in the state's
corresponding switch block.
Can someone else tell what is supposed to happen here?
If this code is falling through intentionally, I will suggest
a patch adding /* fallthrough */ comments for clarity.
When a gscon wants to send a BSSMAP Clear Request, it makes no sense to do it
conditionally depending on the current conn state. Just send it: don't call
gscon_sigtran_send(), directly go for osmo_bsc_sigtran_send().
In particular, if an incoming inter-BSC handover ends in failure, the gscon
state is still ST_INIT, but if the MSC fails to give us a Clear Command, we may
want to ask with a BSSMAP Clear Request.
Otherwise any use of functions in gsm_timers_vty.c will fail because
g_vty_T_defs is never set (so it is NULL).
Check the return value of gsm48_multirate_config() in function
lchan_set_single_amr_mode(). This prevents an invalid AMR mode
from being configured for a logical channel.
Because the VTY parsier limits the AMR mode range to 0-7 this
is just a theoretical issue. However, this fact is beyond the
understanding of a static code analyzer, and the absence of
error handling was indeed setting a bad example.
Use stricter checks for received Global Call Reference.
complete_layer3 returns true if everything succeeded, false otherwise.
However, its caller bsc_compl_l3 returns unix style (0 sucess,
This commit has no real effect since only caller of bsc_compl_l3 never
checks return code, but will check it in the future.
In default example network, there's no cells with those arfcn.
Furthermore, having those seem to prevent some MS to register against
nanoBTS configured by a BSC using those lines.
The 'show lchan' command already shows details about timeslot
state, indicating whether a channel is currently being switched
to a different channel type.
However, this information was not shown by 'show timeslot' yet.
There are TTCN3 BSC tests which use 'show timeslot' and are
seeing sporadic failures when 'show timeout' is invoked during a
channel type transition. With this change, VTY sessions recorded
in pcap files for these tests will contain information about the
current channel type transition state.
conn->fi should actually never be NULL, they are allocated and discarded
simultaneously. So check its null from the start and remove some conditions
below, to remove the coverity warning.
Related: CID 189671
Make sure some RSL cause is set.
Put all lchan release related flags and settings in a sub-struct named
'release' to better indicate what those fields are for. There is no functional
If an lchan is being released and had a SACCH active, there is no reason to
omit the Deact SACCH message ever. All of the callers that passed
do_deact_sacch = false did so for no good reason.
Drop the do_deact_sacch flag everywhere and, when the lchan type matches and
SAPI is still active, simply always send a Deact SACCH message.
The do_deact_sacch flag was carried over from legacy code, by me, mainly
because I never really understood why it was there. I do hope I'm correct now,
asserting that having this flag makes no sense.
In the case where there is a release in error and we skip immediately to the RF
Release state, send all of Deact SACCH, RR Release messages and also signal the
lchan_rtp_fsm as appropriate.
Move that code to static lchan_do_release() and call from both
lchan_fsm_wait_rll_rtp_released_onenter() in the normal case, as well as
lchan_release() when skipping that.
When releasing in error, but we're already in LCHAN_ST_WAIT_RLL_RTP_RELEASED,
those messages have already been sent and we can skip them.
After commit , the code makes sure to disassociate lchan and conn before
invoking the lchan release. However, we only send RR Release if a conn is
present, which clearly is nonsense after .
 commit 8b818a01b00ea3daad4ad58c162ac52b4f08a5cb
"subscr conn: properly forget lchan before release"
Manage sending of RR Release via a flag, set during invoking lchan release.
Add do_rr_release arg to lchan_release(), gscon_release_lchans(). In
lchan_fsm.c, send RR Release only if do_rr_release was passed true; do not care
whether a conn is still associated (because it won't ever be since ).
That way we can intelligently decide what release process makes sense (whether
the lchan terminates the subscriber connection or whether the connection goes
on at another lchan), and still disassociate lchan and conn early.
BTW, this problem wasn't caught by the stock OsmoBSC TTCN3 tests, because the
f_expect_chan_rel() don't care whether an RR Release happens or not. This is
being fixed by Ibc64058f1e214bea585f4e8dcb66f3df8ead3845.
So far this patch should fix BSC_Tests_LCLS.TC_lcls_connect_clear.
When reading the log of OS#3686, I wished for explicit logging of when exactly
an lchan disassociates from a conn. Here is the debug logging I would have
liked to see.
I'm not sure whether we really need to merge this patch...
lchan_fsm_wait_rf_release_ack_onenter() calls gscon_forget_lchan(). At the
point where the conn has no lchan, the lchan must not have a conn. Make sure
that conn is NULL after gscon_forget_lchan().
I hope this fixes OS#3686, it is the only situation I could find where the
disassociation is potentially one-sided.
I get the feeling that this should be hardcoded in gscon_forget_lchan(), but I
dimly remember other situations that are less trivial, where it doesn't
necessarily make sense to forget reciprocically. So only fixing this one now.
Right below this call, we invoke lchan_reset(), and the first thing it does is
gscon_forget_lchan(). Drop this redundant invocation.
See following specs for related information:
* 3GPP TS 52.021 §8.11.3 Get Attribute Response
* 3GPP TS 52.021 §9.4.64 Get Attribute Response Info
Spec compliant format is defined in:
* 3GPP TS 52.021 §8.11.3 "Get Attribute Response"
* 3GPP TS 52.021 §9.4.64 "Get Attribute Response Info".
On nanoBTS, however, reported attribute list is provided directly inside/after
the foh header instead of being enveloped inside the Get Attributes Response Info.
Furthermore, The Get Attributes Response Info can still appear and be at any position
in the reported attribute list, and it only contains the unreported
attribute ID list inside.
nanoBTS actually supports regular formatting. There are a few differences
with spec though:
* The attributes are listed directly in the message instead of being inside
the Get Attributes Response Info after the unsupported attribute ID list.
* The Get Attributes Response Info can be at any position in the
attribute list, and it only contains the unsupported attribute ID list.
As a result, parsing is currently split into 3 main parts or functions:
* Parsing regular (per spec) Get Attributes Response Info attr and get a
pointer to the list of attributes.
* A function that parses the list of attributes, called directly in case
of nanoBTS, and called by the former parser of Get Attributes Response
Info for regular (per spec) OML endpoints.
* A function to parse the unsupported attribute ID list, also used in the
first function to get a pointer to the list of attributes.
* Allow sending Get Attributes message in abis_nm_get_attr.
* Don't try to decode Get Attribute Response Info for nanoBTS, since it
uses a different formatting than the one defined in specs.
its own func
nanoBTS uses same format for the attribute list from Attribute Response
Info, but without using the later as an evelope. By splitting we can
later reuse the code handling the Attribute list.
While at it, take the chance to remove functions parse_attr_resp_info_*
which expect TLVs following an specific order, which is not mandatory.
In future commits, nanoBTS support will be added, which implements its
own format not exactly equal to specs Attribute Response Info.
This function will be merged into another using a "len" variable. This
change makes diffs easier to follow in future patches.
The function cgi_for_msc() provides an easy way to get a cell global id
for an msc/bts combination. This function is currently statically
defined in gsm_08_08.c. Lets move it to gsm_data.c and make it publicly
Coverity points out that conditional checks in set_net_timezone()
depend on each other: The value of 'override' depends on 'hourstr'
being non-NULL. Nest these conditional checks such that this
dependency becomes obvious.
No functional change.
Move code using MDCX via MGCP into separate function to make adding
alternative MDCX variants easier.
These indicators are a legacy of early handover_decision_2.c work, where there
were no separate handover1 and handover2 config commands. No need to restate
the abundantly obvious anymore.
In 'show bts' command only display details of GPRS MO if GPRS is
configured for this BTS.
Fixes: coverity scan CID#189459
Print IDs and IPs of recently rejected BTS devices. Example output:
OsmoBSC> show rejected-bts
Date Site ID BTS ID IP
------------------- ------- ------ ---------------
2018-10-25 09:36:28 1234 0 192.168.1.37
The commit acd29192deed0e1a243b35278b030bde7b1662b5
introduced a bad change to neighbor_ident.vty. Revert that bit.
The chan mode is figured out per-BTS, but may remain uninitialized. Rather log
info about the channel request, like further above.
Separate the cause value passed to further functions from the log string.
The code tried to be nice by composing the RSL cause string and returning the
RSL cause at the same time, which falls on its face when the string composition
happens only within conditional logging.
bssmap_handle_cipher_mode() had code paths doing "goto reject" without
setting a meaningful cause value.