SAI Proposal for Counter enhancement.

[rajkumar38]

Within the current framework of SAI, objects such as vlan, router-interface, tunnel, queue, port, and others come with built-in statistical enumeration. The expectation is that these statistics will be counted by default upon the creation of objects. With this proposal,

• User has flexibility to decide which object and what stat-ids should support counting.
• Helps to optimally use the hardware counting resources.

[kcudnik]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[rlhui]

@JaiOCP , @srikrishnagopu will review this.

[rck-innovium]

@JaiOCP , @srikrishnagopu will review this.

@JaiOCP @srikrishnagopu Can you please review as well as let us know about your preference among the two approaches. We can then do the required SAI .h file additions to the PR.

[JaiOCP]

Quick comment regarding choice of Option 1 and Option 2. Option 1: Doesn't provide granularity of enabling and disabling at granular level. This approach also means that all the counters will be oneof [packet, packet/byte, byte, none]. This does not tend to usecase well as the graluarity of packet or byte count changes for a given counter type. Last problem and probably the deal breaker for this approach is that it doesn't tend well to modern approach of flex counters, where counter as a generic pool can be allocated on a per type basis.

My input will be to go with Option 2. This is more work but is forward looking and works well with flex counter pool HW design.

[rajkumar38]

Quick comment regarding choice of Option 1 and Option 2. Option 1: Doesn't provide granularity of enabling and disabling at granular level. This approach also means that all the counters will be oneof [packet, packet/byte, byte, none]. This does not tend to usecase well as the graluarity of packet or byte count changes for a given counter type. Last problem and probably the deal breaker for this approach is that it doesn't tend well to modern approach of flex counters, where counter as a generic pool can be allocated on a per type basis.

My input will be to go with Option 2. This is more work but is forward looking and works well with flex counter pool HW design.

Thanks @JaiOCP for the inputs. One of the challenge with Option 2 to minimize the number of attributes. To address this, We came up with option 3. Below is the highlight of option 3 and doc is updated with same. Pls review and let know for any comments.

Option 3 Introduce a new attribute value type holding list of stat_id to counter object.

saitypes.h

typedef struct 
{
      /** Object type of the stat enum*/
      sai_object_type_t  object_type;

      /** Stat enum value */
      sai_stat_id_t      stat_enum;

      /** Counter ObjectId associated with stat enum */
      sai_object_id_t    counter_id;

} sai_counter_id_t;

typedef struct _sai_counter_list_t
{
    /** Number of stats */
    uint32_t count;

    /** List of stat-id to counter object */
    sai_counter_id_t *list;

} sai_counter_list_t;


typedef union _sai_attribute_value_t
{
 

    /** @validonly meta->attrvaluetype == SAI_ATTR_VALUE_TYPE_STATID_COUNTER_LIST */
    sai_counter_list_t statidcounterlist;

} sai_attribute_value_t;

saimetadatypes.h

typedef enum _sai_attr_value_type_t
{

    /**
     * @brief Attribute value is STAT enum to COUNTER object list.
     */
    SAI_ATTR_VALUE_TYPE_STATID_COUNTER_LIST
  
} sai_attr_value_type_t;

Introduce an attribute for stat-id counter object list in each of the object file.

sairouterinterface.h Query and update the list only for supported stat enum

 * @brief Router interface counter objects
 * Counter Object list for supported router interface stats enum 
 * @type sai_counter_list_t
 * @flags CREATE_AND_SET
 * @allownull true
 * @default SAI_NULL_OBJECT_ID
 */
 SAI_ROUTER_INTERFACE_ATTR_COUNTER_IDS

[kcudnik]

there will be problem withh sai_counter_id_t, since it contains sai_object_id_t, which cant't be tracked for object dependency if used in list as attribute, this will not pass validation and sanity checks

[kcudnik]

please fix build errors

[tjchadaga]

@JaiOCP, @srikrishnagopu - could you please help complete the review on this?

[kcudnik]

Instead od adding same attributes for all objects would it be better to add global API for this ?

[rajkumar38]

Instead od adding same attributes for all objects would it be better to add global API for this ?

Hi @kcudnik, If we have to manage this by global API, we may have to define attribute in saicounter.h to store the list of OIDs using a specific counter object. IMO this doesn't look correct. May be we can discuss this in next weekly meeting.

[tjchadaga]

@kcudnik - could you please help clarify the proposed change or help sign off on the current one?

[kcudnik]

Instead of adding 2 attributes to all objects add global API that will query those values, no need for definig which objects support that, if some will not support API will retire not supported

[rajkumar38]

Instead of adding 2 attributes to all objects add global API that will query those values, no need for definig which objects support that, if some will not support API will retire not supported

This proposal is not about about query. Here the primary requirement is to let user select on which of the objects ( say router interfaces) or which stat-id's per object , they need statistics to be enabled/disabled. Note that it is not at object type level but at per object instance (OID). NOS can enable/disable the counting and accordingly set these two attributes to pass the configuration to vendor SAI. Without attributes defined, we are not able to visualize how to achieve this using global API. If needed we discuss and go over the proposal again in the community meeting.

[tjchadaga]

Instead of adding 2 attributes to all objects add global API that will query those values, no need for definig which objects support that, if some will not support API will retire not supported

This proposal is not about about query. Here the primary requirement is to let user select on which of the objects ( say router interfaces) or which stat-id's per object , they need statistics to be enabled/disabled. Note that it is not at object type level but at per object instance (OID). NOS can enable/disable the counting and accordingly set these two attributes to pass the configuration to vendor SAI. Without attributes defined, we are not able to visualize how to achieve this using global API. If needed we discuss and go over the proposal again in the community meeting.

@kcudnik - Could you please help clarify further? This comment has been discussed a few times in the community meeting, but there does not seem to be a clear way of achieving this with global API.

[kcudnik]

Instead of adding 2 attributes to all objects add global API that will query those values, no need for definig which objects support that, if some will not support API will retire not supported

This proposal is not about about query. Here the primary requirement is to let user select on which of the objects ( say router interfaces) or which stat-id's per object , they need statistics to be enabled/disabled. Note that it is not at object type level but at per object instance (OID). NOS can enable/disable the counting and accordingly set these two attributes to pass the configuration to vendor SAI. Without attributes defined, we are not able to visualize how to achieve this using global API. If needed we discuss and go over the proposal again in the community meeting.

@kcudnik - Could you please help clarify further? This comment has been discussed a few times in the community meeting, but there does not seem to be a clear way of achieving this with global API.

please clarify which part you have trouble to understand, i think global API for this would be enough, also please resolve conflicts

[tjchadaga]

@kcudnik - please help sign off on this, as discussed in the community meeting.