Skip to content

Enforce N_PRB=2^x3^y5^z in SC-FDMA uplink scheduler #247

Open
PC-OC wants to merge 2 commits into
duranta-project:developfrom
PC-OC:scfdma-uplink-scheduler-rbsize
Open

Enforce N_PRB=2^x3^y5^z in SC-FDMA uplink scheduler #247
PC-OC wants to merge 2 commits into
duranta-project:developfrom
PC-OC:scfdma-uplink-scheduler-rbsize

Conversation

@PC-OC

@PC-OC PC-OC commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

With SC-FDMA, the scheduler in uplink needs to schedule N_PRB=2^x3^y5^z. Not enforcing it may lead to unwanted UE behavior like retransmissions and re-establishments.

TS38.211 - 6.3.1.4

@PC-OC PC-OC added the 5G-NR Perform 5G Tests label Jun 30, 2026
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
@durantabot

Copy link
Copy Markdown
Collaborator

@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from cb94c6c to 8aca922 Compare June 30, 2026 12:31
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c
@rorsc

rorsc commented Jun 30, 2026

Copy link
Copy Markdown
Member

@durantabot

Copy link
Copy Markdown
Collaborator

@rorsc

rorsc commented Jun 30, 2026

Copy link
Copy Markdown
Member

one more thing, there is old code that we should remove or rework:
remove-235.patch

@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from 8aca922 to f37f4c1 Compare July 2, 2026 07:36
@PC-OC

PC-OC commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @rorsc, I have been told that some code enforcing it was already existing. After checking at pre-existing code I found out some cases were not treated so I dealt with them. I let what was done previously as a second safeguard but could be removed if needed.

@rorsc

rorsc commented Jul 2, 2026

Copy link
Copy Markdown
Member

Hi @rorsc, I have been told that some code enforcing it was already existing. After checking at pre-existing code I found out some cases were not treated so I dealt with them. I let what was done previously as a second safeguard but could be removed if needed.

do you refer to the patch that I shared with you here? #247 (comment) or what do you mean with "some code"?

the way it was dealt with is obviously not sufficient. So still, I suggest you remove what was there, and reimplement it anew. The old code makes the binary search function also more complicated.

@PC-OC

PC-OC commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

For the case of using min_rb as rbSize, I have done another LUT. One question has been raised. For the values 271, 272, 273 they do not fit 2^x3^y5^z. So I chose to let the value 270 but in this case we will have rbSize < rb_min.

@rorsc

rorsc commented Jul 2, 2026

Copy link
Copy Markdown
Member

For the case of using min_rb as rbSize, I have done another LUT. One question has been raised. For the values 271, 272, 273 they do not fit 2^x_3^y_5^z. So I chose to let the value 270 but in this case we will have rbSize < rb_min.

Making rb_min=273 is simply fundamentally incompatible with SC-FDMA, because you can't have that number of RBs. So either we simply override, or we assert. I am ok with either behavior, and have the only remark that the earlier we stop, the better.

@PC-OC

PC-OC commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

do you refer to the patch that I shared with you here? #247 (comment) or what do you mean with "some code"?

the way it was dealt with is obviously not sufficient. So still, I suggest you remove what was there, and reimplement it anew. The old code makes the binary search function also more complicated.

Sorry for the imprecision, I was speaking about the nr_find_nb_rb function and its usage. So it is also part of the provided patch. I have applied modifications to it, in order to use the LUTs. I have find that the min_rb condition was also not enforcing N_PRB=2^x3^y5^z. And I've let the check_sc_fdma_rbsize function as second safeguard.

@durantabot

Copy link
Copy Markdown
Collaborator

CI Build: #608 | Failed on the following stages:

Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch_LUTs.h Outdated
@rorsc rorsc mentioned this pull request Jul 2, 2026
@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch 2 times, most recently from 2530497 to 1b8560c Compare July 2, 2026 09:18
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch_default_policies.c Outdated
@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from 1b8560c to a333e1c Compare July 2, 2026 12:07
@durantabot

Copy link
Copy Markdown
Collaborator

@durantabot

Copy link
Copy Markdown
Collaborator

Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from a333e1c to 20add78 Compare July 2, 2026 12:57
Comment thread openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c Outdated
@durantabot

Copy link
Copy Markdown
Collaborator

CI Build: #620 | Failed on the following stages:

