Skip to content

Commit

Permalink
Revert "Use shared egress ACL table for PFCWD in BRCM DNX platform (#…
Browse files Browse the repository at this point in the history 
…3136)"

This reverts commit 4f0d40c.
  • Loading branch information
@zbud-msft
zbud-msft authored Oct 1, 2024
1 parent a40b8a1 commit a3f53ef
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 396 deletions.
76 changes: 9 additions & 67 deletions orchagent/aclorch.cpp
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit a3f53ef

Please sign in to comment.