Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config YAML parser behaviour depends on the line ordering in the YAML file #987

Open
bradunov opened this issue Dec 16, 2024 · 3 comments
Open
Assignees

Comments

@bradunov
Copy link

Issue Description

Config YAML parser behaviour depends on the line ordering in the YAML file.

Setup Details

I am running srsRAN on Azure Linux 3.0 with Foxconn RU. I am using the latest commit (e5d5b44b9). The suspected bug is likely in the YAML parsing library.

Expected Behavior

I want to disable PRACH compression using compr_method_prach: none parameter in ru_ofh section.
This works if compr_method_prach is before the cells key. If it is after, it gets ignored.
YAML parameter values should not depend on the order.

Actual Behaviour

If I put compr_method_prach: none after cells key in ru_ofh, it gets ignored.

Steps to reproduce the problem

I want to disable the PRACH compression. I used a sample configuration file and it worked. Here is the relevant snippet:

ru_ofh:
  compr_method_prach: none
  compr_bitwidth_prach: 16
  compr_method_dl: bfp
  compr_bitwidth_dl: 9
  compr_method_ul: bfp
  compr_bitwidth_ul: 9
  enable_dl_static_compr_hdr: false
  enable_ul_static_compr_hdr: false
  ignore_ecpri_payload_size: true
  iq_scaling: 2.0
  is_dl_broadcast_enabled: false
  is_prach_cp_enabled: true
  ru_bandwidth_MHz: 100
  t1a_max_cp_dl: 470
  t1a_max_cp_ul: 429
  t1a_max_up: 196
  t1a_min_cp_dl: 258
  t1a_min_cp_ul: 285
  t1a_min_up: 50
  ta4_max: 75
  ta4_min: 0
  cells:
  - dl_port_id: [0, 1, 2, 3]
    du_mac_addr: 00:11:22:33:0a:a0
    network_interface: 0000:86:09.0
    prach_port_id: [4, 5, 6, 7]
    ru_mac_addr: 6c:ad:ad:00:0a:a0
    ul_port_id: [0, 1, 2, 3]
    vlan_tag_cp: 3
    vlan_tag_up: 3

The RAN starts with the following message:

Initializing the Open Fronthaul Interface for sector#0: ul_compr=[BFP,9], dl_compr=[BFP,9], prach_compr=[none,16], prach_cp_enabled=true, downlink_broadcast=false

PRACH compression is none, as expected.

I needed to write a script that modifies the YAML file in python. The python script rearranged the order of the config as follows:

ru_ofh:
  cells:
  - dl_port_id: [0, 1, 2, 3]
    du_mac_addr: 00:11:22:33:0a:a0
    network_interface: 0000:86:09.0
    prach_port_id: [4, 5, 6, 7]
    ru_mac_addr: 6c:ad:ad:00:0a:a0
    ul_port_id: [0, 1, 2, 3]
    vlan_tag_cp: 3
    vlan_tag_up: 3
  compr_method_prach: none
  compr_bitwidth_prach: 16
  compr_method_dl: bfp
  compr_bitwidth_dl: 9
  compr_method_ul: bfp
  compr_bitwidth_ul: 9
  enable_dl_static_compr_hdr: false
  enable_ul_static_compr_hdr: false
  ignore_ecpri_payload_size: true
  iq_scaling: 2.0
  is_dl_broadcast_enabled: false
  is_prach_cp_enabled: true
  ru_bandwidth_MHz: 100
  t1a_max_cp_dl: 470
  t1a_max_cp_ul: 429
  t1a_max_up: 196
  t1a_min_cp_dl: 258
  t1a_min_cp_ul: 285
  t1a_min_up: 50
  ta4_max: 75
  ta4_min: 0

Note that it is the same as above, except that cells section is before the rest. Now it seems that the PRACH config was ignored, as the starting message showed:

Initializing the Open Fronthaul Interface for sector#0: ul_compr=[BFP,9], dl_compr=[BFP,9], prach_compr=[BFP,9], prach_cp_enabled=true, downlink_broadcast=false

Notice that the PRAC compression is now BFP, although the config file says compr_method_prach: none.

I also noticed that I can make it work again if I make compr_method_prach: none as a part of the cells config, as shown here:

