cellular-infrastructure
/
osmo-pcu
mirror of https://gerrit.osmocom.org/osmo-pcu
9
0
Fork 1
Code Issues Releases Wiki Activity

Fix Dl EGPRS data blocks being generated occasionally on GPRS TBFs 

Under some circumstances, it could happen that a DL TBF is created as a
GPRS TBF due to not yet having enough information of the MS, and only
after the TBF is created the PCU gains that information and upgrades the
MS mode to "EGPRS". Hence, there's the possibility to run into a
situation where a GPRS TBF is attached to a EGPRS MS.

It may also happen sometimes that despite the TBF and the MS be EGPRS,
there's need to further limit the DL MCS to use, eg. MCS1-4 (GMSK).

As a result, when asking for the current DL (M)CS to use, we must tell
the MS which kind of limitations we want to apply. The later reasoning
was already implemented when GPRS+EGPRS multiplexing was added, but the
former was not being checked. Hence, by further spreading through the
call stack the "req_kind_mode" we match both cases.

Related: OS#4973
Change-Id: Ic0276ce045660713129f0c72f1158a3321c5977f
This commit is contained in:
Pau Espin 2021-01-25 12:05:32 +01:00
parent 201da4e5b2
commit fc464935a4
6 changed files with 62 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
src/gprs_ms.c
View File

