packet_analysis: Avoid shared_ptr copying for analyzer lookups

For deeply encapsulated connections (think AWS traffic mirroring format
like IP,UDP,GENEVE,IP,UDP,VXLAN,ETH,IP,TCP), the Dispatcher::Lookup()
method is fairly visible in profiles when running in bare mode.

This changes the Analyzer::Lookup() and Dispatcher::Lookup() return value
breaking the API in favor of the performance improvement.

Relates to zeek/zeek#3379.

NEWS

@@ -9,6 +9,10 @@ Zeek 6.2.0
 Breaking Changes
 ----------------
 
+- The methods ``Dispatcher::Lookup()`` and ``Analyzer::Lookup()`` in the packet_analysis
+  namespace were changed to return a reference to a std::shared_ptr instead of a copy
+  for performance reasons.
+
 New Functionality
 -----------------
 

src/packet_analysis/Analyzer.cc

@@ -53,23 +53,47 @@ bool Analyzer::IsAnalyzer(const char* name) {
     return packet_mgr->GetComponentName(tag) == name;
 }
 
-AnalyzerPtr Analyzer::Lookup(uint32_t identifier) const { return dispatcher.Lookup(identifier); }
+const AnalyzerPtr& Analyzer::Lookup(uint32_t identifier) const { return dispatcher.Lookup(identifier); }
 
-bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const {
+// Find the next inner analyzer using identifier or via DetectProtocol(),
-    auto inner_analyzer = Lookup(identifier);
+// otherwise return the default analyzer.
-    if ( ! inner_analyzer ) {
+const AnalyzerPtr& Analyzer::FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet,
-        for ( const auto& child : analyzers_to_detect ) {
+                                               uint32_t identifier) const {
-            if ( child->DetectProtocol(len, data, packet) ) {
+    const auto& identifier_based_analyzer = Lookup(identifier);
-                DBG_LOG(DBG_PACKET_ANALYSIS, "Protocol detection in %s succeeded, next layer analyzer is %s",
+    if ( identifier_based_analyzer )
-                        GetAnalyzerName(), child->GetAnalyzerName());
+        return identifier_based_analyzer;
-                inner_analyzer = child;
+
-                break;
+    const auto& detect_based_analyzer = DetectInnerAnalyzer(len, data, packet);
-            }
+    if ( detect_based_analyzer )
+        return detect_based_analyzer;
+
+    return default_analyzer;
+}
+
+// Find the next inner analyzer via DetectProtocol(), otherwise the default analyzer.
+const AnalyzerPtr& Analyzer::FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet) const {
+    const auto& detect_based_analyzer = DetectInnerAnalyzer(len, data, packet);
+    if ( detect_based_analyzer )
+        return detect_based_analyzer;
+
+    return default_analyzer;
+}
+
+// Return an analyzer found via DetectProtocol() for the given data, else nil.
+const AnalyzerPtr& Analyzer::DetectInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet) const {
+    for ( const auto& child : analyzers_to_detect ) {
+        if ( child->DetectProtocol(len, data, packet) ) {
+            DBG_LOG(DBG_PACKET_ANALYSIS, "Protocol detection in %s succeeded, next layer analyzer is %s",
+                    GetAnalyzerName(), child->GetAnalyzerName());
+            return child;
         }
     }
 
-    if ( ! inner_analyzer )
+    return nil;
-        inner_analyzer = default_analyzer;
+}
+
+bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const {
+    const auto& inner_analyzer = FindInnerAnalyzer(len, data, packet, identifier);
 
     if ( ! inner_analyzer ) {
         DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s failed, could not find analyzer for identifier %#x.",
@@ -89,23 +113,12 @@ bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, ui
 
     DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#x.", GetAnalyzerName(),
             identifier);
+
     return inner_analyzer->AnalyzePacket(len, data, packet);
 }
 
 bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet) const {
-    AnalyzerPtr inner_analyzer = nullptr;
+    const auto& inner_analyzer = FindInnerAnalyzer(len, data, packet);
-
-    for ( const auto& child : analyzers_to_detect ) {
-        if ( child->DetectProtocol(len, data, packet) ) {
-            DBG_LOG(DBG_PACKET_ANALYSIS, "Protocol detection in %s succeeded, next layer analyzer is %s",
-                    GetAnalyzerName(), child->GetAnalyzerName());
-            inner_analyzer = child;
-            break;
-        }
-    }
-
-    if ( ! inner_analyzer )
-        inner_analyzer = default_analyzer;
 
     if ( ! inner_analyzer ) {
         DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s stopped, no default analyzer available.", GetAnalyzerName());

src/packet_analysis/Analyzer.h

@@ -9,12 +9,16 @@
 #include "zeek/session/Session.h"
 
 namespace zeek::packet_analysis {
+class Analyzer;
+using AnalyzerPtr = std::shared_ptr<Analyzer>;
 
 /**
  * Main packet analyzer interface.
  */
 class Analyzer {
 public:
+    static inline AnalyzerPtr nil = nullptr;
+
     /**
      * Constructor.
      *
@@ -198,7 +202,7 @@ protected:
      * @return The analyzer registered for the given identifier. Returns a
      * nullptr if no analyzer is registered.
      */
-    AnalyzerPtr Lookup(uint32_t identifier) const;
+    const AnalyzerPtr& Lookup(uint32_t identifier) const;
 
     /**
      * Returns an analyzer based on a script-land definition.
@@ -256,6 +260,11 @@ private:
     void EnqueueAnalyzerViolationInfo(session::Session* session, const char* reason, const char* data, int len,
                                       const zeek::Tag& arg_tag);
 
+    // Internal helpers to find an appropriate next inner analyzer.
+    const AnalyzerPtr& FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const;
+    const AnalyzerPtr& FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet) const;
+    const AnalyzerPtr& DetectInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet) const;
+
     zeek::Tag tag;
     Dispatcher dispatcher;
     AnalyzerPtr default_analyzer = nullptr;
@@ -270,7 +279,4 @@ private:
 
     void Init(const zeek::Tag& tag);
 };
-
-using AnalyzerPtr = std::shared_ptr<Analyzer>;
-
 } // namespace zeek::packet_analysis

src/packet_analysis/Dispatcher.cc

@@ -48,12 +48,12 @@ void Dispatcher::Register(uint32_t identifier, AnalyzerPtr analyzer) {
     table[index] = std::move(analyzer);
 }
 
-AnalyzerPtr Dispatcher::Lookup(uint32_t identifier) const {
+const AnalyzerPtr& Dispatcher::Lookup(uint32_t identifier) const {
     int64_t index = identifier - lowest_identifier;
     if ( index >= 0 && index < static_cast<int64_t>(table.size()) && table[index] != nullptr )
         return table[index];
 
-    return nullptr;
+    return Analyzer::nil;
 }
 
 size_t Dispatcher::Count() const {

src/packet_analysis/Dispatcher.h

@@ -35,7 +35,7 @@ public:
      * @return The analyzer registered for the given identifier. Returns a
      * nullptr if no analyzer is registered.
      */
-    AnalyzerPtr Lookup(uint32_t identifier) const;
+    const AnalyzerPtr& Lookup(uint32_t identifier) const;
 
     /**
      * Returns the number of registered analyzers.