Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[action] [PR:3136] Use shared egress ACL table for PFCWD in BRCM DNX platform (#3136)" #3311

Draft
wants to merge 1 commit into
base: 202405
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 9 additions & 67 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ extern sai_switch_api_t* sai_switch_api;
extern sai_object_id_t gSwitchId;
extern PortsOrch* gPortsOrch;
extern CrmOrch *gCrmOrch;
extern SwitchOrch *gSwitchOrch;
extern string gMySwitchType;

#define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID
Expand All @@ -47,7 +46,6 @@ const int TCP_PROTOCOL_NUM = 6; // TCP protocol number
acl_rule_attr_lookup_t aclMatchLookup =
{
{ MATCH_IN_PORTS, SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS },
{ MATCH_OUT_PORT, SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORT },
{ MATCH_OUT_PORTS, SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORTS },
{ MATCH_SRC_IP, SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP },
{ MATCH_DST_IP, SAI_ACL_ENTRY_ATTR_FIELD_DST_IP },
Expand Down Expand Up @@ -867,23 +865,6 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value)
matchData.data.objlist.count = static_cast<uint32_t>(outPorts.size());
matchData.data.objlist.list = outPorts.data();
}
else if (attr_name == MATCH_OUT_PORT)
{
auto alias = attr_value;
Port port;
if (!gPortsOrch->getPort(alias, port))
{
SWSS_LOG_ERROR("Failed to locate port %s", alias.c_str());
return false;
}
if (port.m_type != Port::PHY)
{
SWSS_LOG_ERROR("Cannot bind rule to %s: OUT_PORT can only match physical interfaces", alias.c_str());
return false;
}

matchData.data.oid = port.m_port_id;
}
else if (attr_name == MATCH_IP_TYPE)
{
if (!processIpType(attr_value, matchData.data.u32))
Expand Down Expand Up @@ -3220,14 +3201,14 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
m_mirrorV6TableId[stage] = "";
}

initDefaultTableTypes(platform, sub_platform);
initDefaultTableTypes();

// Attach observers
m_mirrorOrch->attach(this);
gPortsOrch->attach(this);
}

void AclOrch::initDefaultTableTypes(const string& platform, const string& sub_platform)
void AclOrch::initDefaultTableTypes()
{
SWSS_LOG_ENTER();

Expand Down Expand Up @@ -3314,26 +3295,12 @@ void AclOrch::initDefaultTableTypes(const string& platform, const string& sub_pl
.build()
);

// Use SAI_ACL_BIND_POINT_TYPE_SWITCH in BRCM DNX platforms to use shared egress ACL table for PFCWD.
if (platform == BRCM_PLATFORM_SUBSTRING && sub_platform == BRCM_DNX_PLATFORM_SUBSTRING)
{
addAclTableType(
builder.withName(TABLE_TYPE_PFCWD)
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORT))
.build()
);
}
else
{
addAclTableType(
builder.withName(TABLE_TYPE_PFCWD)
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
.build()
);
}
addAclTableType(
builder.withName(TABLE_TYPE_PFCWD)
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
.build()
);

addAclTableType(
builder.withName(TABLE_TYPE_DROP)
Expand Down Expand Up @@ -4007,20 +3974,6 @@ bool AclOrch::addAclTable(AclTable &newTable)
m_mirrorV6TableId[table_stage] = table_id;
}

// We use SAI_ACL_BIND_POINT_TYPE_SWITCH for PFCWD table in DNX platform.
// This bind type requires to bind the table to switch.
string platform = getenv("platform") ? getenv("platform") : "";
string sub_platform = getenv("sub_platform") ? getenv("sub_platform") : "";
if (platform == BRCM_PLATFORM_SUBSTRING && sub_platform == BRCM_DNX_PLATFORM_SUBSTRING &&
newTable.type.getName() == TABLE_TYPE_PFCWD)
{
if(!gSwitchOrch->bindAclTableToSwitch(ACL_STAGE_EGRESS, newTable.getOid()))
{
return false;
}
newTable.bindToSwitch = true;
}

return true;
}
else
Expand All @@ -4045,18 +3998,6 @@ bool AclOrch::removeAclTable(string table_id)
bool suc = m_AclTables[table_oid].clear();
if (!suc) return false;

// Unbind table from switch if needed.
AclTable &table = m_AclTables.at(table_oid);
if (table.bindToSwitch)
{
// Only bind egress table to switch for now.
assert(table->stage == ACL_STAGE_EGRESS);
if(!gSwitchOrch->unbindAclTableFromSwitch(ACL_STAGE_EGRESS, table.getOid()))
{
return false;
}
}

if (deleteUnbindAclTable(table_oid) == SAI_STATUS_SUCCESS)
{
auto stage = m_AclTables[table_oid].stage;
Expand Down Expand Up @@ -5170,3 +5111,4 @@ void AclOrch::removeAllAclRuleStatus()
m_aclRuleStateTable.del(key);
}
}

