[PATCH] osmo-bsc[master]: doc/examples: tweak osmo-bsc.cfg, add osmo-bsc_custom-sccp.cfg

[Neels Hofmeyr]

Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/3832

to look at the new patch set (#3).

doc/examples: tweak osmo-bsc.cfg, add osmo-bsc_custom-sccp.cfg

Now osmo-bsc.cfg's SCCP addresses work by internal defaults, while
osmo-bsc_custom-sccp.cfg shows how to use custom STP IP address and SCCP point
codes.

Change-Id: Icb41d5adc24b2ee5613be691a201df8f3566e5dd
---
M doc/examples/osmo-bsc/osmo-bsc.cfg
A doc/examples/osmo-bsc/osmo-bsc_custom-sccp.cfg
M osmoappdesc.py
3 files changed, 128 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/32/3832/3

diff --git a/doc/examples/osmo-bsc/osmo-bsc.cfg 
b/doc/examples/osmo-bsc/osmo-bsc.cfg
index 7c10e9d..47123ec 100644
--- a/doc/examples/osmo-bsc/osmo-bsc.cfg
+++ b/doc/examples/osmo-bsc/osmo-bsc.cfg
@@ -1,11 +1,5 @@
-!
-! OsmoBSC (0.9.14+gitr1+3d331c0062bb0c9694dbd4d1eab7adc58138c3ae) 
configuration saved from vty
-!!
-password foo
-!
-!
-line vty
- no login
+! osmo-bsc default configuration
+! (assumes STP to run on 127.0.0.1 and uses default point codes)
 !
 e1_input
  e1_line 0 driver ipa
@@ -15,12 +9,14 @@
  short name OsmoBSC
  long name OsmoBSC
  auth policy closed
+ authorized-regexp .*
  location updating reject cause 13
  encryption a5 0
- neci 1
+ authentication optional
+ neci 0
  paging any use tch 0
  rrlp mode none
- mm info 1
+ mm info 0
  handover 0
  handover window rxlev averaging 10
  handover window rxqual averaging 1
@@ -28,21 +24,25 @@
  handover power budget interval 6
  handover power budget hysteresis 3
  handover maximum distance 
+ dyn_ts_allow_tch_f 0
+ periodic location update 30
  bts 0
-  type nanobts
+  type sysmobts
   band DCS1800
   cell_identity 0
   location_area_code 1
-  training_sequence_code 7
   base_station_id_code 63
   ms max power 15
   cell reselection hysteresis 4
   rxlev access min 0
+  radio-link-timeout 32
   channel allocator ascending
   rach tx integer 9
   rach max transmission 7
-  dtx uplink force
-  dtx downlink
+  channel-descrption attach 1
+  channel-descrption bs-pa-mfrms 5
+  channel-descrption bs-ag-blks-res 1
+  early-classmark-sending forbidden
   ip.access unit_id 0 0
   oml ip.access stream_id 255 line 0
   neighbor-list mode manual-si5
@@ -50,12 +50,14 @@
   neighbor-list add arfcn 200
   si5 neighbor-list add arfcn 10
   si5 neighbor-list add arfcn 20
+  codec-support fr
   gprs mode none
+  no force-combined-si
   trx 0
rf_locked 0
arfcn 871
nominal power 23
-   max_power_red 20
+   max_power_red 0
rsl e1 tei 0
timeslot 0
 phys_chan_config CCCH+SDCCH4
@@ -81,21 +83,24 @@
timeslot 7
 phys_chan_config TCH/F
 hopping enabled 0
-cs7 instance 1
- point-code 3.0.0
- sccp-address bsc_local
-  point-code 3.0.0
- sccp-address msc_remote
-  point-code 1.0.0
-msc
- bsc-addr bsc_local
- msc-addr msc_remote
+msc 0
  ip.access rtp-base 4000
  timeout-ping 20
  timeout-pong 5
- dest 192.168.100.11  0
- access-list-name msc-list
- no access-list-name
+ no timeout-ping advanced
+ no bsc-welcome-text
+ no bsc-msc-lost-text
+ no bsc-grace-text
+ type normal
+ allow-emergency allow
+ amr-config 12_2k forbidden
+ amr-config 10_2k forbidden
+ amr-config 7_95k forbidden
+ amr-config 7_40k forbidden
+ amr-config 6_70k forbidden
+ amr-config 5_90k allowed
+ amr-config 5_15k forbidden
+ amr-config 4_75k forbidden
 bsc
- no access-list-name
- access-list-name bsc-list
+ mid-call-timeout 0
+ no missing-msc-text
diff --git a/doc/examples/osmo-bsc/osmo-bsc_custom-sccp.cfg 
b/doc/examples/osmo-bsc/osmo-bsc_custom-sccp.cfg
new file mode 100644
index 000..f9c0a3c
--- /dev/null
+++ b/doc/examples/osmo-bsc/osmo-bsc_custom-sccp.cfg
@@ -0,0 +1,92 @@
+! osmo-bsc configuration example for custom SCCP addresses
+!
+e1_input
+ e1_line 0 driver ipa
+network
+ network country code 1
+ mobile network code 1
+ short name OsmoBSC
+ long name OsmoBSC
+ auth policy closed
+ authorized-regexp .*
+ location updating reject cause 13
+ encryption a5 0
+ authentication optional
+ neci 0
+ paging any use tch 0
+ rrlp mode none
+ mm info 0
+ handover 0
+ handover window rxlev averaging 10
+ handover window rxqual averaging 1
+ handover window rxlev neighbor averaging 10
+ handover power budget interval 6
+ handover power budget hysteresis 3
+ handover maximum distance 
+ dyn_ts_allow_tch_f 0
+ periodic location update 30
+ bts 0
+  type sysmobts
+  band DCS1800
+  cell_identity 0
+  location_area_code 1
+  base_station_id_code 63
+  ms max power 15
+  cell reselection hysteresis 4
+  rxlev access min 0
+  radio-link-timeout 32
+  channel allocator ascending
+  rach tx integer 9
+  rach max transmission 7
+  channel-descrption attach 1
+  channel-descrption bs-pa-mfrms 5
+  channel-descrption bs-ag-blks-res 1
+  early-classmark-sending forbidden
+  ip.access unit_id 0 0
+  oml ip.access stream_id 255 line 0
+  neighbor-list mode manual-si5
+  neighbor-list add arfcn 100
+  

osmo-bsc[master]: doc/examples: tweak osmo-bsc.cfg, add osmo-bsc_custom-sccp.cfg

[Neels Hofmeyr]

Patch Set 3: Code-Review+2

filename fixed from *scpp* to *sccp*, otherwise identical to previous +2

-- 
To view, visit https://gerrit.osmocom.org/3832
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb41d5adc24b2ee5613be691a201df8f3566e5dd
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: No

[PATCH] osmo-bsc[master]: fix vty tests: vty no longer goes to parent node implicitly

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/3902

fix vty tests: vty no longer goes to parent node implicitly

Fix three instances of VTY tests working because it used to include implicit
'exit' commands to the parent node.

Since libosmocore change-id Id73cba2dd34676bad8a130e9c45e67a272f19588 = commit
d64b6aed235f6e4d84a2cb8e84b32c3179260254, we no longer do this implicit-exit in
interactive VTY shells.

*) in testPingPongConfiguration, the intention is to enter the /msc 0 node.
   Drop prior entry of the 'network' node, which looks like an oversight. So
   far the 'msc 0' caused an implicit 'exit' and thus worked, now fails.

