[PATCH] osmo-bsc[master]: Split paging cases in bssmap_handle_paging() off into helper...

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/.

Stefan Sperling gerrit-no-reply at lists.osmocom.org
Tue Jan 16 14:36:42 UTC 2018


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

Split paging cases in bssmap_handle_paging() off into helper functions.

This is mostly no-op code refactoring which makes it easier to maintain the
code for each paging case and reduces the scope of several local variables.

Also, ensure that paging failures where no matching BTS was found are logged
consistently in all cases. The log level changes from ERROR to NOTICE since
this is not necessarily a fatal condition.

Change-Id: If8fdf425145791f4904a70e295bdc3c7d0f4d5f6
---
M src/osmo-bsc/osmo_bsc_bssap.c
1 file changed, 186 insertions(+), 130 deletions(-)


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

diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 1894561..799cb46 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -273,6 +273,179 @@
 	return gsm48_decode_lai(&lai, mcc, mnc, lac) != 0 ? -1 : 0;
 }
 
+static void
+page_all_bts(struct bsc_msc_data *msc, uint32_t tmsi, const char *mi_string, uint8_t chan_needed)
+{
+	struct gsm_bts *bts;
+	llist_for_each_entry(bts, &msc->network->bts_list, list) {
+		/* ignore errors from page_subscriber(); try all BTS */
+		page_subscriber(msc, bts, tmsi, GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed);
+	}
+}
+
+static void
+page_cgi(struct bsc_msc_data *msc, const uint8_t *data, uint8_t data_length, size_t remain,
+	 uint32_t tmsi, const char *mi_string, uint8_t chan_needed)
+{
+	uint16_t ci;
+	int i = 0;
+	while (remain >= sizeof(struct gsm48_loc_area_id) + sizeof(ci)) {
+		uint16_t mcc, mnc, lac, *ci_be;
+		size_t lai_offset = 1 + i * (sizeof(struct gsm48_loc_area_id) + sizeof(ci));
+		if (decode_lai(&data[lai_offset], &mcc, &mnc, &lac) != 0) {
+			LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Invalid LAI in Cell Identifier List "
+			     "for BSS (0x%x), paging entire BSS anyway (%s)\n",
+			     mi_string, CELL_IDENT_BSS, osmo_hexdump(data, data_length));
+			page_all_bts(msc, tmsi, mi_string, chan_needed);
+			return;
+		}
+		ci_be = (uint16_t *)(&data[lai_offset + sizeof(struct gsm48_loc_area_id)]);
+		ci = osmo_load16be(ci_be);
+		if (mcc == msc->network->country_code && mnc == msc->network->network_code) {
+			int paged = 0;
+			struct gsm_bts *bts;
+			llist_for_each_entry(bts, &msc->network->bts_list, list) {
+				if (bts->location_area_code != lac)
+					continue;
+				if (bts->cell_identity != ci)
+					continue;
+				/* ignore errors from page_subscriber(); keep trying other BTS */
+				page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
+				paged = 1;
+			}
+			if (!paged) {
+				LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: BTS with LAC %d and CI %d not found\n",
+				     mi_string, lac, ci);
+			}
+		} else {
+			LOGP(DMSC, LOGL_DEBUG, "Paging IMSI %s: MCC/MNC in Cell Identifier List "
+			     "(%d/%d) do not match our network (%d/%d)\n", mi_string, mcc, mnc,
+			     msc->network->country_code, msc->network->network_code);
+		}
+		remain -= sizeof(struct gsm48_loc_area_id) + sizeof(ci);
+		i++;
+	}
+}
+
+static void
+page_lac_and_ci(struct bsc_msc_data *msc, const uint8_t *data, size_t remain,
+	 uint32_t tmsi, const char *mi_string, uint8_t chan_needed)
+{
+	uint16_t *lacp_be, *ci_be;
+	lacp_be = (uint16_t *)(&data[1]);
+	ci_be = (uint16_t *)(&data[3]);
+	while (remain >= sizeof(*lacp_be) + sizeof(*ci_be)) {
+		uint16_t lac = osmo_load16be(lacp_be);
+		uint16_t ci = osmo_load16be(ci_be);
+		int paged = 0;
+		struct gsm_bts *bts;
+		llist_for_each_entry(bts, &msc->network->bts_list, list) {
+			if (bts->location_area_code != lac)
+				continue;
+			if (bts->cell_identity != ci)
+				continue;
+			/* ignore errors from page_subscriber(); keep trying other BTS */
+			page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
+			paged = 1;
+		}
+		if (!paged) {
+			LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: BTS with LAC %d and CI %d not found\n",
+			     mi_string, lac, ci);
+		}
+		remain -= sizeof(*lacp_be) + sizeof(*ci_be);
+		lacp_be++;
+		ci_be++;
+	}
+}
+
+static void
+page_ci(struct bsc_msc_data *msc, const uint8_t *data, size_t remain,
+	 uint32_t tmsi, const char *mi_string, uint8_t chan_needed)
+{
+	uint16_t *ci_be = (uint16_t *)(&data[1]);
+	while (remain >= sizeof(*ci_be)) {
+		uint16_t ci = osmo_load16be(ci_be);
+		int paged = 0;
+		struct gsm_bts *bts;
+		llist_for_each_entry(bts, &msc->network->bts_list, list) {
+			if (bts->cell_identity != ci)
+				continue;
+			/* ignore errors from page_subscriber(); keep trying other BTS */
+			page_subscriber(msc, bts, tmsi, GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed);
+			paged = 1;
+		}
+		if (!paged) {
+			LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: BTS with CI %d not found\n",
+			     mi_string, ci);
+		}
+		remain -= sizeof(*ci_be);
+		ci_be++;
+	}
+}
+
+static void
+page_lai_and_lac(struct bsc_msc_data *msc, const uint8_t *data, size_t data_length, size_t remain,
+	 uint32_t tmsi, const char *mi_string, uint8_t chan_needed)
+{
+	int i = 0;
+	while (remain >= sizeof(struct gsm48_loc_area_id)) {
+		uint16_t mcc, mnc, lac;
+		if (decode_lai(&data[1 + i * sizeof(struct gsm48_loc_area_id)], &mcc, &mnc, &lac) != 0) {
+			LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Invalid LAI in Cell Identifier List "
+			     "for BSS (0x%x), paging entire BSS anyway (%s)\n",
+			     mi_string, CELL_IDENT_BSS, osmo_hexdump(data, data_length));
+			page_all_bts(msc, tmsi, mi_string, chan_needed);
+			return;
+		}
+		if (mcc == msc->network->country_code && mnc == msc->network->network_code) {
+			int paged = 0;
+			struct gsm_bts *bts;
+			llist_for_each_entry(bts, &msc->network->bts_list, list) {
+				if (bts->location_area_code != lac)
+					continue;
+				/* ignore errors from page_subscriber(); keep trying other BTS */
+				page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
+				paged = 1;
+			}
+			if (!paged) {
+				LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: BTS with LAC %d not found\n",
+				     mi_string, lac);
+			}
+		} else {
+			LOGP(DMSC, LOGL_DEBUG, "Not paging IMSI %s: MCC/MNC in Cell Identifier List "
+			     "(%d/%d) do not match our network (%d/%d)\n", mi_string, mcc, mnc,
+			     msc->network->country_code, msc->network->network_code);
+		}
+		remain -= sizeof(struct gsm48_loc_area_id);
+		i++;
+	}
+}
+
+static void
+page_lac(struct bsc_msc_data *msc, const uint8_t *data, size_t remain,
+	 uint32_t tmsi, const char *mi_string, uint8_t chan_needed)
+{
+	uint16_t *lacp_be = (uint16_t *)(&data[1]);
+	while (remain >= sizeof(*lacp_be)) {
+		uint16_t lac = osmo_load16be(lacp_be);
+		int paged = 0;
+		struct gsm_bts *bts;
+		llist_for_each_entry(bts, &msc->network->bts_list, list) {
+			if (bts->location_area_code != lac)
+				continue;
+			/* ignore errors from page_subscriber(); keep trying other BTS */
+			page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
+			paged = 1;
+		}
+		if (!paged) {
+			LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: BTS with LAC %d not found\n",
+			     mi_string, lac);
+		}
+		remain -= sizeof(*lacp_be);
+		lacp_be++;
+	}
+}
+
 /* GSM 08.08 § 3.2.1.19 */
 static int bssmap_handle_paging(struct bsc_msc_data *msc,
 				struct msgb *msg, unsigned int payload_length)
