From 2a54caa67131f50fb3435816a0b0a5e47fe46484 Mon Sep 17 00:00:00 2001
From: Kamil Cudnik <kcudnik@gmail.com>
Date: Thu, 21 Nov 2024 15:06:55 +0100
Subject: [PATCH] [meta] Use metadata to check port notification port_id type
 (#1469)

Use metadata for validation instead of hard coded values
---
 meta/Meta.cpp              | 67 +++++++++++++++++++++++++++++---------
 meta/Meta.h                |  5 +++
 unittest/meta/TestMeta.cpp | 24 ++++++++++++++
 3 files changed, 80 insertions(+), 16 deletions(-)

diff --git a/meta/Meta.cpp b/meta/Meta.cpp
index 92ba95f7c..d632eadea 100644
--- a/meta/Meta.cpp
+++ b/meta/Meta.cpp
@@ -8,6 +8,8 @@
 
 #include <inttypes.h>
 
+#include <boost/algorithm/string/join.hpp>
+
 #include <set>
 
 // TODO add validation for all oids belong to the same switch
@@ -6694,24 +6696,14 @@ void Meta::meta_sai_on_port_state_change_single(
 
     auto ot = objectTypeQuery(data.port_id);
 
-    bool valid = false;
+    bool valid = isPortObjectIdValid(ot);
 
-    switch (ot)
+    if (!valid)
     {
-        // TODO hardcoded types, must advance SAI repository commit to get metadata for this
-        case SAI_OBJECT_TYPE_PORT:
-        case SAI_OBJECT_TYPE_BRIDGE_PORT:
-        case SAI_OBJECT_TYPE_LAG:
-
-            valid = true;
-            break;
-
-        default:
-
-            SWSS_LOG_ERROR("data.port_id %s has unexpected type: %s, expected PORT, BRIDGE_PORT or LAG",
-                    sai_serialize_object_id(data.port_id).c_str(),
-                    sai_serialize_object_type(ot).c_str());
-            break;
+        SWSS_LOG_ERROR("data.port_id %s has unexpected type: %s, expected: %s",
+                sai_serialize_object_id(data.port_id).c_str(),
+                sai_serialize_object_type(ot).c_str(),
+                boost::algorithm::join(getValidPortObjectTypes(), ",").c_str());
     }
 
     if (valid && !m_oids.objectReferenceExists(data.port_id))
@@ -7131,3 +7123,46 @@ void Meta::populate(
         }
     }
 }
+
+bool Meta::isPortObjectIdValid(
+        _In_ sai_object_type_t object_type)
+{
+    SWSS_LOG_ENTER();
+
+    auto members = sai_metadata_struct_members_sai_port_oper_status_notification_t;
+
+    for (size_t i = 0; members[i]; i++)
+    {
+        auto* mb = members[i];
+
+        if (mb->membername != std::string("port_id"))
+            continue;
+
+        for (size_t idx = 0; idx < mb->allowedobjecttypeslength; idx++)
+        {
+            if (mb->allowedobjecttypes[idx] == object_type)
+                return true;
+        }
+
+        return false;
+    }
+
+    SWSS_LOG_THROW("port_id member not found on sai_port_oper_status_notification");
+}
+
+std::vector<std::string> Meta::getValidPortObjectTypes()
+{
+    SWSS_LOG_ENTER();
+
+    auto md = sai_metadata_enum_sai_object_type_t;
+
+    std::vector<std::string> v;
+
+    for (size_t i = 0; i < md.valuescount; i++)
+    {
+        if (isPortObjectIdValid((sai_object_type_t)md.values[i]))
+            v.push_back(md.valuesshortnames[i]);
+    }
+
+    return v;
+}
diff --git a/meta/Meta.h b/meta/Meta.h
index c93ae7a59..aac696a68 100644
--- a/meta/Meta.h
+++ b/meta/Meta.h
@@ -315,6 +315,11 @@ namespace saimeta
             static bool is_ipv6_mask_valid(
                     _In_ const uint8_t* mask);
 
+            static bool isPortObjectIdValid(
+                    _In_ sai_object_type_t object_type);
+
+            static std::vector<std::string> getValidPortObjectTypes();
+
         private: // unit tests helpers
 
             bool meta_unittests_get_and_erase_set_readonly_flag(
diff --git a/unittest/meta/TestMeta.cpp b/unittest/meta/TestMeta.cpp
index d31ed4a4d..454f9ff85 100644
--- a/unittest/meta/TestMeta.cpp
+++ b/unittest/meta/TestMeta.cpp
@@ -7,6 +7,8 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/algorithm/string/join.hpp>
+
 #include <memory>
 
 #define VLAN_ID 2
@@ -1850,3 +1852,25 @@ TEST(Meta, remove_prefix_compression_entry)
 
     EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, sai.remove(e));
 }
+
+TEST(Meta, isPortObjectIdValid)
+{
+    EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_PORT), true);
+    EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_BRIDGE_PORT), true);
+    EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_LAG), true);
+
+    EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_TUNNEL),false);
+    EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_NULL), false);
+    EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_VLAN), false);
+}
+
+TEST(Meta, getValidPortObjectTypes)
+{
+    auto v = Meta::getValidPortObjectTypes();
+
+    EXPECT_EQ(v.size(), 3);
+
+    auto s = boost::algorithm::join(v, ",");
+
+    EXPECT_EQ(s, "PORT,LAG,BRIDGE_PORT");
+}