Attention is currently required from: fixeria.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34240 )
Change subject: pcu_sock: print SAPI and msg_id when sending confirmation
......................................................................
Patch Set 1:
(1 comment)
File src/common/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bts/+/34240/comment/76729b33_8bd1dce6
PS1, Line 631: confirmation
> `Sending DATA.cnf` or `Sending data confirmation` would be more informative, IMO. […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34240
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ibd5b4225e597b69eaabaeee437fb637943a55602
Gerrit-Change-Number: 34240
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 31 Aug 2023 09:04:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34059 )
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/54c1e438_4bf7177b
PS4, Line 14: CAUTION: This patch breaks compatibility to current master osmo-pcu (See
: also "Depends")
> Normally such breaking changes should be accompanied with the protocol version bump. […]
We still have the option to bump the version number in this patch and then in the future apply the policy that the version number must be bumped as soon as a detail in PCUIF changes.
What do you think? Shall we bump the version to v.12 here now (osmo-ttcn3-hacks would require a follow up patch)
File src/common/bts.c:
https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/dc7e0a13_2a8ec34f
PS4, Line 733: struct bts_agch_msg_cb *msg_cb
> const
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34059
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
Gerrit-Change-Number: 34059
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 31 Aug 2023 09:03:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/34059
to look at the new patch set (#5).
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
In PCUIF v.11 we use PCU_IF_SAPI_AGCH_2 exclusively. We use this SAPI
to transfer IMMEDIATE ASSIGNMENT messages for uplink and downlink. In
both cases we send a confirmation back to the PCU. For details see
coresponding patch in osmo-pcu.git (see Depends)
CAUTION: This patch breaks compatibility to current master osmo-pcu (See
also "Depends")
Related: OS#5927
Depends: osmo-pcu.git I9effdcec1da91a6e2e7a7c41f95d3300ad1bb292
Depends: osmo-ttcn3-hacks.git Iec00d8144dfb2cd8bcee9093c96a3cc98aea6458
Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
---
M include/osmo-bts/bts.h
M include/osmo-bts/pcu_if.h
M include/osmo-bts/pcuif_proto.h
M src/common/bts.c
M src/common/paging.c
M src/common/pcu_sock.c
6 files changed, 67 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/59/34059/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34059
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
Gerrit-Change-Number: 34059
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: dexter.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/34240
to look at the new patch set (#2).
Change subject: pcu_sock: print SAPI and msg_id when sending confirmation
......................................................................
pcu_sock: print SAPI and msg_id when sending confirmation
At the moment we do not print the SAPI, nor the msg_id when we send a
confirmation back to the PCU
Change-Id: Ibd5b4225e597b69eaabaeee437fb637943a55602
elated: OS#5927
---
M src/common/pcu_sock.c
1 file changed, 15 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/40/34240/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34240
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ibd5b4225e597b69eaabaeee437fb637943a55602
Gerrit-Change-Number: 34240
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria, pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34060 )
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
Patch Set 6:
(4 comments)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/b60a5f50_32e3754c
PS5, Line 445: confirmation
> Same as in the related osmo-bts patch: "Sending confirmation" may be confusing, let's make this clea […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/87ea5cc7_d16ba010
PS5, Line 533: struct gsm_pcu_if_agch *agch;
> const
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/9c80cd95_6371c55a
PS5, Line 551: rc = -EIO;
> So if `agch->confirm == true`, but `rsl_imm_assign_cmd()` fails, we still send the confirmation. […]
That is indeed wrong. Thanks.
https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/7e6b1cb0_4c4c6407
PS5, Line 555: gsm48_imm_ass = (struct gsm48_imm_ass *)agch->data;
> You're setting this pointer, but not using it within this `case`.
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34060
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I709c27adaf09a6766cfde4d76d878626d30ebb3c
Gerrit-Change-Number: 34060
Gerrit-PatchSet: 6
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 31 Aug 2023 09:03:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, pespin.
Hello osmith, Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/34060
to look at the new patch set (#6).
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
In PCUIF v.11 we use PCU_IF_SAPI_AGCH_2 exclusively. We use this SAPI
to transfer IMMEDIATE ASSIGNMENT messages for uplink and downlink. One
new feature of PCU_IF_SAPI_AGCH_2 is that the PCU may ask to send a
confirmation when the MAC block is sent.
CAUTION: This patch breaks compatibility to current master osmo-pcu (See
also "Depends")
Related: OS#5927
Depends: osmo-pcu.git I9effdcec1da91a6e2e7a7c41f95d3300ad1bb292
Change-Id: I709c27adaf09a6766cfde4d76d878626d30ebb3c
---
M include/osmocom/bsc/pcu_if.h
M include/osmocom/bsc/pcuif_proto.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/pcu_sock.c
4 files changed, 60 insertions(+), 13 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/60/34060/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34060
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I709c27adaf09a6766cfde4d76d878626d30ebb3c
Gerrit-Change-Number: 34060
Gerrit-PatchSet: 6
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34192 )
Change subject: pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
......................................................................
Patch Set 6:
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34192/comment/a8677961_26103618
PS1, Line 552: if (pch->confirmed_imm_ass)
> One can already see the consequences of making breaking changes "on the go" without bumping the prot […]
I take your criticism seriously. So the take-away here is that I should have bumped the version number in each patch that changes a detail of the PCUIF.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34192
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
Gerrit-Change-Number: 34192
Gerrit-PatchSet: 6
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-Comment-Date: Thu, 31 Aug 2023 08:44:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34192 )
Change subject: pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
......................................................................
pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
Since osmo-bsc uses RSL (with a propritary Ericsson RBS specific
extension) to send a confirmed IMMEDIATE ASSIGNMENT messages via
PCH, we can not just forward the MAC blocks into the paging queue
without determining whether the MAC block is a PAGING message or an
IMMEDIATE ASSIGNMENT message. the reason for this is that RSL uses
two different message types (IMMEDIATE ASSIGNMENT COMMAND and PAGING
COMMAND) to process IMMEDIATE ASSIGNMENT and PAGING messages.
This means we have to look into the MAC block to make sure whether
the message is a PAGING message or an IMMEDIATE ASSIGNMENT message.
We also need to make sure that the confirm flag is properly executed.
In the case of the IMMEDIATE ASSIGNMENT this means we have to include
(confirm=true) or not include (confirm=false) the RSL_IE_ERIC_MOBILE_ID
into the IMMEDIATE ASSIGNMENT COMMAND message.
In the case of PAGING we directly echo a confirmation after sending
the PAGING COMMAND via RSL when a confirmation is requested.
Related: OS#5927
Depends: osmo-pcu.git Ia202862aafc1f0cb6601574ef61eb9155de11f04
Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
---
M include/osmocom/bsc/abis_rsl.h
M include/osmocom/bsc/pcuif_proto.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/pcu_sock.c
4 files changed, 50 insertions(+), 9 deletions(-)
Approvals:
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/abis_rsl.h b/include/osmocom/bsc/abis_rsl.h
index a6b2cef..023793a 100644
--- a/include/osmocom/bsc/abis_rsl.h
+++ b/include/osmocom/bsc/abis_rsl.h
@@ -68,7 +68,7 @@
/* Ericcson vendor specific RSL extensions */
int rsl_ericsson_imm_assign_cmd(const struct gsm_bts *bts, uint32_t msg_id, uint8_t len,
- const uint8_t *val, uint8_t pag_grp);
+ const uint8_t *val, uint8_t pag_grp, bool confirm);
/* Siemens vendor-specific RSL extensions */
int rsl_siemens_mrpci(struct gsm_lchan *lchan, struct rsl_mrpci *mrpci);
diff --git a/include/osmocom/bsc/pcuif_proto.h b/include/osmocom/bsc/pcuif_proto.h
index bf49b06..aa22447 100644
--- a/include/osmocom/bsc/pcuif_proto.h
+++ b/include/osmocom/bsc/pcuif_proto.h
@@ -271,6 +271,9 @@
char imsi[OSMO_IMSI_BUF_SIZE];
/* GSM mac-block (with immediate assignment message) */
uint8_t data[GSM_MACBLOCK_LEN];
+ /* Set to true in case the receiving end must send a confirmation
+ * when the MAC block (data) has been sent. */
+ bool confirm;
} __attribute__((packed));
struct gsm_pcu_if {
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 5440a23..76948c9 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -996,7 +996,7 @@
/* Chapter 8.5.6 Immediate Assignment Command (with Ericcson vendor specific RSL extension) */
int rsl_ericsson_imm_assign_cmd(const struct gsm_bts *bts, uint32_t msg_id, uint8_t len,
- const uint8_t *val, uint8_t pag_grp)
+ const uint8_t *val, uint8_t pag_grp, bool confirm)
{
struct msgb *msg = rsl_imm_assign_cmd_common(bts, len, val);
if (!msg)
@@ -1008,8 +1008,10 @@
/* ericsson can handle a reference at the end of the message which is used in
* the confirm message. The confirm message is only sent if the trailer is present */
- msgb_put_u8(msg, RSL_IE_ERIC_MOBILE_ID);
- msgb_put_u32(msg, msg_id);
+ if (confirm) {
+ msgb_put_u8(msg, RSL_IE_ERIC_MOBILE_ID);
+ msgb_put_u32(msg, msg_id);
+ }
return abis_rsl_sendmsg(msg);
}
diff --git a/src/osmo-bsc/pcu_sock.c b/src/osmo-bsc/pcu_sock.c
index 6f48f0f..696d5f3 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -506,7 +506,7 @@
}
static int pcu_rx_rr_imm_ass_pch(struct gsm_bts *bts, uint8_t paging_group,
- const struct gsm_pcu_if_pch *pch)
+ const struct gsm_pcu_if_pch *pch, bool confirm)
{
LOG_BTS(bts, DPCU, LOGL_DEBUG, "PCU Sends immediate assignment via PCH (msg_id=0x%08x, IMSI=%s, Paging group=0x%02x)\n",
pch->msg_id, pch->imsi, paging_group);
@@ -516,7 +516,8 @@
* 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 (is_ericsson_bts(bts))
- return rsl_ericsson_imm_assign_cmd(bts, pch->msg_id, sizeof(pch->data), pch->data, paging_group);
+ return rsl_ericsson_imm_assign_cmd(bts, pch->msg_id, sizeof(pch->data), pch->data, paging_group,
+ confirm);
LOG_BTS(bts, DPCU, LOGL_ERROR, "BTS model does not support sending immediate assignment via PCH!\n");
return -ENOTSUP;
@@ -549,11 +550,16 @@
pch = (struct gsm_pcu_if_pch *)data_req->data;
pag_grp = gsm0502_calc_paging_group(&bts->si_common.chan_desc, str_to_imsi(pch->imsi));
-
gsm48_imm_ass = (struct gsm48_imm_ass *)pch->data;
+
if (gsm48_imm_ass->msg_type == GSM48_MT_RR_IMM_ASS)
- return pcu_rx_rr_imm_ass_pch(bts, pag_grp, pch);
- return pcu_rx_rr_paging_pch(bts, pag_grp, pch);
+ return pcu_rx_rr_imm_ass_pch(bts, pag_grp, pch, pch->confirm);
+
+ if (pcu_rx_rr_paging_pch(bts, pag_grp, pch))
+ return -EIO;
+ if (pch->confirm)
+ return pcu_tx_pch_confirm(bts, pch->msg_id);
+ break;
default:
LOG_BTS(bts, DPCU, LOGL_ERROR, "Received PCU data request with "
"unsupported sapi %d\n", data_req->sapi);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34192
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
Gerrit-Change-Number: 34192
Gerrit-PatchSet: 6
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