[cbf] Fix max FC value

orchagent/cbf/cbfnhgorch.cpp

@@ -308,7 +308,7 @@ bool CbfNhg::sync()
     nhg_attr.value.u32 = static_cast<sai_uint32_t>(m_members.size());
     nhg_attrs.push_back(move(nhg_attr));
 
-    if (nhg_attr.value.u32 > gNhgMapOrch->getMaxFcVal())
+    if (nhg_attr.value.u32 > gNhgMapOrch->getMaxNumFcs())
     {
         /* If there are more members than FCs then this may be an error, as some members won't be used. */
         SWSS_LOG_WARN("More CBF NHG members configured than supported Forwarding Classes");

orchagent/cbf/nhgmaporch.cpp

@@ -294,34 +294,34 @@ void NhgMapOrch::decRefCount(const string &index)
 }
 
 /*
- * Get the max FC value supported by the switch.
+ * Get the maximum number of FC classes supported by the switch.
  */
-sai_uint8_t NhgMapOrch::getMaxFcVal()
+sai_uint8_t NhgMapOrch::getMaxNumFcs()
 {
     SWSS_LOG_ENTER();
 
-    static int max_fc_val = -1;
+    static int max_num_fcs = -1;
 
     /*
-     * Get the maximum value allowed for FC if it wasn't already initialized.
+     * Get the maximum number of FC classes if it wasn't already initialized.
      */
-    if (max_fc_val == -1)
+    if (max_num_fcs == -1)
     {
         sai_attribute_t attr;
         attr.id = SAI_SWITCH_ATTR_MAX_NUMBER_OF_FORWARDING_CLASSES;
 
         if (sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr) == SAI_STATUS_SUCCESS)
         {
-            max_fc_val = attr.value.u8;
+            max_num_fcs = attr.value.u8;
         }
         else
         {
             SWSS_LOG_WARN("Switch does not support FCs");
-            max_fc_val = 0;
+            max_num_fcs = 0;
         }
     }
 
-    return static_cast<sai_uint8_t>(max_fc_val);
+    return static_cast<sai_uint8_t>(max_num_fcs);
 }
 
 /*
@@ -343,7 +343,7 @@ pair<bool, unordered_map<sai_uint32_t, sai_int32_t>> NhgMapOrch::getMap(const ve
     }
 
     unordered_map<sai_uint32_t, sai_int32_t> fc_map;
-    sai_uint8_t max_fc_val = getMaxFcVal();
+    sai_uint8_t max_num_fcs = getMaxNumFcs();
 
     /*
     * Create the map while validating that the values are positive
@@ -353,13 +353,13 @@ pair<bool, unordered_map<sai_uint32_t, sai_int32_t>> NhgMapOrch::getMap(const ve
         try
         {
             /*
-             * Check the FC value is valid.
+             * Check the FC value is valid. FC value must be in range [0, max_num_fcs).
              */
             auto fc = stoi(fvField(*it));
 
-            if ((fc < 0) || (fc > max_fc_val))
+            if ((fc < 0) || (fc >= max_num_fcs))
             {
-                SWSS_LOG_ERROR("FC value %d is either negative or greater than max value %d", fc, max_fc_val);
+                SWSS_LOG_ERROR("FC value %d is either negative or greater than max value %d", fc, max_num_fcs - 1);
                 success = false;
                 break;
             }

orchagent/cbf/nhgmaporch.h

@@ -43,9 +43,9 @@ class NhgMapOrch : public Orch
     void decRefCount(const string &key);
 
     /*
-     * Get the max FC value supported by the switch.
+     * Get the maximum number of FC classes supported by the switch.
      */