*) Two instances following comments "# Check searching for outer node's
   commands", which look like they are intended to check for this implicit-exit
   behavior. This is obsolete, drop those parts of the tests.

Change-Id: I77931d6a09c42c443c6936000592f22a7fd06cab
---
M tests/vty_test_runner.py
1 file changed, 0 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/02/3902/1

diff --git a/tests/vty_test_runner.py b/tests/vty_test_runner.py
index 1727e53..3ecfd39 100644
--- a/tests/vty_test_runner.py
+++ b/tests/vty_test_runner.py
@@ -139,14 +139,6 @@
 self.assertTrue(self.vty.verify("exit", ['']))
 self.assertTrue(self.vty.node() is None)
 
-# Check searching for outer node's commands
-self.vty.command("configure terminal")
-self.vty.command('msc 0')
-self.vty.command("bsc")
-self.assertEquals(self.vty.node(), 'config-bsc')
-self.vty.command("msc 0")
-self.assertEquals(self.vty.node(), 'config-msc')
-
 def testUssdNotificationsMsc(self):
 self.vty.enable()
 self.vty.command("configure terminal")
@@ -249,7 +241,6 @@
 def testPingPongConfiguration(self):
 self.vty.enable()
 self.vty.verify("configure terminal", [''])
-self.vty.verify("network", [''])
 self.vty.verify("msc 0", [''])
 
 self.vty.verify("timeout-ping 12", [''])