@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from 20add78 to 69cde6c Compare July 2, 2026 15:43
rorsc added a commit that referenced this pull request Jul 2, 2026
…nto integration_2026_w27

Enforce N_PRB=2^x3^y5^z in SC-FDMA uplink scheduler (#247)

With SC-FDMA, the scheduler in uplink needs to schedule N_PRB=2^x3^y5^z.
Not enforcing it may lead to unwanted UE behavior like retransmissions
and re-establishments.

TS38.211 - 6.3.1.4

Reviewed-by: Robert Schmidt <robert.schmidt@openairinterface.org>
int check_sc_fdma_rbsize(long transform_precoding, uint16_t rb)
{
if (transform_precoding == NR_PUSCH_Config__transformPrecoder_enabled) {
return rb < 274 ? NR_TRANSFORM_PRECODE_RB_LUT[rb] : NR_TRANSFORM_PRECODE_RB_LUT[273];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it makes sense to check RB < 274 since the max BWP is 273 RBs. I would have rather put an assertion. But it's not a blocking point.

@durantabot

Copy link
Copy Markdown
Collaborator

@rorsc

rorsc commented Jul 2, 2026

Copy link
Copy Markdown
Member

there is a segfault in the beginning, something is not right, please check

@PC-OC

PC-OC commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

there is a segfault in the beginning, something is not right, please check

*scc->uplinkConfigCommon->initialUplinkBWP->rach_ConfigCommon->choice.setup->msg3_transformPrecoder can have a null pointer

@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from 69cde6c to cc697c5 Compare July 3, 2026 06:38
PC-OC added 2 commits July 3, 2026 09:15
Signed-off-by: Calvin Peyron <calvin.peyron@open-cells.com>
Signed-off-by: Calvin Peyron <calvin.peyron@open-cells.com>
@PC-OC PC-OC force-pushed the scfdma-uplink-scheduler-rbsize branch from cc697c5 to ce95a4d Compare July 3, 2026 07:16
@durantabot

Copy link
Copy Markdown
Collaborator

CI Build: #639 | Failed on the following stages:

@rorsc rorsc added the retrigger-ci Re-run CI label Jul 3, 2026
@github-actions github-actions Bot removed the retrigger-ci Re-run CI label Jul 3, 2026
@PC-OC

PC-OC commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

CI Build: #639 | Failed on the following stages:

* [RAN-Ubuntu-Image-Builder](https://jenkins-oai.eurecom.fr/job/RAN-GitHub-Container-Parent/639/artifact/test_results-RAN-Ubuntu-Image-Builder.html)

0.803 fatal: unable to access '': The requested URL returned error: 502
0.805 fatal: clone of 'https://gitlab.eurecom.fr/mosaic5g/flexric.git' into submodule path '/oai-ran/openair2/E2AP/flexric' failed
0.805 Failed to clone 'openair2/E2AP/flexric' a second time, aborting

@durantabot

Copy link
Copy Markdown
Collaborator

else
ul_bler_options->harq_round_max = *gpd(params, np, MACRLC_UL_HARQ_ROUND_MAX)->u8ptr;
RC.nrmac[j]->min_grant_prb = *gpd(params, np, MACRLC_MIN_GRANT_PRB)->u16ptr;
long uses_sc_fdma = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's exactly wrong. check_sc_fdma_rbsize() uses NR_PUSCH_Config__transformPrecoder_enabled to check if SC-FDMA is enabled. that constant is 0, so here, by default and if msg3_transformPrecoder is not set, will be set to enabled when it should be disabled.

please fix

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IOW, initialize with NR_PUSCH_Config__transformPrecoder_disabled

}
int new_min = check_sc_fdma_rbsize(uses_sc_fdma, RC.nrmac[j]->min_grant_prb);
// NR_PUSCH_Config__transformPrecoder_enabled = 0 | NR_PUSCH_Config__transformPrecoder_disabled = 1, so !uses_sc_fdma should be used
if (!uses_sc_fdma && RC.nrmac[j]->min_grant_prb != new_min) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know that you have that comment, but it's really confusing that !uses_sc_fdma means "we have SC-FDMA"

one suggestion I have is to rename the variable and check

sc_fdma == NR_PUSCH_Config__transformPrecoder_enabled

then it's clear and the 0/1 confusion is not visible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5G-NR Perform 5G Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants