Commit Graph

534 Commits

Author SHA1 Message Date
Pau Espin 32e6710487 vty: command.c: Fix: single-choice optional args are no longer passed incomplete to vty func
For instance, take command "single0 [one]":
If user executes "single0 on", VTY func will receive argv[0]="one"
instead of argv[0]="on".

Related: OS#4045
Change-Id: I5f4e2d16c62a2d22717989c6acc77450957168cb
2019-06-14 17:44:21 +02:00
Pau Espin 7e1b03f763 vty: command.c: Fix: multi-choice args are no longer passed incomplete to vty func
For instance, take command "multi0 (one|two|three)":
If user executes "multi0 tw", VTY func will receive argv[0]="two"
instead of argv[0]="tw".

Fixes: OS#4045
Change-Id: I91b6621ac3d87fda5412a9b415e7bfb4736c8a9a
2019-06-14 17:44:16 +02:00
Pau Espin 14aadd5b6f vty: tests: Verify incomplete optional parameters are passed to vty funcs
The test shows that in the case were "single0 on" is executed, VTY
function should return complete "single0 one" but it doesn't.

Related: OS#4045
Change-Id: Ib5b9dc07e2b280dc95011b3926afb1d490cadd81
2019-06-14 12:43:19 +02:00
Pau Espin 6df2e44404 vty: command.c: Fix is_cmd_ambiguous() returning always 0
inner block defined variable "enum match_type ret" was being masking
outter block variable "int ret = 0". The ret variable was being given
non zero values only inside the inner block, so that change was done on
the inner variable and not the outer one, which is returned.

Fixes: 5314c513f2
Change-Id: Iec87d7db49a096d07e38ff8a060b923a52bfd6ba
2019-06-11 21:50:17 +02:00
Oliver Smith 186f878266 gsm48_decode_bcd_number2: fix ENOSPC edge case
Return ENOSPC if the decoding buffer is one byte too small, instead of
returning 0 and silently truncating the string. Add a new "truncated"
variable to detect if the loop breaks in the final iteration.

The string is not truncated if there is exactly one 0xf ('\0') higher
nibble remaining. This is covered by the existing test case "long
15-digit (maximum) MSISDN, limited buffer".

Related: OS#4049
Change-Id: Ie05900aca50cc7fe8a45d17844dbfcd905fd82fe
2019-06-07 11:01:51 +02:00
Vadim Yanitskiy 8c9befeaee vty_transcript_test.vty: add choice auto-complete tests
This patch is a result of discussion we had in [1]. The key idea
is that libosmovty should properly auto-complete the commands
containing choice, such as the following one:

  multi0 (one|two|three)

[1] If9b0c0d031477ca87786aab5c269d00748e896c8

Right now, sending the following command:

  (osmo-foo-bar)# multi0 th

would basically match the following vector:

  multi0 three

however the resulting argv would be:

  ["multi0", "th"]

Moreover, sending the following command:

  (osmo-foo-bar)# multi0 t

would basically match the following vectors:

  multi0 two
  multi0 three

because both start from 't', so the resulting argv would be:

  ["multi0", "t"]

which is ambiguous!

The expected output is:

  (osmo-foo-bar)# multi0 th
  ok argc=1 three

  (osmo-foo-bar)# multi0 t
  % Ambiguous command.

This is going to be fixed in the follow up patches.

Change-Id: I83c3aef813173952641035862c534ef16384780e
2019-06-06 06:26:30 +00:00
Vadim Yanitskiy 1dc82643c5 gsm48_encode_bcd_number(): clarify optional LHV header initialization
Change-Id: Iafd911dd55691b3715391e3899cd6971245c8d7f
2019-05-29 08:48:46 +00:00
Vadim Yanitskiy e4799f5603 gsm48_decode_bcd_number2(): return -EINVAL if LV has too big length
Change-Id: Ie07b2e8bc2f9628904e88448b4ee63b359655123
2019-05-28 06:50:41 +07:00
Vadim Yanitskiy 7194087457 gsm48_decode_bcd_number2(): fix: return -ENOSPC on truncation
The documentation of gsm48_decode_bcd_number2() clearly states that
the output truncation is a erroneous case, so it should actually
return negative in such cases. Let's return -ENOSPC.

Change-Id: I75680f232001ba419a587fed4c24f32c70c3ad2b
2019-05-28 06:50:41 +07:00
Vadim Yanitskiy 2cd1dda631 gsm48_decode_bcd_number2(): fix output truncation
Thanks to the new unit test for BCD number encoding / decoding, it was
discovered that gsm48_decode_bcd_number2() does not properly handle
encoded LV if the output buffer size is equal to the original MSISDN
length + 1 (\0-terminator): one digit is lost.