6 changes: 1 addition & 5 deletions orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

#define RULE_PRIORITY "PRIORITY"
#define MATCH_IN_PORTS "IN_PORTS"
#define MATCH_OUT_PORT "OUT_PORT"
#define MATCH_OUT_PORTS "OUT_PORTS"
#define MATCH_SRC_IP "SRC_IP"
#define MATCH_DST_IP "DST_IP"
Expand Down Expand Up @@ -452,9 +451,6 @@ class AclTable
// Set to store the not configured ACL table port alias
set<string> pendingPortSet;

// Is the ACL table bound to switch?
bool bindToSwitch = false;

private:
sai_object_id_t m_oid = SAI_NULL_OBJECT_ID;
AclOrch *m_pAclOrch = nullptr;
Expand Down Expand Up @@ -534,7 +530,7 @@ class AclOrch : public Orch, public Observer
void doAclRuleTask(Consumer &consumer);
void doAclTableTypeTask(Consumer &consumer);
void init(vector<TableConnector>& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch);
void initDefaultTableTypes(const string& platform, const string& sub_platform);
void initDefaultTableTypes();

void queryMirrorTableCapability();
void queryAclActionCapability();
Expand Down
20 changes: 1 addition & 19 deletions orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,25 +676,7 @@ bool OrchDaemon::init()
SAI_QUEUE_ATTR_PAUSE_STATUS,
};

bool pfcDlrInit = gSwitchOrch->checkPfcDlrInitEnable();

// Override pfcDlrInit if needed, and this change is only for PFC tests.
if(getenv("PFC_DLR_INIT_ENABLE"))
{
string envPfcDlrInit = getenv("PFC_DLR_INIT_ENABLE");
if(envPfcDlrInit == "1")
{
pfcDlrInit = true;
SWSS_LOG_NOTICE("Override PfcDlrInitEnable to true");
}
else if(envPfcDlrInit == "0")
{
pfcDlrInit = false;
SWSS_LOG_NOTICE("Override PfcDlrInitEnable to false");
}
}

if(pfcDlrInit)
if(gSwitchOrch->checkPfcDlrInitEnable())
{
m_orchList.push_back(new PfcWdSwOrch<PfcWdDlrHandler, PfcWdDlrHandler>(
m_configDb,
Expand Down
95 changes: 17 additions & 78 deletions orchagent/pfcactionhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,58 +344,19 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,

// Egress table/rule creation
table_type = TABLE_TYPE_PFCWD;

// Use shared egress acl table for BRCM DNX platform.
string platform = getenv("platform") ? getenv("platform") : "";
string sub_platform = getenv("sub_platform") ? getenv("sub_platform") : "";
shared_egress_acl_table = (platform == BRCM_PLATFORM_SUBSTRING &&
sub_platform == BRCM_DNX_PLATFORM_SUBSTRING);

if (shared_egress_acl_table)
m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr;
found = m_aclTables.find(m_strEgressTable);
if (found == m_aclTables.end())
{
Port p;
if (!gPortsOrch->getPort(port, p))
{
SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, port);
return;
}
m_strEgressRule = "Egress_Rule_PfcWdAclHandler_" + p.m_alias + "_" + queuestr;
m_strEgressTable = "EgressTable_PfcWdAclHandler";
found = m_aclTables.find(m_strEgressTable);
if (found == m_aclTables.end())
{
// First time of handling PFC, create ACL table and also ACL rule.
createPfcAclTable(port, m_strEgressTable, false);
shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strEgressRule, m_strEgressTable);
createPfcAclRule(newRule, queueId, m_strEgressTable, port);
}
else
{
// ACL table already exists. Add ACL rule if needed.
AclRule* rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule);
if (rule == nullptr)
{
shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strEgressRule, m_strEgressTable);
createPfcAclRule(newRule, queueId, m_strEgressTable, port);
}
}
// First time of handling PFC for this queue, create ACL table, and bind
createPfcAclTable(port, m_strEgressTable, false);
shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strRule, m_strEgressTable);
createPfcAclRule(newRule, queueId, m_strEgressTable, port);
}
else
{
m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr;
found = m_aclTables.find(m_strEgressTable);
if (found == m_aclTables.end())
{
// First time of handling PFC for this queue, create ACL table, and bind
createPfcAclTable(port, m_strEgressTable, false);
shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strRule, m_strEgressTable);
createPfcAclRule(newRule, queueId, m_strEgressTable, port);
}
else
{
// Otherwise just bind ACL table with the port
found->second.bind(port);
}
// Otherwise just bind ACL table with the port
found->second.bind(port);
}
}

Expand All @@ -421,20 +382,8 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
}

if (shared_egress_acl_table)
{
rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule);
if (rule == nullptr)
{
SWSS_LOG_THROW("Egress ACL Rule does not exist for rule %s", m_strEgressRule.c_str());
}
gAclOrch->removeAclRule(m_strEgressTable, m_strEgressRule);
}
else
{
auto found = m_aclTables.find(m_strEgressTable);
found->second.unbind(port);
}
auto found = m_aclTables.find(m_strEgressTable);
found->second.unbind(port);
}

void PfcWdAclHandler::clear()
Expand Down Expand Up @@ -469,11 +418,7 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b
return;
}

// Link port only for ingress ACL table or unshared egress ACL table.
if (ingress || !shared_egress_acl_table)
{
aclTable.link(port);
}
aclTable.link(port);

if (ingress)
{
Expand Down Expand Up @@ -507,24 +452,18 @@ void PfcWdAclHandler::createPfcAclRule(shared_ptr<AclRulePacket> rule, uint8_t q
attr_value = to_string(queueId);
rule->validateAddMatch(attr_name, attr_value);

// Add MATCH_IN_PORTS as match criteria for ingress table and MATCH_OUT_PORT as match creiteria for shared egress table.
if (strTable == INGRESS_TABLE_DROP || shared_egress_acl_table)
// Add MATCH_IN_PORTS as match criteria for ingress table
if (strTable == INGRESS_TABLE_DROP)
{
Port p;
if (strTable == INGRESS_TABLE_DROP)
{
attr_name = MATCH_IN_PORTS;
}
else if (shared_egress_acl_table) {
attr_name = MATCH_OUT_PORT;
}

attr_name = MATCH_IN_PORTS;

if (!gPortsOrch->getPort(portOid, p))
{
SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, portOid);
return;
}

attr_value = p.m_alias;
rule->validateAddMatch(attr_name, attr_value);
}
Expand Down
3 changes: 0 additions & 3 deletions orchagent/pfcactionhandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,9 @@ class PfcWdAclHandler: public PfcWdLossyHandler
// class shared dict: ACL table name -> ACL table
static std::map<std::string, AclTable> m_aclTables;

bool shared_egress_acl_table = false;

string m_strIngressTable;
string m_strEgressTable;
string m_strRule;
string m_strEgressRule;
void createPfcAclTable(sai_object_id_t port, string strTable, bool ingress);
void createPfcAclRule(shared_ptr<AclRulePacket> rule, uint8_t queueId, string strTable, sai_object_id_t port);
void updatePfcAclRule(shared_ptr<AclRule> rule, uint8_t queueId, string strTable, vector<sai_object_id_t> port);
Expand Down
48 changes: 0 additions & 48 deletions orchagent/switchorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1522,51 +1522,3 @@ bool SwitchOrch::querySwitchCapability(sai_object_type_t sai_object, sai_attr_id
}
}
}

// Bind ACL table (with bind type switch) to switch
bool SwitchOrch::bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id)
{
sai_attribute_t attr;
if ( stage == ACL_STAGE_INGRESS ) {
attr.id = SAI_SWITCH_ATTR_INGRESS_ACL;
} else {
attr.id = SAI_SWITCH_ATTR_EGRESS_ACL;
}
attr.value.oid = table_id;
sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
string stage_str = (stage == ACL_STAGE_INGRESS) ? "ingress" : "egress";
if (status == SAI_STATUS_SUCCESS)
{
SWSS_LOG_NOTICE("Bind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
return true;
}
else
{
SWSS_LOG_ERROR("Failed to bind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
return false;
}
}

// Unbind ACL table from swtich
bool SwitchOrch::unbindAclTableFromSwitch(acl_stage_type_t stage,sai_object_id_t table_id)
{
sai_attribute_t attr;
if ( stage == ACL_STAGE_INGRESS ) {
attr.id = SAI_SWITCH_ATTR_INGRESS_ACL;
} else {
attr.id = SAI_SWITCH_ATTR_EGRESS_ACL;
}
attr.value.oid = SAI_NULL_OBJECT_ID;
sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
string stage_str = (stage == ACL_STAGE_INGRESS) ? "ingress" : "egress";
if (status == SAI_STATUS_SUCCESS)
{
SWSS_LOG_NOTICE("Unbind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
return true;
}
else
{
SWSS_LOG_ERROR("Failed to unbind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
return false;
}
}
3 changes: 0 additions & 3 deletions orchagent/switchorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ class SwitchOrch : public Orch
return (m_fatalEventCount != 0);
}

bool bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id);
bool unbindAclTableFromSwitch(acl_stage_type_t stage, sai_object_id_t table_id);

private:
void doTask(Consumer &consumer);
void doTask(swss::SelectableTimer &timer);
Expand Down
Loading
Loading