Skip to content

Commit

Permalink
[orchagent]: Fix issue with bind ACL table group to port (sonic-net#386)
Browse files Browse the repository at this point in the history
Port object creates ACL table group, binds it to particular port
and stores group ID. AclOrch class was operating with copies of
Port objects. ID of created ACL table group was stored in object copy.
Which caused resoures leak after copy was destroyed. For each ACL table new
ACL table group was created. Only one ACL table was bind to
group at the same time.

- Move port bind implementation into PortOrch class to give access to real port object.
- Make Port class objects stateless.
  • Loading branch information
oleksandrivantsiv authored and Shuotian Cheng committed Nov 17, 2017
1 parent afa3e9f commit 5bc06f5
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 117 deletions.
1 change: 0 additions & 1 deletion orchagent/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ endif

orchagent_SOURCES = \
main.cpp \
port.cpp \
orchdaemon.cpp \
orch.cpp \
notifications.cpp \
Expand Down
14 changes: 3 additions & 11 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -934,22 +934,14 @@ bool AclTable::bind(sai_object_id_t portOid)

assert(ports.find(portOid) != ports.end());

Port port;
bool found = gPortsOrch->getPort(portOid, port);
if (!found)
{
SWSS_LOG_ERROR("Failed to get port: %lx", portOid);
return false;
}
assert(port.m_type == Port::PHY);

sai_object_id_t group_member_oid;
sai_status_t status = port.bindAclTable(group_member_oid, m_oid);
if (status != SAI_STATUS_SUCCESS) {
if (!gPortsOrch->bindAclTable(portOid, m_oid, group_member_oid))
{
return false;
}

ports[portOid] = group_member_oid;

return true;
}

Expand Down
99 changes: 0 additions & 99 deletions orchagent/port.cpp

This file was deleted.

4 changes: 0 additions & 4 deletions orchagent/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ class Port
return !(*this == o);
}

// Output parameter:
// group_member_oid - the newly created group member OID for the table in a table group
sai_status_t bindAclTable(sai_object_id_t& group_member_oid, sai_object_id_t table_oid);

std::string m_alias;
Type m_type;
int m_index = 0; // PHY_PORT: index
Expand Down
89 changes: 89 additions & 0 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,95 @@ bool PortsOrch::setPortFec(sai_object_id_t id, sai_port_fec_mode_t mode)
return true;
}

bool PortsOrch::bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_object_id_t &group_member_oid)
{
sai_status_t status;
sai_object_id_t groupOid;

Port p;
if (!getPort(id, p))
{
return false;
}

auto &port = m_portList.find(p.m_alias)->second;

// If port ACL table group does not exist, create one
if (port.m_acl_table_group_id == 0)
{
sai_object_id_t bp_list[] = { SAI_ACL_BIND_POINT_TYPE_PORT };

vector<sai_attribute_t> group_attrs;
sai_attribute_t group_attr;

group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_ACL_STAGE;
group_attr.value.s32 = SAI_ACL_STAGE_INGRESS; // TODO: double check
group_attrs.push_back(group_attr);

group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_ACL_BIND_POINT_TYPE_LIST;
group_attr.value.objlist.count = 1;
group_attr.value.objlist.list = bp_list;
group_attrs.push_back(group_attr);

group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_TYPE;
group_attr.value.s32 = SAI_ACL_TABLE_GROUP_TYPE_PARALLEL;
group_attrs.push_back(group_attr);

status = sai_acl_api->create_acl_table_group(&groupOid, gSwitchId, (uint32_t)group_attrs.size(), group_attrs.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create ACL table group, rv:%d", status);
return false;
}

port.m_acl_table_group_id = groupOid;

// Bind this ACL group to port OID
sai_attribute_t port_attr;
port_attr.id = SAI_PORT_ATTR_INGRESS_ACL;
port_attr.value.oid = groupOid;

status = sai_port_api->set_port_attribute(port.m_port_id, &port_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to bind port %lx(%s) to ACL table group %lx, rv:%d",
port.m_port_id, port.m_alias.c_str(), groupOid, status);
return false;
}

SWSS_LOG_NOTICE("Create ACL table group and bind port %s to it", port.m_alias.c_str());
}
else
{
groupOid = port.m_acl_table_group_id;
}

// Create an ACL group member with table_oid and groupOid
vector<sai_attribute_t> member_attrs;

sai_attribute_t member_attr;
member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_GROUP_ID;
member_attr.value.oid = groupOid;
member_attrs.push_back(member_attr);

member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_ID;
member_attr.value.oid = table_oid;
member_attrs.push_back(member_attr);

member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_PRIORITY;
member_attr.value.u32 = 100; // TODO: double check!
member_attrs.push_back(member_attr);

status = sai_acl_api->create_acl_table_group_member(&group_member_oid, gSwitchId, (uint32_t)member_attrs.size(), member_attrs.data());
if (status != SAI_STATUS_SUCCESS) {
SWSS_LOG_ERROR("Failed to create member in ACL table group %lx for ACL table group %lx, rv:%d",
table_oid, groupOid, status);
return false;
}

return true;
}

bool PortsOrch::setPortPvid(Port &port, sai_uint32_t pvid)
{
SWSS_LOG_ENTER();
Expand Down
1 change: 1 addition & 0 deletions orchagent/portsorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class PortsOrch : public Orch, public Subject

bool setHostIntfsOperStatus(sai_object_id_t id, bool up);
void updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_t status);
bool bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_object_id_t &group_member_oid);
private:
unique_ptr<Table> m_counterTable;
unique_ptr<Table> m_portTable;
Expand Down
3 changes: 1 addition & 2 deletions orchagent/qosorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,7 @@ sai_object_id_t QosOrch::initSystemAclTable()

sai_object_id_t group_member_oid;
// Note: group member OID is discarded
status = port.bindAclTable(group_member_oid, acl_table_id);
if (status != SAI_STATUS_SUCCESS)
if (!gPortsOrch->bindAclTable(port.m_port_id, acl_table_id, group_member_oid))
{
SWSS_LOG_ERROR("Failed to bind the system ACL table globally, rv:%d", status);
throw runtime_error("Failed to bind the system ACL table globally");
Expand Down

0 comments on commit 5bc06f5

Please sign in to comment.