Skip to content

[sonic-swss] Add support for COPP#57

Merged
stcheng merged 1 commit intosonic-net:masterfrom
hrachya-m:copp.07-29-16
Aug 13, 2016
Merged

[sonic-swss] Add support for COPP#57
stcheng merged 1 commit intosonic-net:masterfrom
hrachya-m:copp.07-29-16

Conversation

@lguohan
Copy link
Contributor

@lguohan lguohan commented Aug 6, 2016

  • Added new class CoppOrch
  • Added schema for COPP_TABLE

Signed-off-by: hrachya@mellanox.com

{
attr.id = SAI_HOSTIF_TRAP_GROUP_ATTR_POLICER;
attr.value.oid = SAI_NULL_OBJECT_ID;
if (SAI_STATUS_SUCCESS != (sai_status = sai_hostif_api->get_trap_group_attribute(m_trap_map[key], 1, &attr)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not necessary to use get api to retrieve the trap group ID from the SAI as you know if you have created the trap group or not, and should cache the trap group ID. In the orchagent, we should not use get API unless we have to. For example, the hw_lane ID, number of ports, and etc. on. Everything else should be cached in the orchagent.

@lguohan
Copy link
Contributor Author

lguohan commented Aug 6, 2016

@hrachya-mellanox , @stcheng

@hrachya-m
Copy link

I've addressed Guohan's comments:

  • styling
  • policer_id is bound to the trap_group, for case when adding policer to existing trap group.
  • removed get__attrib calls. Instead policer values are cached now.
  • consolidated trap_attrs in one function.

SWSS_LOG_DEBUG("trap group name:%s:", key.c_str());
if (m_trap_group_map.find(key) == m_trap_group_map.end())
{
return SAI_NULL_OBJECT_ID;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SAI error codes shall only be returned in the SAI implementation not here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 'null' value, not error code. it's used by caller function to detect whether there is cached object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So my suggestion is that similar to the neighorch.h, we will have a hasPolicer() function and getPolicerId() function, which seems to be clearer.

@lguohan
Copy link
Contributor Author

lguohan commented Aug 11, 2016

In portsOrch.cpp, line 42 - line 72, should be moved to in this copp orch

void CoppOrch::getTrapIdList(vector<string> &trap_id_name_list, vector<sai_hostif_trap_id_t> &trap_id_list) const
{
SWSS_LOG_ENTER();
for(auto trap_id_str : trap_id_name_list)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave a space between for and '(', make this consistent across the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants