lte,gtpu: fix and refactor gtpu tunnel management during handover

srsenb/hdr/stack/upper/gtpu.h

@@ -50,7 +50,7 @@ public:
   // A UE should have <= 3 DRBs active, and each DRB should have two tunnels active at the same time at most
   const static size_t MAX_TUNNELS_PER_UE = 6;
 
-  enum class tunnel_state { pdcp_active, buffering, forward_to, forwarded_from };
+  enum class tunnel_state { pdcp_active, buffering, forward_to, forwarded_from, inactive };
 
   struct tunnel {
     uint16_t rnti      = SRSRAN_INVALID_RNTI;
@@ -101,13 +101,13 @@ public:
 
   void activate_tunnel(uint32_t teid);
   void suspend_tunnel(uint32_t teid);
+  void deactivate_tunnel(uint32_t teid);
   void set_tunnel_priority(uint32_t first_teid, uint32_t second_teid);
   void handle_rx_pdcp_sdu(uint32_t teid);
   void buffer_pdcp_sdu(uint32_t teid, uint32_t pdcp_sn, srsran::unique_byte_buffer_t sdu);
   void setup_forwarding(uint32_t rx_teid, uint32_t tx_teid);
 
   bool remove_tunnel(uint32_t teid);
-  bool remove_bearer(uint16_t rnti, uint32_t lcid);
   bool remove_rnti(uint16_t rnti);
 
 private:

srsenb/src/stack/upper/gtpu.cc

@@ -165,6 +165,9 @@ bool gtpu_tunnel_manager::remove_tunnel(uint32_t teidin)
 {
   tunnel& tun = tunnels[teidin];
 
+  // update forwarding paths if required
+  deactivate_tunnel(teidin);
+
   // erase keeping the relative order
   auto& ue      = ue_teidin_db[tun.rnti];
   auto  lcid_it = std::lower_bound(ue.begin(), ue.end(), lcid_tunnel{tun.lcid, tun.teid_in});
@@ -176,28 +179,10 @@ bool gtpu_tunnel_manager::remove_tunnel(uint32_t teidin)
   return true;
 }
 
-bool gtpu_tunnel_manager::remove_bearer(uint16_t rnti, uint32_t lcid)
-{
-  logger.info("Removing rnti=0x%x,lcid=%d", rnti, lcid);
-  bool removed = false;
-  for (srsran::span<lcid_tunnel> to_rem = find_rnti_lcid_tunnels(rnti, lcid); not to_rem.empty();
-       to_rem                           = find_rnti_lcid_tunnels(rnti, lcid)) {
-    uint32_t teid = to_rem.front().teid;
-    bool     ret  = remove_tunnel(teid);
-    srsran_expect(ret,
-                  "Inconsistency detected between internal data structures for rnti=0x%x,lcid=%d," TEID_IN_FMT,
-                  rnti,
-                  lcid,
-                  teid);
-    removed |= ret;
-  }
-  return removed;
-}
-
 bool gtpu_tunnel_manager::remove_rnti(uint16_t rnti)
 {
   if (not ue_teidin_db.contains(rnti)) {
-    logger.warning("removing rnti. rnti=0x%x not found.", rnti);
+    logger.warning("Removing rnti. rnti=0x%x not found.", rnti);
     return false;
   }
   logger.info("Removing rnti=0x%x", rnti);
@@ -249,10 +234,28 @@ void gtpu_tunnel_manager::suspend_tunnel(uint32_t teid)
   tun.state = tunnel_state::buffering;
 }
 
+void gtpu_tunnel_manager::deactivate_tunnel(uint32_t teid)
+{
+  tunnel& tun = tunnels[teid];
+
+  if (tun.state == tunnel_state::forwarded_from) {
+    // Deactivate respective MME->SeNB forwarding tunnel
+    for (auto lcid_tun : find_rnti_lcid_tunnels(tun.rnti, tun.lcid)) {
+      if (lcid_tun.teid != tun.teid_in) {
+        const gtpu_tunnel_manager::tunnel* mmeenb_tun = find_tunnel(lcid_tun.teid);
+        if (mmeenb_tun->state == gtpu_tunnel_manager::tunnel_state::forward_to and mmeenb_tun->fwd_tunnel == &tun) {
+          deactivate_tunnel(mmeenb_tun->teid_in);
+        }
+      }
+    }
+  }
+
+  tun.state = tunnel_state::inactive;
+}
+
 void gtpu_tunnel_manager::set_tunnel_priority(uint32_t before_teid, uint32_t after_teid)
 {
   tunnel& before_tun = tunnels[before_teid];
-  tunnel& after_tun  = tunnels[after_teid];
 
   // GTPU should not forward SDUs from main tunnel until the SeNB-TeNB tunnel has been flushed
   suspend_tunnel(after_teid);
@@ -518,11 +521,15 @@ void gtpu::set_tunnel_status(uint32_t teidin, bool dl_active)
 
 void gtpu::rem_bearer(uint16_t rnti, uint32_t lcid)
 {
-  if (tunnels.find_rnti_lcid_tunnels(rnti, lcid).empty()) {
+  srsran::span<gtpu_tunnel_manager::lcid_tunnel> lcid_tuns = tunnels.find_rnti_lcid_tunnels(rnti, lcid);
-    logger.error("Removing non-existent bearer rnti=0x%x,lcid=%d", rnti, lcid);
+  if (lcid_tuns.empty()) {
+    logger.info("Removing bearer rnti=0x%x,lcid=%d without any active tunnels", rnti, lcid);
     return;
   }
-  tunnels.remove_bearer(rnti, lcid);
+  do {
+    rem_tunnel(lcid_tuns.front().teid);
+    lcid_tuns = tunnels.find_rnti_lcid_tunnels(rnti, lcid);
+  } while (not lcid_tuns.empty());
 }
 
 void gtpu::mod_bearer_rnti(uint16_t old_rnti, uint16_t new_rnti)
@@ -532,27 +539,33 @@ void gtpu::mod_bearer_rnti(uint16_t old_rnti, uint16_t new_rnti)
 
 void gtpu::rem_tunnel(uint32_t teidin)
 {
-  if (not tunnels.has_teid(teidin)) {
+  const gtpu_tunnel_manager::tunnel* tun = tunnels.find_tunnel(teidin);
+  if (tun == nullptr) {
     logger.warning("Removing tunnel - " TEID_IN_FMT " does not exist", teidin);
     return;
   }
+  if (tun->state == gtpu_tunnel_manager::tunnel_state::forwarded_from) {
+    // TS 36.300, Sec 10.1.2.2.1 - Path Switch upon handover
+    // END MARKER should be forwarded to TeNB if forwarding is activated
+    send_end_marker(tun->teid_in);
+  } else if (tun->state == gtpu_tunnel_manager::tunnel_state::forward_to) {
+    // Delete respective forwarding SeNB-TeNB tunnel
+    send_end_marker(tun->fwd_tunnel->teid_in);
+    rem_tunnel(tun->fwd_tunnel->teid_in);
+  }
   tunnels.remove_tunnel(teidin);
 }
 
 void gtpu::rem_user(uint16_t rnti)
 {
-  const auto* tun_lst = tunnels.find_rnti_tunnels(rnti);
+  const gtpu_tunnel_manager::ue_lcid_tunnel_list* tun_lst = tunnels.find_rnti_tunnels(rnti);
   if (tun_lst == nullptr) {
     logger.info("Removing user - rnti=0x%x not found.", rnti);
     return;
   }
-  for (gtpu_tunnel_manager::lcid_tunnel tun_elem : *tun_lst) {
+  while (not tun_lst->empty()) {
-    const gtpu_tunnel* tun = tunnels.find_tunnel(tun_elem.teid);
+    // Note: May send End-Marker to active forwarding tunnels during their removal
-    if (tun != nullptr and tun->state == gtpu_tunnel_manager::tunnel_state::forwarded_from) {
+    rem_tunnel(tun_lst->front().teid);
-      // In case of forwarding tunnel tx endpoint, send one extra End Marker on removal
-      send_end_marker(tun->teid_in);
-      rem_tunnel(tun->teid_in);
-    }
   }
   tunnels.remove_rnti(rnti);
 }
@@ -560,7 +573,7 @@ void gtpu::rem_user(uint16_t rnti)
 void gtpu::handle_end_marker(const gtpu_tunnel& rx_tunnel)
 {
   uint16_t rnti = rx_tunnel.rnti;
-  logger.info("Received GTPU End Marker for " TEID_IN_FMT ", rnti=0x%x.", rx_tunnel.teid_in, rnti);
+  logger.info("Rx GTPU End Marker, " TEID_IN_FMT ", rnti=0x%x.", rx_tunnel.teid_in, rnti);
 
   if (rx_tunnel.state == gtpu_tunnel_state::forward_to) {
     // TS 36.300, Sec 10.1.2.2.1 - Path Switch upon handover
@@ -765,12 +778,12 @@ void gtpu::echo_response(in_addr_t addr, in_port_t port, uint16_t seq)
  ***************************************************************************/
 bool gtpu::send_end_marker(uint32_t teidin)
 {
-  logger.info("TX GTPU End Marker.");
   const gtpu_tunnel* tx_tun = tunnels.find_tunnel(teidin);
   if (tx_tun == nullptr) {
     logger.error("TEID=%d not found to send the end marker to", teidin);
     return false;
   }
+  logger.info("Tx GTPU End Marker, " TEID_IN_FMT ", rnti=0x%x", teidin, tx_tun->rnti);
 
   gtpu_header_t        header = {};
   unique_byte_buffer_t pdu    = make_byte_buffer();
@@ -792,7 +805,12 @@ bool gtpu::send_end_marker(uint32_t teidin)
   servaddr.sin_addr.s_addr    = htonl(tx_tun->spgw_addr);
   servaddr.sin_port           = htons(GTPU_PORT);
 
-  return sendto(fd, pdu->msg, pdu->N_bytes, MSG_EOR, (struct sockaddr*)&servaddr, sizeof(struct sockaddr_in)) > 0;
+  bool success =
+      sendto(fd, pdu->msg, pdu->N_bytes, MSG_EOR, (struct sockaddr*)&servaddr, sizeof(struct sockaddr_in)) > 0;
+  if (success) {
+    tunnels.deactivate_tunnel(tx_tun->teid_in);
+  }
+  return success;
 }
 
 /****************************************************************************

srsenb/test/upper/gtpu_test.cc

@@ -358,6 +358,13 @@ int test_gtpu_direct_tunneling(tunnel_test_event event)
   }
   srsran::span<uint8_t> encoded_data2{tenb_pdcp.last_sdu->msg + 20u, tenb_pdcp.last_sdu->msg + 30u};
   TESTASSERT(std::all_of(encoded_data2.begin(), encoded_data2.end(), [N_pdus](uint8_t b) { return b == N_pdus - 1; }));
+  if (event != tunnel_test_event::ue_removal_no_marker) {
+    // The User is removed in SeNB in case it hasn't and there was no reestablishment
+    if (std::uniform_int_distribution<uint32_t>{0, 1}(g) > 0) {
+      senb_gtpu.rem_bearer(0x46, 3);
+    }
+    senb_gtpu.rem_user(0x46);
+  }
 
   return SRSRAN_SUCCESS;
 }