Attention is currently required from: neels, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31176 )
Change subject: support for Ericsson RBS E1 CCU
......................................................................
Patch Set 24:
(7 comments)
File configure.ac:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/4a9491fb_432ad9df
PS24, Line 90: PKG_CHECK_MODULES(LIBOSMOABIS, libosmoabis >= 1.4.0)
So the E1 CCU feature is configurable via `--enable-er-e1-ccu` and is disabled by default, but you're requiring `libosmo{abis,trau}` unconditionally. Maybe depend on these libraries only if the feature is enabled?
File contrib/jenkins.sh:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/9ef8c98e_73a7cf6b
PS24, Line 59: PCU_CONFIG="$PCU_CONFIG --enable-werror --enable-sanitize"
You should add `--enable-er-e1-ccu` here, so that the conditional code is also built by Jenkins.
File contrib/osmo-pcu.spec.in:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/e5acb7d9_eea5f3d8
PS24, Line 36: 1.3.0
You require 1.4.0 in `configure.ac`, but 1.3.0 here?
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/2e962ef1_e28a0ec4
PS24, Line 49: configure
Do we want to build RPM and DEB packages with `--enable-er-e1-ccu`?
File src/Makefile.am:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/c418f2b4_faf756d3
PS24, Line 218: EXTRA_DIST
Why do you put `.[ch]` files to `EXTRA_DIST`? This is unusual.
File src/ericsson-rbs/er_ccu_descr.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/e449870c_37b3ceaa
PS24, Line 3: #include <stdint.h>
missing `<stdbool.h>`
File src/ericsson-rbs/er_ccu_if.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/f54e5d0d_af70b8b2
PS24, Line 412: tall_ccu_ctx = ctx;
Either obtain a child ctx here, or simply use `extern void *tall_pcu_ctx;`.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31176
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I5c0a76667339ca984a12cbd2052f5d9e5b0f9c4d
Gerrit-Change-Number: 31176
Gerrit-PatchSet: 24
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 07 Mar 2023 12:55:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31684 )
Change subject: pcu_sock: rename rc to fd
......................................................................
pcu_sock: rename rc to fd
The variable rc holds the return code of accept(), which returns a file
descriptor on success. So lets call the variable "fd" to make this
clear.
Change-Id: Ic8d22c2af18477f110a3a9115434314ebca95b25
---
M src/osmo-bsc/pcu_sock.c
1 file changed, 18 insertions(+), 5 deletions(-)
Approvals:
laforge: 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 486f364..1d7b061 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -911,14 +911,14 @@
struct gsm_bts_trx *trx;
int j;
socklen_t len;
- int rc;
+ int fd;
/* FIXME: allow multiple BTS */
bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);
len = sizeof(un_addr);
- rc = accept(bfd->fd, (struct sockaddr *) &un_addr, &len);
- if (rc < 0) {
+ fd = accept(bfd->fd, (struct sockaddr *) &un_addr, &len);
+ if (fd < 0) {
LOG_BTS(bts, DPCU, LOGL_ERROR, "Failed to accept a new connection\n");
return -1;
}
@@ -927,11 +927,11 @@
LOG_BTS(bts, DPCU, LOGL_NOTICE, "PCU connects but we already have another active connection ?!?\n");
/* We already have one PCU connected, this is all we support */
osmo_fd_read_disable(&state->listen_bfd);
- close(rc);
+ close(fd);
return 0;
}
- osmo_fd_setup(conn_bfd, rc, OSMO_FD_READ, pcu_sock_cb, state, 0);
+ osmo_fd_setup(conn_bfd, fd, OSMO_FD_READ, pcu_sock_cb, state, 0);
if (osmo_fd_register(conn_bfd) != 0) {
LOG_BTS(bts, DPCU, LOGL_ERROR, "Failed to register new connection fd\n");
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31684
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ic8d22c2af18477f110a3a9115434314ebca95b25
Gerrit-Change-Number: 31684
Gerrit-PatchSet: 2
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-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31578 )
Change subject: pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH
......................................................................
pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH
When the IMMEDIATE ASSIGNMENT is sent from the PCU to the BSC using the
"direct TLLI" method, the TLLI (and the last three digits of the IMSI)
is prepended to the MAC block. Currently we are taking the fields apart
manually using offsets. The code for this is difficult to read and the
method is error prone. Let's define a struct that we can just overlay
to access the fields directly. Let's also transfer the full IMSI.
Change-Id: Id6acbd243adf26169e5e8319dd66bb68dd6a3c22
Related: OS#5198
---
M include/osmocom/bsc/pcuif_proto.h
M src/osmo-bsc/pcu_sock.c
2 files changed, 38 insertions(+), 10 deletions(-)
Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/pcuif_proto.h b/include/osmocom/bsc/pcuif_proto.h
index 7e13b5c..ef7fe39 100644
--- a/include/osmocom/bsc/pcuif_proto.h
+++ b/include/osmocom/bsc/pcuif_proto.h
@@ -3,6 +3,8 @@
#include <osmocom/gsm/l1sap.h>
#include <arpa/inet.h>
+#include <osmocom/gsm/protocol/gsm_04_08.h>
+#include <osmocom/gsm/protocol/gsm_23_003.h>
#define PCU_SOCK_DEFAULT "/tmp/pcu_bts"
@@ -270,6 +272,17 @@
} cgi_ps;
} __attribute__ ((packed));
+/* Struct to send a (confirmed) IMMEDIATE ASSIGNMENT message via PCH. The struct is sent as a data request
+ * (data_req) under SAPI PCU_IF_SAPI_PCH_DT. */
+struct gsm_pcu_if_pch_dt {
+ /* TLLI as reference for confirmation */
+ uint32_t tlli;
+ /* IMSI (to derive paging group) */
+ char imsi[OSMO_IMSI_BUF_SIZE];
+ /* GSM mac-block (with immediate assignment message) */
+ uint8_t data[GSM_MACBLOCK_LEN];
+} __attribute__((packed));
+
struct gsm_pcu_if {
/* context based information */
uint8_t msg_type; /* message type */
diff --git a/src/osmo-bsc/pcu_sock.c b/src/osmo-bsc/pcu_sock.c
index 51b59c6..187da18 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -535,9 +535,9 @@
static int pcu_rx_data_req(struct gsm_bts *bts, uint8_t msg_type,
struct gsm_pcu_if_data *data_req)
{
- uint32_t tlli = -1;
uint8_t pag_grp;
int rc = 0;
+ struct gsm_pcu_if_pch_dt *pch_dt;
LOGP(DPCU, LOGL_DEBUG, "Data request received: sapi=%s arfcn=%d "
"block=%d data=%s\n", sapi_string[data_req->sapi],
@@ -558,27 +558,25 @@
/* DT = direct TLLI. A tlli is prefixed so that the BSC/BTS can confirm the sending of the downlink
* IMMEDIATE ASSIGNMENT towards the PCU using this TLLI as a reference. */
- if (data_req->len < 8) {
+ if (data_req->len < sizeof(struct gsm_pcu_if_pch_dt)) {
LOGP(DPCU, LOGL_ERROR, "Received PCU data request with invalid/small length %d\n",
data_req->len);
break;
}
- /* Extract 4 byte TLLI */
- memcpy(&tlli, data_req->data, 4);
+ pch_dt = (struct gsm_pcu_if_pch_dt *)data_req->data;
+ pag_grp = gsm0502_calc_paging_group(&bts->si_common.chan_desc, str_to_imsi(pch_dt->imsi));
- /* Extract 3 byte paging group */
- pag_grp = extract_paging_group(bts, data_req->data + 4);
-
- LOGP(DPCU, LOGL_DEBUG, "PCU Sends immediate assignment via PCH (tlli=0x%08x, pag_grp=0x%02x)\n",
- tlli, pag_grp);
+ LOGP(DPCU, LOGL_DEBUG, "PCU Sends immediate assignment via PCH (TLLI=0x%08x, IMSI=%s, Paging group=0x%02x)\n",
+ pch_dt->tlli, pch_dt->imsi, pag_grp);
/* NOTE: Sending an IMMEDIATE ASSIGNMENT via PCH became necessary with GPRS in order to be able to
* 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) {
- rc = rsl_ericsson_imm_assign_cmd(bts, tlli, data_req->len - 7, data_req->data + 7, pag_grp);
+ rc = rsl_ericsson_imm_assign_cmd(bts, pch_dt->tlli, sizeof(pch_dt->data),
+ pch_dt->data, pag_grp);
} else {
LOGP(DPCU, LOGL_ERROR, "BTS model does not support sending immediate assignment via PCH!\n");
rc = -ENOTSUP;
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31578
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Id6acbd243adf26169e5e8319dd66bb68dd6a3c22
Gerrit-Change-Number: 31578
Gerrit-PatchSet: 12
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: laforge, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31687 )
Change subject: pcu_sock: only allow Ericsson RBS to connect to external PCU
......................................................................
Patch Set 2: Code-Review-1
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31687/comment/6dc41cd7_c6193fdd
PS2, Line 964: return -EINVAL;
So when this condition is true, osmo-bsc would fail to start printing:
%% Error creating PCU socket `%s' for BTS %u
But nothing specific *why* it failed. Please print something here.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31687
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I18d190d661b06015419078382915c4606be01b04
Gerrit-Change-Number: 31687
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 07 Mar 2023 12:23:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: dexter.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31650 )
Change subject: pcu_l1_if: get rid of strange paging group calculation
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31650
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I40d7fc14c9591b3de091e425faaf325421c70a0e
Gerrit-Change-Number: 31650
Gerrit-PatchSet: 7
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 07 Mar 2023 12:11:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment