Change in osmo-pcu[master]: Update MCS selection for retransmission
Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/13402 ) Change subject: Update MCS selection for retransmission .. Update MCS selection for retransmission In 3GPP TS 44.060 the selection of MCS for retransmissions is defined as separate tables (8.1.1.1 and 8.1.1.2) depending on the value of resegmentation bit (which is opposite to the way EGPRS_ARQ are defined in the source code). Let's follow the same idea and explicitly check for resegmentation bit value and use separate tables. This also makes it easier to add proper support for special cases (MCS-6-9 and MCS-5-7) and padding in future independently for different ARQ types. The code is also moved to c to avoid unnecessary conversions to and from cpp class. Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 --- M src/coding_scheme.c M src/coding_scheme.h M src/gprs_coding_scheme.cpp M src/gprs_coding_scheme.h M src/tbf_dl.cpp 5 files changed, 40 insertions(+), 50 deletions(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/coding_scheme.c b/src/coding_scheme.c index a4ae882..eaa4953 100644 --- a/src/coding_scheme.c +++ b/src/coding_scheme.c @@ -127,3 +127,39 @@ const char *mode_name(enum mcs_kind val) { return get_value_string(mode_names, val); } + +/* FIXME: take into account padding and special cases of commanded MCS (MCS-6-9 and MCS-5-7) */ +enum CodingScheme get_retx_mcs(enum CodingScheme initial_mcs, enum CodingScheme commanded_mcs, bool resegment_bit) +{ + OSMO_ASSERT(mcs_is_edge(initial_mcs)); + OSMO_ASSERT(mcs_is_edge(commanded_mcs)); + OSMO_ASSERT(NUM_SCHEMES - MCS1 == 9); + + if (resegment_bit) { /* 3GPP TS 44.060 Table 8.1.1.1, reflected over antidiagonal */ + enum CodingScheme egprs_reseg[NUM_SCHEMES - MCS1][NUM_SCHEMES - MCS1] = { + { MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1 }, + { MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3 }, + { MCS1, MCS1, MCS1, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4 }, + { MCS2, MCS2, MCS2, MCS2, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS6, MCS6, MCS6, MCS9 }, + { MCS2, MCS2, MCS2, MCS2, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS6, MCS6, MCS8, MCS8 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS6, MCS6, MCS6, MCS9 }, + }; + return egprs_reseg[mcs_chan_code(initial_mcs)][mcs_chan_code(commanded_mcs)]; + } else { /* 3GPP TS 44.060 Table 8.1.1.2, reflected over antidiagonal */ + enum CodingScheme egprs_no_reseg[NUM_SCHEMES - MCS1][NUM_SCHEMES - MCS1] = { + { MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1 }, + { MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3 }, + { MCS4, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4 }, + { MCS5, MCS5, MCS5, MCS5, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS9 }, + { MCS5, MCS5, MCS5, MCS5, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS8, MCS8 }, + { MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS9 }, + }; + return egprs_no_reseg[mcs_chan_code(initial_mcs)][mcs_chan_code(commanded_mcs)]; + } +} diff --git a/src/coding_scheme.h b/src/coding_scheme.h index aac4bba..24db86d 100644 --- a/src/coding_scheme.h +++ b/src/coding_scheme.h @@ -32,6 +32,7 @@ extern const struct value_string mcs_names[]; const char *mcs_name(enum CodingScheme val); +enum CodingScheme get_retx_mcs(enum CodingScheme initial_mcs, enum CodingScheme commanded_mcs, bool resegment_bit); bool mcs_is_gprs(enum CodingScheme cs); bool mcs_is_edge(enum CodingScheme cs); diff --git a/src/gprs_coding_scheme.cpp b/src/gprs_coding_scheme.cpp index a149f81..0c22670 100644 --- a/src/gprs_coding_scheme.cpp +++ b/src/gprs_coding_scheme.cpp @@ -21,41 +21,6 @@ #include "gprs_coding_scheme.h" -#define MAX_NUM_ARQ 2 /* max. number of ARQ */ -#define MAX_NUM_MCS 9 /* max. number of MCS */ - -/* - * 44.060 Table 8.1.1.1 and Table 8.1.1.2 - * It has 3 level indexing. 0th level is ARQ type - * 1st level is Original MCS( index 0 corresponds to MCS1 and so on) - * 2nd level is MS MCS (index 0 corresponds to MCS1 and so on) - */ -static enum CodingScheme egprs_mcs_retx_tbl[MAX_NUM_ARQ] -
Change in osmo-pcu[master]: Update MCS selection for retransmission
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13402 ) Change subject: Update MCS selection for retransmission .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/13402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 Gerrit-Change-Number: 13402 Gerrit-PatchSet: 4 Gerrit-Owner: Max Gerrit-Reviewer: Daniel Willmann Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: osmith Gerrit-Comment-Date: Sat, 30 Mar 2019 09:32:01 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-pcu[master]: Update MCS selection for retransmission
Max has posted comments on this change. ( https://gerrit.osmocom.org/13402 ) Change subject: Update MCS selection for retransmission .. Patch Set 4: (1 comment) https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c File src/coding_scheme.c: https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c@131 PS1, Line 131: /* FIXME: take into account padding and special cases of commanded MCS (MCS-6-9 and MCS-5-7) */ > commanded MCS? do you mean demanded_mcs? Done -- To view, visit https://gerrit.osmocom.org/13402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 Gerrit-Change-Number: 13402 Gerrit-PatchSet: 4 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 27 Mar 2019 13:03:32 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-pcu[master]: Update MCS selection for retransmission
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/13402 to look at the new patch set (#4). Change subject: Update MCS selection for retransmission .. Update MCS selection for retransmission In 3GPP TS 44.060 the selection of MCS for retransmissions is defined as separate tables (8.1.1.1 and 8.1.1.2) depending on the value of resegmentation bit (which is opposite to the way EGPRS_ARQ are defined in the source code). Let's follow the same idea and explicitly check for resegmentation bit value and use separate tables. This also makes it easier to add proper support for special cases (MCS-6-9 and MCS-5-7) and padding in future independently for different ARQ types. The code is also moved to c to avoid unnecessary conversions to and from cpp class. Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 --- M src/coding_scheme.c M src/coding_scheme.h M src/gprs_coding_scheme.cpp M src/gprs_coding_scheme.h M src/tbf_dl.cpp 5 files changed, 40 insertions(+), 50 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/02/13402/4 -- To view, visit https://gerrit.osmocom.org/13402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 Gerrit-Change-Number: 13402 Gerrit-PatchSet: 4 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-CC: Pau Espin Pedrol
Change in osmo-pcu[master]: Update MCS selection for retransmission
Max has posted comments on this change. ( https://gerrit.osmocom.org/13402 ) Change subject: Update MCS selection for retransmission .. Patch Set 3: (1 comment) https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c File src/coding_scheme.c: https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c@136 PS1, Line 136: OSMO_ASSERT(NUM_SCHEMES - MCS1 == 9); > May be worth adding an OSMO_STATIC_ASSERT() macro in libosmocore Excellent idea! Please update this code once the macro is available in libosmocore. -- To view, visit https://gerrit.osmocom.org/13402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 Gerrit-Change-Number: 13402 Gerrit-PatchSet: 3 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 27 Mar 2019 12:48:42 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-pcu[master]: Update MCS selection for retransmission
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/13402 ) Change subject: Update MCS selection for retransmission .. Patch Set 1: (2 comments) https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c File src/coding_scheme.c: https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c@131 PS1, Line 131: /* FIXME: take into account padding and special cases of commanded MCS (MCS-6-9 and MCS-5-7) */ commanded MCS? do you mean demanded_mcs? https://gerrit.osmocom.org/#/c/13402/1/src/coding_scheme.c@136 PS1, Line 136: OSMO_ASSERT(NUM_SCHEMES - MCS1 == 9); That should be actually a compile-time assert. Have a look at "_Static_assert": https://en.cppreference.com/w/c/language/_Static_assert It seems for c++ "static_assert" is used instead. May be worth adding an OSMO_STATIC_ASSERT() macro in libosmocore to make it automatically c/c++ compatible, as explained in https://stackoverflow.com/a/54993033 -- To view, visit https://gerrit.osmocom.org/13402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 Gerrit-Change-Number: 13402 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Mon, 25 Mar 2019 16:37:56 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-pcu[master]: Update MCS selection for retransmission
Max has uploaded this change for review. ( https://gerrit.osmocom.org/13402 Change subject: Update MCS selection for retransmission .. Update MCS selection for retransmission In 3GPP TS 44.060 the selection of MCS for retransmissions is defined as separate tables (8.1.1.1 and 8.1.1.2) depending on the value of resegmentation bit (which is opposite to the way EGPRS_ARQ are defined in the source code). Let's follow the same idea and explicitly check for resegmentation bit value and use separate tables. This also makes it easier to add proper support for special cases (MCS-6-9 and MCS-5-7) and padding in future independently for different ARQ types. The code is also moved to c to avoid unnecessary conversions to and from cpp class. Change-Id: Ia73baeefee7a58834f0fc50e3b8bf8d5e3eb7815 --- M src/coding_scheme.c M src/coding_scheme.h M src/gprs_coding_scheme.cpp M src/gprs_coding_scheme.h M src/tbf_dl.cpp 5 files changed, 40 insertions(+), 50 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/02/13402/1 diff --git a/src/coding_scheme.c b/src/coding_scheme.c index a4ae882..48a74cd 100644 --- a/src/coding_scheme.c +++ b/src/coding_scheme.c @@ -127,3 +127,39 @@ const char *mode_name(enum mcs_kind val) { return get_value_string(mode_names, val); } + +/* FIXME: take into account padding and special cases of commanded MCS (MCS-6-9 and MCS-5-7) */ +enum CodingScheme get_retx_mcs(enum CodingScheme initial_mcs, enum CodingScheme demanded_mcs, bool resegment_bit) +{ + OSMO_ASSERT(mcs_is_edge(initial_mcs)); + OSMO_ASSERT(mcs_is_edge(demanded_mcs)); + OSMO_ASSERT(NUM_SCHEMES - MCS1 == 9); + + if (resegment_bit) { /* 3GPP TS 44.060 Table 8.1.1.1, reflected over antidiagonal */ + enum CodingScheme egprs_reseg[NUM_SCHEMES - MCS1][NUM_SCHEMES - MCS1] = { + { MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1 }, + { MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3 }, + { MCS1, MCS1, MCS1, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4 }, + { MCS2, MCS2, MCS2, MCS2, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS6, MCS6, MCS6, MCS9 }, + { MCS2, MCS2, MCS2, MCS2, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS6, MCS6, MCS8, MCS8 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS6, MCS6, MCS6, MCS9 }, + }; + return egprs_reseg[mcs_chan_code(initial_mcs)][mcs_chan_code(demanded_mcs)]; + } else { /* 3GPP TS 44.060 Table 8.1.1.2, reflected over antidiagonal */ + enum CodingScheme egprs_no_reseg[NUM_SCHEMES - MCS1][NUM_SCHEMES - MCS1] = { + { MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1 }, + { MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2, MCS2 }, + { MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3, MCS3 }, + { MCS4, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4, MCS4 }, + { MCS5, MCS5, MCS5, MCS5, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS9 }, + { MCS5, MCS5, MCS5, MCS5, MCS5, MCS5, MCS7, MCS7, MCS7 }, + { MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS8, MCS8 }, + { MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS6, MCS9 }, + }; + return egprs_no_reseg[mcs_chan_code(initial_mcs)][mcs_chan_code(demanded_mcs)]; + } +} diff --git a/src/coding_scheme.h b/src/coding_scheme.h index aac4bba..f93a4a2 100644 --- a/src/coding_scheme.h +++ b/src/coding_scheme.h @@ -32,6 +32,7 @@ extern const struct value_string mcs_names[]; const char *mcs_name(enum CodingScheme val); +enum CodingScheme get_retx_mcs(enum CodingScheme initial_mcs, enum CodingScheme demanded_mcs, bool resegment_bit); bool mcs_is_gprs(enum CodingScheme cs); bool mcs_is_edge(enum CodingScheme cs); diff --git a/src/gprs_coding_scheme.cpp b/src/gprs_coding_scheme.cpp index a149f81..0c22670 100644 --- a/src/gprs_coding_scheme.cpp +++ b/src/gprs_coding_scheme.cpp @@ -21,41 +21,6 @@ #include "gprs_coding_scheme.h" -#define MAX_NUM_ARQ 2 /* max. number of ARQ */ -#define MAX_NUM_MCS 9 /* max. number of MCS */ - -/* - * 44.060 Table 8.1.1.1 and Table 8.1.1.2 - * It has 3 level indexing. 0th level is ARQ type - * 1st level is Original MCS( index 0 corresponds to MCS1 and so on) - * 2nd level is MS MCS (index 0 corresponds to MCS1 and so on) - */ -static enum CodingScheme egprs_mcs_retx_tbl[MAX_NUM_ARQ] - [MAX_NUM_MCS][MAX_NUM_MCS] = { - { -