@@ -389,19 +380,6 @@
 self.assertEquals(self.vty.node(), 'config')
 self.assertTrue(self.vty.verify('exit', ['']))
 self.assertTrue(self.vty.node() is None)
-
-# Check searching for outer node's commands
-self.vty.command('configure terminal')
-self.vty.command('mgcp')
-self.vty.command('nat')
-self.assertEquals(self.vty.node(), 'config-nat')
-self.vty.command('mgcp')
-self.assertEquals(self.vty.node(), 'config-mgcp')
-self.vty.command('nat')
-self.assertEquals(self.vty.node(), 'config-nat')
-self.vty.command('bsc 0')
-self.vty.command('mgcp')
-self.assertEquals(self.vty.node(), 'config-mgcp')
 
 def testRewriteNoRewrite(self):
 self.vty.enable()

-- 
To view, visit https://gerrit.osmocom.org/3902
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I77931d6a09c42c443c6936000592f22a7fd06cab
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 

[PATCH] libosmo-sccp[master]: vty: add 'asp' / 'local-ip' command

[Neels Hofmeyr]

Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/3826

to look at the new patch set (#2).

vty: add 'asp' / 'local-ip' command

We can set the ASP's remote IP (i.e. where to reach osmo-stp), but so far the
only way to specify the local IP address to bind to can only be set from C code
(e.g. the simple client). Allow setting the local address via VTY.

For example, this is desired for the osmo-gsm-tester, to not use arbitrary IP
addresses.

Change-Id: I3f71897dfacafcf3126e51894d6ca756b02dcd7d
---
M src/osmo_ss7_vty.c
1 file changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/26/3826/2

diff --git a/src/osmo_ss7_vty.c b/src/osmo_ss7_vty.c
index 1fb35a1..8d13865 100644
--- a/src/osmo_ss7_vty.c
+++ b/src/osmo_ss7_vty.c
@@ -556,6 +556,16 @@
return CMD_SUCCESS;
 }
 
+DEFUN(asp_local_ip, asp_local_ip_cmd,
+   "local-ip A.B.C.D",
+   "Specify Local IP Address from which to contact ASP\n"
+   "Local IP Address from which to contact of ASP\n")
+{
+   struct osmo_ss7_asp *asp = vty->index;
+   osmo_talloc_replace_string(asp, >cfg.local.host, argv[0]);
+   return CMD_SUCCESS;
+}
+
 DEFUN(asp_remote_ip, asp_remote_ip_cmd,
"remote-ip A.B.C.D",
"Specify Remote IP Address of ASP\n"
@@ -633,6 +643,8 @@
osmo_ss7_asp_protocol_name(asp->cfg.proto), VTY_NEWLINE);
if (asp->cfg.description)
vty_out(vty, "  description %s%s", asp->cfg.description, 
VTY_NEWLINE);
+   if (asp->cfg.local.host)
+   vty_out(vty, "  local-ip %s%s", asp->cfg.local.host, 
VTY_NEWLINE);
if (asp->cfg.remote.host)
vty_out(vty, "  remote-ip %s%s", asp->cfg.remote.host, 
VTY_NEWLINE);
if (asp->cfg.qos_class)
@@ -1728,6 +1740,7 @@
install_element(L_CS7_NODE, _cs7_asp_cmd);
install_element(L_CS7_ASP_NODE, _description_cmd);
install_element(L_CS7_ASP_NODE, _remote_ip_cmd);
+   install_element(L_CS7_ASP_NODE, _local_ip_cmd);
install_element(L_CS7_ASP_NODE, _qos_class_cmd);
install_element(L_CS7_ASP_NODE, _block_cmd);
install_element(L_CS7_ASP_NODE, _shutdown_cmd);

