pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/33886 )
Change subject: rlcmac: Mark received BSNs falling out of the V(N)/RBB when V(R) is raised
......................................................................
rlcmac: Mark received BSNs falling out of the V(N)/RBB when V(R) is raised
Problem this patch is fixing:
The current RLCMAC window code ported from osmo-pcu is never
invalidating the BSNs which have been received after they are not
needed.
As a result, when the DL TBF keeps sending data for a long time, and
finally BSN wraps around 127->0, when this implementation receives the
BSN=0, it will find it is already received and hence will discard it,
and then keep asking for BSN=0 nevertheless in PKT DL ACK, causing an
endless loop where PCU stays submitting the same block forever.
Explanation of the solution:
The V(N) array contains the status of the previous WS (64 in GPRS) data
blocks. This array is used to construct the RRB signaled to the peer
during PKT DL ACK/NACK messages together with the SSN (start sequence
number), which in our case is mainly V(R), aka one block higher than the
highest received block in the rx window.
Hence, whenever PKT DL ACK/NACK is transmitted, it contains an RRB
ranging [V(R)-1,...V(R)-WS)] mod SNS (SNS=128 in GPRS).
The receive window is basically [V(Q) <= BSN < V(R)] mod SNS, as is of
size 64.
The V(R) is increased whenever a highest new block arrives which is in the
receive window (guaranteeing it will be increased to at most V(Q)+64.
Since we are only announcing state of blocks from V(R)..V(R)-WS, and
blocks received which are before that BSN are dropped since don't fall
inside the rx window, we can securely mark as invalid those blocks
falling behind V(R)-WS whenever we increase V(R).
Related: OS#6102
Change-Id: I962111995e741a7e9c230b2dd4904c2fa9a828e9
---
M include/osmocom/gprs/rlcmac/rlc_window_dl.h
M src/rlcmac/rlc_window_dl.c
M tests/rlcmac/rlcmac_prim_test.err
3 files changed, 55 insertions(+), 11 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/86/33886/1
diff --git a/include/osmocom/gprs/rlcmac/rlc_window_dl.h b/include/osmocom/gprs/rlcmac/rlc_window_dl.h
index 58faaa4..93ccbe4 100644
--- a/include/osmocom/gprs/rlcmac/rlc_window_dl.h
+++ b/include/osmocom/gprs/rlcmac/rlc_window_dl.h
@@ -62,7 +62,6 @@
uint16_t gprs_rlcmac_rlc_dl_window_raise_v_q(struct gprs_rlcmac_rlc_dl_window *dlw);
void gprs_rlcmac_rlc_dl_window_receive_bsn(struct gprs_rlcmac_rlc_dl_window *dlw, uint16_t bsn);
-bool gprs_rlcmac_rlc_dl_window_invalidate_bsn(struct gprs_rlcmac_rlc_dl_window *dlw, uint16_t bsn);
static inline uint16_t gprs_rlcmac_rlc_dl_window_ssn(const struct gprs_rlcmac_rlc_dl_window *dlw)
{
diff --git a/src/rlcmac/rlc_window_dl.c b/src/rlcmac/rlc_window_dl.c
index 77153b6..eab01cd 100644
--- a/src/rlcmac/rlc_window_dl.c
+++ b/src/rlcmac/rlc_window_dl.c
@@ -69,6 +69,11 @@
return gprs_rlcmac_rlc_v_n_mark(v_n, bsn, GPRS_RLCMAC_RLC_DL_BSN_MISSING);
}
+void gprs_rlcmac_rlc_v_n_mark_invalid(struct gprs_rlcmac_rlc_v_n *v_n, int bsn)
+{
+ return gprs_rlcmac_rlc_v_n_mark(v_n, bsn, GPRS_RLCMAC_RLC_DL_BSN_INVALID);
+}
+
/*************
* UL WINDOW
*************/
@@ -220,8 +225,14 @@
/* Positive offset, so raise. */
if (offset_v_r < (gprs_rlcmac_rlc_window_sns(w) >> 1)) {
while (offset_v_r--) {
- if (offset_v_r) /* all except the received block */
- gprs_rlcmac_rlc_v_n_mark_missing(&dlw->v_n, gprs_rlcmac_rlc_dl_window_v_r(dlw));
+ const uint16_t v_r = gprs_rlcmac_rlc_dl_window_v_r(dlw);
+ const uint16_t bsn_no_longer_in_ws = gprs_rlcmac_rlc_window_mod_sns_bsn(w, v_r - gprs_rlcmac_rlc_window_ws(w));
+ LOGRLCMAC(LOGL_DEBUG, "- Mark BSN %u as INVALID\n", bsn_no_longer_in_ws);
+ gprs_rlcmac_rlc_v_n_mark_invalid(&dlw->v_n, bsn_no_longer_in_ws);
+ if (offset_v_r) {/* all except the received block */
+ LOGRLCMAC(LOGL_DEBUG, "- Mark BSN %u as MISSING\n", v_r);
+ gprs_rlcmac_rlc_v_n_mark_missing(&dlw->v_n, v_r);
+ }
gprs_rlcmac_rlc_dl_window_raise_v_r_to(dlw, 1);
}
LOGRLCMAC(LOGL_DEBUG, "- Raising V(R) to %d\n", gprs_rlcmac_rlc_dl_window_v_r(dlw));
@@ -255,11 +266,3 @@
gprs_rlcmac_rlc_v_n_mark_received(&dlw->v_n, bsn);
gprs_rlcmac_rlc_dl_window_raise_v_r(dlw, bsn);
}
-
-bool gprs_rlcmac_rlc_dl_window_invalidate_bsn(struct gprs_rlcmac_rlc_dl_window *dlw, uint16_t bsn)
-{
- bool was_valid = gprs_rlcmac_rlc_v_n_is_received(&dlw->v_n, bsn);
- gprs_rlcmac_rlc_v_n_mark_missing(&dlw->v_n, bsn);
-
- return was_valid;
-}
diff --git a/tests/rlcmac/rlcmac_prim_test.err b/tests/rlcmac/rlcmac_prim_test.err
index ec707d2..006efc3 100644
--- a/tests/rlcmac/rlcmac_prim_test.err
+++ b/tests/rlcmac/rlcmac_prim_test.err
@@ -885,6 +885,7 @@
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) Got CS-1 RLC data block: FBI=1, BSN=0, SPB=0, S/P=1 RRBP=1, E=0, bitoffs=24
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) BSN 0 storing in window (0..63)
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) data_length=20, data=19 43 c0 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
+DLGLOBAL DEBUG - Mark BSN 64 as INVALID
DLGLOBAL DEBUG - Raising V(R) to 1
DLGLOBAL DEBUG - Taking block 0 out, raising V(Q) to 1
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) Assembling frames: (len=20)
@@ -923,6 +924,7 @@
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) Got CS-1 RLC data block: FBI=1, BSN=0, SPB=0, S/P=1 RRBP=1, E=0, bitoffs=24
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) BSN 0 storing in window (0..63)
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) data_length=20, data=19 43 c0 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
+DLGLOBAL DEBUG - Mark BSN 64 as INVALID
DLGLOBAL DEBUG - Raising V(R) to 1
DLGLOBAL DEBUG - Taking block 0 out, raising V(Q) to 1
DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) Assembling frames: (len=20)
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/33886
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I962111995e741a7e9c230b2dd4904c2fa9a828e9
Gerrit-Change-Number: 33886
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/33885 )
Change subject: rlcmac: rlc_window_dl: Fix RBB generation
......................................................................
rlcmac: rlc_window_dl: Fix RBB generation
We want to do mod sns on the BSN, not on the boolean returned by
gprs_rlcmac_rlc_v_n_is_received().
This is a typo which occurred while porting the code from osmo-pcu.git
void gprs_rlc_ul_window::update_rbb(char *rbb), where the following line
is used:
"""
if (m_v_n.is_received((ssn()-1-i) & mod_sns()))
"""
Change-Id: I37c8fd5c2528f035f077c3e05105f913922ffd84
---
M src/rlcmac/rlc_window_dl.c
1 file changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/85/33885/1
diff --git a/src/rlcmac/rlc_window_dl.c b/src/rlcmac/rlc_window_dl.c
index a2bef5f..77153b6 100644
--- a/src/rlcmac/rlc_window_dl.c
+++ b/src/rlcmac/rlc_window_dl.c
@@ -163,7 +163,7 @@
unsigned int i;
for (i = 0; i < ws; i++) {
- if (gprs_rlcmac_rlc_v_n_is_received(&dlw->v_n, ssn - 1 - i) & mod_sns)
+ if (gprs_rlcmac_rlc_v_n_is_received(&dlw->v_n, (ssn - 1 - i) & mod_sns))
rbb[ws - 1 - i] = 'R';
else
rbb[ws - 1 - i] = 'I';
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/33885
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I37c8fd5c2528f035f077c3e05105f913922ffd84
Gerrit-Change-Number: 33885
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/33528 )
Change subject: USSD: fix handling of ussd-DataCodingScheme != 0x0f
......................................................................
USSD: fix handling of ussd-DataCodingScheme != 0x0f
The usual Data Coding Scheme value in the mobile-originated USSD
request (processUnstructuredSS-Request) is 0x0f, which means:
0000 .... = Coding Group: Coding Group 0 (Language using the GSM 7 bit default alphabet)
.... 1111 = Language: unspecified
However some modems are known to use a different default value, if
not specified explicitly (AT+CUSD has optional DCS parameter):
0000 .... = Coding Group: Coding Group 0 (Language using the GSM 7 bit default alphabet)
.... 0000 = Language: German (0)
In function rx_proc_ss_req(), we should not be using req.ussd_text,
because this field has been deprecated and may contain unexpected
data. For example, in the abovementioned case it would contain the
7 bit encoded ussd-String 'aa510c061b01'O and osmo-hlr would indeed
fail to find a matching route for a non-ASCII string.
Instead of relaying on gsm0480_parse_facility_ie(), let's check the
Data Coding Scheme value and decode the request string ourselves.
Expect the Coding Group 0, but be more tolerant to the indicated
language: print a warning and treat it as '1111'B (unspecified).
Change-Id: Ib7bac660b1a7942adcfbe7b14f162c95061a25db
Related: OS#6075
---
M src/hlr_ussd.c
1 file changed, 66 insertions(+), 2 deletions(-)
Approvals:
laforge: Looks good to me, approved
neels: 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/src/hlr_ussd.c b/src/hlr_ussd.c
index 8d5237f..1c0107b 100644
--- a/src/hlr_ussd.c
+++ b/src/hlr_ussd.c
@@ -122,9 +122,40 @@
talloc_free(rt);
}
-static struct hlr_ussd_route *ussd_route_lookup_7bit(struct hlr *hlr, const char *ussd_code)
+static struct hlr_ussd_route *ussd_route_lookup_for_req(struct hlr *hlr, const struct ss_request *req)
{
+ const uint8_t cgroup = req->ussd_data_dcs >> 4;
+ const uint8_t lang = req->ussd_data_dcs & 0x0f;
+ char ussd_code[GSM0480_USSD_7BIT_STRING_LEN];
struct hlr_ussd_route *rt;
+
+ ussd_code[0] = '\0';
+
+ /* We support only the Coding Group 0 (GSM 7-bit default alphabeet). In fact,
+ * the USSD request is usually limited to [*#0-9], so we don't really need to
+ * support other coding groups and languages. */
+ switch (cgroup) {
+ case 0:
+ /* The Language is usually set to '1111'B (unspecified), but some UEs
+ * are known to indicate '0000'B (German). */
+ if (lang != 0x0f) {
+ LOGP(DSS, LOGL_NOTICE, "USSD DataCodingScheme (0x%02x): "
+ "the Language is usually set to 15 (unspecified), "
+ "but the request indicates %u - ignoring this\n",
+ req->ussd_data_dcs, lang);
+ /* do not abort, attempt to decode as if it was '1111'B */
+ }
+
+ gsm_7bit_decode_n_ussd(&ussd_code[0], sizeof(ussd_code),
+ req->ussd_data, (req->ussd_data_len * 8) / 7);
+ break;
+ default:
+ LOGP(DSS, LOGL_ERROR, "USSD DataCodingScheme (0x%02x): "
+ "Coding Group %u is not supported, expecting Coding Group 0\n",
+ req->ussd_data_dcs, cgroup);
+ return NULL;
+ }
+
llist_for_each_entry(rt, &hlr->ussd_routes, list) {
if (!strncmp(ussd_code, rt->prefix, strlen(rt->prefix))) {
LOGP(DSS, LOGL_DEBUG, "Found %s '%s' (prefix '%s') for USSD "
@@ -603,7 +634,7 @@
} else {
/* VLR->EUSE: MO USSD. VLR is known ('conn'), EUSE is to be resolved */
struct hlr_ussd_route *rt;
- rt = ussd_route_lookup_7bit(hlr, (const char *) req.ussd_text);
+ rt = ussd_route_lookup_for_req(hlr, &req);
if (rt) {
if (rt->is_external) {
ss->is_external = true;
--
To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/33528
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Change-Id: Ib7bac660b1a7942adcfbe7b14f162c95061a25db
Gerrit-Change-Number: 33528
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33848 )
Change subject: vty: Allow space in network name
......................................................................
Patch Set 2:
(3 comments)
Patchset:
PS2:
> Not sure if other osmo programs may also need the same patch?
Which programs? What do you mean?
PS2:
Oh, we ended up submitting two similar patches yesterday.
File src/libmsc/msc_vty.c:
https://gerrit.osmocom.org/c/osmo-msc/+/33848/comment/a3ace18f_e55d310f
PS2, Line 134: .NAME
Adding a dot is not enough, you also need to do `argv_concat()`. Done in my patch.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33848
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie85e98790d6917a30c84333203cbc00c4ceb97e8
Gerrit-Change-Number: 33848
Gerrit-PatchSet: 2
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 21 Jul 2023 12:37:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/33866 )
Change subject: simtrace2-cardem-pcsc: mark reset events in GSMTAP trace
......................................................................
simtrace2-cardem-pcsc: mark reset events in GSMTAP trace
At the moment only APDUs are logged to GSMTAP. It is not uncommon that a
card is resetted by the UE multiple times during normal operation. When
the trace lacks the reset events (ATR) it becomes difficult to follow in
which state the card actually is. Let't mark reset events by sending the
ATR via GSMTAP (like simtrace2_sniff already does it)
Related: OS#6094
Change-Id: I6b4d82b6ee369c95eeca8f7d59478452395fbe54
---
M host/src/simtrace2-cardem-pcsc.c
1 file changed, 19 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/host/src/simtrace2-cardem-pcsc.c b/host/src/simtrace2-cardem-pcsc.c
index 374966d..4acf268 100644
--- a/host/src/simtrace2-cardem-pcsc.c
+++ b/host/src/simtrace2-cardem-pcsc.c
@@ -106,6 +106,9 @@
LOGCI(ci, LOGL_NOTICE, "%s Resetting card in reader...\n",
reset == COLD_RESET ? "Cold" : "Warm");
osim_card_reset(card, reset == COLD_RESET ? true : false);
+
+ /* Mark reset event in GSMTAP wireshark trace */
+ osmo_st2_gsmtap_send_apdu(GSMTAP_SIM_ATR, card->atr, card->atr_len);
}
last_status_flags = flags;
--
To view, visit https://gerrit.osmocom.org/c/simtrace2/+/33866
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-Change-Id: I6b4d82b6ee369c95eeca8f7d59478452395fbe54
Gerrit-Change-Number: 33866
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33299 )
Change subject: ASCI: Send only NLN on Paging request type 1 rest octets
......................................................................
ASCI: Send only NLN on Paging request type 1 rest octets
Do not send notifications here. The notifications are sent on the NCH.
The NLN is used to indicate a change on the NCH, so that the MS will
read the NCH then.
If NLN is not sent towards MS, it will not read NCH to get an updated
list of ongoing calls. Also, it will not allow making VGCS/VBS calls and
might indicate that ASCI is not supported in this call. (Tested with
GPH-610R.)
Change-Id: I22e584b70fd14d8bdabb28cf5de3d4647f37425a
Related: OS#5781
---
M src/common/paging.c
1 file changed, 28 insertions(+), 6 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/common/paging.c b/src/common/paging.c
index 2b20786..d4a3aed 100644
--- a/src/common/paging.c
+++ b/src/common/paging.c
@@ -704,7 +704,6 @@
int paging_gen_msg(struct paging_state *ps, uint8_t *out_buf, struct gsm_time *gt,
int *is_empty)
{
- const struct asci_notification *notif;
struct llist_head *group_q;
struct gsm_bts *bts = ps->bts;
int group;
@@ -733,13 +732,17 @@
/* we intentioanally don't try to add notifications here, as ETWS is more critical */
len = fill_paging_type_1(out_buf, empty_id_lv, 0, NULL, 0, &p1ro, NULL);
} else if (llist_empty(group_q)) {
+ struct p1_rest_octets p1ro;
+ memset(&p1ro, 0, sizeof(p1ro));
+ /* Use NLN to notify MS about ongoing VGCS/VBS calls.
+ * This is required to make the phone read the NCH to get an updated list of ongoing calls.
+ * Without this the phone will not allow making VGCS/VBS calls. */
+ p1ro.nln_pch.present = (bts->asci.pos_nch >= 0);
+ p1ro.nln_pch.nln = bts->asci.nln;
+ p1ro.nln_pch.nln_status = bts->asci.nln_status;
/* There is nobody to be paged, send Type1 with two empty ID */
//DEBUGP(DPAG, "Tx PAGING TYPE 1 (empty)\n");
- /* for now, we only send VGCS/VBS notfication if we have nothing else to page;
- * this is the safe choice. For other situations with mobile identities in the
- * paging type 1, we'd need to check if there's sufficient space in the rest octets. */
- notif = bts_asci_notification_get_next(bts);
- len = fill_paging_type_1(out_buf, empty_id_lv, 0, NULL, 0, NULL, notif);
+ len = fill_paging_type_1(out_buf, empty_id_lv, 0, NULL, 0, &p1ro, NULL);
*is_empty = 1;
} else {
struct paging_record *pr[4];
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33299
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I22e584b70fd14d8bdabb28cf5de3d4647f37425a
Gerrit-Change-Number: 33299
Gerrit-PatchSet: 10
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
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: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged