Skip to content

Commit 5bc06f5

Browse files
oleksandrivantsivShuotian Cheng
authored and
Shuotian Cheng
committed
[orchagent]: Fix issue with bind ACL table group to port (sonic-net#386)
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.
1 parent afa3e9f commit 5bc06f5

File tree

7 files changed

+94
-117
lines changed

7 files changed

+94
-117
lines changed

orchagent/Makefile.am

-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ endif
1919

2020
orchagent_SOURCES = \
2121
main.cpp \
22-
port.cpp \
2322
orchdaemon.cpp \
2423
orch.cpp \
2524
notifications.cpp \

orchagent/aclorch.cpp

+3-11
Original file line numberDiff line numberDiff line change
@@ -934,22 +934,14 @@ bool AclTable::bind(sai_object_id_t portOid)
934934

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

937-
Port port;
938-
bool found = gPortsOrch->getPort(portOid, port);
939-
if (!found)
940-
{
941-
SWSS_LOG_ERROR("Failed to get port: %lx", portOid);
942-
return false;
943-
}
944-
assert(port.m_type == Port::PHY);
945-
946937
sai_object_id_t group_member_oid;
947-
sai_status_t status = port.bindAclTable(group_member_oid, m_oid);
948-
if (status != SAI_STATUS_SUCCESS) {
938+
if (!gPortsOrch->bindAclTable(portOid, m_oid, group_member_oid))
939+
{
949940
return false;
950941
}
951942

952943
ports[portOid] = group_member_oid;
944+
953945
return true;
954946
}
955947

orchagent/port.cpp

-99
This file was deleted.

orchagent/port.h

-4
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ class Port
6060
return !(*this == o);
6161
}
6262

63-
// Output parameter:
64-
// group_member_oid - the newly created group member OID for the table in a table group
65-
sai_status_t bindAclTable(sai_object_id_t& group_member_oid, sai_object_id_t table_oid);
66-
6763
std::string m_alias;
6864
Type m_type;
6965
int m_index = 0; // PHY_PORT: index

orchagent/portsorch.cpp

+89
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,95 @@ bool PortsOrch::setPortFec(sai_object_id_t id, sai_port_fec_mode_t mode)
388388
return true;
389389
}
390390

391+
bool PortsOrch::bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_object_id_t &group_member_oid)
392+
{
393+
sai_status_t status;
394+
sai_object_id_t groupOid;
395+
396+
Port p;
397+
if (!getPort(id, p))
398+
{
399+
return false;
400+
}
401+
402+
auto &port = m_portList.find(p.m_alias)->second;
403+
404+
// If port ACL table group does not exist, create one
405+
if (port.m_acl_table_group_id == 0)
406+
{
407+
sai_object_id_t bp_list[] = { SAI_ACL_BIND_POINT_TYPE_PORT };
408+
409+
vector<sai_attribute_t> group_attrs;
410+
sai_attribute_t group_attr;
411+
412+
group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_ACL_STAGE;
413+
group_attr.value.s32 = SAI_ACL_STAGE_INGRESS; // TODO: double check
414+
group_attrs.push_back(group_attr);
415+
416+
group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_ACL_BIND_POINT_TYPE_LIST;
417+
group_attr.value.objlist.count = 1;
418+
group_attr.value.objlist.list = bp_list;
419+
group_attrs.push_back(group_attr);
420+
421+
group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_TYPE;
422+
group_attr.value.s32 = SAI_ACL_TABLE_GROUP_TYPE_PARALLEL;
423+
group_attrs.push_back(group_attr);
424+
425+
status = sai_acl_api->create_acl_table_group(&groupOid, gSwitchId, (uint32_t)group_attrs.size(), group_attrs.data());
426+
if (status != SAI_STATUS_SUCCESS)
427+
{
428+
SWSS_LOG_ERROR("Failed to create ACL table group, rv:%d", status);
429+
return false;
430+
}
431+
432+
port.m_acl_table_group_id = groupOid;
433+
434+
// Bind this ACL group to port OID
435+
sai_attribute_t port_attr;
436+
port_attr.id = SAI_PORT_ATTR_INGRESS_ACL;
437+
port_attr.value.oid = groupOid;
438+
439+
status = sai_port_api->set_port_attribute(port.m_port_id, &port_attr);
440+
if (status != SAI_STATUS_SUCCESS)
441+
{
442+
SWSS_LOG_ERROR("Failed to bind port %lx(%s) to ACL table group %lx, rv:%d",
443+
port.m_port_id, port.m_alias.c_str(), groupOid, status);
444+
return false;
445+
}
446+
447+
SWSS_LOG_NOTICE("Create ACL table group and bind port %s to it", port.m_alias.c_str());
448+
}
449+
else
450+
{
451+
groupOid = port.m_acl_table_group_id;
452+
}
453+
454+
// Create an ACL group member with table_oid and groupOid
455+
vector<sai_attribute_t> member_attrs;
456+
457+
sai_attribute_t member_attr;
458+
member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_GROUP_ID;
459+
member_attr.value.oid = groupOid;
460+
member_attrs.push_back(member_attr);
461+
462+
member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_ID;
463+
member_attr.value.oid = table_oid;
464+
member_attrs.push_back(member_attr);
465+
466+
member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_PRIORITY;
467+
member_attr.value.u32 = 100; // TODO: double check!
468+
member_attrs.push_back(member_attr);
469+
470+
status = sai_acl_api->create_acl_table_group_member(&group_member_oid, gSwitchId, (uint32_t)member_attrs.size(), member_attrs.data());
471+
if (status != SAI_STATUS_SUCCESS) {
472+
SWSS_LOG_ERROR("Failed to create member in ACL table group %lx for ACL table group %lx, rv:%d",
473+
table_oid, groupOid, status);
474+
return false;
475+
}
476+
477+
return true;
478+
}
479+
391480
bool PortsOrch::setPortPvid(Port &port, sai_uint32_t pvid)
392481
{
393482
SWSS_LOG_ENTER();

orchagent/portsorch.h

+1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ class PortsOrch : public Orch, public Subject
5656

5757
bool setHostIntfsOperStatus(sai_object_id_t id, bool up);
5858
void updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_t status);
59+
bool bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_object_id_t &group_member_oid);
5960
private:
6061
unique_ptr<Table> m_counterTable;
6162
unique_ptr<Table> m_portTable;

orchagent/qosorch.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -684,8 +684,7 @@ sai_object_id_t QosOrch::initSystemAclTable()
684684

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

0 commit comments

Comments
 (0)