-- 
To view, visit https://gerrit.osmocom.org/3826
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f71897dfacafcf3126e51894d6ca756b02dcd7d
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder

osmo-gsm-tester[master]: modem: Fix race condition when connect() is called more than...

[Pau Espin Pedrol]

Patch Set 1:

I'll test the patch tomorrow morning and then we can merge

-- 
To view, visit https://gerrit.osmocom.org/3901
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4bb1c6b72c23cd860c33bee7851bca3d0ac0e1b
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

[PATCH] osmo-gsm-tester[master]: modem: Fix race condition when connect() is called more than...

[Pau Espin Pedrol]

Review at  https://gerrit.osmocom.org/3901

modem: Fix race condition when connect() is called more than once

An issue was spotted recently in tests run in production which call
modem.connect() twice, first time to check that we are unable to connect
with a wrong KI, then that we can connect with the correct KI.

As there's no current easy way to wait for "rejected" signal, we
basically wait for some time before checking if we were unable to
connect. It seems that sometimes waiting for 30 seconds is not enough,
and then a scan() async method is still in progress when we decide to
call connect() again, which will powercycle the modem. If the scan()
method returns at that time, then we try to access interfaces of the
modem which are no longer available because the modem is powered off at
that time. This triggers an exception and test fails.

To correct way to fix this is to make sure we disable pending Scan()
dbus method before powercycling the modem and starting a new Scan()
method.

The sleep time is also increased to make sure we let enough time to
register. It seems it can take about 15 seconds to do a full scan, and
sometimes during first scan iteration the BTS is still not found,
probably because it's still starting.

Related: OS#2510

Change-Id: Ic4bb1c6b72c23cd860c33bee7851bca3d0ac0e1b
---
M src/osmo_gsm_tester/ofono_client.py
M suites/aoip_encryption/register_a5_0_authreq.py
M suites/aoip_encryption/register_a5_1_authreq.py
3 files changed, 11 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/01/3901/1

diff --git a/src/osmo_gsm_tester/ofono_client.py 
b/src/osmo_gsm_tester/ofono_client.py
index e3858f9..bf2af70 100644
--- a/src/osmo_gsm_tester/ofono_client.py
+++ b/src/osmo_gsm_tester/ofono_client.py
@@ -346,13 +346,16 @@
 }
 self.dbus.watch_interfaces()
 
+def cancel_pending_dbus_methods(sef):
+self.cancellable.cancel()
+# Cancel op is applied as a signal coming from glib mainloop, so we
+# need to run it and wait for the callbacks to handle cancellations.
+poll_glib()
+
 def cleanup(self):
 self.dbg('cleanup')
 if self.cancellable:
-self.cancellable.cancel()
-# Cancel op is applied as a signal coming from glib mainloop, so we
-# need to run it and wait for the callbacks to handle 
cancellations.
-poll_glib()
+self.cancel_pending_dbus_methods()
 self.cancellable = None
 self.dbus.cleanup()
 self.dbus = None
