Attention is currently required from: laforge.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-sigtran/+/39690?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: sccp: Allow marking UNIT-DATA.req Sequence Control param non-presence, set SLS
......................................................................
sccp: Allow marking UNIT-DATA.req Sequence Control param non-presence, set SLS
The public API for SCCP UNIT-DATA.req was missing a way to state whether
the "Sequence Control" Parameter was present or not in the primitive.
This was originated most probably by the fact that the field is always
present for SUA CLDT message generated and sent over the wire.
Still, the presence of such field during UNIT-DATA.req actually
indicates the Protocol Class to be used/set on the message.
Hence, it is important to provide it.
Once the Protocol Class to transmit the message is known, we can then
generate a proper SLS.
Change-Id: I834e91c1ec337713b0e684cf4fd7de854150bef6
---
M include/osmocom/sigtran/sccp_sap.h
M src/sccp_helpers.c
M src/sccp_sclc.c
3 files changed, 39 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/90/39690/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39690?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I834e91c1ec337713b0e684cf4fd7de854150bef6
Gerrit-Change-Number: 39690
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Attention is currently required from: laforge.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39690?usp=email )
Change subject: sccp: Allow marking UNIT-DATA.req Sequence Control param non-presence, set SLS
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
FYI: All users of SCCP UNIT-DATA.req (through osmo_sccp_tx_unitdata(_msg)()) are using it to send RESET/RESET-ACK and PAGING messages, so it's fine to set the OSMO_SCU_UNITDATA_REQ_P_SEQUENCE_CONTROL_NOT_PRESENT in osmo_sccp_tx_unitdata() so they end up as ProtoClass 0 imho.
I used the NOT_PRESENT define in order to avoid breaking ABI, let me know if you think I should be adding a "bool in_seq_control_present" instead.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39690?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I834e91c1ec337713b0e684cf4fd7de854150bef6
Gerrit-Change-Number: 39690
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 04 Mar 2025 20:20:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39690?usp=email )
Change subject: sccp: Allow marking UNIT-DATA.req Sequence Control param non-presence, set SLS
......................................................................
sccp: Allow marking UNIT-DATA.req Sequence Control param non-presence, set SLS
The public API for SCCP UNIT-DATA.req was missing a way to state whether
the "Sequence Control" Parameter was present or not in the primitive.
This was originated most probably by the fact that the field is always
present for SUA CLDT message generated and sent over the wire.
Still, the presence of such field during UNIT-DATA.req actually
indicates the Protocol Class to be used/set on the message.
Hence, it is important to provide it.
Once the Protocol Class to transmit the message is known, we can then
generate a proper SLS.
Change-Id: I834e91c1ec337713b0e684cf4fd7de854150bef6
---
M include/osmocom/sigtran/sccp_sap.h
M src/sccp_helpers.c
M src/sccp_sclc.c
3 files changed, 39 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/90/39690/1
diff --git a/include/osmocom/sigtran/sccp_sap.h b/include/osmocom/sigtran/sccp_sap.h
index d0fb040..42c8af3 100644
--- a/include/osmocom/sigtran/sccp_sap.h
+++ b/include/osmocom/sigtran/sccp_sap.h
@@ -236,6 +236,13 @@
};
/* OSMO_SCU_PRIM_N_UNITDATA */
+/* NOTE: Idenally there should have been a "bool in_sequence_control_present",
+ * but that was found too late, so instead set:
+ * "in_sequence_control = OSMO_SCU_UNITDATA_PARAM_SEQUENCE_CONTROL_NOT_PRESENT"
+ * to mark the field not present (aka select Protocol Class 0).
+ * See ITU-T Q.711 6.2.1 for more information.
+ */
+#define OSMO_SCU_UNITDATA_REQ_P_SEQUENCE_CONTROL_NOT_PRESENT 0xffffffff
struct osmo_scu_unitdata_param {
struct osmo_sccp_addr called_addr;
struct osmo_sccp_addr calling_addr;
diff --git a/src/sccp_helpers.c b/src/sccp_helpers.c
index 2a04b9d..6dd9ab8 100644
--- a/src/sccp_helpers.c
+++ b/src/sccp_helpers.c
@@ -72,6 +72,7 @@
param = &prim->u.unitdata;
memcpy(¶m->calling_addr, calling_addr, sizeof(*calling_addr));
memcpy(¶m->called_addr, called_addr, sizeof(*called_addr));
+ param->in_sequence_control = OSMO_SCU_UNITDATA_REQ_P_SEQUENCE_CONTROL_NOT_PRESENT;
osmo_prim_init(&prim->oph, SCCP_SAP_USER, OSMO_SCU_PRIM_N_UNITDATA, PRIM_OP_REQUEST, msg);
msg->l2h = msgb_put(msg, len);
diff --git a/src/sccp_sclc.c b/src/sccp_sclc.c
index 186efbd..e0a2844 100644
--- a/src/sccp_sclc.c
+++ b/src/sccp_sclc.c
@@ -35,9 +35,6 @@
* SUA which classic SCCP cannot handle (like IP addresses in GT).
* However, all SCCP features can be expressed in SUA.
*
- * The code only supports Class 2. No support for Class 3 is intended,
- * but patches are of course always welcome.
- *
* Missing other features:
* * Segmentation/Reassembly support
* * T(guard) after (re)start
@@ -67,18 +64,47 @@
{
struct xua_msg *xua = xua_msg_alloc();
struct osmo_scu_unitdata_param *udpar = &prim->u.unitdata;
+ uint32_t seq_ctrl = udpar->in_sequence_control;
+ uint32_t proto_class;
if (!xua)
return NULL;
+ if (seq_ctrl == OSMO_SCU_UNITDATA_REQ_P_SEQUENCE_CONTROL_NOT_PRESENT) {
+ /* ITU-T Q.711 6.2.1 Class 0:
+ * "The SCCP user can invoke this service by means of the parameter
+ * "sequence control" in the N-UNITDATA request primitive being absent" */
+ proto_class = 0;
+ /* Erase the mark, we are anyway not using it in protocol_class=0. */
+ seq_ctrl = 0;
+ /* ITU-T Q.714 (1.1.2.1 Protocol class 0):
+ * "They are transferred independently of each other.
+ * Therefore, they may be delivered to the SCCP user out-of-sequence."
+ */
+ xua->mtp.sls = rand() & 0xf;
+ } else {
+ /* "ITU-T Q.711 6.2.1 Class 1:
+ * "The SCCP user can invoke this service by means of the parameter
+ * "sequence control" in the N-UNITDATA request primitive being present." */
+ proto_class = 1;
+ /* ITU-T Q.714 (1.1.2.2 Protocol class 1):
+ * "The Signalling Link Selection (SLS) parameter in the MTP-TRANSFER
+ * request primitive is chosen by the originating SCCP based on the value
+ * of the sequence control parameter. The SLS shall be identical for a
+ * stream of NSDUs with the same sequence control parameter".
+ * SLS is 4 bits, as described in ITU Q.704 Figure 3.
+ */
+ xua->mtp.sls = udpar->in_sequence_control & 0x0f;
+ }
+
switch (msg_type) {
case SUA_CL_CLDT:
xua->hdr = XUA_HDR(SUA_MSGC_CL, SUA_CL_CLDT);
xua_msg_add_u32(xua, SUA_IEI_ROUTE_CTX, 0); /* FIXME */
- xua_msg_add_u32(xua, SUA_IEI_PROTO_CLASS, 0);
+ xua_msg_add_u32(xua, SUA_IEI_PROTO_CLASS, proto_class);
xua_msg_add_sccp_addr(xua, SUA_IEI_SRC_ADDR, &udpar->calling_addr);
xua_msg_add_sccp_addr(xua, SUA_IEI_DEST_ADDR, &udpar->called_addr);
- xua_msg_add_u32(xua, SUA_IEI_SEQ_CTRL, udpar->in_sequence_control);
+ xua_msg_add_u32(xua, SUA_IEI_SEQ_CTRL, seq_ctrl);
/* optional: importance, ... correlation id? */
if (!prim)
goto prim_needed;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39690?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I834e91c1ec337713b0e684cf4fd7de854150bef6
Gerrit-Change-Number: 39690
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39689?usp=email )
Change subject: sccp: Apply SLS on locally-originated transmitted Connection-Oriented messages
......................................................................
sccp: Apply SLS on locally-originated transmitted Connection-Oriented messages
Change-Id: I1956acf631867b12b647d14c10057ded20d7fa2f
---
M src/sccp_scoc.c
1 file changed, 34 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/89/39689/1
diff --git a/src/sccp_scoc.c b/src/sccp_scoc.c
index c11dddf..8222bde 100644
--- a/src/sccp_scoc.c
+++ b/src/sccp_scoc.c
@@ -102,6 +102,11 @@
uint32_t sccp_class;
uint32_t release_cause; /* WAIT_CONN_CONF */
+ /* SLS to be used to transmit all Connection-oriented messages
+ * (ITU-T Q.714 1.1.2.3 Protocol class 2).
+ * SLS is 4 bits, as described in ITU Q.704 Figure 3 */
+ uint8_t tx_co_mtp_sls;
+
struct msgb *opt_data_cache;
/* incoming (true) or outgoing (false) */
@@ -450,6 +455,26 @@
#define INIT_TIMER(x, fn, priv) do { (x)->cb = fn; (x)->data = priv; } while (0)
+/* Generate an SLS to be used for Connection-oriented messages on this SCCP connection.
+ * SLS is 4 bits, as described in ITU Q.704 Figure 3.
+ * ITU-T Q.714 1.1.2.3 Protocol class 2:
+ * "Messages belonging to a given signalling connection shall contain the same
+ * value of the SLS field to ensure sequencing as described in 1.1.2.2"
+ */
+static uint8_t sccp_conn_gen_tx_co_mtp_sls(const struct sccp_connection *conn)
+{
+ /* Implementation: Derive the SLS from conn->conn_id. */
+ const uint32_t id = conn->conn_id;
+ uint8_t sls;
+
+ /* First shrink it to 1 byte: */
+ sls = ((id >> (3*8)) ^ (id >> (2*8)) ^ (id >> (1*8)) ^ (id)) & 0xff;
+ /* Now shrink it 8 -> 4 bits: */
+ sls = ((sls >> 4) ^ sls) & 0x0f;
+
+ return sls;
+}
+
/* allocate + init a SCCP Connection with given ID */
static struct sccp_connection *conn_create_id(struct osmo_sccp_user *user, uint32_t conn_id)
{
@@ -472,6 +497,8 @@
INIT_TIMER(&conn->t_int, int_tmr_cb, conn);
INIT_TIMER(&conn->t_rep_rel, rep_rel_tmr_cb, conn);
+ conn->tx_co_mtp_sls = sccp_conn_gen_tx_co_mtp_sls(conn);
+
/* this might change at runtime, as it is not a constant :/ */
sccp_scoc_fsm.log_subsys = DLSCCP;
@@ -715,6 +742,13 @@
if (!xua)
return NULL;
+ /* amend this with point code information; Many CO msgs
+ * includes neither called nor calling party address! */
+ xua->mtp.dpc = conn->remote_pc;
+
+ /* Apply SLS calculated for the connection (ITU-T Q.714 1.1.2.3). */
+ xua->mtp.sls = conn->tx_co_mtp_sls;
+
switch (msg_type) {
case SUA_CO_CORE: /* Connect Request == SCCP CR */
xua->hdr = XUA_HDR(SUA_MSGC_CO, SUA_CO_CORE);
@@ -835,9 +869,6 @@
if (!xua)
return -ENOMEM;
- /* amend this with point code information; Many CO msgs
- * includes neither called nor calling party address! */
- xua->mtp.dpc = conn->remote_pc;
sccp_scrc_rx_scoc_conn_msg(conn->inst, xua);
xua_msg_free(xua);
return 0;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39689?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I1956acf631867b12b647d14c10057ded20d7fa2f
Gerrit-Change-Number: 39689
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Attention is currently required from: dexter.
fixeria has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/39670?usp=email )
Change subject: filesystem: do not decode short TransRecEF records
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39670?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ib1dc4d7ce306f1f0b080bb4b6abc36e72431d3fa
Gerrit-Change-Number: 39670
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 04 Mar 2025 18:28:05 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: laforge.
falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/libosmo-abis/+/39669?usp=email )
Change subject: trau_frame encode8_hr: simplify setting of C5 for odd parity
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> it will bite us in the butt if we ever want to change anything about this function, and the strange assumption no longer holds true.
I am afraid I don't follow. Suppose we decide that no sane user application is going to set only C4 (`fr->c_bits[3]`) while leaving the other 4 C-bits in that group uninitialized/garbage, and we change the function to always use all of `fr->c_bits[]`, like we already do for OSMO_TRAU8_DATA. (In fact, I have a patch that does just that - would you like me to submit it as a merge candidate?) In that case, the local function for computing odd parity *still* goes away just like with the present patch - so I don't see where the "bite in the butt" would happen.
But I have no particular attachment to the present patch. I produced it because it seemed like a sensible code simplification/clean-up, but it does not change any functionality or behavior, so if you (and/or other senior maintainers) don't like it, then fine, no big deal.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/39669?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I30c7dfdaaadd0fd4cb084cf02c66c0f19a40ae42
Gerrit-Change-Number: 39669
Gerrit-PatchSet: 1
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 04 Mar 2025 17:53:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>