[MERGED] openbsc[master]: ensure that acc_ramp_init() is only called once

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Apr 16 15:19:15 UTC 2018


Harald Welte has submitted this change and it was merged.

Change subject: ensure that acc_ramp_init() is only called once
......................................................................


ensure that acc_ramp_init() is only called once

There are plans to register signal handlers in acc_ramp_init(). Once we
do that, the acc_ramp_init() function should only be called once to
avoid duplicate signal handlers on the handler list.

However, the acc_ramp_init() function currently serves a dual-purpose:

1) Initialize the acc_ramp structure for a bts
2) Enable or disable ACC ramping

Add new functions to support use case 2, and call acc_ramp_init()
just once while reading the configuration file. The VTY commands
which enable/disable ACC ramping use the new APIs instead.

Also, rename acc_ramp_start() to acc_ramp_trigger() and tweak its
semantics so that it can always be called regardless of what the
current configuration settings are. This prepares us for triggering
ACC ramping upon events other than "RSL link-up".

This is a port of osmo-bsc commit ea33341cf7b52d432be98f2280b4a5f3129ef667.
Also remove a call to acc_ramp_init() which should have been removed in
openbsc commit ebc1e39d919f5f919cb176ee9c6cbbccc8d620b1

Change-Id: Iadd25016e6478a9dc5da1db42e6192ce0f5cc746
Related: OS2591
---
M openbsc/include/openbsc/acc_ramp.h
M openbsc/src/libbsc/acc_ramp.c
M openbsc/src/libbsc/bsc_init.c
M openbsc/src/libbsc/bsc_vty.c
4 files changed, 43 insertions(+), 35 deletions(-)

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/openbsc/include/openbsc/acc_ramp.h b/openbsc/include/openbsc/acc_ramp.h
index cb063bf..efb12b0 100644
--- a/openbsc/include/openbsc/acc_ramp.h
+++ b/openbsc/include/openbsc/acc_ramp.h
@@ -79,6 +79,18 @@
 };
 
 /*!
+ * Enable or disable ACC ramping.
+ * When enabled, ramping begins once acc_ramp_start() is called.
+ * When disabled, an ACC ramping process in progress will continue
+ * unless acc_ramp_abort() is called as well.
+ * \param[in] acc_ramp Pointer to acc_ramp structure.
+ */
+static inline void acc_ramp_set_enabled(struct acc_ramp *acc_ramp, bool enable)
+{
+	acc_ramp->acc_ramping_enabled = enable;
+}
+
+/*!
  * Return true if ACC ramping is currently enabled, else false.
  * \param[in] acc_ramp Pointer to acc_ramp structure.
  */
@@ -141,9 +153,9 @@
 	rach_control->t3 |= acc_ramp_get_barred_t3(acc_ramp);
 }
 
-void acc_ramp_init(struct acc_ramp *acc_ramp, bool enable, struct gsm_bts *bts);
+void acc_ramp_init(struct acc_ramp *acc_ramp, struct gsm_bts *bts);
 int acc_ramp_set_step_size(struct acc_ramp *acc_ramp, unsigned int step_size);
 int acc_ramp_set_step_interval(struct acc_ramp *acc_ramp, unsigned int step_interval);
 void acc_ramp_set_step_interval_dynamic(struct acc_ramp *acc_ramp);
-void acc_ramp_start(struct acc_ramp *acc_ramp);
+void acc_ramp_trigger(struct acc_ramp *acc_ramp);
 void acc_ramp_abort(struct acc_ramp *acc_ramp);
diff --git a/openbsc/src/libbsc/acc_ramp.c b/openbsc/src/libbsc/acc_ramp.c
index 9b3f90b..e887723 100644
--- a/openbsc/src/libbsc/acc_ramp.c
+++ b/openbsc/src/libbsc/acc_ramp.c
@@ -140,28 +140,20 @@
  * Initialize an acc_ramp data structure.
  * Storage for this structure must be provided by the caller.
  *
- * If ACC ramping is enabled, all ACCs are denied by default.
- * A subsequent call to acc_ramp_start() will begin the ramping process.
- * If ACC ramping is disabled, all ACCs will be allowed by default,
- * and there is no need to do anything else.
+ * By default, ACC ramping is disabled and all ACCs are allowed.
  *
  * \param[in] acc_ramp Pointer to acc_ramp structure to be initialized.
- * \param[in] enable Indicates whether ACC ramping should be enabled or disabled.
  * \param[in] bts BTS which uses this ACC ramp data structure.
  */
