Attention is currently required from: neels, laforge, pespin, fixeria.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31618
to look at the new patch set (#13).
Change subject: pcu_sock: handle multiple BTSs with one BSC co-located PCU (in theory)
......................................................................
pcu_sock: handle multiple BTSs with one BSC co-located PCU (in theory)
The current PCU implementation has never been tested with multiple BTS
attached to it. This is due to the fact that it has been used
exclusively in an BTS co-located setup where naturally only one BTS is
present. The PCU sock protocol supports multiple BTSs in theory and we
should handle this correctly.
Related: OS#5198
Change-Id: I0b42c2c130106f6ffca2dd08d079e1a7bda41f0b
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/gsm_data.h
M include/osmocom/bsc/pcu_if.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/bts_vty.c
M src/osmo-bsc/pcu_sock.c
M src/osmo-bsc/timeslot_fsm.c
7 files changed, 143 insertions(+), 98 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/18/31618/13
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31618
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I0b42c2c130106f6ffca2dd08d079e1a7bda41f0b
Gerrit-Change-Number: 31618
Gerrit-PatchSet: 13
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31619 )
(
7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: pcu_sock: drop usage of PCUIF flag PCU_IF_FLAG_DT
......................................................................
pcu_sock: drop usage of PCUIF flag PCU_IF_FLAG_DT
The flag PCU_IF_FLAG_DT is no longer needed. The PCU implementation will
distinguish by the version number instead.
Change-Id: I0dc20e351deb14906b2edffc39499bad9659cc35
Related: OS#5198
---
M include/osmocom/bsc/pcuif_proto.h
M src/osmo-bsc/pcu_sock.c
2 files changed, 13 insertions(+), 2 deletions(-)
Approvals:
fixeria: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/pcuif_proto.h b/include/osmocom/bsc/pcuif_proto.h
index ef7fe39..e92ed49 100644
--- a/include/osmocom/bsc/pcuif_proto.h
+++ b/include/osmocom/bsc/pcuif_proto.h
@@ -46,7 +46,6 @@
/* flags */
#define PCU_IF_FLAG_ACTIVE (1 << 0)/* BTS is active */
#define PCU_IF_FLAG_SYSMO (1 << 1)/* access PDCH of sysmoBTS directly */
-#define PCU_IF_FLAG_DT (1 << 2)/* use TLLI for confirmation directly */
#define PCU_IF_FLAG_CS1 (1 << 16)
#define PCU_IF_FLAG_CS2 (1 << 17)
#define PCU_IF_FLAG_CS3 (1 << 18)
diff --git a/src/osmo-bsc/pcu_sock.c b/src/osmo-bsc/pcu_sock.c
index 2d77efb..5f61356 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -204,7 +204,6 @@
info_ind->version = PCU_IF_VERSION;
info_ind->flags |= PCU_IF_FLAG_ACTIVE;
info_ind->flags |= PCU_IF_FLAG_SYSMO;
- info_ind->flags |= PCU_IF_FLAG_DT;
/* RAI */
info_ind->mcc = bts->network->plmn.mcc;
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31619
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I0dc20e351deb14906b2edffc39499bad9659cc35
Gerrit-Change-Number: 31619
Gerrit-PatchSet: 13
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31803 )
Change subject: pcu_sock: check BTS type properly in pcu_info_update()
......................................................................
pcu_sock: check BTS type properly in pcu_info_update()
When updating the BTS information in the bsc co-located PCU, first check
if the BTS has a BSC co-located PCU at all. Also check if the BTS is E1
based since those type of BTS require extra information about the E1
connection.
Related: OS#5198
Change-Id: I8da26debc0e27f24fae4ee88f22f8875de13bc84
---
M src/osmo-bsc/pcu_sock.c
1 file changed, 22 insertions(+), 7 deletions(-)
Approvals:
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/osmo-bsc/pcu_sock.c b/src/osmo-bsc/pcu_sock.c
index 453d5a7..2d77efb 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -362,13 +362,13 @@
__attribute__((weak)) void pcu_info_update(struct gsm_bts *bts)
{
if (pcu_connected(bts)) {
- /* In cases where the CCU is connected via an E1 line, we transmit the connection parameters for the
- * PDCH before we announce the other BTS related parameters. At the moment Ericsson RBS is the only
- * E1 BTS we support. */
- if (is_ericsson_bts(bts))
- pcu_tx_e1_ccu_ind(bts);
-
- pcu_tx_info_ind(bts);
+ if (bsc_co_located_pcu(bts)) {
+ /* In cases where the CCU is connected via an E1 line, we transmit the connection parameters for the
+ * PDCH before we announce the other BTS related parameters. */
+ if (is_e1_bts(bts))
+ pcu_tx_e1_ccu_ind(bts);
+ pcu_tx_info_ind(bts);
+ }
}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31803
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8da26debc0e27f24fae4ee88f22f8875de13bc84
Gerrit-Change-Number: 31803
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31802 )
Change subject: pcu_sock: use is_ericsson_bts() to check for ericsson BTS
......................................................................
pcu_sock: use is_ericsson_bts() to check for ericsson BTS
Do not access bts->type directly, we have is_ericsson_bts() to do that
Related: OS#5198
Change-Id: I274a9f0f1208dc17713ba2e1c7a1110eeb133cca
---
M src/osmo-bsc/pcu_sock.c
1 file changed, 13 insertions(+), 1 deletion(-)
Approvals:
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/osmo-bsc/pcu_sock.c b/src/osmo-bsc/pcu_sock.c
index b6e626d..453d5a7 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -573,7 +573,7 @@
* assign downlink TBFs directly through the paging channel. However, this method never became part
* of the RSL specs. This means that each BTS vendor has to come up with a proprietary method. At
* the moment we only support Ericsson RBS here. */
- if (bts->type == GSM_BTS_TYPE_RBS2000) {
+ if (is_ericsson_bts(bts)) {
rc = rsl_ericsson_imm_assign_cmd(bts, pch_dt->tlli, sizeof(pch_dt->data),
pch_dt->data, pag_grp);
} else {
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31802
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I274a9f0f1208dc17713ba2e1c7a1110eeb133cca
Gerrit-Change-Number: 31802
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31801 )
(
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: bts: add function to check if a BTS has a BSC co located PCU
......................................................................
bts: add function to check if a BTS has a BSC co located PCU
At the momemnt we use is_ericsson_bts() to check if the BTS uses a BSC
co-located PCU, this is a bit ambiguous, lets have a function that
explicitly checks for a BSC co-located PCU and nothing else.
Change-Id: I23ed4219e5ebd188867c17f387ca877efa9bc3b0
Related: OS#5198
---
M include/osmocom/bsc/bts.h
M src/osmo-bsc/timeslot_fsm.c
2 files changed, 27 insertions(+), 1 deletion(-)
Approvals:
pespin: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h
index ea396cc..ccca186 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -751,6 +751,18 @@
return false;
}
+static inline bool bsc_co_located_pcu(const struct gsm_bts *bts)
+{
+ switch (bts->type) {
+ case GSM_BTS_TYPE_RBS2000:
+ return true;
+ default:
+ break;
+ }
+
+ return false;
+}
+
static inline const struct osmo_location_area_id *bts_lai(struct gsm_bts *bts)
{
static struct osmo_location_area_id lai;
diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c
index aa5e0db..790ad77 100644
--- a/src/osmo-bsc/timeslot_fsm.c
+++ b/src/osmo-bsc/timeslot_fsm.c
@@ -341,7 +341,7 @@
return;
}
- if (is_ericsson_bts(bts) && !pcu_connected(bts)) {
+ if (bsc_co_located_pcu(bts) && !pcu_connected(bts)) {
LOG_TS(ts, LOGL_DEBUG, "PCU not connected: not activating PDCH.\n");
return;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31801
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I23ed4219e5ebd188867c17f387ca877efa9bc3b0
Gerrit-Change-Number: 31801
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31969 )
Change subject: l1gprs: fix NULL pointer dereference in l1gprs_unregister_tbf()
......................................................................
Patch Set 1:
(1 comment)
File src/shared/l1gprs.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/31969/comment/82ae0b89_a4b08368
PS1, Line 138: "%s(): " LOG_TBF_FMT " not found\n",
> Yes but my point is that passing that LOG_TBF_FMT here makes the code more difficult to understand b […]
Following this logic using macros in general makes the code complicated because "you're not sure what you pass matches with what you have in the macro until you look at it". So you want me to expand the `LOG_TBF_FMT` macro here. I disagree and prefer to keep it in the format string. If we ever change `LOG_TBF_FMT`, we're sure that this logging statement is also consistent with the new format (because we'll see compiler warnings).
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31969
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Ie3e7b5a8a205d4410de458dec2fde466493d31ce
Gerrit-Change-Number: 31969
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 20 Mar 2023 09:26:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment