sched: fix handling of error indication and make it part of the ue event manager

Author: frankist

lib/scheduler/logging/scheduler_event_logger.cpp

@@ -18,6 +18,9 @@ void scheduler_event_logger::log_impl()
   if (mode == debug) {
     logger.debug("Processed slot events:{}", to_c_str(fmtbuf));
     fmtbuf.clear();
+  } else if (mode == info) {
+    logger.info("Processed slot events:{}", to_c_str(fmtbuf));
+    fmtbuf.clear();
   }
 }
 
@@ -92,6 +95,20 @@ void scheduler_event_logger::enqueue_impl(const sched_ue_delete_message& ue_requ
   }
 }
 
+void scheduler_event_logger::enqueue_impl(const error_indication_event& err_ind)
+{
+  if (mode == debug) {
+    fmt::format_to(fmtbuf,
+                   "\n- ErrorIndication: slot={}{}{}{}",
+                   err_ind.sl_tx,
+                   err_ind.outcome.pdcch_discarded ? ", PDCCH discarded" : "",
+                   err_ind.outcome.pdsch_discarded ? ", PDSCH discarded" : "",
+                   err_ind.outcome.pusch_and_pucch_discarded ? ", PUSCH and PUCCH discarded" : "");
+  } else if (mode == info) {
+    fmt::format_to(fmtbuf, "{}ErrorIndication slot={}", separator(), err_ind.sl_tx);
+  }
+}
+
 void scheduler_event_logger::enqueue_impl(const sr_event& sr)
 {
   if (mode == debug) {

lib/scheduler/logging/scheduler_event_logger.h

@@ -77,6 +77,10 @@ class scheduler_event_logger
     ph_db_range                ph;
     optional<p_cmax_dbm_range> p_cmax;
   };
+  struct error_indication_event {
+    slot_point                            sl_tx;
+    scheduler_slot_handler::error_outcome outcome;
+  };
 
   scheduler_event_logger() :
     logger(srslog::fetch_basic_logger("SCHED")),
@@ -120,6 +124,8 @@ class scheduler_event_logger
   void enqueue_impl(const ue_reconf_event& ue_request);
   void enqueue_impl(const sched_ue_delete_message& ue_request);
 
+  void enqueue_impl(const error_indication_event& err_ind);
+
   void enqueue_impl(const sr_event& sr);
   void enqueue_impl(const bsr_event& bsr);
   void enqueue_impl(const harq_ack_event& harq_ev);

lib/scheduler/ue_scheduling/ue_event_manager.cpp

@@ -424,6 +424,114 @@ void ue_event_manager::handle_dl_buffer_state_indication(const dl_buffer_state_i
   dl_bo_mng->handle_dl_buffer_state_indication(bs);
 }
 
+static void handle_discarded_pusch(const cell_slot_resource_allocator& prev_slot_result, ue_repository& ue_db)
+{
+  for (const ul_sched_info& grant : prev_slot_result.result.ul.puschs) {
+    ue* u = ue_db.find_by_rnti(grant.pusch_cfg.rnti);
+    if (u == nullptr) {
+      // UE has been removed.
+      continue;
+    }
+
+    // - The lower layers will not attempt to decode the PUSCH and will not send any CRC indication.
+    ul_harq_process& h_ul = u->get_pcell().harqs.ul_harq(grant.pusch_cfg.harq_id);
+    if (not h_ul.empty()) {
+      if (h_ul.tb().nof_retxs == 0) {
+        // Given that the PUSCH grant was discarded before it reached the PHY, the "new_data" flag was not handled
+        // and the UL softbuffer was not reset. To avoid mixing different TBs in the softbuffer, it is important to
+        // reset the UL HARQ process.
+        h_ul.reset();
+      } else {
+        // To avoid a long UL HARQ timeout window (due to lack of CRC indication), it is important to force a NACK
+        // in the UL HARQ process.
+        h_ul.crc_info(false);
+      }
+    }
+
+    // - The lower layers will not attempt to decode any UCI in the PUSCH and will not send any UCI indication.
+    if (grant.uci.has_value() and grant.uci->harq.has_value() and grant.uci->harq->harq_ack_nof_bits > 0) {
+      // To avoid a long DL HARQ timeout window (due to lack of UCI indication), it is important to NACK the
+      // DL HARQ processes with UCI falling in this slot.
+      u->get_pcell().harqs.dl_ack_info_cancelled(prev_slot_result.slot);
+    }
+  }
+}
+
+static void handle_discarded_pucch(const cell_slot_resource_allocator& prev_slot_result, ue_repository& ue_db)
+{
+  for (const auto& pucch : prev_slot_result.result.ul.pucchs) {
+    ue* u = ue_db.find_by_rnti(pucch.crnti);
+    if (u == nullptr) {
+      // UE has been removed.
+      continue;
+    }
+    bool has_harq_ack = false;
+    switch (pucch.format) {
+      case pucch_format::FORMAT_1:
+        has_harq_ack = pucch.format_1.harq_ack_nof_bits > 0;
+        break;
+      case pucch_format::FORMAT_2:
+        has_harq_ack = pucch.format_2.harq_ack_nof_bits > 0;
+        break;
+      default:
+        break;
+    }
+
+    // - The lower layers will not attempt to decode the PUCCH and will not send any UCI indication.
+    if (has_harq_ack) {
+      // To avoid a long DL HARQ timeout window (due to lack of UCI indication), it is important to force a NACK in
+      // the DL HARQ processes with UCI falling in this slot.
+      u->get_pcell().harqs.dl_ack_info_cancelled(prev_slot_result.slot);
+    }
+  }
+}
+
+void ue_event_manager::handle_error_indication(slot_point                            sl_tx,
+                                               du_cell_index_t                       cell_index,
+                                               scheduler_slot_handler::error_outcome event)
+{
+  common_events.emplace(INVALID_DU_UE_INDEX, [this, sl_tx, cell_index, event]() {
+    // Handle Error Indication.
+
+    const cell_slot_resource_allocator* prev_slot_result = du_cells[cell_index].res_grid->get_history(sl_tx);
+    if (prev_slot_result == nullptr) {
+      logger.warning("cell={}, slot={}: Discarding error indication. Cause: Scheduler results associated with the slot "
+                     "of the error indication have already been erased",
+                     cell_index,
+                     sl_tx);
+      return;
+    }
+
+    // In case DL PDCCHs were skipped, there will be the following consequences:
+    // - The UE will not decode the PDSCH and will not send the respective UCI.
+    // - The UE won't update the HARQ NDI, if new HARQ TB.
+    // - The UCI indication coming later from the lower layers will likely contain a HARQ-ACK=DTX.
+    // In case UL PDCCHs were skipped, there will be the following consequences:
+    // - The UE will not decode the PUSCH.
+    // - The UE won't update the HARQ NDI, if new HARQ TB.
+    // - The CRC indication coming from the lower layers will likely be CRC=KO.
+    // - Any UCI in the respective PUSCH will be likely reported as HARQ-ACK=DTX.
+    // In neither of the cases, the HARQs will timeout, because we did not lose the UCI/CRC indications in the lower
+    // layers. We do not need to cancel associated PUSCH grant (in UL PDCCH case) because it is important that
+    // the PUSCH "new_data" flag reaches the lower layers, telling them whether the UL HARQ buffer needs to be reset or
+    // not. Cancelling HARQ retransmissions is dangerous as it increases the chances of NDI ambiguity.
+
+    // In case of PDSCH grants being discarded, there will be the following consequences:
+    // - If the PDCCH was not discarded,the UE will fail to decode the PDSCH and will send an HARQ-ACK=NACK. The
+    // scheduler will retransmit the respective DL HARQ. No actions required.
+
+    // In case of PUCCH and PUSCH grants being discarded.
+    if (event.pusch_and_pucch_discarded) {
+      handle_discarded_pusch(*prev_slot_result, ue_db);
+
+      handle_discarded_pucch(*prev_slot_result, ue_db);
+    }
+
+    // Log event.
+    ev_logger.enqueue(scheduler_event_logger::error_indication_event{sl_tx, event});
+  });
+}
+
 void ue_event_manager::process_common(slot_point sl, du_cell_index_t cell_index)
 {
   bool new_slot_detected = last_sl != sl;
@@ -495,12 +603,14 @@ void ue_event_manager::run(slot_point sl, du_cell_index_t cell_index)
   process_cell_specific(cell_index);
 }
 
-void ue_event_manager::add_cell(const cell_configuration& cell_cfg_, ue_srb0_scheduler& srb0_sched)
+void ue_event_manager::add_cell(cell_resource_allocator& cell_res_grid, ue_srb0_scheduler& srb0_sched)
 {
-  srsran_assert(not cell_exists(cell_cfg_.cell_index), "Overwriting cell configurations not supported");
+  const du_cell_index_t cell_index = cell_res_grid.cell_index();
+  srsran_assert(not cell_exists(cell_index), "Overwriting cell configurations not supported");
 
-  du_cells[cell_cfg_.cell_index].cfg        = &cell_cfg_;
-  du_cells[cell_cfg_.cell_index].srb0_sched = &srb0_sched;
+  du_cells[cell_index].cfg        = &cell_res_grid.cfg;
+  du_cells[cell_index].res_grid   = &cell_res_grid;
+  du_cells[cell_index].srb0_sched = &srb0_sched;
 }
 
 bool ue_event_manager::cell_exists(du_cell_index_t cell_index) const

lib/scheduler/ue_scheduling/ue_event_manager.h

@@ -35,7 +35,7 @@ class ue_event_manager final : public sched_ue_configuration_handler,
   ue_event_manager(ue_repository& ue_db, scheduler_metrics_handler& metrics_handler, scheduler_event_logger& ev_logger);
   ~ue_event_manager();
 
-  void add_cell(const cell_configuration& cell_cfg_, ue_srb0_scheduler& srb0_sched);
+  void add_cell(cell_resource_allocator& cell_res_grid, ue_srb0_scheduler& srb0_sched);
 
   /// UE Add/Mod/Remove interface.
   void handle_ue_creation(ue_config_update_event ev) override;
@@ -52,6 +52,9 @@ class ue_event_manager final : public sched_ue_configuration_handler,
   /// Scheduler DL buffer state indication handler interface.
   void handle_dl_buffer_state_indication(const dl_buffer_state_indication_message& bs) override;
 
+  void
+  handle_error_indication(slot_point sl_tx, du_cell_index_t cell_index, scheduler_slot_handler::error_outcome event);
+
   /// Process events for a given slot and cell index.
   void run(slot_point sl, du_cell_index_t cell_index);
 
@@ -106,6 +109,8 @@ class ue_event_manager final : public sched_ue_configuration_handler,
   struct du_cell {
     const cell_configuration* cfg = nullptr;
 
+    cell_resource_allocator* res_grid = nullptr;
+
     /// Reference to SRB0 and other bearers scheduler
     ue_srb0_scheduler* srb0_sched = nullptr;
   };

lib/scheduler/ue_scheduling/ue_scheduler_impl.cpp

@@ -29,7 +29,7 @@ void ue_scheduler_impl::add_cell(const ue_scheduler_cell_params& params)
 {
   ue_res_grid_view.add_cell(*params.cell_res_alloc);
   cells[params.cell_index] = std::make_unique<cell>(expert_cfg, params, ue_db);
-  event_mng.add_cell(params.cell_res_alloc->cfg, cells[params.cell_index]->srb0_sched);
+  event_mng.add_cell(*params.cell_res_alloc, cells[params.cell_index]->srb0_sched);
   ue_alloc.add_cell(params.cell_index, *params.pdcch_sched, *params.uci_alloc, *params.cell_res_alloc);
 }
 
@@ -166,122 +166,3 @@ void ue_scheduler_impl::run_slot(slot_point slot_tx, du_cell_index_t cell_index)
   // TODO: remove this.
   puxch_grant_sanitizer(*cells[cell_index]->cell_res_alloc);
 }
