sched: improve comments in PUCCH allocator

Signed-off-by: Carlo Galiotto <[email protected]>

Author: carlo-gal

include/srsran/ran/pucch/pucch_configuration.h

@@ -202,7 +202,8 @@ struct pucch_config {
   static_vector<uint8_t, 8> dl_data_to_ul_ack;
 
   /// PUCCH resource max UCI payload, depending on the format. The index defines the format.
-  /// \remark All the resources of the same format are configured with the same max UCI payload.
+  /// \remark The UCI payload is the same for all UE's PUCCH resources belonging to the same format, regardless of
+  /// whether they are used for HARQ-ACK or CSI.
   /// \remark For Format 0 and 1, only the max number of HARQ-ACK bits are considered.
   static_vector<unsigned, 5> format_max_payload{0, 0, 0, 0, 0};
 

lib/du_manager/ran_resource_management/du_pucch_resource_manager.cpp

@@ -289,7 +289,8 @@ bool du_pucch_resource_manager::alloc_resources(cell_group_config& cell_grp_cfg)
   }
 
   // Update the PUCCH max payload.
-  // With Format 1, we can have up to 2 HARQ-ACK bits (SR doesn't count as part of the payload).
+  // As per TS 38.231, Section 9.2.1, with PUCCH Format 1, we can have up to 2 HARQ-ACK bits (SR doesn't count as part
+  // of the payload).
   constexpr static unsigned pucch_f1_max_harq_payload = 2U;
   cell_grp_cfg.cells[0].serv_cell_cfg.ul_config->init_ul_bwp.pucch_cfg.value().format_max_payload[pucch_format_to_uint(
       pucch_format::FORMAT_1)]                        = pucch_f1_max_harq_payload;

lib/scheduler/config/serving_cell_config_factory.cpp

@@ -647,7 +647,8 @@ uplink_config srsran::config_helpers::make_default_ue_uplink_config(const cell_c
   }
 
   // Compute the max UCI payload per format.
-  // With Format 1, we can have up to 2 HARQ-ACK bits (SR doesn't count as part of the payload).
+  // As per TS 38.231, Section 9.2.1, with PUCCH Format 1, we can have up to 2 HARQ-ACK bits (SR doesn't count as part
+  // of the payload).
   constexpr static unsigned pucch_f1_max_harq_payload                        = 2U;
   pucch_cfg.format_max_payload[pucch_format_to_uint(pucch_format::FORMAT_1)] = pucch_f1_max_harq_payload;
   const auto& res_f2 = std::get<pucch_format_2_3_cfg>(res_basic_f2.format_params);

lib/scheduler/pucch_scheduling/pucch_allocator_impl.cpp

