[syncd] Comparison logic add support to LABEL attribute with higher p…

…riority (sonic-net#764) * [syncd] Add best candidate finder based on attr label * [meta] Support chardata attr value on set validation * [tests] Add unittests for lag label attribute comparison logic * [tests] Add no lag label to lag label unittests * [tests] Export color function
Metaswitch · Jan 22, 2021 · 667c33d · 667c33d
1 parent aaf5b98
commit 667c33d
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 3 deletions.
diff --git a/meta/Meta.cpp b/meta/Meta.cpp
@@ -4963,7 +4963,6 @@ sai_status_t Meta::meta_generic_validation_set(
     switch (md.attrvaluetype)
     {
         case SAI_ATTR_VALUE_TYPE_BOOL:
-            // case SAI_ATTR_VALUE_TYPE_CHARDATA:
         case SAI_ATTR_VALUE_TYPE_UINT8:
         case SAI_ATTR_VALUE_TYPE_INT8:
         case SAI_ATTR_VALUE_TYPE_UINT16:
@@ -4979,6 +4978,28 @@ sai_status_t Meta::meta_generic_validation_set(
             // primitives
             break;
 
+        case SAI_ATTR_VALUE_TYPE_CHARDATA:
+
+            {
+                size_t len = strnlen(value.chardata, sizeof(sai_attribute_value_t::chardata)/sizeof(char));
+
+                // for some attributes, length can be zero
+
+                for (size_t i = 0; i < len; ++i)
+                {
+                    char c = value.chardata[i];
+
+                    if (c < 0x20 || c > 0x7e)
+                    {
+                        META_LOG_ERROR(md, "invalid character 0x%02x in chardata", c);
+
+                        return SAI_STATUS_INVALID_PARAMETER;
+                    }
+                }
+
+                break;
+            }
+
         case SAI_ATTR_VALUE_TYPE_IP_ADDRESS:
 
             {

diff --git a/syncd/BestCandidateFinder.cpp b/syncd/BestCandidateFinder.cpp
@@ -1392,6 +1392,78 @@ std::shared_ptr<SaiObj> BestCandidateFinder::findCurrentBestMatchForGenericObjec
     return nullptr;
 }
 
+std::shared_ptr<SaiObj> BestCandidateFinder::findCurrentBestMatchForGenericObjectUsingLabel(
+        _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
+        _In_ const std::vector<sai_object_compare_info_t> &candidateObjects)
+{
+    SWSS_LOG_ENTER();
+
+    switch (temporaryObj->getObjectType())
+    {
+        case SAI_OBJECT_TYPE_LAG:
+            return findCurrentBestMatchForGenericObjectUsingLabel(temporaryObj, candidateObjects, SAI_LAG_ATTR_LABEL);
+
+        case SAI_OBJECT_TYPE_VIRTUAL_ROUTER:
+            return findCurrentBestMatchForGenericObjectUsingLabel(temporaryObj, candidateObjects, SAI_VIRTUAL_ROUTER_ATTR_LABEL);
+
+        default:
+            return nullptr;
+    }
+}
+
+std::shared_ptr<SaiObj> BestCandidateFinder::findCurrentBestMatchForGenericObjectUsingLabel(
+        _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
+        _In_ const std::vector<sai_object_compare_info_t> &candidateObjects,
+        _In_ sai_attr_id_t attrId)
+{
+    SWSS_LOG_ENTER();
+
+    auto labelAttr = temporaryObj->tryGetSaiAttr(attrId);
+
+    if (!labelAttr)
+    {
+        // no label attribute on that object
+        return nullptr;
+    }
+
+    auto label = labelAttr->getStrAttrValue();
+
+    std::vector<sai_object_compare_info_t> sameLabel;
+
+    for (auto& co: candidateObjects)
+    {
+        if (co.obj->hasAttr(attrId) && co.obj->getSaiAttr(attrId)->getStrAttrValue() == label)
+        {
+            sameLabel.push_back(co);
+        }
+    }
+
+    if (sameLabel.size() == 0)
+    {
+        // no objects with that label, fallback to attr count
+        return nullptr;
+    }
+
+    if (sameLabel.size() == 1)
+    {
+        SWSS_LOG_NOTICE("matched object by label '%s' for %s:%s",
+            label.c_str(),
+            temporaryObj->m_str_object_type.c_str(),
+            temporaryObj->m_str_object_id.c_str());
+
+        return sameLabel.at(0).obj;
+    }
+
+    SWSS_LOG_WARN("same label '%s' found on multiple objects for %s:%s, selecting one with most common atributes",
+            label.c_str(),
+            temporaryObj->m_str_object_type.c_str(),
+            temporaryObj->m_str_object_id.c_str());
+
+    std::sort(sameLabel.begin(), sameLabel.end(), compareByEqualAttributes);
+
+    return sameLabel.at(0).obj;
+}
+
 std::shared_ptr<SaiObj> BestCandidateFinder::findCurrentBestMatchForGenericObjectUsingGraph(
         _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
         _In_ const std::vector<sai_object_compare_info_t> &candidateObjects)
@@ -1403,7 +1475,7 @@ std::shared_ptr<SaiObj> BestCandidateFinder::findCurrentBestMatchForGenericObjec
     candidate = findCurrentBestMatchForGenericObjectUsingPreMatchMap(temporaryObj, candidateObjects);
 
     if (candidate != nullptr)
-            return candidate;
+        return candidate;
 
     switch (temporaryObj->getObjectType())
     {
@@ -1815,6 +1887,13 @@ std::shared_ptr<SaiObj> BestCandidateFinder::findCurrentBestMatchForGenericObjec
         return candidateObjects.begin()->obj;
     }
 
+    auto labelCandidate = findCurrentBestMatchForGenericObjectUsingLabel(
+            temporaryObj,
+            candidateObjects);
+
+    if (labelCandidate != nullptr)
+        return labelCandidate;
+
     /*
      * If we have more than 1 object matched actually more preferred
      * object would be the object with most CREATE_ONLY attributes matching

diff --git a/syncd/BestCandidateFinder.h b/syncd/BestCandidateFinder.h
@@ -95,6 +95,15 @@ namespace syncd
                     _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
                     _In_ const std::vector<sai_object_compare_info_t> &candidateObjects);
 
+            std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingLabel(
+                    _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
+                    _In_ const std::vector<sai_object_compare_info_t> &candidateObjects);
+
+            std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingLabel(
+                    _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
+                    _In_ const std::vector<sai_object_compare_info_t> &candidateObjects,
+                    _In_ sai_attr_id_t attrId);
+
             std::shared_ptr<SaiObj> findCurrentBestMatchForGenericObjectUsingGraph(
                     _In_ const std::shared_ptr<const SaiObj> &temporaryObj,
                     _In_ const std::vector<sai_object_compare_info_t> &candidateObjects);

diff --git a/tests/BCM56850.pl b/tests/BCM56850.pl
@@ -623,8 +623,40 @@ sub test_bulk_set_multiple
     play "test_bulk_set_multiple_B.rec", 0;
 }
 
+sub test_lag_label
+{
+    fresh_start;
+
+    play "lag_label_A.rec";
+    play "lag_label_B.rec";
+
+    open (my $H, "<", "applyview.log") or die "failed to open applyview.log $!";
+
+    my $line = <$H>;
+
+    close ($H);
+
+    chomp$line;
+
+    if (not $line =~ /ASIC_OPERATIONS: (\d+)/ or $1 != 8)
+    {
+        print color('red') . "expected 8 ASIC_OPERATIONS count on first line, but got: '$line'" . color('reset') . "\n";
+        exit 1;
+    }
+}
+
+sub test_no_lag_label
+{
+    fresh_start;
+
+    play "no_lag_label_A.rec";
+    play "no_lag_label_B.rec", 2;
+}
+
 # RUN TESTS
 
+test_no_lag_label;
+test_lag_label;
 test_bulk_set_multiple;
 test_depreacated_enums;
 test_brcm_buffer_pool_zmq_sync_flag;

diff --git a/tests/BCM56850/lag_label_A.rec b/tests/BCM56850/lag_label_A.rec
@@ -0,0 +1,9 @@
+2020-12-31.21:34:31.861834|a|INIT_VIEW
+2020-12-31.21:34:31.862379|A|SAI_STATUS_SUCCESS
+2020-12-31.21:34:31.864591|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000761|SAI_LAG_ATTR_LABEL=foo|SAI_LAG_ATTR_PORT_VLAN_ID=2|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=5
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000762|SAI_LAG_ATTR_LABEL=car|SAI_LAG_ATTR_PORT_VLAN_ID=2|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=5
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000763|SAI_LAG_ATTR_LABEL=bar|SAI_LAG_ATTR_PORT_VLAN_ID=3|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=7
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000764|SAI_LAG_ATTR_LABEL=baz|SAI_LAG_ATTR_PORT_VLAN_ID=3|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=7
+2020-12-31.21:34:41.215885|a|APPLY_VIEW
+2020-12-31.21:34:41.216326|A|SAI_STATUS_SUCCESS
diff --git a/tests/BCM56850/lag_label_B.rec b/tests/BCM56850/lag_label_B.rec
@@ -0,0 +1,9 @@
+2020-12-31.21:34:31.861834|a|INIT_VIEW
+2020-12-31.21:34:31.862379|A|SAI_STATUS_SUCCESS
+2020-12-31.21:34:31.864591|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000765|SAI_LAG_ATTR_LABEL=bar|SAI_LAG_ATTR_PORT_VLAN_ID=2|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=5
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000766|SAI_LAG_ATTR_LABEL=baz|SAI_LAG_ATTR_PORT_VLAN_ID=2|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=5
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000767|SAI_LAG_ATTR_LABEL=foo|SAI_LAG_ATTR_PORT_VLAN_ID=3|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=7
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000768|SAI_LAG_ATTR_LABEL=car|SAI_LAG_ATTR_PORT_VLAN_ID=3|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=7
+2020-12-31.21:34:41.215885|a|APPLY_VIEW
+2020-12-31.21:34:41.216326|A|SAI_STATUS_SUCCESS
diff --git a/tests/BCM56850/no_lag_label_A.rec b/tests/BCM56850/no_lag_label_A.rec
@@ -0,0 +1,7 @@
+2020-12-31.21:34:31.861834|a|INIT_VIEW
+2020-12-31.21:34:31.862379|A|SAI_STATUS_SUCCESS
+2020-12-31.21:34:31.864591|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000763|SAI_LAG_ATTR_PORT_VLAN_ID=2|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=5
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000764|SAI_LAG_ATTR_PORT_VLAN_ID=3|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=7
+2020-12-31.21:34:41.215885|a|APPLY_VIEW
+2020-12-31.21:34:41.216326|A|SAI_STATUS_SUCCESS
diff --git a/tests/BCM56850/no_lag_label_B.rec b/tests/BCM56850/no_lag_label_B.rec
@@ -0,0 +1,7 @@
+2020-12-31.21:34:31.861834|a|INIT_VIEW
+2020-12-31.21:34:31.862379|A|SAI_STATUS_SUCCESS
+2020-12-31.21:34:31.864591|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000763|SAI_LAG_ATTR_LABEL=bar|SAI_LAG_ATTR_PORT_VLAN_ID=2|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=5
+2020-05-27.17:28:54.352190|c|SAI_OBJECT_TYPE_LAG:oid:0x2000000000764|SAI_LAG_ATTR_LABEL=baz|SAI_LAG_ATTR_PORT_VLAN_ID=3|SAI_LAG_ATTR_DEFAULT_VLAN_PRIORITY=7
+2020-12-31.21:34:41.215885|a|APPLY_VIEW
+2020-12-31.21:34:41.216326|A|SAI_STATUS_SUCCESS
diff --git a/tests/utils.pm b/tests/utils.pm
@@ -180,7 +180,7 @@ sub sync_fresh_start
 BEGIN
 {
     our @ISA    = qw(Exporter);
-    our @EXPORT = qw/
+    our @EXPORT = qw/ color
     kill_syncd flush_redis start_syncd play fresh_start start_syncd_warm request_warm_shutdown
     sync_start_syncd sync_fresh_start sync_start_syncd_warm sync_start_syncd sync_play
     /;