@@ -280,15 +453,11 @@
 	struct tlv_parsed tp;
 	char mi_string[GSM48_MI_SIZE];
 	uint32_t tmsi = GSM_RESERVED_TMSI;
-	uint16_t lac, *lacp_be;
-	uint16_t mcc;
-	uint16_t mnc;
 	uint8_t data_length;
 	int remain;
 	const uint8_t *data;
 	uint8_t chan_needed = RSL_CHANNEED_ANY;
 	uint8_t cell_ident;
-	struct gsm_bts *bts;
 
 	tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, payload_length - 1, 0, 0);
 	remain = payload_length - 1;
@@ -353,136 +522,29 @@
 	cell_ident = data[0] & 0xf;
 	remain -= 1; /* cell ident consumed */
 
-	/* Default fallback: page entire BSS */
-	lac = GSM_LAC_RESERVED_ALL_BTS;
-
 	switch (cell_ident) {
 	case CELL_IDENT_NO_CELL:
 		LOGP(DMSC, LOGL_NOTICE, "Ignoring no-op paging request for IMSI %s\n", mi_string);
 		return 0; /* nothing to do */
 
-	case CELL_IDENT_WHOLE_GLOBAL: {
-		uint16_t ci;
-		int i = 0;
-		while (remain >= sizeof(struct gsm48_loc_area_id) + sizeof(ci)) {
-			uint16_t *ci_be;
-			size_t lai_offset = 1 + i * (sizeof(struct gsm48_loc_area_id) + sizeof(ci));
-			if (decode_lai(&data[lai_offset], &mcc, &mnc, &lac) != 0) {
-				LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Invalid LAI in Cell Identifier List "
-				     "for BSS (0x%x), paging entire BSS anyway (%s)\n",
-				     mi_string, CELL_IDENT_BSS, osmo_hexdump(data, data_length));
-				lac = GSM_LAC_RESERVED_ALL_BTS;
-				break;
-			}
-			ci_be = (uint16_t *)(&data[lai_offset + sizeof(struct gsm48_loc_area_id)]);
-			ci = osmo_load16be(ci_be);
-			if (mcc == msc->network->country_code && mnc == msc->network->network_code) {
-				llist_for_each_entry(bts, &msc->network->bts_list, list) {
-					if (bts->location_area_code != lac)
-						continue;
-					if (bts->cell_identity != ci)
-						continue;
-					if (page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed) < 0)
-						break;
-				}
-			} else {
-				LOGP(DMSC, LOGL_DEBUG, "Not paging IMSI %s: MCC/MNC in Cell Identifier List "
-				     "(%d/%d) do not match our network (%d/%d)\n", mi_string, mcc, mnc,
-				     msc->network->country_code, msc->network->network_code);
-			}
-			remain -= sizeof(struct gsm48_loc_area_id) + sizeof(ci);
-			i++;
-		}
-	}
-
-	case CELL_IDENT_LAC_AND_CI: {
-		uint16_t ci, *ci_be;
-		lacp_be = (uint16_t *)(&data[1]);
-		ci_be = (uint16_t *)(&data[3]);
-		while (remain >= sizeof(*lacp_be) + sizeof(*ci_be)) {
-			lac = osmo_load16be(lacp_be);
-			ci = osmo_load16be(ci_be);
-
-			llist_for_each_entry(bts, &msc->network->bts_list, list) {
-				if (bts->location_area_code != lac)
-					continue;
-				if (bts->cell_identity != ci)
-					continue;
-				if (page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed) < 0)
-					break;
-			}
-
-			remain -= sizeof(*lacp_be) + sizeof(*ci_be);
-			lacp_be++;
-			ci_be++;
-		}
+	case CELL_IDENT_WHOLE_GLOBAL:
+		page_cgi(msc, data, data_length, remain, tmsi, mi_string, chan_needed);
 		break;
