Merge "[HICN-167] fixes on timeouts and nacks"

3 files changed, 44 insertions, 24 deletions

diff --git a/libtransport/src/hicn/transport/interfaces/rtc_socket_producer.cc b/libtransport/src/hicn/transport/interfaces/rtc_socket_producer.cc
index 67fcc83e3..57ac081dc 100644
--- a/libtransport/src/hicn/transport/interfaces/rtc_socket_producer.cc
+++ b/libtransport/src/hicn/transport/interfaces/rtc_socket_producer.cc
@@ -145,7 +145,7 @@ void RTCProducerSocket::produce(const uint8_t *buf, size_t buffer_size) {
   payload->append(buffer_size + TIMESTAMP_LEN);
   content_object.appendPayload(std::move(payload));
 
-  content_object.setLifetime(1000);  // XXX this should be set by the APP
+  content_object.setLifetime(500);  // XXX this should be set by the APP
 
   content_object.setPathLabel(prodLabel_);
   portal_->sendContentObject(content_object);
diff --git a/libtransport/src/hicn/transport/protocols/rtc.cc b/libtransport/src/hicn/transport/protocols/rtc.cc
index 98abbe35b..e3dcbbdbd 100644
--- a/libtransport/src/hicn/transport/protocols/rtc.cc
+++ b/libtransport/src/hicn/transport/protocols/rtc.cc
@@ -105,6 +105,7 @@ void RTCTransportProtocol::reset() {
   actualSegment_ = 0;
   inflightInterestsCount_ = 0;
   while (interestRetransmissions_.size() != 0) interestRetransmissions_.pop();
+  lastSegNacked_ = 0;
   nackedByProducer_.clear();
   nackedByProducerMaxSize_ = 512;
 
@@ -433,8 +434,8 @@ void RTCTransportProtocol::scheduleAppNackRtx(std::vector<uint32_t> &nacks) {
       continue;
     }
     // packetLost_++;
-    // XXX here I need to avoid the retrasmission for packet that were nacked by
-    // the network
+    // XXX here I need to avoid the retrasmission for packet that were
+    // nacked by the network
     interestRetransmissions_.push(nacks[i]);
   }
 
@@ -450,7 +451,8 @@ void RTCTransportProtocol::onTimeout(Interest::Ptr &&interest) {
     inflightInterestsCount_--;
   }
 
-  if (inflightInterests_[pkt].retransmissions < HICN_MAX_RTX) {
+  if (inflightInterests_[pkt].retransmissions < HICN_MAX_RTX &&
+          lastSegNacked_ <= segmentNumber) {
     interestRetransmissions_.push(segmentNumber);
   }
 
@@ -466,7 +468,9 @@ bool RTCTransportProtocol::checkIfProducerIsActive(
   if (productionRate == 0) {
     // the producer socket is not active
     // in this case we consider only the first nack
-    if (nack_timer_used_) return false;
+    if (nack_timer_used_) {
+        return false;
+    }
 
     nack_timer_used_ = true;
     // actualSegment_ should be the one in the nack, which will the next in
@@ -482,7 +486,6 @@ bool RTCTransportProtocol::checkIfProducerIsActive(
     });
     return false;
   }
-
   return true;
 }
 
@@ -492,6 +495,7 @@ void RTCTransportProtocol::onNack(const ContentObject &content_object) {
   uint32_t productionRate = *(++payload);
   uint32_t nackSegment = content_object.getName().getSuffix();
 
+
   gotNack_ = true;
   // we synch the estimated production rate with the actual one
   estimatedBw_ = (double)productionRate;
@@ -506,25 +510,18 @@ void RTCTransportProtocol::onNack(const ContentObject &content_object) {
     computeMaxWindow(productionRate, 0);
     increaseWindow();
 
+    while (interestRetransmissions_.size() != 0) interestRetransmissions_.pop();
+    lastSegNacked_ = productionSeg;
+
     if (nackedByProducer_.size() >= nackedByProducerMaxSize_)
       nackedByProducer_.erase(nackedByProducer_.begin());
     nackedByProducer_.insert(nackSegment);
 
   } else if (productionSeg < nackSegment) {
-    gotFutureNack_++;
     // we are asking stuff in the future
-    // example
-    // 10    12    13    14    15    16    17
-    //       ^                  ^           ^
-    //       in prod            nack        actual
-    // in this example we sent up to segment 17 and we get a nack for segment 15
-    // this means that we will get nack also for 16 17
-    // and valid data for 13 14
-    // so the next segment to ask is 15, because 13 and 14 will can back anyway
-    // we go back only in the case that the actual segment is really bigger than
-    // nack segment, other we do nothing
-
-    actualSegment_ = min(actualSegment_, nackSegment);
+    gotFutureNack_++;
+
+    actualSegment_ = productionSeg + 1;
 
     computeMaxWindow(productionRate, 0);
     decreaseWindow();
@@ -535,6 +532,24 @@ void RTCTransportProtocol::onNack(const ContentObject &content_object) {
   }  // equal should not happen
 }
 
+void RTCTransportProtocol::onNackForRtx(const ContentObject &content_object) {
+  uint32_t *payload = (uint32_t *)content_object.getPayload()->data();
+  uint32_t productionSeg = *payload;
+  uint32_t nackSegment = content_object.getName().getSuffix();
+
+  if (productionSeg > nackSegment) {
+    // we are asking for stuff produced in the past
+    actualSegment_ = max(productionSeg + 1, actualSegment_);
+
+    while (interestRetransmissions_.size() != 0) interestRetransmissions_.pop();
+    lastSegNacked_ = productionSeg;
+
+  } else if (productionSeg < nackSegment) {
+
+    actualSegment_ = productionSeg + 1;
+  }  // equal should not happen
+}
+
 void RTCTransportProtocol::onContentObject(
     Interest::Ptr &&interest, ContentObject::Ptr &&content_object) {
   auto payload = content_object->getPayload();
@@ -546,16 +561,16 @@ void RTCTransportProtocol::onContentObject(
   if (payload_size == HICN_NACK_HEADER_SIZE) {
     // Nacks always come form the producer, so we set the producerPathLabel_;
     producerPathLabel_ = content_object->getPathLabel();
+    schedule_next_interest = checkIfProducerIsActive(*content_object);
     if (inflightInterests_[pkt].retransmissions == 0) {
       // discard nacks for rtx packets
       inflightInterestsCount_--;
-      schedule_next_interest = checkIfProducerIsActive(*content_object);
-      // if checkIfProducerIsActive returns true, we did all we need to do
+      // if checkIfProducerIsActive returns false, we did all we need to do
       // inside that function, no need to call onNack
-      if (!schedule_next_interest) onNack(*content_object);
+      if (schedule_next_interest) onNack(*content_object);
       updateDelayStats(*content_object);
     } else {
-      schedule_next_interest = checkIfProducerIsActive(*content_object);
+      if (schedule_next_interest) onNackForRtx(*content_object);
     }
 
   } else {
diff --git a/libtransport/src/hicn/transport/protocols/rtc.h b/libtransport/src/hicn/transport/protocols/rtc.h
index 0bb9d9b2e..51a288574 100644
--- a/libtransport/src/hicn/transport/protocols/rtc.h
+++ b/libtransport/src/hicn/transport/protocols/rtc.h
@@ -38,7 +38,7 @@
 // controller constant
 #define HICN_ROUND_LEN \
   200  // ms interval of time on which we take decisions / measurements
-#define HICN_MAX_RTX 128
+#define HICN_MAX_RTX 3
 #define HICN_MIN_RTT_WIN 30  // rounds
 
 // cwin
@@ -126,6 +126,8 @@ class RTCTransportProtocol : public TransportProtocol, public Reassembly {
   // is not active)
   bool checkIfProducerIsActive(const ContentObject &content_object);
   void onNack(const ContentObject &content_object);
+  //funtcion used to handle nacks for retransmitted interests
+  void onNackForRtx(const ContentObject &content_object);
   void onContentObject(Interest::Ptr &&interest,
                        ContentObject::Ptr &&content_object) override;
   void returnContentToApplication(const ContentObject &content_object);
@@ -158,6 +160,9 @@ class RTCTransportProtocol : public TransportProtocol, public Reassembly {
   uint32_t inflightInterestsCount_;
   std::queue<uint32_t> interestRetransmissions_;
   std::vector<sentInterest> inflightInterests_;
+  uint32_t lastSegNacked_; //indicates the last segment id in a past Nack.
+                           //we do not ask for retransmissions for samething
+                           //that is older than this value.
   uint32_t nackedByProducerMaxSize_;
   std::set<uint32_t>
       nackedByProducer_;  // this is used to avoid retransmissions from the