@@ -263,7 +263,11 @@ std::optional<unsigned> pucch_allocator_impl::alloc_ded_pucch_harq_ack_ue(cell_r
   // Get the slot allocation grid considering the PDSCH delay (k0) and the PUCCH delay wrt PDSCH (k1).
   cell_slot_resource_allocator& pucch_slot_alloc = res_alloc[k0 + k1 + res_alloc.cfg.ntn_cs_koffset];
 
-  resource_manager.reset_last_ue_allocation();
+  // The PUCCH allocation may result in a temporary reservation of PUCCH resources, which need to be released in case of
+  // failure or in case the multiplexing results in a different final PUCCH resource. If we don't reset the previous
+  // record, we could release the resources that have been allocated for other UEs of allocated for this UE in a
+  // different slot.
+  resource_manager.reset_latest_reserved_res_tracker();
 
   slot_point sl_ack = pucch_slot_alloc.slot;
 
@@ -280,7 +284,7 @@ std::optional<unsigned> pucch_allocator_impl::alloc_ded_pucch_harq_ack_ue(cell_r
     std::optional<unsigned> pucch_res_ind =
         multiplex_and_allocate_pucch(pucch_slot_alloc, new_bits, *existing_grant_it, ue_cell_cfg);
     if (not pucch_res_ind) {
-      resource_manager.cancel_last_ue_allocations(sl_ack, crnti, ue_cell_cfg);
+      resource_manager.cancel_last_ue_res_reservations(sl_ack, crnti, ue_cell_cfg);
     }
     return pucch_res_ind;
   } else {
@@ -304,7 +308,7 @@ void pucch_allocator_impl::pucch_allocate_sr_opportunity(cell_slot_resource_allo
   }
 
   if (pucch_grants_alloc_grid[sl_tx.to_uint()].full()) {
-    logger.info("rnti={}: PUCCH HARQ-ACK allocation for slot={} skipped. Cause: scheduler cache full",
+    logger.info("rnti={}: PUCCH HARQ-ACK allocation for slot={} skipped. Cause: PUCCH allocator grant list is full",
                 crnti,
                 pucch_slot_alloc.slot);
     return;
@@ -368,7 +372,11 @@ void pucch_allocator_impl::pucch_allocate_csi_opportunity(cell_slot_resource_all
 {
   const slot_point sl_tx = pucch_slot_alloc.slot;
 
-  resource_manager.reset_last_ue_allocation();
+  // The PUCCH allocation may result in a temporary reservation of PUCCH resources, which need to be released in case of
+  // failure or in case the multiplexing results in a different final PUCCH resource. If we don't reset the previous
+  // record, we could release the resources that have been allocated for other UEs of allocated for this UE in a
+  // different slot.
+  resource_manager.reset_latest_reserved_res_tracker();
 
   // [Implementation-defined] We only allow a max number of PUCCH + PUSCH grants per slot.
   if (pucch_slot_alloc.result.ul.pucchs.size() >=
@@ -392,19 +400,20 @@ void pucch_allocator_impl::pucch_allocate_csi_opportunity(cell_slot_resource_all
   if (existing_grant_it != pucch_grants_alloc_grid[sl_tx.to_uint()].end() and existing_grant_it->has_common_pucch) {
     // Allocation of dedicated + common resources are handled by allocating PUCCH common on existing CSI, not the other
     // way around. If the function enters the path, it means it too early to start scheduling the CSI.
-    logger.info("rnti={}: CSI occasion allocation for slot={} skipped. Cause: existing PUCCH common grant",
+    logger.info("rnti={}: CSI occasion allocation for slot={} skipped. Cause: There is a PUCCH common grant"
+                "allocated at this slot",
                 crnti,
                 pucch_slot_alloc.slot);
     return;
   }
 
   // Handle case of existing PUCCHs with possible multiplexing.
   pucch_uci_bits bits_for_uci = existing_grant_it->pucch_grants.get_uci_bits();
-  srsran_assert(bits_for_uci.csi_part1_nof_bits == 0, "PUCCH grant for CSI already been allocated");
+  srsran_assert(bits_for_uci.csi_part1_nof_bits == 0, "PUCCH grant for CSI has already been allocated");
   bits_for_uci.csi_part1_nof_bits = csi_part1_nof_bits;
   auto alloc_outcome = multiplex_and_allocate_pucch(pucch_slot_alloc, bits_for_uci, *existing_grant_it, ue_cell_cfg);
   if (not alloc_outcome.has_value()) {
-    resource_manager.cancel_last_ue_allocations(sl_tx, crnti, ue_cell_cfg);
+    resource_manager.cancel_last_ue_res_reservations(sl_tx, crnti, ue_cell_cfg);
   }
 }
 
@@ -988,7 +997,12 @@ pucch_allocator_impl::find_common_and_ded_harq_res_available(cell_slot_resource_
   // As per Section 9.2.1, TS 38.213, this is the max value of \f$\Delta_{PRI}\f$, which is a 3-bit unsigned.
   const unsigned max_d_pri = 7;
   for (unsigned d_pri = 0; d_pri != max_d_pri + 1; ++d_pri) {
-    resource_manager.reset_last_ue_allocation();
+    // The PUCCH allocation may result in a temporary reservation of PUCCH resources, which need to be released in case
+    // of failure or in case the multiplexing results in a different final PUCCH resource. If we don't reset the
+    // previous record, we could release the resources that have been allocated for other UEs of allocated for this UE
+    // in a different slot.
+    // Reset at each iteration, as a new iteration indicates that the previous allocation failed.
+    resource_manager.reset_latest_reserved_res_tracker();
 
     // r_PUCCH, as per Section 9.2.1, TS 38.213.
     const unsigned r_pucch = get_pucch_default_resource_index(start_cce_idx, nof_coreset_cces, d_pri);
@@ -1030,7 +1044,7 @@ pucch_allocator_impl::find_common_and_ded_harq_res_available(cell_slot_resource_
     pucch_res_ind = multiplex_and_allocate_pucch(pucch_alloc, bits_for_uci, existing_grants, ue_cell_cfg, true);
 
     if (not pucch_res_ind.has_value()) {
-      resource_manager.cancel_last_ue_allocations(pucch_alloc.slot, rnti, ue_cell_cfg);
+      resource_manager.cancel_last_ue_res_reservations(pucch_alloc.slot, rnti, ue_cell_cfg);
       continue;
     }
 
@@ -1057,7 +1071,7 @@ std::optional<unsigned> pucch_allocator_impl::allocate_harq_grant(cell_slot_reso
   }
 
   if (pucch_grants_alloc_grid[sl_tx.to_uint()].full()) {
-    logger.info("rnti={}: PUCCH HARQ-ACK allocation for slot={} skipped. Cause: scheduler cache full",
+    logger.info("rnti={}: PUCCH HARQ-ACK allocation for slot={} skipped. Cause: PUCCH allocator grant list is full",
                 crnti,
                 pucch_slot_alloc.slot);
     return std::nullopt;
@@ -1108,7 +1122,7 @@ void pucch_allocator_impl::allocate_csi_grant(cell_slot_resource_allocator& pucc
   }
 
   if (pucch_grants_alloc_grid[sl_tx.to_uint()].full()) {
-    logger.info("rnti={}: PUCCH HARQ-ACK allocation for slot={} skipped. Cause: scheduler cache full",
+    logger.info("rnti={}: PUCCH HARQ-ACK allocation for slot={} skipped. Cause: PUCCH allocator grant list is full",
                 crnti,
                 pucch_slot_alloc.slot);
     return;

lib/scheduler/pucch_scheduling/pucch_resource_manager.cpp

@@ -253,13 +253,13 @@ bool pucch_resource_manager::release_csi_resource(slot_point                   s
   return true;
 }
 
-void pucch_resource_manager::reset_last_ue_allocation()
+void pucch_resource_manager::reset_latest_reserved_res_tracker()
 {
   last_ue_allocations.rnti       = rnti_t::INVALID_RNTI;
   last_ue_allocations.harq_set_0 = false;
   last_ue_allocations.harq_set_1 = false;
   last_ue_allocations.sr         = false;
-  last_ue_allocations.sr         = false;
+  last_ue_allocations.csi        = false;
 }
 
 void pucch_resource_manager::set_new_resource_allocation(rnti_t crnti, pucch_resource_usage res_type)
@@ -311,9 +311,9 @@ bool pucch_resource_manager::is_resource_allocated(rnti_t rnti, pucch_resource_u
   }
 }
 
-void pucch_resource_manager::cancel_last_ue_allocations(slot_point                   slot_tx,
-                                                        rnti_t                       crnti,
-                                                        const ue_cell_configuration& ue_cell_cfg)
+void pucch_resource_manager::cancel_last_ue_res_reservations(slot_point                   slot_tx,
+                                                             rnti_t                       crnti,
+                                                             const ue_cell_configuration& ue_cell_cfg)
 {
   if (crnti != last_ue_allocations.rnti) {
     srsran_assertion_failure("Trying to cancel a UE allocation that was not the last one");

lib/scheduler/pucch_scheduling/pucch_resource_manager.h

@@ -126,13 +126,19 @@ class pucch_resource_manager
   /// \return True if the resource for the UE was found in the allocation records for the given slot.
   bool release_csi_resource(slot_point slot_sr, rnti_t crnti, const ue_cell_configuration& ue_cell_cfg);
 
-  void reset_last_ue_allocation();
+  /// \brief Reset the record of the PUCCH resources reserved to UE at the current slot.
+  /// \remark This function doesn't release the resources, it only resets the record of which resources have been
+  /// reserved.
+  void reset_latest_reserved_res_tracker();
 
+  /// \brief Adds a records that a given resource has been reserved for the current slot for a given UE.
   void set_new_resource_allocation(rnti_t rnti, pucch_resource_usage res_type);
 
+  /// \brief Checks whether a given resource has been reserved for the current slot for a given UE.
   [[nodiscard]] bool is_resource_allocated(rnti_t rnti, pucch_resource_usage res_type) const;
 
-  void cancel_last_ue_allocations(slot_point slot_tx, rnti_t crnti, const ue_cell_configuration& ue_cell_cfg);
+  /// \brief Releases the resources that have been recorded as reserved for the current slot for a given UE.
+  void cancel_last_ue_res_reservations(slot_point slot_tx, rnti_t crnti, const ue_cell_configuration& ue_cell_cfg);
 
 private:
   /// Size of the ring buffer of pucch_resource_manager. This size sets a limit on how far in advance a PUCCH can be
@@ -149,7 +155,7 @@ class pucch_resource_manager
   // As per Section 9.2.1, TS 38.213, this is given by the number of possible values of r_PUCCH, which is 16.
   static const size_t MAX_COMMON_PUCCH_RESOURCES{16};
 
-  // Tracks usage of PUCCH resources.
+  // Tracks reservation of a PUCCH resource by a given UE and for a given usage (e.g., HARQ, SR, CSI).
   struct resource_tracker {
     rnti_t               rnti;
     pucch_resource_usage resource_usage;
@@ -158,15 +164,19 @@ class pucch_resource_manager
   using pucch_res_record_array  = std::array<resource_tracker, MAX_PUCCH_RESOURCES>;
   using common_res_record_array = std::array<bool, MAX_COMMON_PUCCH_RESOURCES>;
 
-  // Record for the RNTI and PUCCH resource indicator used for a given resource at a given slot.
+  // Record for the RNTI and PUCCH resource indicator used for a given resource at a given slot; this information is
+  // used to keep track of which resources are available and which UE is using them. This information is preserved over
+  // the slots.
   struct rnti_pucch_res_id_slot_record {
     common_res_record_array used_common_resources;
     pucch_res_record_array  ues_using_pucch_res;
   };
 
-  // Collects the information of what PUCCH cell resources have been allocated to a UE at given slot.
-  // This info is used/updated by the PUCCH allocator and called for a new UE or new allocation for the same UE.
-  struct ue_allocation_tracker {
+  // Keeps track of which PUCCH cell resources have been allocated to a UE at the current slot.
+  // This info is needed to release some resources after the PUCCH multiplexing (by the PUCCH allocator), during which
+  // resources can be reserved and then released, depending on the multiplexing algorithm.
+  // The lifespan of this information lasts for a single UE PUCCH allocation.
+  struct current_slot_ue_allocations {
     rnti_t rnti       = rnti_t::INVALID_RNTI;
     bool   harq_set_0 = false;
     bool   harq_set_1 = false;
@@ -194,7 +204,7 @@ class pucch_resource_manager
   // Ring buffer of rnti_pucch_res_id_slot_record for PUCCH resources.
   std::array<rnti_pucch_res_id_slot_record, RES_MANAGER_RING_BUFFER_SIZE> resource_slots;
 
-  ue_allocation_tracker last_ue_allocations;
+  current_slot_ue_allocations last_ue_allocations;
 
   // Keeps track of the last slot_point used by the resource manager.
   slot_point last_sl_ind;