-	}
 
-	case CELL_IDENT_CI: {
-		uint16_t *ci_be = (uint16_t *)(&data[1]);
-		while (remain >= sizeof(*ci_be)) {
-			uint16_t ci = osmo_load16be(ci_be);
-
-			llist_for_each_entry(bts, &msc->network->bts_list, list) {
-				if (bts->cell_identity == ci)
-					break;
-			}
-
-			if (bts) {
-				/* ignore errors from page_subscriber(); keep trying other BTS */
-				page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
-			} else {
-				LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: BTS with cell identifier %d not found\n",
-				     mi_string, ci);
-			}
-			remain -= sizeof(*ci_be);
-			ci_be++;
-		}
+	case CELL_IDENT_LAC_AND_CI:
+		page_lac_and_ci(msc, data, remain, tmsi, mi_string, chan_needed);
 		break;
-	}
 
-	case CELL_IDENT_LAI_AND_LAC: {
-		int i = 0;
-		while (remain >= sizeof(struct gsm48_loc_area_id)) {
-			if (decode_lai(&data[1 + i * sizeof(struct gsm48_loc_area_id)], &mcc, &mnc, &lac) != 0) {
-				LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Invalid LAI in Cell Identifier List "
-				     "for BSS (0x%x), paging entire BSS anyway (%s)\n",
-				     mi_string, CELL_IDENT_BSS, osmo_hexdump(data, data_length));
-				lac = GSM_LAC_RESERVED_ALL_BTS;
-				break;
-			}
-			if (mcc == msc->network->country_code && mnc == msc->network->network_code) {
-				llist_for_each_entry(bts, &msc->network->bts_list, list) {
-					if (bts->location_area_code != lac)
-						continue;
-					/* ignore errors from page_subscriber(); keep trying other BTS */
-					page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
-				}
-			} else {
-				LOGP(DMSC, LOGL_DEBUG, "Not paging IMSI %s: MCC/MNC in Cell Identifier List "
-				     "(%d/%d) do not match our network (%d/%d)\n", mi_string, mcc, mnc,
-				     msc->network->country_code, msc->network->network_code);
-			}
-			remain -= sizeof(struct gsm48_loc_area_id);
-			i++;
-		}
+	case CELL_IDENT_CI:
+		page_ci(msc, data, remain, tmsi, mi_string, chan_needed);
 		break;