@@ -517,6 +520,8 @@
 'Connect to MCC+MNC'
 if (mcc_mnc is not None) and (len(mcc_mnc) != 2 or None in mcc_mnc):
 raise log.Error('mcc_mnc value is invalid. It should be None or 
contain both valid mcc and mnc values:', mcc_mnc=mcc_mnc)
+# if test called connect() before and async scanning has not finished, 
we need to get rid of it:
+self.cancel_pending_dbus_methods()
 self.power_cycle()
 self.register_attempts = 0
 if self.is_connected(mcc_mnc):
diff --git a/suites/aoip_encryption/register_a5_0_authreq.py 
b/suites/aoip_encryption/register_a5_0_authreq.py
index be8f8a1..051f5e2 100755
--- a/suites/aoip_encryption/register_a5_0_authreq.py
+++ b/suites/aoip_encryption/register_a5_0_authreq.py
@@ -31,7 +31,7 @@
 print('Attempt connection with wrong KI...')
 ms.connect(msc.mcc_mnc())
 
-sleep(30) # TODO: read pcap or CTRL interface and look for Rejected? 
(gsm_a.dtap.msg_mm_type == 0x04)
+sleep(40) # TODO: read pcap or CTRL interface and look for Rejected? 
(gsm_a.dtap.msg_mm_type == 0x04)
 print('Asserting modem did not register')
 # FIXME: this can fail because ofono qmi signals registered before being 
accepted by network. See OS#2458
 # assert not ms.is_connected(msc.mcc_mnc())
diff --git a/suites/aoip_encryption/register_a5_1_authreq.py 
b/suites/aoip_encryption/register_a5_1_authreq.py
index dd41348..11ee006 100755
--- a/suites/aoip_encryption/register_a5_1_authreq.py
+++ b/suites/aoip_encryption/register_a5_1_authreq.py
@@ -31,7 +31,7 @@
 print('Attempt connection with wrong KI...')
 ms.connect(msc.mcc_mnc())
 
-sleep(30) # TODO: read pcap or CTRL interface and look for Rejected? 
(gsm_a.dtap.msg_mm_type == 0x04)
+sleep(40) # TODO: read pcap or CTRL interface and look for Rejected? 
(gsm_a.dtap.msg_mm_type == 0x04)
 print('Asserting modem did not register')
 # FIXME: this can fail because ofono qmi signals registered before being 
accepted by network. See OS#2458
 # assert not ms.is_connected(msc.mcc_mnc())

-- 
To view, visit https://gerrit.osmocom.org/3901
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic4bb1c6b72c23cd860c33bee7851bca3d0ac0e1b
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 

libosmo-netif[master]: src: _snprintf() helper functions always nul-terminate buffers

[Pau Espin Pedrol]

Patch Set 1:

> Because I got confused. I remember your original patch is trying to >ensure 
> that we leave room for the nul-termination.

If I recall correctly, it does so in case at some point snprintf returns < 0, 
in wich case the NUll terminated buffer cannot be ensured.

> Anyway, bottom line is: I don't we need this patch at all.
> Given that snprintf() output is always nul-terminated, the last snprintf() 
> call already guarantees that we deliver a nul-terminated >buffer.

It only guarantees that in case there's no error returned.

What if msg->len == 0? Then we need to null-terminate the buffer.

-- 
To view, visit https://gerrit.osmocom.org/3830
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls

[Pau Espin Pedrol]

Patch Set 3: Code-Review+1

-- 
To view, visit https://gerrit.osmocom.org/3825
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
Gerrit-PatchSet: 3
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

[PATCH] openbsc[master]: libmsc: annotate esme route in the sms object from deliver_t...

[Pablo Neira Ayuso]

Review at  https://gerrit.osmocom.org/3900

libmsc: annotate esme route in the sms object from deliver_to_esme()

Annotate this esme route, so we can use it to return -EINPROGRESS to
skip sending premature RP-ACK to the mobile station, in case we're
handling sms routes through SMPP.

Now that we have this information in place, we use it wherever possible
to avoid kludgy checks on sms->receiver.

sms_free() already releases references to this object, so we should be
fine with this.

Fixes: 4e5b90a594f9 ("libmsc: remove 'deferred' parameter in 
sms_route_mt_sms()")
Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484
---
M openbsc/src/libmsc/gsm_04_11.c
M openbsc/src/libmsc/smpp_openbsc.c
2 files changed, 11 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/00/3900/1

diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index ad788f2..98c362f 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -520,7 +520,10 @@
 
rc = sms_route_mt_sms(conn, gsms);
 
-   /* This SMS got routed through SMPP or no receiver exists. */
+   /* This SMS got routed through SMPP. */
+   if (gsms->smpp.esme)
+   return -EINPROGRESS;
+
if (!gsms->receiver)
return rc;
 
@@ -614,8 +617,10 @@
return gsm411_send_rp_ack(trans, rph->msg_ref);
else if (rc > 0)
return gsm411_send_rp_error(trans, rph->msg_ref, rc);
-   else
-   return rc;
+   else if (rc == -EINPROGRESS)
+   rc = 0;
+
+   return rc;
 }
 
 /* Receive a 04.11 RP-DATA message in accordance with Section 7.3.1.2 */
diff --git a/openbsc/src/libmsc/smpp_openbsc.c 
b/openbsc/src/libmsc/smpp_openbsc.c
index af2d1be..02b2aba 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -714,6 +714,9 @@
if (ret < 0)
return ret;
 
+   smpp_esme_get(esme);
+   sms->smpp.esme = esme;
+
return smpp_cmd_enqueue(esme, conn->subscr, sms,
deliver.sequence_number);
 }