-
-void ue_scheduler_impl::handle_error_indication(slot_point                            sl_tx,
-                                                du_cell_index_t                       cell_index,
-                                                scheduler_slot_handler::error_outcome event)
-{
-  if (cells[cell_index] == nullptr or cells[cell_index]->cell_res_alloc == nullptr) {
-    logger.error("cell={}: Discarding error indication. Cause: cell with provided index is not configured", cell_index);
-    return;
-  }
-  cell_resource_allocator& res_grid = *cells[cell_index]->cell_res_alloc;
-
-  const cell_slot_resource_allocator* prev_slot_result = res_grid.get_history(sl_tx);
-  if (prev_slot_result == nullptr) {
-    logger.warning("cell={}, slot={}: Discarding error indication. Cause: Scheduler results associated with the slot "
-                   "of the error indication have already been erased",
-                   cell_index,
-                   sl_tx);
-    return;
-  }
-
-  // In case DL PDCCHs were skipped, there will be the following consequences:
-  // - The UE will not decode the PDSCH and send the respective UCI. The UCI indication coming later from the lower
-  // layers will likely contain a HARQ-ACK=DTX. No action needed. We do not cancel the respective DL HARQ, because
-  // doing so would likely increase the chance of NDI ambiguity on the UE side.
-
-  // In case UL PDCCHs were skipped, there will be the following consequences:
-  // - The UE won't send the associated PUSCHs, which means that the lower layers will later forward CRC indications
-  // with CRC=KO, and the respective UL HARQs will need to be retransmitted. No action needed. We do not cancel
-  // the respective PUSCH grant, because it is important that the PUSCH "new_data" flag reaches the lower layers,
-  // telling them whether the UL HARQ buffer needs to be reset or not.
-  // - Any UCI in the respective PUSCH will be reported as DTX, and the DL HARQ will be retransmitted. No action
-  // needed.
-
-  // In case of PDSCH grants being discarded, there will be the following consequences:
-  // - The UE will fail to decode the PDSCH and will send an HARQ-ACK=NACK. The scheduler will retransmit the
-  // respective DL HARQ. No actions required.
-
-  // In case of PUCCH and PUSCH grants being discarded.
-  if (event.pusch_and_pucch_discarded) {
-    for (const ul_sched_info& grant : prev_slot_result->result.ul.puschs) {
-      ue* u = ue_db.find_by_rnti(grant.pusch_cfg.rnti);
-      if (u == nullptr) {
-        // UE has been removed.
-        continue;
-      }
-
-      // - The lower layers will not attempt to decode the PUSCH and will not send any CRC indication.
-      ul_harq_process& h_ul = u->get_pcell().harqs.ul_harq(grant.pusch_cfg.harq_id);
-      if (not h_ul.empty()) {
-        if (h_ul.tb().nof_retxs == 0) {
-          // Given that the PUSCH grant was discarded before it reached the PHY, the "new_data" flag was not handled
-          // and the UL softbuffer was not reset. To avoid mixing different TBs in the softbuffer, it is important to
-          // reset the UL HARQ process.
-          h_ul.reset();
-        } else {
-          // To avoid a long UL HARQ timeout window (due to lack of CRC indication), it is important to force a NACK in
-          // the UL HARQ process.
-          h_ul.crc_info(false);
-        }
-      }
-
-      // - The lower layers will not attempt to decode any UCI in the PUSCH and will not send any UCI indication.
-      if (grant.uci.has_value() and grant.uci->harq.has_value() and grant.uci->harq->harq_ack_nof_bits > 0) {
-        // To avoid a long DL HARQ timeout window (due to lack of UCI indication), it is important to NACK the
-        // DL HARQ processes with UCI falling in this slot.
-        u->get_pcell().harqs.dl_ack_info_cancelled(sl_tx);
-      }
-    }
-    for (const auto& pucch : prev_slot_result->result.ul.pucchs) {
-      ue* u = ue_db.find_by_rnti(pucch.crnti);
-      if (u == nullptr) {
-        // UE has been removed.
-        continue;
-      }
-      bool has_harq_ack = false;
-      switch (pucch.format) {
-        case pucch_format::FORMAT_1:
-          has_harq_ack = pucch.format_1.harq_ack_nof_bits > 0;
-          break;
-        case pucch_format::FORMAT_2:
-          has_harq_ack = pucch.format_2.harq_ack_nof_bits > 0;
-          break;
-        default:
-          break;
-      }
-
-      // - The lower layers will not attempt to decode the PUCCH and will not send any UCI indication.
-      if (has_harq_ack) {
-        // To avoid a long DL HARQ timeout window (due to lack of UCI indication), it is important to force a NACK in
-        // the DL HARQ processes with UCI falling in this slot.
-        u->get_pcell().harqs.dl_ack_info_cancelled(sl_tx);
-      }
-    }
-  }
-
-  if (event.pdsch_discarded) {
-    for (const dl_msg_alloc& grant : prev_slot_result->result.dl.ue_grants) {
-      // Given that the PDSCH grant was discarded before it reached the PHY, the "new_data" flag was not handled
-      // and the DL buffer was not reset. To avoid mixing different TBs in the buffer, it is important to
-      // reset the DL HARQ process.
-      ue* u = ue_db.find_by_rnti(grant.pdsch_cfg.rnti);
-      if (u == nullptr) {
-        // UE has been removed.
-        continue;
-      }
-      ue_cell* ucell = u->find_cell(cell_index);
-      if (ucell == nullptr) {
-        // UE cell does not exist.
-        continue;
-      }
-      dl_harq_process& h_dl = ucell->harqs.dl_harq(grant.pdsch_cfg.harq_id);
-      for (unsigned cw = 0; cw != grant.pdsch_cfg.codewords.size(); ++cw) {
-        if (h_dl.is_waiting_ack(cw)) {
-          h_dl.cancel_harq_retxs(cw);
-        }
-      }
-    }
-  }
-}

lib/scheduler/ue_scheduling/ue_scheduler_impl.h

@@ -45,7 +45,10 @@ class ue_scheduler_impl final : public ue_scheduler
 
   void handle_error_indication(slot_point                            sl_tx,
                                du_cell_index_t                       cell_index,
-                               scheduler_slot_handler::error_outcome event) override;
+                               scheduler_slot_handler::error_outcome event) override
+  {
+    event_mng.handle_error_indication(sl_tx, cell_index, event);
+  }
 
   sched_ue_configuration_handler& get_ue_configurator() override { return event_mng; }