-	}
+
+	case CELL_IDENT_LAI_AND_LAC:
+		page_lai_and_lac(msc, data, data_length, remain, tmsi, mi_string, chan_needed);
+		break;
 
 	case CELL_IDENT_LAC:
-		lacp_be = (uint16_t *)(&data[1]);
-		while (remain >= sizeof(*lacp_be)) {
-			lac = osmo_load16be(lacp_be);
-			llist_for_each_entry(bts, &msc->network->bts_list, list) {
-				if (bts->location_area_code != lac)
-					continue;
-				/* ignore errors from page_subscriber(); keep trying other BTS */
-				page_subscriber(msc, bts, tmsi, lac, mi_string, chan_needed);
-			}
-			remain -= sizeof(*lacp_be);
-			lacp_be++;
-		}
+		page_lac(msc, data, remain, tmsi, mi_string, chan_needed);
 		break;
 
 	case CELL_IDENT_BSS:
@@ -491,20 +553,14 @@
 			     " has invalid length: %u, paging entire BSS anyway (%s)\n",
 			     mi_string, CELL_IDENT_BSS, data_length, osmo_hexdump(data, data_length));
 		}
-		llist_for_each_entry(bts, &msc->network->bts_list, list) {
-			/* ignore errors from page_subscriber(); try all BTS */
-			page_subscriber(msc, bts, tmsi, GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed);
-		}
+		page_all_bts(msc, tmsi, mi_string, chan_needed);
 		break;
 
 	default:
 		LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: unimplemented Cell Identifier List (0x%x),"
 		     " paging entire BSS instead (%s)\n",
 		     mi_string, cell_ident, osmo_hexdump(data, data_length));
-		llist_for_each_entry(bts, &msc->network->bts_list, list) {
-			/* ignore errors from page_subscriber(); try all BTS */
-			page_subscriber(msc, bts, tmsi, GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed);
-		}
+		page_all_bts(msc, tmsi, mi_string, chan_needed);
 		break;
 	}
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If8fdf425145791f4904a70e295bdc3c7d0f4d5f6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>



More information about the gerrit-log mailing list