For example, decoding of 15-digit long MSISDN to a buffer of size
16 (15 digits + 1 for \0) would give us only 14 digits.

The problem was that 'output_len' was being decremented before
checking the remaining buffer length and writing a digit to it.
As a result, the maximum length was always one byte shorter.

Change-Id: I61d49387fedbf7b238e21540a5eff22f6861e27a
Fixes: OS#4025
2019-05-28 06:50:41 +07:00
Vadim Yanitskiy aa0683d9f8 gsm0408/gsm0408_test.c: introduce BCD number encoding / decoding test
So far, both gsm48_encode_bcd_number() and gsm48_decode_bcd_number2()
did not have any unit test coverage. Let's fill this gap by testing
the following scenarios:

  - encoding / decoding of a regular 9-digit MSISDN;
  - encoding / decoding of a MSISDN with optional LHV;
  - encoding / decoding of a long 15-digit MSISDN;
  - encoding / decoding of a MSISDN to a buffer:
    - with exactly matching size,
    - with lower size (truncation);
  - decoding LV buffer with incorrect length,
  - encoding / decoding an empty input buffer.

As it turns out, gsm48_decode_bcd_number2() does not properly
handle encoded LV if the output buffer size is equal to the
original MSISDN length + 1 (\0-terminator): one digit is lost.

For example, decoding of 15-digit long MSISDN to a buffer of size
16 (15 digits + 1 for \0) would give us only 14 digits. This is
reflected in the unit test output:

  Decoding HEX (buffer limit=16) '0821436587092143f5'...
    Expected: (rc=0) '123456789012345'
      Actual: (rc=0) '12345678901234'

Moreover, if the output buffer is shorter than decoded number,
gsm48_decode_bcd_number2() silently truncates it and returns 0,
while its description states, that the rc should reflect this.

To be fixed in the follow-up patches.

Change-Id: I4b2c330cf8ffe4427c0bee7d5f3b74be56ecd85d
Related: OS#4025
2019-05-28 06:50:41 +07:00
Harald Welte c53afd95c5 oap_client: Fix license: GPLv2+ instead of AGPLv3+
libosmo{core,gsm,vty} code is GPLv2+.  The OAP code originated in
osmo-msc.git and was moved here without changing the license. That
was a mistake, it always was meant to be under GPLv2-or-later after
moving to libosmocore.git.

Copyright is with sysmocom, so I as the managing director can
approve the license change.

Change-Id: I08311fa8214c15f8df8945b9894226608cf96f15
2019-05-27 23:26:45 +02:00
Harald Welte 8006f5393e TLV: Add one-shot TLV encoder
So far, the TLV code contained two types of functions
* tlp_parse() to parse all TLVs according to definition into tlvp_parsed
* various helper functions to encode individual TLVs during message
  generation

This patch implements the inverse of tlv_parse(): tlv_encode(), which
takes a full 'struct tlv_pared' and encodes all IEs found in it.  The
order of IEs is in numerically ascending order of the tag.

As many protocols have different IE/TLV ordering requirements, let's add
a tlv_encode_ordered() function where the caller can specify the TLV
ordering during the one-shot encode.

Change-Id: I761a30bf20355a9f80a4a8e0c60b0b0f78515efe
2019-05-19 07:33:32 +00:00
Alexander Couzens cc72cc45a4 add osmo_stat_item_inc/osmo_stat_item_dec to set it relative
Change-Id: Id2462c4866bd22bc2338c9c8f69b775f88ae7511
2019-05-07 13:20:57 +00:00
Oliver Smith affbb20646 GSUP: add inter-MSC handover related msgs and IEs
Based on a draft created by Neels, which is the result of reading a MAP
trace of two MSCs negotiating inter-MSC handovers, and of reading the
TS 29.002, TS 29.010 and related specs:
https://lists.osmocom.org/pipermail/openbsc/2019-January/012653.html

I figured out that the "Handover Number" mentioned in the specifications
is the same as the MSISDN IE that we already have, so we can use that
instead of creating a new IE (example usage in tests/gsup/gsup_test.c).

Create a new OSMO_GSUP_MSGT_E_ROUTING_ERROR message type, which the GSUP
server uses to tell a client that its message could not be forwarded to
the destination (see [1]). MAP has no related message.

[1]: Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 (osmo-hlr.git)

