neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27656 )
Change subject: assignment_fsm: always mark MGCP ci as completed
......................................................................
assignment_fsm: always mark MGCP ci as completed
When the assignment fails, we roll back the MSC side RTP endpoint at the
MGW that may have been created before the failure occured. On success,
we clear the mgcp_ci pointer so it is not rolled back.
Always clear this, not only when lchan_changed == true. That is because
a channel Mode Modify may also have added a voice stream that should
retain the newly created RTP endpoint at the MGW.
If no voice stream is involved, the pointer should already be NULL.
There is no reason to have a condition for clearing this pointer.
Just always clear it.
This fixes all voice calls that modify a TCH lchan from signalling-only
to voice mode. Before this patch, their MSC side RTP endpoint was DLCX'd
right on assignment success.
In particular, this fixes Emergency Calls (which usually get a TCH lchan
assigned right from the start, and hence do a Mode Modify to add voice).
Related: SYS#5916
Change-Id: I5ab10ee7fd9c5d7608e8a06893881d990943feed
---
M src/osmo-bsc/assignment_fsm.c
1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/56/27656/1
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index 8d5e841..c7fc335 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -283,11 +283,11 @@
}
}
- if (lchan_changed) {
- /* Rembered this only for error handling: should assignment fail, assignment_reset() will release
- * the MGW endpoint right away. If successful, the conn continues to use the endpoint. */
- conn->assignment.created_ci_for_msc = NULL;
+ /* Rembered this only for error handling: should assignment fail, assignment_reset() will release
+ * the MGW endpoint right away. If successful, the conn continues to use the endpoint. */
+ conn->assignment.created_ci_for_msc = NULL;
+ if (lchan_changed) {
/* New RTP information is now accepted */
conn->user_plane.msc_assigned_cic = conn->assignment.req.msc_assigned_cic;
osmo_strlcpy(conn->user_plane.msc_assigned_rtp_addr, conn->assignment.req.msc_rtp_addr,
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27656
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I5ab10ee7fd9c5d7608e8a06893881d990943feed
Gerrit-Change-Number: 27656
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27657 )
Change subject: assignment_fsm: always update RTP info
......................................................................
assignment_fsm: always update RTP info
When the assignment succeeds, the assignment_request has taken effect
and whatever it requested should now be in effect.
Hence copy the new RTP information from the assignment_request to the
conn's storage indicating what is currently used, always, not only when
lchan_changed == true. The RTP information may have changed also from a
Mode Modify where the lchan stays the same but changes its mode of use,
e.g. from signalling to voice.
When there is no RTP involved, this data is zero or empty, so there is
no harm in copying it, always.
Related: SYS#5916
Change-Id: I0788d1f013b8f820f559b6ed58a5f9bb8a02e0b4
---
M src/osmo-bsc/assignment_fsm.c
1 file changed, 6 insertions(+), 7 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/57/27657/1
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index c7fc335..7deca65 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -287,13 +287,12 @@
* the MGW endpoint right away. If successful, the conn continues to use the endpoint. */
conn->assignment.created_ci_for_msc = NULL;
- if (lchan_changed) {
- /* New RTP information is now accepted */
- conn->user_plane.msc_assigned_cic = conn->assignment.req.msc_assigned_cic;
- osmo_strlcpy(conn->user_plane.msc_assigned_rtp_addr, conn->assignment.req.msc_rtp_addr,
- sizeof(conn->user_plane.msc_assigned_rtp_addr));
- conn->user_plane.msc_assigned_rtp_port = conn->assignment.req.msc_rtp_port;
- }
+ /* New RTP information is now accepted. If there is no RTP stream, this information is zero / empty. Either way
+ * store the result of this assignment. */
+ conn->user_plane.msc_assigned_cic = conn->assignment.req.msc_assigned_cic;
+ osmo_strlcpy(conn->user_plane.msc_assigned_rtp_addr, conn->assignment.req.msc_rtp_addr,
+ sizeof(conn->user_plane.msc_assigned_rtp_addr));
+ conn->user_plane.msc_assigned_rtp_port = conn->assignment.req.msc_rtp_port;
assignment_count_result(CTR_ASSIGNMENT_COMPLETED);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27657
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I0788d1f013b8f820f559b6ed58a5f9bb8a02e0b4
Gerrit-Change-Number: 27657
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: neels.
iedemam has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27081 )
Change subject: stats: new trackers for lchan life duration
......................................................................
Patch Set 19:
(12 comments)
Patchset:
PS18:
> Hi Neels, […]
For now I'm using deciseconds (not decaseconds as in my typo above). I'd like to have the underlying stat values be milliseconds for clarity but if this isn't possible to have independent timer frequency and increment values, it's not a deal-breaker.
Patchset:
PS19:
Hi all,
I think all feedback has now been addressed. I still want to do additional testing now that we're really close so please do not merge, even if everything looks good.
Thanks,
-Michael
File include/osmocom/bsc/bts.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/e2f91611_256e6d9e
PS18, Line 61: BTS_CTR_CHAN_SDCCH_ACTIVE_MILLISECONDS_TOTAL,
> (1)
Done
File include/osmocom/bsc/gsm_data.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/61943723_41ef4818
PS18, Line 890: /* Interval timing to capture duration per activation and cummulative active time */
> "cumulative"
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/15f55d51_92d8594f
PS18, Line 891: struct osmo_time_cc active_ms;
> (1) […]
Done
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/134c40f4_93716e8e
PS18, Line 1038: "Cumulative number of milliseconds of SDCCH channel activity" },
> (1)
Done
File src/osmo-bsc/bts_trx_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/31ba6abc_315990ef
PS18, Line 576: VTY_NEWLINE);
> (3) […]
This specific VTY output is the primary motivation for this entire patch. I've re-added the osmo_time_cc_reset() so each lchan activation can be tracked and output.
I started with option b but you argued against it so I refactored everything to only use a single osmo_time_cc. I agree this is cleaner and now believe it matches my functional goal and your implementation approach.
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/1959db04_7d21fd8b
PS18, Line 3844: vty_out(vty, "%s", VTY_NEWLINE);
> (2) […]
Agreed. I've opted to show total activations and then avg lifespan over past hour.
File src/osmo-bsc/lchan_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/d6bb8d10_2c515814
PS18, Line 208: struct rate_ctr *rate_ctr;
> osmocom style requires that this local variable be defined at the start of the scope, at line 203 ab […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/a09ec898_31d87a43
PS18, Line 224: if (lchan->active_ms.cfg.rate_ctr) {
> (could just set the flag no matter if rate_ctr is set or not. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/56f4b207_a0e53313
PS18, Line 546: .active_ms = lchan->active_ms
> please add a trailing comma. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/e385d5ae_3588b8d7
PS18, Line 556: if (lchan->active_ms.cfg.rate_ctr) {
> (could just set the flag no matter if rate_ctr is set or not. […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27081
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
Gerrit-Change-Number: 27081
Gerrit-PatchSet: 19
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 06 Apr 2022 12:59:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: iedemam <michael(a)kapsulate.com>
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels.
Hello Jenkins Builder, neels, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/27081
to look at the new patch set (#19).
Change subject: stats: new trackers for lchan life duration
......................................................................
stats: new trackers for lchan life duration
This patch adds two stats which track cummulative lchan lifetime by
type TCH and SDCCH. These new counters will accomplish two things:
1) Provide a glanceable way to see if lchan durations look healthy. When
examining a site, short-lived (<5s) and long-lived (>30s) TCH lchans
are difficult to tell apart. If we only see short-lived TCH lchans,
there is most likely an RF or signaling problem to investigate. This
new counter will expose channel ages in the VTY output
2) Provide a more accurate count for Erlangs per site. Currently, we
are basing Erlangs on active TCH channel counts per stats period. This
method skews high very quickly. Each active TCH in that period
translates into the full 10s of activity. This counter should improve
accuracy by two orders of magnitude.
Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_trx_vty.c
M src/osmo-bsc/bts_vty.c
M src/osmo-bsc/gsm_data.c
M src/osmo-bsc/lchan_fsm.c
7 files changed, 95 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/81/27081/19
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27081
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
Gerrit-Change-Number: 27081
Gerrit-PatchSet: 19
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newpatchset
osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27655 )
Change subject: contrib/osmo-bsc.spec.in: fix 4trx example paths
......................................................................
contrib/osmo-bsc.spec.in: fix 4trx example paths
The paths were wrong in my previous attempt, add the proper paths now.
I've verified that this file builds, and adjusted the current nightly
rpm spec uploaded to OBS with this change manually this time so it
doesn't block.
Fix for:
File not found: /home/abuild/rpmbuild/BUILDROOT/osmo-bsc-1.8.0.90.e0187.202204061142-1.1.x86_64/etc/osmocom/osmo-bsc-4trx-fh.confmerge
File not found: /home/abuild/rpmbuild/BUILDROOT/osmo-bsc-1.8.0.90.e0187.202204061142-1.1.x86_64/etc/osmocom/osmo-bsc-4trx.cfg
Fixes: e0187bc3 ("contrib/osmo-bsc.spec.in: add new config files")
Change-Id: I92a7d85b64a9659db7ef71af5b00110b582dc4d9
---
M contrib/osmo-bsc.spec.in
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/55/27655/1
diff --git a/contrib/osmo-bsc.spec.in b/contrib/osmo-bsc.spec.in
index 86c8ea6..00f9d8a 100644
--- a/contrib/osmo-bsc.spec.in
+++ b/contrib/osmo-bsc.spec.in
@@ -122,14 +122,14 @@
%dir %{_docdir}/%{name}/examples/osmo-bsc
%{_docdir}/%{name}/examples/osmo-bsc/osmo-bsc.cfg
%{_docdir}/%{name}/examples/osmo-bsc/osmo-bsc_custom-sccp.cfg
+%{_docdir}/%{name}/examples/osmo-bsc/osmo-bsc-4trx-fh.confmerge
+%{_docdir}/%{name}/examples/osmo-bsc/osmo-bsc-4trx.cfg
%{_docdir}/%{name}/examples/osmo-bsc/osmo-bsc-minimal.cfg
%dir %{_docdir}/%{name}/examples/osmo-bsc/ericsson
%dir %{_docdir}/%{name}/examples/osmo-bsc/nokia
%dir %{_docdir}/%{name}/examples/osmo-bsc/siemens
%{_docdir}/%{name}/examples/osmo-bsc/*/osmo-bsc*.cfg
%dir %{_sysconfdir}/osmocom
-%config(noreplace) %{_sysconfdir}/osmocom/osmo-bsc-4trx-fh.confmerge
-%config(noreplace) %{_sysconfdir}/osmocom/osmo-bsc-4trx.cfg
%config(noreplace) %{_sysconfdir}/osmocom/osmo-bsc.cfg
%{_unitdir}/%{name}.service
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27655
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I92a7d85b64a9659db7ef71af5b00110b582dc4d9
Gerrit-Change-Number: 27655
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newchange