-    static sai_uint8_t getMaxFcVal();
+    static sai_uint8_t getMaxNumFcs();
 
 private:
     /*

orchagent/qosorch.cpp

@@ -808,7 +808,7 @@ bool DscpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &
 {
     SWSS_LOG_ENTER();
 
-    sai_uint8_t max_fc_val = NhgMapOrch::getMaxFcVal();
+    sai_uint8_t max_num_fcs = NhgMapOrch::getMaxNumFcs();
 
     sai_attribute_t list_attr;
     list_attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST;
@@ -835,10 +835,11 @@ bool DscpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &
             }
             list_attr.value.qosmap.list[ind].key.dscp = static_cast<sai_uint8_t>(value);
 
+            // FC value must be in range [0, max_num_fcs)
             value = stoi(fvValue(*i));
-            if ((value < 0) || (value > max_fc_val))
+            if ((value < 0) || (value >= max_num_fcs))
             {
-                SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %d", value, max_fc_val);
+                SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %d", value, max_num_fcs - 1);
                 delete[] list_attr.value.qosmap.list;
                 return false;
             }
@@ -901,7 +902,7 @@ bool ExpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &t
 {
     SWSS_LOG_ENTER();
 
-    sai_uint8_t max_fc_val = NhgMapOrch::getMaxFcVal();
+    sai_uint8_t max_num_fcs = NhgMapOrch::getMaxNumFcs();
 
     sai_attribute_t list_attr;
     list_attr.id = SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST;
@@ -928,10 +929,11 @@ bool ExpToFcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &t
             }
             list_attr.value.qosmap.list[ind].key.mpls_exp = static_cast<sai_uint8_t>(value);
 
+            // FC value must be in range [0, max_num_fcs)
             value = stoi(fvValue(*i));
-            if ((value < 0) || (value > max_fc_val))
+            if ((value < 0) || (value >= max_num_fcs))
             {
-                SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %hu", value, max_fc_val);
+                SWSS_LOG_ERROR("FC value %d is either negative, or bigger than max value %hu", value, max_num_fcs - 1);
                 delete[] list_attr.value.qosmap.list;
                 return false;
             }
@@ -1660,7 +1662,7 @@ task_process_status QosOrch::handlePortQosMapTable(Consumer& consumer)
         }
 
         sai_uint8_t old_pfc_enable = 0;
-        if (!gPortsOrch->getPortPfc(port.m_port_id, &old_pfc_enable)) 
+        if (!gPortsOrch->getPortPfc(port.m_port_id, &old_pfc_enable))
         {
             SWSS_LOG_ERROR("Failed to retrieve PFC bits on port %s", port_name.c_str());
         }

tests/test_nhg.py

@@ -1980,7 +1980,7 @@ def data_validation_test():
             # Test validation errors
             nhg_maps = [
                 ('-1', '0'), # negative FC
-                ('64', '0'), # greater than max FC value
+                ('63', '0'), # greater than max FC value
                 ('a', '0'), # non-integer FC
                 ('0', '-1'), # negative NH index
                 ('0', 'a'), # non-integer NH index

tests/test_qos_map.py

@@ -139,7 +139,7 @@ def test_dscp_to_fc(self, dvs):
         self.init_test(dvs)
 
         # Create a DSCP_TO_FC map
-        dscp_map = [(str(i), str(i)) for i in range(0, 64)]
+        dscp_map = [(str(i), str(i)) for i in range(0, 63)]
         self.dscp_ps.set("AZURE", swsscommon.FieldValuePairs(dscp_map))
 
         self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1)
@@ -153,7 +153,7 @@ def test_dscp_to_fc(self, dvs):
         assert(fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_DSCP_TO_FORWARDING_CLASS")
 
         # Modify the map
-        dscp_map = [(str(i), '0') for i in range(0, 64)]
+        dscp_map = [(str(i), '0') for i in range(0, 63)]
         self.dscp_ps.set("AZURE", swsscommon.FieldValuePairs(dscp_map))
         time.sleep(1)
 
@@ -174,7 +174,7 @@ def test_dscp_to_fc(self, dvs):
             ('-1', '0'), # negative DSCP
             ('64', '0'), # DSCP greater than max value
             ('0', '-1'), # negative FC
-            ('0', '64'), # FC greater than max value
+            ('0', '63'), # FC greater than max value
             ('a', '0'), # non-integer DSCP
             ('0', 'a'), # non-integet FC
         ]
@@ -228,7 +228,7 @@ def test_exp_to_fc(self, dvs):
             ('-1', '0'), # negative EXP
             ('8', '0'), # EXP greater than max value
             ('0', '-1'), # negative FC
-            ('0', '64'), # FC greater than max value
+            ('0', '63'), # FC greater than max value
             ('a', '0'), # non-integer EXP
             ('0', 'a'), # non-integet FC
         ]
@@ -258,7 +258,7 @@ def test_per_port_cbf_binding(self, dvs):
         self.init_test(dvs)
 
         # Create a DSCP_TO_FC map
-        dscp_map = [(str(i), str(i)) for i in range(0, 64)]
+        dscp_map = [(str(i), str(i)) for i in range(0, 63)]
         self.dscp_ps.set("AZURE", swsscommon.FieldValuePairs(dscp_map))
         self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1)
         dscp_map_id = self.get_qos_id()