@ -778,13 +778,46 @@ void ms_update_l1_meas(struct GprsMs *ms, const struct pcu_l1_meas *meas)
}
}
enum CodingScheme ms_current_cs_dl(const struct GprsMs *ms)
/* req_mcs_kind acts as a set filter, where EGPRS means any and GPRS is the most restrictive */
enum CodingScheme ms_current_cs_dl(const struct GprsMs *ms, enum mcs_kind req_mcs_kind)
{
enum CodingScheme cs = ms->current_cs_dl;
enum CodingScheme orig_cs = ms->current_cs_dl;
struct gprs_rlcmac_bts *bts = ms->bts;
size_t unencoded_octets;
enum CodingScheme cs;
if (!ms->bts)
return cs;
/* It could be that a TBF requests a GPRS CS despite the MS currently
being upgraded to EGPRS (hence reporting MCS). That could happen
because the TBF was created early in the process where we didn't have
yet enough information about the MS, and only AFTER it was created we
upgraded the MS to be EGPRS capable.
As a result, when the MS is queried for the target CS here, we could be
returning an MCS despite the current TBF being established as GPRS,
but we rather stick to the TBF type we assigned to the MS rather than
magically sending EGPRS data blocks to a GPRS TBF.
It could also be that the caller requests specific MCS kind
explicitly too due to scheduling restrictions (GPRS+EGPRS multiplexing). */
if (req_mcs_kind == EGPRS_GMSK && mcs_is_edge(orig_cs) && orig_cs > MCS4) {
cs = bts_cs_dl_is_supported(bts, MCS4) ? MCS4 :
bts_cs_dl_is_supported(bts, MCS3) ? MCS3 :
bts_cs_dl_is_supported(bts, MCS2) ? MCS2 :
MCS1;
} else if (req_mcs_kind == GPRS && mcs_is_edge(orig_cs)) { /* GPRS */
int i;
cs = orig_cs > MCS4 ? MCS4 : orig_cs;
cs -= (MCS1 - CS1); /* MCSx -> CSx */
/* Find suitable CS starting from equivalent MCS which is supported by BTS: */
for (i = mcs_chan_code(cs); !bts_cs_dl_is_supported(bts, CS1 + i); i--);
OSMO_ASSERT(i >= 0 && i <= 3); /* CS1 is always supported */
cs = CS1 + i;
} else {
cs = orig_cs;
}
if (orig_cs != cs)
LOGPMS(ms, DRLCMACDL, LOGL_INFO, "MS (mode=%s) suggests transmitting "
"DL %s, downgrade to %s in order to match TBF & scheduler requirements\n",
mode_name(ms_mode(ms)), mcs_name(orig_cs), mcs_name(cs));
unencoded_octets = llc_queue_octets(&ms->llc_queue);

2
src/gprs_ms.h
View File

@ -114,7 +114,7 @@ void ms_set_ms_class(struct GprsMs *ms, uint8_t ms_class_);
void ms_set_egprs_ms_class(struct GprsMs *ms, uint8_t ms_class_);
void ms_set_ta(struct GprsMs *ms, uint8_t ta_);
enum CodingScheme ms_current_cs_dl(const struct GprsMs *ms);
enum CodingScheme ms_current_cs_dl(const struct GprsMs *ms, enum mcs_kind req_mcs_kind);
enum CodingScheme ms_max_cs_ul(const struct GprsMs *ms);
enum CodingScheme ms_max_cs_dl(const struct GprsMs *ms);
void ms_set_current_cs_dl(struct GprsMs *ms, enum CodingScheme scheme);

2
src/pcu_vty_functions.cpp
View File

@ -133,7 +133,7 @@ static int show_ms(struct vty *vty, GprsMs *ms)
vty_out(vty, " Timing advance (TA): %d%s", ms_ta(ms), VTY_NEWLINE);
vty_out(vty, " Coding scheme uplink: %s%s", mcs_name(ms_current_cs_ul(ms)),
VTY_NEWLINE);
vty_out(vty, " Coding scheme downlink: %s%s", mcs_name(ms_current_cs_dl(ms)),
vty_out(vty, " Coding scheme downlink: %s%s", mcs_name(ms_current_cs_dl(ms, ms_mode(ms))),
VTY_NEWLINE);
vty_out(vty, " Mode: %s%s", mode_name(ms_mode(ms)), VTY_NEWLINE);
vty_out(vty, " MS class: %d%s", ms_ms_class(ms), VTY_NEWLINE);

3
src/tbf.cpp
View File

@ -194,11 +194,12 @@ uint8_t gprs_rlcmac_tbf::ms_class() const
enum CodingScheme gprs_rlcmac_tbf::current_cs() const
{
enum CodingScheme cs;
enum mcs_kind req_mcs_kind = is_egprs_enabled() ? EGPRS : GPRS;
if (direction == GPRS_RLCMAC_UL_TBF)
cs = ms_current_cs_ul(m_ms);
else
cs = ms_current_cs_dl(m_ms);
cs = ms_current_cs_dl(m_ms, req_mcs_kind);
return cs;
}

44
src/tbf_dl.cpp
View File

@ -448,29 +448,24 @@ int gprs_rlcmac_dl_tbf::take_next_bsn(uint32_t fn,
{
int bsn;
int data_len2, force_data_len = -1;
enum CodingScheme force_cs = UNKNOWN;
enum CodingScheme tx_cs;
/* Scheduler may be fine with sending any kind of data, but if
the selected TBF is GPRS-only, then let's filter out EGPRS
here */
if (!is_egprs_enabled())
req_mcs_kind = GPRS;
/* search for a nacked or resend marked bsn */
bsn = m_window.resend_needed();
if (previous_bsn >= 0) {
force_cs = m_rlc.block(previous_bsn)->cs_current_trans;
if (!mcs_is_edge(force_cs))
tx_cs = m_rlc.block(previous_bsn)->cs_current_trans;
if (!mcs_is_edge(tx_cs))
return -1;
force_data_len = m_rlc.block(previous_bsn)->len;
} else if (bsn < 0 && is_egprs_enabled() && req_mcs_kind == EGPRS_GMSK) {
/* New data to be sent for EGPRS TBF but we are required to downgrade to
* MCS1-4, because USF for GPRS-only MS will be sent */
force_cs = ms_current_cs_dl(m_ms);
if (force_cs > MCS4) {
force_cs = bts_cs_dl_is_supported(bts, MCS4) ? MCS4 :
bts_cs_dl_is_supported(bts, MCS3) ? MCS3 :
bts_cs_dl_is_supported(bts, MCS2) ? MCS2 :
MCS1;
LOGPTBFDL(this, LOGL_DEBUG,
"Force downgrading DL %s -> %s due to USF for GPRS-only MS\n",
mcs_name(ms_current_cs_dl(m_ms)), mcs_name(force_cs));
}
} else {
tx_cs = ms_current_cs_dl(ms(), req_mcs_kind);
}
if (bsn >= 0) {
@ -484,15 +479,14 @@ int gprs_rlcmac_dl_tbf::take_next_bsn(uint32_t fn,
if (is_egprs_enabled()) {
/* Table 8.1.1.2 and Table 8.1.1.1 of 44.060 */
m_rlc.block(bsn)->cs_current_trans = get_retx_mcs(m_rlc.block(bsn)->cs_init,
ms_current_cs_dl(ms()),
m_rlc.block(bsn)->cs_current_trans = get_retx_mcs(m_rlc.block(bsn)->cs_init, tx_cs,
bts->pcu->vty.dl_arq_type == EGPRS_ARQ1);
LOGPTBFDL(this, LOGL_DEBUG,
"initial_cs_dl(%s) last_mcs(%s) demanded_mcs(%s) cs_trans(%s) arq_type(%d) bsn(%d)\n",
mcs_name(m_rlc.block(bsn)->cs_init),
mcs_name(m_rlc.block(bsn)->cs_last),
mcs_name(ms_current_cs_dl(ms())),
mcs_name(tx_cs),
mcs_name(m_rlc.block(bsn)->cs_current_trans),
the_pcu->vty.dl_arq_type, bsn);
@ -537,14 +531,12 @@ int gprs_rlcmac_dl_tbf::take_next_bsn(uint32_t fn,
return take_next_bsn(fn, previous_bsn, req_mcs_kind, may_combine);
} else if (have_data()) {
/* The window has space left, generate new bsn */
enum CodingScheme new_cs;
new_cs = force_cs ? force_cs : current_cs();
LOGPTBFDL(this, LOGL_DEBUG,
"Sending new block at BSN %d, CS=%s%s\n",
m_window.v_s(), mcs_name(new_cs),
force_cs ? " (forced)" : "");
m_window.v_s(), mcs_name(tx_cs),
force_data_len != -1 ? " (forced)" : "");
bsn = create_new_bsn(fn, new_cs);
bsn = create_new_bsn(fn, tx_cs);
} else if (bts->pcu->vty.dl_tbf_preemptive_retransmission && !m_window.window_empty()) {
/* The window contains unacked packages, but not acked.
* Mark unacked bsns as RESEND */
@ -558,8 +550,8 @@ int gprs_rlcmac_dl_tbf::take_next_bsn(uint32_t fn,
/* Nothing left to send, create dummy LLC commands */
LOGPTBFDL(this, LOGL_DEBUG,
"Sending new dummy block at BSN %d, CS=%s\n",
m_window.v_s(), mcs_name(current_cs()));
bsn = create_new_bsn(fn, current_cs());
m_window.v_s(), mcs_name(tx_cs));
bsn = create_new_bsn(fn, tx_cs);
/* Don't send a second block, so don't set cs_current_trans */
}

6
tests/ms/MsTest.cpp
View File

@ -522,11 +522,11 @@ static void test_ms_cs_selection()
OSMO_ASSERT(!ms_is_idle(ms));
OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms)) == 3);
OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms, ms_mode(ms))) == 3);
the_pcu->vty.cs_downgrade_threshold = 200;
OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms)) == 2);
OSMO_ASSERT(mcs_chan_code(ms_current_cs_dl(ms, ms_mode(ms))) == 2);
talloc_free(dl_tbf);
talloc_free(bts);
@ -536,7 +536,7 @@ static void test_ms_cs_selection()
static void dump_ms(const GprsMs *ms, const char *pref)
{
printf("%s MS DL %s/%s, UL %s/%s, mode %s, <%s>\n", pref,
mcs_name(ms_current_cs_dl(ms)), mcs_name(ms_max_cs_dl(ms)),
mcs_name(ms_current_cs_dl(ms, ms_mode(ms))), mcs_name(ms_max_cs_dl(ms)),
mcs_name(ms_current_cs_ul(ms)), mcs_name(ms_max_cs_ul(ms)),
mode_name(ms_mode(ms)),
ms_is_idle(ms) ? "IDLE" : "ACTIVE");