Attention is currently required from: fixeria.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35052?usp=email )
Change subject: msc: split off f_mo_call_establish__handle_assignment_request()
......................................................................
Patch Set 2:
(1 comment)
File msc/BSC_ConnectionHandler.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35052/comment/2b87c228_f11f…
PS2, Line 1690: valueof(ts_CodecFR
> i did consider that. […]
reading this again, I actually do not want to move the csd decision outside of the function. There are more conditions on cpars.csd in the new function.
It would be the opposite of the concept of cpars.
Actually it would make sense to add the codec_chosen arg to cpars instead.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35052?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I98c6171a592dfe1573e15136c4ecf4ff234048d7
Gerrit-Change-Number: 35052
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 12 Jun 2024 03:27:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37189?usp=email )
Change subject: Osmocom_CTRL: counters: return -1 for absent objects
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> imho f_ctrl_get_ratectr_abs() is conceived to be used when a rate ctr should be obtained, and fail o […]
I thought about that actually.
- Our tests currently pass, hence this patch affects no callers at all.
(any absent object would currently setverdict(fail) unconditionally)
- Future tests implicitly expect counters >= 0, and would fail if they get -1 instead, simply by verifying the expected counters as we already do.
- If a test tests for a before-after diff of a counter, it will get unexpected diff values when a counter returns -1. (One corner case is expecting that a counter diff remains zero, which should always also have a phase of the test where the same counter increments; one of these phases will fail on an absent object.)
So, no additional code needed to handle -1 return values, in all practical situations.
That is why I did not continue to carry the on_error param further out to f_ctrl_get_ratectr_abs().
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37189?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Id00bbfe721d453e5f551e6d47e1b1d03d219700b
Gerrit-Change-Number: 37189
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 12 Jun 2024 02:55:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37188?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: nft-kpi: remove X34 drifting: adjust delay by elapsed time
......................................................................
nft-kpi: remove X34 drifting: adjust delay by elapsed time
Related: SYS#6773
Change-Id: I04f572890c04a48bb19c59f613a492ef96624baa
---
M include/osmocom/hnbgw/hnbgw.h
M src/osmo-hnbgw/nft_kpi.c
2 files changed, 49 insertions(+), 6 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h
index 36358b1..a647abd 100644
--- a/include/osmocom/hnbgw/hnbgw.h
+++ b/include/osmocom/hnbgw/hnbgw.h
@@ -499,6 +499,7 @@
struct {
bool active;
struct osmo_timer_list get_counters_timer;
+ struct timespec next_timer;
} nft_kpi;
};
diff --git a/src/osmo-hnbgw/nft_kpi.c b/src/osmo-hnbgw/nft_kpi.c
index 09b9d3b..bbdee2a 100644
--- a/src/osmo-hnbgw/nft_kpi.c
+++ b/src/osmo-hnbgw/nft_kpi.c
@@ -690,15 +690,47 @@
/* main thread */
static void nft_kpi_get_counters_schedule(void)
{
- unsigned long period_s;
+ struct timespec now;
+ struct timespec period;
+ struct timespec diff;
+ struct timespec *next = &g_hnbgw->nft_kpi.next_timer;
unsigned long period_us = osmo_tdef_get(hnbgw_T_defs, -34, OSMO_TDEF_US, 1000000);
- if (period_us < 1)
- period_us = 1;
- period_s = period_us / 1000000;
- period_us %= 1000000;
+ period.tv_sec = period_us / 1000000;
+ period.tv_nsec = (period_us % 1000000) * 1000;
+
+ /* Try to keep the period of getting counters close to the configured period, i.e. don't drift by the time it
+ * takes to read counters. */
+ osmo_clock_gettime(CLOCK_MONOTONIC, &now);
+
+ if (!next->tv_sec && !next->tv_nsec) {
+ /* Not yet initialized. Schedule to get counters one 'period' from 'now':
+ * Set 'next' to 'now', and the period is added by timespecadd() below.
+ * (We could retrieve counters immediately -- but at startup counters are then queried even before the
+ * nft table was created by the maintenance thread. That is not harmful, but it causes an ugly error
+ * message in the logs. So rather wait one period.)
+ */
+ *next = now;
+ }
+ timespecadd(next, &period, next);
+ if (timespeccmp(next, &now, <)) {
+ /* The time that has elapsed since last scheduling counter retrieval is already more than the configured
+ * period. Continue counting the time period from 'now', and ask for counters right now. */
+ timespecsub(&now, next, &diff);
+ LOGP(DNFT, LOGL_NOTICE, "nft-kpi: retrieving counters took %ld.%06ld s longer"
+ " than the timeout configured in timer hnbgw X34.\n", diff.tv_sec, diff.tv_nsec / 1000);
+ *next = now;
+ nft_kpi_get_counters_cb(NULL);
+ return;
+ }
+
+ /* next > now, wait for the remaining time. */
+ timespecsub(next, &now, &diff);
+ LOGP(DNFT, LOGL_DEBUG, "nft-kpi: scheduling timer: period is %ld.%06ld s, next occurrence in %ld.%06ld s\n",
+ period.tv_sec, period.tv_nsec / 1000,
+ diff.tv_sec, diff.tv_nsec / 1000);
osmo_timer_setup(&g_hnbgw->nft_kpi.get_counters_timer, nft_kpi_get_counters_cb, NULL);
- osmo_timer_schedule(&g_hnbgw->nft_kpi.get_counters_timer, period_s, period_us);
+ osmo_timer_schedule(&g_hnbgw->nft_kpi.get_counters_timer, diff.tv_sec, diff.tv_nsec / 1000);
}
/* from main(), initialize all worker threads and other nft state. */
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37188?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I04f572890c04a48bb19c59f613a492ef96624baa
Gerrit-Change-Number: 37188
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37187?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: nft-kpi: log errors of counter retrieval
......................................................................
nft-kpi: log errors of counter retrieval
Make sure errors of getting counters from nft are logged.
Some context: we'll try again each X34 period, hence this is only a
problem when the error persists.
For example, when the get-counters thread is faster than the maintenance
thread can even create the nft table initially, this error will show.
We could add a definite check whether the maintenance thread has created
the tables yet, and wait for that event. But this complexity is not
really needed: it is fine to just fail getting counters once or twice.
Related: SYS#6773
Change-Id: I84340482e4a5bfcac158a21c9378a9511fa5ea10
---
M src/osmo-hnbgw/nft_kpi.c
1 file changed, 30 insertions(+), 1 deletion(-)
Approvals:
pespin: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/osmo-hnbgw/nft_kpi.c b/src/osmo-hnbgw/nft_kpi.c
index 9eff35e..09b9d3b 100644
--- a/src/osmo-hnbgw/nft_kpi.c
+++ b/src/osmo-hnbgw/nft_kpi.c
@@ -990,7 +990,14 @@
break;
case NFT_THREAD_GET_COUNTERS:
- main_thread_handle_get_counters_resp(req);
+ if (req->rc) {
+ /* Maybe we requested counters before the table was created by the maintenance thread. If so,
+ * it's not harmful and we can just try again next time. */
+ LOGP(DNFT, LOGL_ERROR, "Retrieving counters from nftables failed. Trying again (timer X34).\n");
+ } else {
+ main_thread_handle_get_counters_resp(req);
+ }
+ /* Schedule the next counter retrieval. */
nft_kpi_get_counters_schedule();
break;
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37187?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I84340482e4a5bfcac158a21c9378a9511fa5ea10
Gerrit-Change-Number: 37187
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: laforge.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37186?usp=email )
Change subject: dbg log: nft kpi: clarify nr of rate ctrs vs nr of hnbp
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS3:
> not sure we can expect the reader to be familiar with what a "hnbp" is, but anyway, better to give t […]
"hNodeB" makes more sense indeed
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37186?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I84346d3151f3040967f39a3a6e6db2e29bc1e2ec
Gerrit-Change-Number: 37186
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 12 Jun 2024 02:25:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: laforge, neels.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/37186?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Verified+1 by Jenkins Builder
Change subject: dbg log: nft kpi: clarify nr of rate ctrs vs nr of hnbp
......................................................................
dbg log: nft kpi: clarify nr of rate ctrs vs nr of hnbp
Related: SYS#6773
Change-Id: I84346d3151f3040967f39a3a6e6db2e29bc1e2ec
---
M src/osmo-hnbgw/nft_kpi.c
1 file changed, 13 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/86/37186/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37186?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I84346d3151f3040967f39a3a6e6db2e29bc1e2ec
Gerrit-Change-Number: 37186
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/37188?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
The change is no longer submittable: Verified is unsatisfied now.
Change subject: nft-kpi: remove X34 drifting: adjust delay by elapsed time
......................................................................
nft-kpi: remove X34 drifting: adjust delay by elapsed time
Related: SYS#6773
Change-Id: I04f572890c04a48bb19c59f613a492ef96624baa
---
M include/osmocom/hnbgw/hnbgw.h
M src/osmo-hnbgw/nft_kpi.c
2 files changed, 49 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/88/37188/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37188?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I04f572890c04a48bb19c59f613a492ef96624baa
Gerrit-Change-Number: 37188
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/37187?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
The change is no longer submittable: Verified is unsatisfied now.
Change subject: nft-kpi: log errors of counter retrieval
......................................................................
nft-kpi: log errors of counter retrieval
Make sure errors of getting counters from nft are logged.
Some context: we'll try again each X34 period, hence this is only a
problem when the error persists.
For example, when the get-counters thread is faster than the maintenance
thread can even create the nft table initially, this error will show.
We could add a definite check whether the maintenance thread has created
the tables yet, and wait for that event. But this complexity is not
really needed: it is fine to just fail getting counters once or twice.
Related: SYS#6773
Change-Id: I84340482e4a5bfcac158a21c9378a9511fa5ea10
---
M src/osmo-hnbgw/nft_kpi.c
1 file changed, 30 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/87/37187/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37187?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I84340482e4a5bfcac158a21c9378a9511fa5ea10
Gerrit-Change-Number: 37187
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newpatchset
