Bug 1231975 - Part 4: Add some logging and simplification in TestNrSocket. r=drno draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 14 Apr 2016 09:24:47 -0500
changeset 357941 d611712e2a7f0a29063ee5765dc66fb7ed9e5b0f
parent 357940 590238bca757ff3e7c13202f5db1971084fe538d
child 357942 fd5d972f042baf0007d09579fe43893c1a787836
push id16896
push userbcampen@mozilla.com
push dateFri, 29 Apr 2016 21:42:02 +0000
reviewersdrno
bugs1231975
milestone49.0a1
Bug 1231975 - Part 4: Add some logging and simplification in TestNrSocket. r=drno MozReview-Commit-ID: ASbgh62xqQU
media/mtransport/test_nr_socket.cpp
--- a/media/mtransport/test_nr_socket.cpp
+++ b/media/mtransport/test_nr_socket.cpp
@@ -381,35 +381,45 @@ int TestNrSocket::recvfrom(void *buf, si
 
   if (readable_socket_) {
     // If any of the external sockets got data, see if it will be passed through
     r = readable_socket_->recvfrom(buf, maxlen, len, 0, from);
     readable_socket_ = nullptr;
     if (!r) {
       PortMapping *port_mapping_used;
       ingress_allowed = allow_ingress(*from, &port_mapping_used);
-      if (ingress_allowed && nat_->refresh_on_ingress_ && port_mapping_used) {
-        port_mapping_used->last_used_ = PR_IntervalNow();
+      if (ingress_allowed) {
+        r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s received from %s via %s",
+              internal_socket_->my_addr().as_string,
+              from->as_string,
+              port_mapping_used->external_socket_->my_addr().as_string);
+        if (nat_->refresh_on_ingress_) {
+          port_mapping_used->last_used_ = PR_IntervalNow();
+        }
       }
     }
   } else {
     // If no external socket has data, see if there's any data that was sent
     // directly to the TestNrSocket, and eat it if it isn't supposed to get
     // through.
     r = internal_socket_->recvfrom(buf, maxlen, len, flags, from);
     if (!r) {
       // We do not use allow_ingress() here because that only handles traffic
       // landing on an external port.
       ingress_allowed = (!nat_->enabled_ ||
                          nat_->is_an_internal_tuple(*from));
       if (!ingress_allowed) {
         r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
-                                     "Not behind the same NAT",
-                                     internal_socket_->my_addr().as_string,
-                                     from->as_string);
+              "Not behind the same NAT",
+              internal_socket_->my_addr().as_string,
+              from->as_string);
+      } else {
+        r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s received from %s",
+              internal_socket_->my_addr().as_string,
+              from->as_string);
       }
     }
   }
 
   // Kinda lame that we are forced to give the app a readable callback and then
   // say "Oh, never mind...", but the alternative is to totally decouple the
   // callbacks from STS and the callbacks the app sets. On the bright side, this
   // speeds up unit tests where we are verifying that ingress is forbidden,
@@ -420,22 +430,19 @@ int TestNrSocket::recvfrom(void *buf, si
     r = R_WOULDBLOCK;
   }
 
   return r;
 }
 
 bool TestNrSocket::allow_ingress(const nr_transport_addr &from,
                                  PortMapping **port_mapping_used) const {
-  *port_mapping_used = nullptr;
-  if (!nat_->enabled_)
-    return true;
-
-  if (nat_->is_an_internal_tuple(from))
-    return true;
+  // This is only called for traffic arriving at a port mapping
+  MOZ_ASSERT(nat_->enabled_);
+  MOZ_ASSERT(!nat_->is_an_internal_tuple(from));
 
   *port_mapping_used = get_port_mapping(from, nat_->filtering_type_);
   if (!(*port_mapping_used)) {
     r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
                                  "Filtered",
                                  internal_socket_->my_addr().as_string,
                                  from.as_string);
     return false;