-- 
To view, visit https://gerrit.osmocom.org/3900
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 

openbsc[master]: libmsc: Either route report to ESME or send it, not both

[Pablo Neira Ayuso]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/3885/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:

Line 693:   if (rc == 0) {
> Pablo, I'm not sure how you would like to make this test here.
Keith, could you try this patch instead to see if it cures the problem for you?

https://gerrit.osmocom.org/#/c/3899/


-- 
To view, visit https://gerrit.osmocom.org/3885
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e86a34f5d3087c9c25479192d9a690922113da2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: Yes

openbsc[master]: libmsc: Either route report to ESME or send it, not both

[Pablo Neira Ayuso]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/3885/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:

Line 693:   if (rc == 0) {
> The uninitialized return value is a real issue, I just sent a patch to addr
Actually, this can be a real issue from gsm340_rx_tpdu().  Given rc determines 
what to do.


-- 
To view, visit https://gerrit.osmocom.org/3885
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e86a34f5d3087c9c25479192d9a690922113da2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: Yes

openbsc[master]: libmsc: Either route report to ESME or send it, not both

[Pablo Neira Ayuso]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/3885/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:

Line 693:   if (rc == 0) {
> Took a brief look at the code which I don't know well, and at it looks such
The uninitialized return value is a real issue, I just sent a patch to address 
this. This may result in bogus log messages indication "Failed to send status 
report!". I just sent a patch to address this.

Regarding sms_free(sms_report), this is just called at the end of this 
function, once this object is not needed at all. Given we don't return after 
sms_route_mt_sms() call, I see no problem with it.


-- 
To view, visit https://gerrit.osmocom.org/3885
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e86a34f5d3087c9c25479192d9a690922113da2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: Yes

[PATCH] openbsc[master]: libmsc: sms_route_mt_sms() may return uninitialized return v...

[Pablo Neira Ayuso]

Review at  https://gerrit.osmocom.org/3899

libmsc: sms_route_mt_sms() may return uninitialized return value

If smpp_first is set off and there is a local receiver for this sms,
then return 0.

Without this patch, we return 'rc' which is uninitialized in the
scenario that I'm describing above.

Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 22 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/99/3899/1

diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index ddef444..ad788f2 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -363,31 +363,32 @@
/* determine gsms->receiver based on dialled number */
gsms->receiver = subscr_get_by_extension(conn->network->subscr_group,
 gsms->dst.addr);
-   if (!gsms->receiver) {
-#ifdef BUILD_SMPP
-   /* Avoid a second look-up */
-   if (smpp_first) {
-   
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
-   return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
-   }
+   if (gsms->receiver)
+   return 0;
 
-   rc = smpp_try_deliver(gsms, conn);
-   if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
-   
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
-   } else if (rc < 0) {
-   LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
-subscr_name(conn->subscr), rc);
-   rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
-   /* rc will be logged by gsm411_send_rp_error() */
-   rate_ctr_inc(>bts->network->msc_ctrs->ctr[
-   MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
-   }
-#else
-   rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+#ifdef BUILD_SMPP
+   /* Avoid a second look-up */
+   if (smpp_first) {

rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
-#endif
+   return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
}
 
+   rc = smpp_try_deliver(gsms, conn);
+   if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
+   
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+   } else if (rc < 0) {
+   LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
+subscr_name(conn->subscr), rc);
+   rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
+   /* rc will be logged by gsm411_send_rp_error() */
+   rate_ctr_inc(>bts->network->msc_ctrs->ctr[
+   MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
+   }
+#else
+   rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+   rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+#endif
+
return rc;
 }
 

-- 
To view, visit https://gerrit.osmocom.org/3899
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 

libosmo-netif[master]: src: _snprintf() helper functions always nul-terminate buffers

[Pablo Neira Ayuso]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/3830/1/src/osmux.c
File src/osmux.c:

Line 941:   /* This _snprintf() variant always nul-terminates the buffer. */
> I still don't get why you say at least some variants of snprintf don't cont
Because I got confused. I remember your original patch is trying to ensure that 
we leave room for the nul-termination.

Anyway, bottom line is: I don't we need this patch at all.

Given that snprintf() output is always nul-terminated, the last snprintf() call 
already guarantees that we deliver a nul-terminated buffer.

So all this logic this patch adds is entirely superfluous.

I'm going to abbandon this patch.


-- 
To view, visit https://gerrit.osmocom.org/3830
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

[PATCH] libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls

[Pablo Neira Ayuso]

Hello Pau Espin Pedrol, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/3825

to look at the new patch set (#3).

osmux: fix buffer management mess in snprintf() calls

SNPRINTF_BUFFER_SIZE() looks too complex, previous version maintains two
different variables to account for the remaining space in the buffer,
one of them is always decremented based on what snprintf() returns,
which may result in underflow. These variables are swapped - not used
consistently - all over this code.

Replace this macro by a simplified version, with one single parameter to
account for remaining space. This macro also deals with two corner
cases:

1) snprintf() fails, actually never happens in practise, but
   documentation indicates it may return -1, so let's catch this case
   from here to stick to specs.

2) There is not enough space in the buffer, in that case, keep
   increasing offset, so we know how much would have been printed, just
   like snprintf() does.

Thanks to Pau Espin for reporting, and Holger for clues on this.
I have run osmux_test and, at quick glance, it looks good.

Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
---
M src/osmux.c
M src/rtp.c
M tests/osmux/osmux_test.c
3 files changed, 44 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/25/3825/3

diff --git a/src/osmux.c b/src/osmux.c
index b3c43e2..b18246f 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -846,19 +846,20 @@
h->rtp_ssrc = rtp_ssrc;
 }
 
-#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset)   \
-   size -= ret;\
-   if (ret > len)  \
-   ret = len;  \
+#define SNPRINTF_BUFFER_SIZE(ret, remain, offset)  \
+   if (ret < 0)\
+   ret = 0;\
offset += ret;  \
-   len -= ret;
+   if (ret > remain)   \
+   ret = remain;   \
+   remain -= ret;
 
 static int osmux_snprintf_header(char *buf, size_t size, struct osmux_hdr 
*osmuxh)
 {
+   unsigned int remain = size, offset = 0;
int ret;
-   int len = size, offset = 0;
 
-   ret = snprintf(buf, len, "OSMUX seq=%03u ccid=%03u "
+   ret = snprintf(buf, remain, "OSMUX seq=%03u ccid=%03u "
 "ft=%01u ctr=%01u "
 "amr_f=%01u amr_q=%01u "
 "amr_ft=%02u amr_cmr=%02u ",
@@ -866,7 +867,7 @@
osmuxh->ft, osmuxh->ctr,
osmuxh->amr_f, osmuxh->amr_q,
osmuxh->amr_ft, osmuxh->amr_cmr);
-   SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+   SNPRINTF_BUFFER_SIZE(ret, remain, offset);
 
return offset;
 }
@@ -874,19 +875,19 @@
 static int osmux_snprintf_payload(char *buf, size_t size,
  const uint8_t *payload, int payload_len)
 {
+   unsigned int remain = size, offset = 0;
int ret, i;
-   int len = size, offset = 0;
 
-   ret = snprintf(buf+offset, len, "[ ");
-   SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+   ret = snprintf(buf + offset, remain, "[ ");
+   SNPRINTF_BUFFER_SIZE(ret, remain, offset);
 
for (i=0; ilen, len = size;
-   struct osmux_hdr *osmuxh;
+   unsigned int remain = size;
int this_len, msg_off = 0;
+   struct osmux_hdr *osmuxh;
+   unsigned int offset = 0;
+   int msg_len = msg->len;
+   int ret;
 
while (msg_len > 0) {
if (msg_len < sizeof(struct osmux_hdr)) {
@@ -915,10 +917,8 @@
return -1;
}
 
-   ret = osmux_snprintf_header(buf+offset, size, osmuxh);
-   if (ret < 0)
-   break;
-   SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+   ret = osmux_snprintf_header(buf + offset, remain, osmuxh);
+   SNPRINTF_BUFFER_SIZE(ret, remain, offset);
 
this_len = sizeof(struct osmux_hdr) +
   

libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls

[Pablo Neira Ayuso]

Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/3825/2//COMMIT_MSG
Commit Message:

Line 31: As in snprintf(), caller should not assume the buffer is 
nul-terminated.
> I think this is still not true and should be removed.
Right, it's sprintf() the one that does not guarantee this.

I'm going to submit a new version rewriting commit description.


-- 
To view, visit https://gerrit.osmocom.org/3825
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

[MERGED] libosmo-netif[master]: rtp: return offset in osmo_rtp_snprintf()

[Pablo Neira Ayuso]

Pablo Neira Ayuso has submitted this change and it was merged.

Change subject: rtp: return offset in osmo_rtp_snprintf()
..


rtp: return offset in osmo_rtp_snprintf()

Instead of the result of the last snprintf() call.

Change-Id: I10066d73387be96a4e1f3349d700405beb138076
---
M src/rtp.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/rtp.c b/src/rtp.c
index 7ac5862..44fc217 100644
--- a/src/rtp.c
+++ b/src/rtp.c
@@ -222,5 +222,5 @@
ret = snprintf(buf+offset, len, "]");
SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-   return ret;
+   return offset;
 }

-- 
To view, visit https://gerrit.osmocom.org/3824
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I10066d73387be96a4e1f3349d700405beb138076
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-Reviewer: Pau Espin Pedrol 

[MERGED] osmo-gsm-tester[master]: resource: Fix list comparison in item_matches

[Pau Espin Pedrol]

Pau Espin Pedrol has submitted this change and it was merged.

Change subject: resource: Fix list comparison in item_matches
..


resource: Fix list comparison in item_matches

In following commits, cipher attribute containing a list of supported
ciphers is introdued in osmo-gsm-tester. While developing the feature,
it was found that resources containing lists are not being handled
correctly.

Previous implementation regarding lists in several ways:
- In the list coe path, both item and wanted_item are expected to be
lists. Python doesn't support to check for sublists using the "in"
operand.
- want_item basically contains the scenario dic, and item is each of the
items available as resource of a given type. Thus, we want to check that
item supports the subset of features requested by the scenario, ie. that
the list in the scenario is a subset of the ones in the3 item and not
the opposite.

The following failing scenario is going to be checked during "make check"
when the cipher attribute is added: BTS supporting cipher a50 and a51,
and scenario requesting a50 succeeds. If this commit is remove, the test
no longer passes, and it fails with:
osmo_gsm_tester.resource.NoResourceExn: No matching resource available for
bts = {'type': 'osmo-bts-sysmo', 'ciphers': ['a5 1']}

Change-Id: I27b372aa5906feac2843f24f5cdd0d9578d44b4d
---
M src/osmo_gsm_tester/resource.py
1 file changed, 5 insertions(+), 2 deletions(-)

Approvals:
  Neels Hofmeyr: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo_gsm_tester/resource.py b/src/osmo_gsm_tester/resource.py
index c55140a..da543f7 100644
--- a/src/osmo_gsm_tester/resource.py
+++ b/src/osmo_gsm_tester/resource.py
@@ -441,9 +441,12 @@
 return True
 
 if is_list(wanted_item):
-# multiple possible values
-if item not in wanted_item:
+if not is_list(item):
 return False
+# multiple possible values
+for val in wanted_item:
+if val not in item:
+return False
 return True
 
 return item == wanted_item

-- 
To view, visit https://gerrit.osmocom.org/3721
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I27b372aa5906feac2843f24f5cdd0d9578d44b4d
Gerrit-PatchSet: 3
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol