Change in osmo-bts[master]: abis.c: Fix mess with priv->bsc_oml_host

[pespin]

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/25458 )

Change subject: abis.c: Fix mess with priv->bsc_oml_host
......................................................................

abis.c: Fix mess with priv->bsc_oml_host

The pointer was used as "struct bsc_oml_host" sometimes, and other times
as "struct llist_head". It just worked because bsc_oml_host->list is the
first item in the script. The code was really confusing, also because
the bts list of items has a name really similar to the one currently
assigned. Let's rename the currently assigned address to "current_bsc",
store it always as "struct bsc_oml_host*" and finally use llist_entry
helpers when needed.

The related code is also moved to a helper function to enclose there the
logic to get next BSC in list. This change actually changes the logic
where a remote address is removed from VTY, since now the next address
in list is picked at the time, and later when reconnecting the list is
forwarded another time, meaning one address will be skipped.
This could be considered a bug, but this situation is really special
and anyway the entire logic will be changed in new commits where we'll
keep reconnecting in loop without exiting when reaching the end of the
list, so we are fine with it. Think of this commit as a preparation
commit for next ones.

Change-Id: I3cc8a4148b3d63bc185b72dab8108105a6644910
---
M include/osmo-bts/abis.h
M src/common/abis.c
2 files changed, 32 insertions(+), 20 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, approved
  fixeria: Looks good to me, but someone else must approve
  osmith: Looks good to me, but someone else must approve



diff --git a/include/osmo-bts/abis.h b/include/osmo-bts/abis.h
index 1939faf..40707cd 100644
--- a/include/osmo-bts/abis.h
+++ b/include/osmo-bts/abis.h
@@ -9,7 +9,7 @@
 enum abis_link_fsm_event {
 	ABIS_LINK_EV_SIGN_LINK_OML_UP,
 	ABIS_LINK_EV_SIGN_LINK_DOWN,
-	ABIS_LINK_EV_VTY_RM_ADDR,
+	ABIS_LINK_EV_VTY_RM_ADDR, /* data: struct bsc_oml_host* being removed */
 };
 
 void abis_init(struct gsm_bts *bts);
diff --git a/src/common/abis.c b/src/common/abis.c
index 551670c..c230a4b 100644
--- a/src/common/abis.c
+++ b/src/common/abis.c
@@ -76,7 +76,7 @@
 };
 
 struct abis_link_fsm_priv {
-	struct llist_head *bsc_oml_host;
+	struct bsc_oml_host *current_bsc;
 	struct gsm_bts *bts;
 	char *model_name;
 	int line_ctr;
@@ -115,30 +115,45 @@
 	}
 }
 
+static int pick_next_bsc(struct osmo_fsm_inst *fi)
+{
+	struct abis_link_fsm_priv *priv = fi->priv;
+	struct gsm_bts *bts = priv->bts;
+	struct bsc_oml_host *last;
+
+	if (llist_empty(&bts->bsc_oml_hosts)) {
+		LOGPFSML(fi, LOGL_ERROR, "List of BSCs to connect to is empty!\n");
+		return -1;
+	}
+
+	last = (struct bsc_oml_host *)llist_last_entry(&bts->bsc_oml_hosts, struct bsc_oml_host, list);
+
+	if (!priv->current_bsc) /* Pick first one: */
+		priv->current_bsc = (struct bsc_oml_host *)llist_first_entry(&bts->bsc_oml_hosts, struct bsc_oml_host, list);
+	else if (priv->current_bsc != last)
+		priv->current_bsc = (struct bsc_oml_host *)llist_entry(priv->current_bsc->list.next, struct bsc_oml_host, list);
+	else
+		return -1; /* We are so far not starting over the list when we reach the list, but only exit */
+
+	return 0;
+}
+
 static void abis_link_connecting_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
 {
 	struct e1inp_line *line;
 	struct abis_link_fsm_priv *priv = fi->priv;
 	struct gsm_bts *bts = priv->bts;
-	struct bsc_oml_host *bsc_oml_host;
 
-	if (priv->bsc_oml_host) {
-		/* Get a BSC host from the list and move the list head one position forward. */
-		bsc_oml_host = (struct bsc_oml_host *)priv->bsc_oml_host;
-		if (priv->bsc_oml_host == llist_last(&bts->bsc_oml_hosts))
-			priv->bsc_oml_host = NULL;
-		else
-			priv->bsc_oml_host = priv->bsc_oml_host->next;
-	} else {
-		LOGP(DABIS, LOGL_FATAL, "No BSC available, A-bis connection establishment failed\n");
+	if (pick_next_bsc(fi) < 0) {
+		LOGPFSML(fi, LOGL_FATAL, "No BSC available, A-bis connection establishment failed\n");
 		osmo_fsm_inst_state_chg(fi, ABIS_LINK_ST_FAILED, 0, 0);
 		return;
 	}
 
-	LOGP(DABIS, LOGL_NOTICE, "A-bis connection establishment to BSC (%s) in progress...\n", bsc_oml_host->addr);
+	LOGP(DABIS, LOGL_NOTICE, "A-bis connection establishment to BSC (%s) in progress...\n", priv->current_bsc->addr);
 
 	/* patch in various data from VTY and other sources */
-	line_ops.cfg.ipa.addr = bsc_oml_host->addr;
+	line_ops.cfg.ipa.addr = priv->current_bsc->addr;
 	osmo_get_macaddr(bts_dev_info.mac_addr, "eth0");
 	bts_dev_info.site_id = bts->ip_access.site_id;
 	bts_dev_info.bts_id = bts->ip_access.bts_id;
@@ -239,13 +254,11 @@
 
 	OSMO_ASSERT(event == ABIS_LINK_EV_VTY_RM_ADDR);
 
-	if (priv->bsc_oml_host == data) {
+	if (priv->current_bsc == data) {
 		if (llist_count(&bts->bsc_oml_hosts) <= 1)
-			priv->bsc_oml_host = NULL;
-		else if (priv->bsc_oml_host == llist_last(&bts->bsc_oml_hosts))
-			priv->bsc_oml_host = priv->bsc_oml_host->prev;
+			priv->current_bsc = NULL;
 		else
-			priv->bsc_oml_host = priv->bsc_oml_host->next;
+			pick_next_bsc(fi);
 	}
 }
 
@@ -477,7 +490,6 @@
 
 	abis_link_fsm_priv = talloc_zero(bts->abis_link_fi, struct abis_link_fsm_priv);
 	OSMO_ASSERT(abis_link_fsm_priv);
-	abis_link_fsm_priv->bsc_oml_host = bts->bsc_oml_hosts.next;
 	abis_link_fsm_priv->bts = bts;
 	abis_link_fsm_priv->model_name = model_name;
 	bts->abis_link_fi->priv = abis_link_fsm_priv;

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/25458
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I3cc8a4148b3d63bc185b72dab8108105a6644910
Gerrit-Change-Number: 25458
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210920/69eb33c5/attachment.htm>