-void acc_ramp_init(struct acc_ramp *acc_ramp, bool enable, struct gsm_bts *bts)
+void acc_ramp_init(struct acc_ramp *acc_ramp, struct gsm_bts *bts)
 {
 	acc_ramp->bts = bts;
-	acc_ramp->acc_ramping_enabled = enable;
+	acc_ramp_set_enabled(acc_ramp, false);
 	acc_ramp->step_size = ACC_RAMP_STEP_SIZE_DEFAULT;
 	acc_ramp->step_interval_sec = ACC_RAMP_STEP_INTERVAL_MIN;
 	acc_ramp->step_interval_is_fixed = false;
+	allow_all_enabled_accs(acc_ramp);
 	osmo_timer_setup(&acc_ramp->step_timer, do_acc_ramping_step, acc_ramp);
-
-	if (acc_ramp->acc_ramping_enabled)
-		barr_all_enabled_accs(acc_ramp);
-	else
-		allow_all_enabled_accs(acc_ramp);
 }
 
 /*!
@@ -211,28 +203,34 @@
 }
 
 /*!
- * Begin the ramping process. Perform at least one ramping step to allow 'step_size' ACCs.
- * If 'step_size' is ACC_RAMP_STEP_SIZE_MAX, all ACCs will be allowed immediately.
+ * Determine if ACC ramping should be started according to configuration, and
+ * if ACC ramping is enabled, begin the ramping process.
+ * Perform at least one ramping step to allow 'step_size' ACCs.
+ * If 'step_size' is ACC_RAMP_STEP_SIZE_MAX, or if ACC ramping is disabled,
+ * all ACCs will be allowed immediately.
  * \param[in] acc_ramp Pointer to acc_ramp structure.
  */
-void acc_ramp_start(struct acc_ramp *acc_ramp)
+void acc_ramp_trigger(struct acc_ramp *acc_ramp)
 {
-	/* Abort any previously running ramping process. */
+	/* Abort any previously running ramping process and allow all available ACCs. */
 	acc_ramp_abort(acc_ramp);
 
-	/* Set all availble ACCs to barred and start ramping up. */
-	barr_all_enabled_accs(acc_ramp);
-	do_acc_ramping_step(acc_ramp);
+	if (acc_ramp_is_enabled(acc_ramp)) {
+		/* Set all available ACCs to barred and start ramping up. */
+		barr_all_enabled_accs(acc_ramp);
+		do_acc_ramping_step(acc_ramp);
+	}
 }
 
 /*!
- * Abort the ramping process. If ramping is disabled or has already finished,
- * then this function has no effect.
+ * Abort the ramping process and allow all available ACCs immediately.
  * \param[in] acc_ramp Pointer to acc_ramp structure.
  */
 void acc_ramp_abort(struct acc_ramp *acc_ramp)
 {
 	if (osmo_timer_pending(&acc_ramp->step_timer))
 		osmo_timer_del(&acc_ramp->step_timer);
+
+	allow_all_enabled_accs(acc_ramp);
 }
 
diff --git a/openbsc/src/libbsc/bsc_init.c b/openbsc/src/libbsc/bsc_init.c
index 6147257..120955b 100644
--- a/openbsc/src/libbsc/bsc_init.c
+++ b/openbsc/src/libbsc/bsc_init.c
@@ -311,19 +311,16 @@
 		trx->bts->location_area_code,
 		trx->bts->cell_identity, trx->bts->bsic);
 
-	/*
-	 * Re-initialize ACC ramping to ensure ACCs are barred/allowed
-	 * according to our current VTY configuration.
-	 */
-	acc_ramp_init(&trx->bts->acc_ramp, acc_ramp_is_enabled(&trx->bts->acc_ramp), trx->bts);
-
 	if (trx->bts->type == GSM_BTS_TYPE_NOKIA_SITE) {
 		rsl_nokia_si_begin(trx);
 	}
 
-	/* Configure ACC ramping before sending system information to BTS. */
-	if (acc_ramp_is_enabled(&trx->bts->acc_ramp))
-		acc_ramp_start(&trx->bts->acc_ramp);
+	/*
+	 * Trigger ACC ramping before sending system information to BTS.
+	 * This ensures that RACH control in system information is configured correctly.
+	 */
+	acc_ramp_trigger(&trx->bts->acc_ramp);
+
 	gsm_bts_trx_set_system_infos(trx);
 
 	if (trx->bts->type == GSM_BTS_TYPE_NOKIA_SITE) {
diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c
index f78247b..a076397 100644
--- a/openbsc/src/libbsc/bsc_vty.c
+++ b/openbsc/src/libbsc/bsc_vty.c
@@ -1655,7 +1655,7 @@
 		 * Initalize bts->acc_ramp here. Else we could segfault while
 		 * processing a configuration file with ACC ramping settings.
 		 */
-		acc_ramp_init(&bts->acc_ramp, false, bts);
+		acc_ramp_init(&bts->acc_ramp, bts);
 	} else
 		bts = gsm_bts_num(gsmnet, bts_nr);
 
@@ -3042,7 +3042,8 @@
 {
 	struct gsm_bts *bts = vty->index;
 
-	acc_ramp_init(&bts->acc_ramp, true, bts);
+	if (!acc_ramp_is_enabled(&bts->acc_ramp))
+		acc_ramp_set_enabled(&bts->acc_ramp, true);
 
 	/* ACC ramping takes effect when the BTS reconnects. */
 	return CMD_SUCCESS;
@@ -3057,7 +3058,7 @@
 
 	if (acc_ramp_is_enabled(&bts->acc_ramp)) {
 		acc_ramp_abort(&bts->acc_ramp);
-		acc_ramp_init(&bts->acc_ramp, false, bts);
+		acc_ramp_set_enabled(&bts->acc_ramp, false);
 		gsm_bts_set_system_infos(bts);
 	}
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iadd25016e6478a9dc5da1db42e6192ce0f5cc746
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>



More information about the gerrit-log mailing list