Related: OS#3774
Change-Id: Ic00b0601eacff6d72927cea51767801142ee75db
2019-04-26 20:36:39 +02:00
Neels Hofmeyr 9838c9070f GSUP: add Message Class IE
osmo-msc and osmo-hlr have distinct subsystems handling incoming GSUP messages.
So far we decide entirely by message type which code path should handle a GSUP
message. Thus no GSUP message type may be re-used across subsystems.

If we add a GSUP message to indicate a routing error, it would have to be a
distinct message type for subscriber management, another one for SMS, another
one for USSD...

To allow introducing common message types, introduce a GSUP Message Class IE.

In the presence of this IE, GSUP handlers can trivially direct a received
message to the right code path. If it is missing, handlers can fall back to the
previous switch(message_type) method.

Change-Id: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
2019-04-13 21:38:58 +00:00
Neels Hofmeyr ecef7ec3c3 add osmo_{escape,quote}_str_buf2() for standard args ordering
To be able to append an escaped or quoted string using
OSMO_STRBUF_APPEND_NOLEN(), the function signature must have the buf and len as
first args, like most other *_buf() functions.

Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.

A recent patch [1] has changed the return value of osmo_escape_str_buf() to
char*, removing the const. However, the functions may return const strings,
hence re-add the const. The new signatures always return the non-const buffer.

To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.

I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:

