ConnTrackers: Set conn_id earlier for CONN_TRACE consistency

Summary: ConnID should be set as soon as the ConnTracker is created to avoid CONN_TRACEs with upid=0.

Test Plan: Manually checked the CONN_TRACE logs.

Reviewers: #stirling, yzhao

Reviewed By: #stirling, yzhao

Subscribers: yzhao
Signed-off-by: Omid Azizi <oazizi@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D10842

GitOrigin-RevId: d11ace03d65615ddd91d5d3827546d5f3db46717

src/stirling/source_connectors/socket_tracer/conn_tracker.h

@@ -355,13 +355,13 @@ class ConnTracker : NotCopyMoveable {
   DataStream* resp_data();
 
   /**
-   * @return Returns the latest timestamp of all BPF events received by this tracker (using BPF
+   * Returns the latest timestamp of all BPF events received by this tracker (using BPF
    * timestamp).
    */
   uint64_t last_bpf_timestamp_ns() { return last_bpf_timestamp_ns_; }
 
   /**
-   * @return Returns the a timestamp the last time an event was added to this tracker (using
+   * Returns the a timestamp the last time an event was added to this tracker (using
    * steady_clock).
    */
   std::chrono::time_point<std::chrono::steady_clock> last_update_timestamp() {

src/stirling/source_connectors/socket_tracer/conn_trackers_manager.cc

@@ -29,24 +29,22 @@ namespace stirling {
 // ConnTrackerGenerations
 //-----------------------------------------------------------------------------
 
-std::pair<ConnTracker*, bool> ConnTrackerGenerations::GetOrCreate(uint64_t tsid,
+std::pair<ConnTracker*, bool> ConnTrackerGenerations::GetOrCreate(conn_id_t conn_id,
                                                                   ConnTrackerPool* tracker_pool) {
-  std::unique_ptr<ConnTracker>& conn_tracker_ptr = generations_[tsid];
+  std::unique_ptr<ConnTracker>& conn_tracker_ptr = generations_[conn_id.tsid];
   bool created = false;
 
-  // If conn_trackers[conn_id.tsid] does not exist, a new one will be created in the map.
-  // New trackers always have TSID == 0, while BPF should never generate a TSID of zero,
-  // so we use this as a way of detecting new trackers.
   if (conn_tracker_ptr == nullptr) {
     conn_tracker_ptr = tracker_pool->Pop();
     created = true;
+    conn_tracker_ptr->SetConnID(conn_id);
 
     // If there is a another generation for this conn map key,
     // one of them needs to be marked for death.
     if (oldest_generation_ != nullptr) {
       // If the inserted conn_tracker is not the last generation, then mark it for death.
       // This can happen because the events draining from the perf buffers are not ordered.
-      if (tsid < oldest_generation_->conn_id().tsid) {
+      if (conn_id.tsid < oldest_generation_->conn_id().tsid) {
         VLOG(1) << "Marking for death because not last generation.";
         conn_tracker_ptr->MarkForDeath();
       } else {
@@ -118,12 +116,11 @@ ConnTracker& ConnTrackersManager::GetOrCreateConnTracker(struct conn_id_t conn_i
   DCHECK_NE(conn_map_key, 0) << "Connection map key cannot be 0, pid must be wrong";
 
   ConnTrackerGenerations& conn_trackers = conn_id_tracker_generations_[conn_map_key];
-  auto [conn_tracker_ptr, created] = conn_trackers.GetOrCreate(conn_id.tsid, &trackers_pool_);
+  auto [conn_tracker_ptr, created] = conn_trackers.GetOrCreate(conn_id, &trackers_pool_);
 
   if (created) {
     active_trackers_.push_back(conn_tracker_ptr);
     conn_tracker_ptr->manager_ = this;
-    conn_tracker_ptr->SetConnID(conn_id);
 
     stats_.Increment(StatKey::kTotal);
     stats_.Increment(StatKey::kCreated);

src/stirling/source_connectors/socket_tracer/conn_trackers_manager.h

@@ -50,7 +50,7 @@ class ConnTrackerGenerations {
    *
    * @return The pointer to the conn_tracker and whether the tracker was newly created.
    */
-  std::pair<ConnTracker*, bool> GetOrCreate(uint64_t tsid, ConnTrackerPool* tracker_pool);
+  std::pair<ConnTracker*, bool> GetOrCreate(conn_id_t conn_id, ConnTrackerPool* tracker_pool);
 
   /**
    * Return true if a ConnTracker created at the specified time stamp exists.

src/stirling/source_connectors/socket_tracer/conn_trackers_manager_test.cc

@@ -122,13 +122,13 @@ class ConnTrackerGenerationsTest : public ::testing::Test {
   }
 
   std::pair<ConnTracker*, bool> GetOrCreateTracker(uint64_t tsid) {
-    auto [tracker, created] = tracker_gens_.GetOrCreate(tsid, &tracker_pool);
-    if (created) {
-      struct conn_id_t conn_id = {};
-      conn_id.tsid = tsid;
-      tracker->SetConnID(conn_id);
-    }
-    return {tracker, created};
+    struct conn_id_t conn_id = {};
+    conn_id.upid.pid = 1;
+    conn_id.upid.start_time_ticks = 1;
+    conn_id.fd = 1;
+    conn_id.tsid = tsid;
+
+    return tracker_gens_.GetOrCreate(conn_id, &tracker_pool);
   }
 
   int CleanupTrackers() {