Merge pull request #36351 from azat/netlink-strict

Strict taskstats parser

programs/server/Server.cpp

@@ -1507,7 +1507,8 @@ int Server::main(const std::vector<std::string> & /*args*/)
     }
 
 #if defined(OS_LINUX)
-    if (!TasksStatsCounters::checkIfAvailable())
+    auto tasks_stats_provider = TasksStatsCounters::findBestAvailableProvider();
+    if (tasks_stats_provider == TasksStatsCounters::MetricsProvider::None)
     {
         LOG_INFO(log, "It looks like this system does not have procfs mounted at /proc location,"
             " neither clickhouse-server process has CAP_NET_ADMIN capability."
@@ -1518,6 +1519,10 @@ int Server::main(const std::vector<std::string> & /*args*/)
             " It also doesn't work if you run clickhouse-server inside network namespace as it happens in some containers.",
             executable_path);
     }
+    else
+    {
+        LOG_INFO(log, "Tasks stats provider: {}", TasksStatsCounters::metricsProviderString(tasks_stats_provider));
+    }
 
     if (!hasLinuxCapability(CAP_SYS_NICE))
     {

src/Common/TaskStatsInfoGetter.cpp

@@ -265,26 +265,24 @@ void TaskStatsInfoGetter::getStat(::taskstats & out_stats, pid_t tid) const
 {
     NetlinkMessage answer = query(netlink_socket_fd, taskstats_family_id, tid, TASKSTATS_CMD_GET, TASKSTATS_CMD_ATTR_PID, &tid, sizeof(tid));
 
-    for (const NetlinkMessage::Attribute * attr = &answer.payload.attribute;
-        attr < answer.end();
-        attr = attr->next())
-    {
-        if (attr->header.nla_type == TASKSTATS_TYPE_AGGR_TGID || attr->header.nla_type == TASKSTATS_TYPE_AGGR_PID)
-        {
-            for (const NetlinkMessage::Attribute * nested_attr = reinterpret_cast<const NetlinkMessage::Attribute *>(attr->payload);
-                nested_attr < attr->next();
-                nested_attr = nested_attr->next())
-            {
-                if (nested_attr->header.nla_type == TASKSTATS_TYPE_STATS)
-                {
-                    out_stats = unalignedLoad<::taskstats>(nested_attr->payload);
-                    return;
-                }
-            }
-        }
-    }
+    const NetlinkMessage::Attribute * attr = &answer.payload.attribute;
+    if (attr->header.nla_type != TASKSTATS_TYPE_AGGR_PID)
+        throw Exception("Expected TASKSTATS_TYPE_AGGR_PID", ErrorCodes::NETLINK_ERROR);
 
-    throw Exception("There is no TASKSTATS_TYPE_STATS attribute in the Netlink response", ErrorCodes::NETLINK_ERROR);
+    /// TASKSTATS_TYPE_AGGR_PID
+    const NetlinkMessage::Attribute * nested_attr = reinterpret_cast<const NetlinkMessage::Attribute *>(attr->payload);
+    if (nested_attr->header.nla_type != TASKSTATS_TYPE_PID)
+        throw Exception("Expected TASKSTATS_TYPE_PID", ErrorCodes::NETLINK_ERROR);
+    if (nested_attr == nested_attr->next())
+        throw Exception("No TASKSTATS_TYPE_STATS packet after TASKSTATS_TYPE_PID", ErrorCodes::NETLINK_ERROR);
+    nested_attr = nested_attr->next();
+    if (nested_attr->header.nla_type != TASKSTATS_TYPE_STATS)
+        throw Exception("Expected TASKSTATS_TYPE_STATS", ErrorCodes::NETLINK_ERROR);
+
+    out_stats = unalignedLoad<::taskstats>(nested_attr->payload);
+
+    if (attr->next() != answer.end())
+        throw Exception("Unexpected end of response", ErrorCodes::NETLINK_ERROR);
 }
 
 

src/Common/ThreadProfileEvents.cpp

@@ -67,6 +67,20 @@ namespace ProfileEvents
 namespace DB
 {
 
+const char * TasksStatsCounters::metricsProviderString(MetricsProvider provider)
+{
+    switch (provider)
+    {
+        case MetricsProvider::None:
+            return "none";
+        case MetricsProvider::Procfs:
+            return "procfs";
+        case MetricsProvider::Netlink:
+            return "netlink";
+    }
+    __builtin_unreachable();
+}
+
 bool TasksStatsCounters::checkIfAvailable()
 {
     return findBestAvailableProvider() != MetricsProvider::None;

src/Common/ThreadProfileEvents.h

@@ -176,7 +176,17 @@ extern PerfEventsCounters current_thread_counters;
 class TasksStatsCounters
 {
 public:
+    enum class MetricsProvider
+    {
+        None,
+        Procfs,
+        Netlink,
+    };
+
+    static const char * metricsProviderString(MetricsProvider provider);
     static bool checkIfAvailable();
+    static MetricsProvider findBestAvailableProvider();
+
     static std::unique_ptr<TasksStatsCounters> create(UInt64 tid);
 
     void reset();
@@ -186,16 +196,8 @@ private:
     ::taskstats stats;  //-V730_NOINIT
     std::function<::taskstats()> stats_getter;
 
-    enum class MetricsProvider
-    {
-        None,
-        Procfs,
-        Netlink
-    };
-
     explicit TasksStatsCounters(UInt64 tid, MetricsProvider provider);
 
-    static MetricsProvider findBestAvailableProvider();
     static void incrementProfileEvents(const ::taskstats & prev, const ::taskstats & curr, ProfileEvents::Counters & profile_events);
 };