(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
  "this string is trunca"
and now this becomes, sic:
  "this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.

(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
  their content (i.e. code that usually writes out strings in full length
  "suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.

Adjust tests to accomodate above changes.

[1] 4a62eda225
    Ibf85f79e93244f53b2684ff6f1095c5b41203e05

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
2019-04-12 01:00:16 +02:00
Neels Hofmeyr 8531d6695f tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()
In OSMO_STRBUF_APPEND, use local variable names that are less likely to shadow
other local variables: prefix with _sb_.

In OSMO_STRBUF_APPEND, add a check to add to .pos only if it is not NULL.

Add OSMO_STRBUF_APPEND_NOLEN(), which works for function signatures that don't
return a length. This is useful for any osmo_*_buf() string writing functions,
so that these write directly to the strbuf.

Change-Id: I108cadf72deb3a3bcab9a07e50572d9da1ab0359
2019-04-11 07:40:25 +02:00
Neels Hofmeyr d79ccc65f7 add osmo_str_startswith()
Move from a static implementation in tdef_vty.c to utils.c, I also want to use
this in osmo-msc.

The point is that the telnet VTY allows unambiguous partly matches of keyword
args. For example, if I have a command definition of:

    compare (apples|oranges)

then it is perfectly legal as for the vty parser to write only

    compare app

One could expect the VTY to then pass the unambiguous match of "apples" to the
parsing function, but that is not the case.

Hence a VTY function implementation is faced with parsing a keyword of "app"
instead of the expected "apples".

This is actually a very widespread bug in our VTY implementations, which assume
that exactly one full keyword will always be found. I am now writing new
commands in a way that are able to manage only the starts of keywords.

Arguably, strstr(a, b) == a does the same thing, but it searches the entire
string unnecessarily.

Change-Id: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
2019-04-11 05:36:36 +00:00
Neels Hofmeyr d28aa0c2f1 fsm_dealloc_test: no need for ST_DESTROYING
A separate ST_DESTROYING state originally helped with certain deallocation
scenarios. But now that fsm.c avoids re-entering osmo_fsm_inst_term() twice and
gracefully handles FSM instance deallocations for termination cascades, it is
actually just as safe without a separate ST_DESTROYING state. ST_DESTROYING was
used to flag deallocation and prevent entering osmo_fsm_inst_term() twice,
which works only in a very limited range of scenarios.

Remove ST_DESTROYING from fsm_dealloc_test.c to show that all tested scenarios
still clean up gracefully.

Change-Id: I05354e6cad9b82ba474fa50ffd41d481b3c697b4
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 1f9cc01861 fsm: support graceful osmo_fsm_inst_term() cascades
Add global flag osmo_fsm_term_safely() -- if set to true, enable the following
behavior:

Detect osmo_fsm_inst_term() occuring within osmo_fsm_inst_term():
- collect deallocations until the outermost osmo_fsm_inst_term() is done.
- call osmo_fsm_inst_free() *after* dispatching the parent event.

If a struct osmo_fsm_inst enters osmo_fsm_inst_term() while another is already
within osmo_fsm_inst_term(), do not directly deallocate it, but talloc-reparent
it to a separate talloc context, to be deallocated with the outermost FSM inst.

The effect is that all osmo_fsm_inst freed within an osmo_fsm_inst_term()
cascade will stay allocated until all osmo_fsm_inst_term() are complete and all
of them will be deallocated at the same time.

Mark the deferred deallocation state as __thread in an attempt to make cascaded
deallocation handling threadsafe.  Keep the enable/disable flag separate, so
that it is global and not per-thread.

The feature is showcased by fsm_dealloc_test.c: with this feature, all of those
wild deallocation scenarios succeed.

Make fsm_dealloc_test a normal regression test in testsuite.at.

Rationale:

It is difficult to gracefully handle deallocations of groups of FSM instances
that reference each other. As soon as one child dispatching a cleanup event
causes its parent to deallocate before fsm.c was ready for it, deallocation
will hit a use-after-free. Before this patch, by using parent_term events and
distinct "terminating" FSM states, parent/child FSMs can be taught to wait for
all children to deallocate before deallocating the parent. But as soon as a
non-child / non-parent FSM instance is involved, or actually any other
cleanup() action that triggers parent FSMs or parent talloc contexts to become
unused, it is near impossible to think of all possible deallocation events
ricocheting, and to avoid running into freeing FSM instances that were still in
the middle of osmo_fsm_inst_term(), or FSM instances to enter
osmo_fsm_inst_term() more than once. This patch makes deallocation of "all
possible" setups of complex cross referencing FSM instances easy to handle
correctly, without running into use-after-free or double free situations, and,
notably, without changing calling code.

Change-Id: I8eda67540a1cd444491beb7856b9fcd0a3143b18
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 3b414a4adc fsm: add flag to ensure osmo_fsm_inst_term() happens only once
To prevent re-entering osmo_fsm_inst_term() twice for the same osmo_fsm_inst,
add flag osmo_fsm_inst.proc.terminating. osmo_fsm_inst_term() sets this to
true, or exits if it already is true.

Update fsm_dealloc_test.err for illustration. It is not relevant for unit
testing yet, just showing the difference.

Change-Id: I0c02d76a86f90c49e0eae2f85db64704c96a7674
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 223d66a414 add fsm_dealloc_test.c
Despite efforts to properly handle "GONE" events and entering a ST_DESTROYING
only once, so far this test runs straight into a heap use-after-free. With
current fsm.c, it is hard to resolve the situation with the objects named
"other" also causing deallocations besides the FSM instance parent/child
relations.

For illustration, add an "expected" test output file fsm_dealloc_test.err,
making this pass will follow in a subsequent patch.

Change-Id: If801907c541bca9f524c9e5fd22ac280ca16979a
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 0e8df1c7e4 add osmo_use_count API
Provide a common implementation of use counting that supports naming each user
as well as counting more than just one use per user, depending on the rules the
caller implies.

In osmo-msc, we were originally using a simple int counter to see whether a
connection is still in use or should be discarded. For clarity, we later added
names to each user in the form of a bitmask of flags, to figure out exactly
which users are still active: for logging and to debug double get / double put
bugs. This however is still not adequate, since there may be more than one CM
Service Request pending. Also, it is a specialized implementation that is not
re-usable.

With this generalized implementation, we can:

- fix the problem of inadequate counting of multiple concurrent CM Service
  Requests (more than one use count per user category),
- directly use arbitrary names for uses like __func__ or "foo" (no need to
  define enums and value_string[]s),
- re-use the same code for e.g. vlr_subscr and get fairly detailed VLR
  susbscriber usage logging for free.

Change-Id: Ife31e6798b4e728a23913179e346552a7dd338c0
2019-04-08 13:47:17 +00:00
Neels Hofmeyr 0c7826e9bd add osmo_sockaddr_str API
For handling RTP IP addresses and ports, osmo-mgw, osmo-bsc and osmo-msc
so far have their own separate shims and code duplication around
inet_ntoa(), htons(), sockaddr conversions etc. Unify and standardize
with this common API.

In the MGW endpoint FSM that was introduced in osmo-bsc and which I
would like to re-use for osmo-msc (upcoming patch moving that to
osmo-mgw), it has turned out that using char* IP address and uint16_t
port number types are a convenient common denominator for logging,
MGCP message composition and GSM48. Ongoing osmo-msc work also uses this
for MNCC.

This is of course potentially useful for any other IP+port combinations
besides RTP stream handling.

Needless to say that most current implementations will probably stay
with their current own conversion code for a long time; for current
osmo-{bsc,msc,mgw} work (MGW endpoint FSM) though, I would like to move
to this API here.

Change-Id: Id617265337f09dfb6ddfe111ef5e578cd3dc9f63
2019-04-08 13:47:17 +00:00
Harald Welte 98ed3393cd osmo_escape_str_buf: Always copy, don't return input string pointer
osmo_escape_str_buf() used to have the somewhat odd semantics that
if no escaping was needed, it would return the original pointer without
making any copy to the output buffer.  While this seems like an elegant
optimization, it is a very strange behavior and it works differently
than all of our other *_buf() functions.  Let's unify the API and
turn osmo_escape_str_buf() into a strlcpy() if no escaping is needed.

Change-Id: I3a02bdb27008a73101c2db41ac04248960ed4064
2019-03-29 16:42:25 +00:00
Harald Welte d488c7256f tests: use -no-install libtool flag to avoid ./lt-* scripts
This ensures that the rpath of the generated binaries is set to
use only the just-compiled libosmo{core,gsm,vty}.so and not any
system-wide installed libraries while avoiding the ugly shell script
wrapper.

Change-Id: I9b9ae0ed277ba71519661a66a70b7f86971e4511
2019-03-19 00:06:35 +01:00
Philipp Maier 94d79fdeef gsm0808_utils: fix gsm48 multirate to S-bit converter
The function gsm0808_sc_cfg_from_gsm48_mr_cfg() is used to convert a
gsm48 multirate struct into a set of S-bits (S0 to S15). However, the
conversion function currently does not take into account that bit S1
actually stands for four rates at once (Config-NB-Code = 1). Lets make
sure that S1 is only set when the multirate configuration permits all
four required rates.

Change-Id: I6ad531d4e70c2252e32e2bbaca8e14a7ec6d9840
Related: SYS#4470
2019-03-11 09:08:31 +01:00
Philipp Maier 3713af8655 gsm0808_utils: fix gsm48 multirate configuration generator
The function gsm0808_sc_cfg_from_gsm48_mr_cfg() takes an S15 to S0
bitmask and converts that bitmask into an AMR multirate configuration
struct.

Unfortunately the current implementation implements 3GPP TS 28.062,
Table 7.11.3.1.3-2 wrongly in some aspects. Lets fix this.

- Fix wrong interpretation of the bitpatterns
- 5,15K is invalid and must never be selected
- Make sure that no more than 4 rates are selected in the active set
- Extend unit-test

Change-Id: I6fd7f4073b84093742c322752f2fd878d1071e15
Related: SYS#4470
2019-03-11 09:08:31 +01:00
Neels Hofmeyr 3a5045302f add gsm0808_cell_id_from_cgi(), gsm0808_cell_id_to_cgi()
CGI to Cell ID: for example, for Paging, osmo-msc has a CGI for a subscriber
and needs to send out a Cell Identifier IE. Makes sense to add this conversion
here.

Cell ID to CGI: for a Layer 3 Complete, a subscriber sends the current cell in
the form of a Cell Identifier, which we store as a CGI, if necessary enriched
with the local PLMN.

Add enum with bitmask values to identify parts of a CGI, for the return value
of gsm0808_cell_id_to_cgi(). Can't use enum CELL_IDENT for that, because it
doesn't have a value for just a PLMN (and is not a bitmask).

Change-Id: Ib9af67b100c4583342a2103669732dab2e577b04
2019-03-08 05:25:07 +01:00
Neels Hofmeyr d4b79c8772 fsm: add osmo_fsm_inst_state_chg_keep_or_start_timer()
During FSM design for osmo-msc, I noticed that the current behavior that
keep_timer=true doesn't guarantee a running timer can make FSM design a bit
complex, especially when using osmo_tdef for timeout definitions.

A desirable keep_timer=true behavior is one that keeps the previous timer
running, but starts a timer if no timer is running yet.

The simplest example is: a given state repeatedly transitions back to itself,
but wants to set a timeout only on first entering, avoiding to restart the
timeout on re-entering.

Another example is a repeated transition between two or more states, where the
first time we enter this group a timeout should start, but it should not
restart from scratch on every transition.

When using osmo_tdef timeout definitions for this, so far separate meaningless
states have to be introduced that merely set a fixed timeout.

To simplify, add osmo_fsm_inst_state_chg_keep_or_start_timer(), and use this in
osmo_tdef_fsm_inst_state_chg() when both keep_timer == true *and* T != 0.

In tdef_test.ok, the changes show that on first entering state L, the previous
T=1 is now kept with a large remaining timeout. When entering state L from O,
where no timer was running, this time L's T123 is started.

Change-Id: Id647511a4b18e0c4de0e66fb1f35dc9adb9177db
2019-03-07 23:10:21 +01:00
Neels Hofmeyr 4ea698233a fix tdef_test.c: do call the function-to-test in all cases
Always call osmo_tdef_fsm_inst_state_chg(), also when no timeout is defined.

When there is no timeout defined for a state, tdef_test.c tries to be smart and
print different output. In that mess, I missed the fact that
osmo_tdef_fsm_inst_state_chg() isn't always called as it should. In the same
mess, the resulting state was never printed until the preceding patch, which
helped to hide this bug.

Change-Id: I1d953d99854422bff8eb32f051e9c6147bc836b6
2019-03-06 07:00:30 +01:00
Neels Hofmeyr 4f54c6c48d tdef_test: tweak output to prepare for a fix
- Always print the state after a state transition.
  This shows that actually state transitions are missing for states that have
  no timer defined. This is a bug in tdef_test.c, to be fixed subsequently.

- Instead of total time passed since start, print the individual fake time
  intervals. Omit initial useless zero fake time advance.

- Add two more state transitions, back out from and into a state that has no
  timeout set.

Change-Id: Icb31af96d37741e256ff07868f3d4f5c48cdda74
2019-03-06 06:59:47 +01:00
Neels Hofmeyr 5734bff3b0 represent negative T-timers as Osmocom-specific X-timers
fi->T values are int, i.e. can be negative. Do not log them as unsigned, but
define a distinct timer class "Xnnnn" for negative T values: i.e. for T == -1,
print "Timeout of X1" instead of "Timeout of T4294967295".

The negative T timer number space is useful to distinguish freely invented
timers from proper 3GPP defined T numbers. So far I was using numbers like
T993210 or T9999 for invented T, but X1, X2 etc. is a better solution. This way
we can make sure to not accidentally define an invented timer number that
actually collides with a proper 3GPP specified timer number that the author was
not aware of at the time of writing.

Add OSMO_T_FMT and OSMO_T_FMT_ARGS() macros as standardized timer number print
format. Use that in fsm.c, tdef_vty.c, and adjust vty tests accordingly.

Mention the two timer classes in various API docs and VTY online-docs.

Change-Id: I3a59457623da9309fbbda235fe18fadd1636bff6
2019-03-06 00:51:15 +01:00
Max a478590250 coding: check gsm0503_rach_*() results
Check return value of RACH encode/decode functions and fail test on
unexpected results.

Change-Id: I41bfa808e3c064a11152e7ce8ee77a01d38a0744
Related: OS#1854
2019-03-05 11:30:39 +01:00
Neels Hofmeyr 050f2d3259 log: fsm: allow logging the timeout on state change
Add a flag that adds timeout info to osmo_fsm_inst state change logging.

To not affect unit testing, make this an opt-in feature that is disabled by
default -- mostly because osmo_fsm_inst_state_chg_keep_timer() will produce
non-deterministic logging depending on timing (logs remaining time).

Unit tests that don't verify log output and those that use fake time may also
enable this feature. Do so in fsm_test.c.

The idea is that in due course we will add osmo_fsm_log_timeouts(true) calls to
all of our production applications' main() initialization.

Change-Id: I089b81021a1a4ada1205261470da032b82d57872
2019-02-26 20:57:58 +00:00
Harald Welte 35042a2919 NS: Factor out gprs_nsvc_start_test() and use it
This function performs  sending a NS-ALIVE PDU and starting Tns-Test,
let's use it in all places where we used to do that.

As part of this, also fix a bug where the sendto() return value (number
of bytes sent) would actually propagate up all the way to
gprs_ns_rx_reset() return value, which in turn affects the test results
on stdout.

Change-Id: I4d303117f77fabb74bbb91887b9914a81c2a084a
2019-02-26 12:18:30 +01:00
Max 5ec0cf5475 LCLS: add string dump helpers
Add functions to dump LCLS (without GCR) and GCR. Dumping entire struct
results in inconveniently long string hence the separate functions. Both
use talloc functions so they expect caller to take care of providing
proper allocation context and freeing memory.

Change-Id: Ic3609224c8f3282d667e75f68bc20327e36eb9e6
2019-02-26 09:16:38 +00:00
Harald Welte cf665fc6bc gsm0808: Add unit tests for test_create_clear_command2()
Change-Id: Ie3f34b78edc91a013152742bebbd839586a787fe
Related: OS#3805
2019-02-18 13:34:02 +00:00
Neels Hofmeyr 2cbe25f484 add OSMO_STRBUF_PRINTF()
We are using macros like this or different workarounds in libmsc. In the course
of implementing inter-MSC handover, I am encountering yet another such
situation of appending multiple strings to a limited char buffer. Standardize.

Add a unit test to utils_test.c.

Change-Id: I2497514e26c5e7a5d88985fc7e58343be1a027b2
2019-02-17 20:40:17 +00:00
Max 450f5ac910 Enable remote SIM protocol log level
It's defined in logging.h for quite some time but is not actually
enabled alongside with other internal logging categories.

Change-Id: I0e7a2add6293a072752900608c8ba34cc3850f31
2019-02-14 19:19:57 +01:00
Neels Hofmeyr 7b740f72c3 platform independence fix: tdef range tests
Run INT_MAX and ULONG_MAX related tests only manually, remove from automatic
testing. This will hopefully fix recent build failures on various platforms.

Add a 64 bit output example for expected results when invoking
`./tdef_test range'. This is not checked automatically and merely serves for
manual reference.

For vty tests, use 32bit max values instead of INT_MAX and ULONG_MAX.

Change-Id: I6242243bde1d7ddebb858512a1f0b07f4ec3e5c2
2019-02-06 01:05:37 +00:00
Harald Welte b0708d3e76 bitvec: Add bitvec_tailroom_bits() function
This is similar to msgb_tailroom(): It returns the amount of space
left at the end of the bit vector (compared to the current cursor).

The function returns the number of bits left in the bitvec.

Change-Id: I8980a6b6d1973b67a2d9ad411c878d956fb428d1
2019-02-05 11:16:44 +01:00
Harald Welte ae7966d145 bitvec: Add bitvec_bytes_used() function
This new bitvec API function returns the number of bytes used in a given
bit-vector.

Change-Id: Id4bd7f7543f5b0f4f6f876e283bd065039c37646
2019-02-05 09:24:17 +00:00
Neels Hofmeyr 0fd615fd7b add osmo_tdef API, originally adopted from osmo-bsc T_def
Move T_def from osmo-bsc to libosmocore as osmo_tdef. Adjust naming to be more
consistent. Upgrade to first class API:
- add timer grouping
- add generic vty support
- add mising API doc
- add C test
- add VTY transcript tests, also as examples for using the API

From osmo_fsm_inst_state_chg() API doc, cross reference to osmo_tdef API.

The root reason for moving to libosmocore is that I want to use the
mgw_endpoint_fsm in osmo-msc for inter-MSC handover, and hence want to move the
FSM to libosmo-mgcp-client. This FSM uses the T_def from osmo-bsc. Though the
mgw_endpoint_fsm's use of T_def is minimal, I intend to use the osmo_tdef API
in osmo-msc (and probably elsewhere) as well. libosmocore is the most sensible
place for this.

osmo_tdef provides:

- a list of Tnnnn (GSM) timers with description, unit and default value.
- vty UI to allow users to configure non-default timeouts.
- API to tie T timers to osmo_fsm states and set them on state transitions.

- a few standard units (minute, second, millisecond) as well as a custom unit
  (which relies on the timer's human readable description to indicate the
  meaning of the value).
- conversion for standard units: for example, some GSM timers are defined in
  minutes, while our FSM definitions need timeouts in seconds. Conversion is
  for convenience only and can be easily avoided via the custom unit.

By keeping separate osmo_tdef arrays, several groups of timers can be kept
separately. The VTY tests in tests/tdef/ showcase different schemes:

- tests/vty/tdef_vty_test_config_root.c:
  Keep several timer definitions in separately named groups: showcase the
  osmo_tdef_vty_groups*() API. Each timer group exists exactly once.

- tests/vty/tdef_vty_test_config_subnode.c:
  Keep a single list of timers without separate grouping.
  Put this list on a specific subnode below the CONFIG_NODE.
  There could be several separate subnodes with timers like this, i.e.
  continuing from this example, sets timers could be separated by placing
  timers in specific config subnodes instead of using the global group name.

- tests/vty/tdef_vty_test_dynamic.c:
  Dynamically allocate timer definitions per each new created object.
  Thus there can be an arbitrary number of independent timer definitions, one
  per allocated object.

T_def was introduced during the recent osmo-bsc refactoring for inter-BSC
handover, and has proven useful:

- without osmo_tdef, each invocation of osmo_fsm_inst_state_chg() needs to be
  programmed with the right timeout value, for all code paths that invoke this
  state change. It is a likely source of errors to get one of them wrong.  By
  defining a T timer exactly for an FSM state, the caller can merely invoke the
  state change and trust on the original state definition to apply the correct
  timeout.

- it is helpful to have a standardized config file UI to provide user
  configurable timeouts, instead of inventing new VTY commands for each
  separate application of T timer numbers.

Change-Id: Ibd6b1ed7f1bd6e1f2e0fde53352055a4468f23e5
2019-02-04 18:52:16 +01:00
Neels Hofmeyr b55f4d2df2 vty: enable optional-multi-choice syntax: [(one|two)]
Since very recently we sensibly handle commands like

  cmd ([one]|[two]|[three])

as optional multi-choice arguments. In addition, support the more obvious
syntax of

  cmd [(one|two|three)]

Internally, the tokens are mangled to [one] [two] and [three], which is how the
rest of the code detects optional args, and makes sense in terms of UI:

  > cmd ?
  [one]
  [two]
  [three]

(i.e. optional arguments are always shown in braces in '?' listings)

Before this patch, commands defined with a syntax like [(one|two)], would lead
to an assertion (shows as "multiple") during program startup.

Change-Id: I952b3c00f97e2447f2308b0ec6f5f1714692b5b2
2019-02-04 16:43:57 +00:00
Neels Hofmeyr c197809deb vty: enable optional-multi-choice syntax: ([one]|[two])
Add basic optional multi-choice argument support.

The VTY detects optional arguments by square braces.

 > cmd ?
 [optional-arg]
 > cmd optional-arg
 ok
 > cmd
 ok

However, within multi-choice args, these braces were so far not treated as
optional:

 > list
 cmd2 ([one]|[two]|[three])
 > cmd2
 % Command incomplete

In preparation for I952b3c00f97e2447f2308b0ec6f5f1714692b5b2 which will enable
the more obvious syntax of

  cmd [(one|two)]

for reasons of internal implementation, first support a syntax of

  cmd ([one]|[two])

The internal vty implementation always needs square braces around each option.
There is currently no good way to prevent developers from defining braces
inside multi-arguments, so it is easiest to allow and handle them:

 > list
 cmd2 ([one]|[two]|[three])
 > cmd2
 ok

The VTY doesn't guard against a mix like

 cmd (one|[two])

With this patch, a multi-choice command is treated as optional iff the first
element is in square brackets. The remaining elements' square brackets have no
effect besides confusing the user. This is not explicitly checked against.

In general, I would prefer to check all of these details, but the current VTY
code with its endless code duplication and obscure string mangling just doesn't
provide that luxury. There are numerous worse errors hidden in there.

Change-Id: I9a8474bd89ddc2155c58bfca7bd038d586aaa60a
2019-02-04 16:43:57 +00:00
Neels Hofmeyr faa49e24cb add generic vty_transcript_test.c, vty_transcript_test.vty
I want to tweak general VTY features and need to cover with a transcript test
to show the differences. Start by showing the current situation of optional
and multi-choice arguments.

Change-Id: I5a79c83fabd02aba6406b6e0d620969c4bd0cc1d
2019-02-04 16:43:57 +00:00
Neels Hofmeyr bd5a1dc84f osmo_fsm_inst_state_chg(): set T also for zero timeout
Before this patch, if timeout_secs == 0 was passed to
osmo_fsm_inst_state_chg(), the previous T value remained set in the
osmo_fsm_inst->T.

For example:

  osmo_fsm_inst_state_chg(fi, ST_X, 23, 42);
  // timer == 23 seconds; fi->T == 42
  osmo_fsm_inst_state_chg(fi, ST_Y, 0, 0);
  // no timer; fi->T == 42!

Instead, always set to the T value passed to osmo_fsm_inst_state_chg().

Adjust osmo_fsm_inst_state_chg() API doc; need to rephrase to accurately
describe the otherwise unchanged behaviour independently from T.

Verify in fsm_test.c.

Rationale: it is confusing to have a T number remaining from some past state,
especially since the user explicitly passed a T number to
osmo_fsm_inst_state_chg(). (Usually we are passing timeout_secs=0, T=0).

I first thought this behavior was introduced with
osmo_fsm_inst_state_chg_keep_timer(), but in fact osmo_fsm_inst_state_chg()
behaved this way from the start.

This shows up in the C test for the upcoming tdef API, where the test result
printout was showing some past T value sticking around after FSM state
transitions. After this patch, there will be no such confusion.

Change-Id: I65c7c262674a1bc5f37faeca6aa0320ab0174f3c
2019-01-29 10:25:26 +00:00
Neels Hofmeyr 760c58f226 build: add missing logging_vty_test.vty to EXTRA_DIST
It was introduced and forgotten to add to EXTRA_DIST in:

"logging vty: add VTY transcript test"
commit 3a9ff11e57
change-Id I948e832a33131f8eab98651d6010ceb0ccbc9a9c

Change-Id: I1bcedf3097f02b2adc679560d1cbceb27dbc345e
2019-01-29 01:02:01 +01:00