ru_ofh:
  cells:
  - dl_port_id: [0, 1, 2, 3]
    du_mac_addr: 00:11:22:33:0a:a0
    network_interface: 0000:86:09.0
    prach_port_id: [4, 5, 6, 7]
    ru_mac_addr: 6c:ad:ad:00:0a:a0
    ul_port_id: [0, 1, 2, 3]
    vlan_tag_cp: 3
    vlan_tag_up: 3
    compr_method_prach: none
    compr_bitwidth_prach: 16
    compr_method_dl: bfp
    compr_bitwidth_dl: 9
    compr_method_ul: bfp
    compr_bitwidth_ul: 9
    enable_dl_static_compr_hdr: false
    enable_ul_static_compr_hdr: false
    ignore_ecpri_payload_size: true
    iq_scaling: 2.0
    is_dl_broadcast_enabled: false
    is_prach_cp_enabled: true
    ru_bandwidth_MHz: 100
    t1a_max_cp_dl: 470
    t1a_max_cp_ul: 429
    t1a_max_up: 196
    t1a_min_cp_dl: 258
    t1a_min_cp_ul: 285
    t1a_min_up: 50
    ta4_max: 75
    ta4_min: 0

I suspect that your code loads the default values, including compr_method_prach first, and then when you parse the cells section you delete all the default values and populate with the ones within the cell. The problem is that you seem to delete the default values even if they are not populated in the cells section.

Also, at least according to this documentation, compr_method_prach is not a part of the cells section, so the last configuration option should not be accepted.

@AlaiaL
Copy link
Contributor

AlaiaL commented Dec 16, 2024

Hi bradunov,

Also, at least according to this documentation, compr_method_prach is not a part of the cells section, so the last configuration option should not be accepted.

As you said the code supports configuring the compression parameters per cell, so it is ok to configure them there. We added this support in case that you have multiple cells with different RUs that support different parameters. You can configure all the parameters in the ru_ofh section but gps_alpha and gps_beta . We will update the documentation to make it more clear.

I suspect that your code loads the default values, including compr_method_prach first, and then when you parse the cells section you delete all the default values and populate with the ones within the cell. The problem is that you seem to delete the default values even if they are not populated in the cells section.

I will check this. As you said, it seems something related to the order of parsing.

@AlaiaL AlaiaL self-assigned this Dec 16, 2024
@AlaiaL
Copy link
Contributor

AlaiaL commented Dec 17, 2024

Hi bradunov,

I checked it and it the order was important. There is a proposal to fix this, if you want to test it , I leave here a diff:

diff --git a/apps/units/flexible_o_du/split_7_2/helpers/ru_ofh_config_cli11_schema.cpp b/apps/units/flexible_o_du/split_7_2/helpers/ru_ofh_config_cli11_schema.cpp
index b3958d6..63c24fa 100644
--- a/apps/units/flexible_o_du/split_7_2/helpers/ru_ofh_config_cli11_schema.cpp
+++ b/apps/units/flexible_o_du/split_7_2/helpers/ru_ofh_config_cli11_schema.cpp
@@ -215,7 +215,16 @@
   add_option(app, "--gps_beta", ofh_cfg.gps_Beta, "GPS Beta")->capture_default_str()->check(CLI::Range(-32768, 32767));
 
   // Common cell parameters.
-  configure_cli11_ru_ofh_base_cell_args(app, config.base_cell_cfg);
+  auto base_cell_group = app.add_option_group("base_cell");
+  configure_cli11_ru_ofh_base_cell_args(*base_cell_group, config.base_cell_cfg);
+  base_cell_group->parse_complete_callback([&config, &app]() {
+    for (auto& cell : config.config.cells) {
+      cell.cell = config.base_cell_cfg;
+    };
+    if (app.get_option("--cells")->get_callback_run()) {
+      app.get_option("--cells")->run_callback();
+    }
+  });
 
   // Cell parameters.
   add_option_cell(


I think you can apply it directly, but if not, just do it manually.

Thanks

@bradunov
Copy link
Author

Hi AlaiaL,

I've checked the patch and it works. Please let me know when you push it so I can update my config files.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants