Skip to content

Commit

Permalink
Updated PFCWD to use single ACL table for PFCWD and MUX (sonic-net#1620)
Browse files Browse the repository at this point in the history
Updated PFCWD to use single ACL table for PFCWD and MUX
  • Loading branch information
vmittal-msft authored and DavidZagury committed Mar 4, 2021
1 parent a307c91 commit 30103e3
Show file tree
Hide file tree
Showing 8 changed files with 294 additions and 33 deletions.
105 changes: 93 additions & 12 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ static acl_table_type_lookup_t aclTableTypeLookUp =
{ TABLE_TYPE_CTRLPLANE, ACL_TABLE_CTRLPLANE },
{ TABLE_TYPE_DTEL_FLOW_WATCHLIST, ACL_TABLE_DTEL_FLOW_WATCHLIST },
{ TABLE_TYPE_DTEL_DROP_WATCHLIST, ACL_TABLE_DTEL_DROP_WATCHLIST },
{ TABLE_TYPE_MCLAG, ACL_TABLE_MCLAG }
{ TABLE_TYPE_MCLAG, ACL_TABLE_MCLAG },
{ TABLE_TYPE_DROP, ACL_TABLE_DROP }
};

static acl_stage_type_lookup_t aclStageLookUp =
Expand Down Expand Up @@ -620,6 +621,23 @@ bool AclRule::remove()
return res;
}

void AclRule::updateInPorts()
{
SWSS_LOG_ENTER();
sai_attribute_t attr;
sai_status_t status;

attr.id = SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS;
attr.value = m_matches[SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS];
attr.value.aclfield.enable = true;

status = sai_acl_api->set_acl_entry_attribute(m_ruleOid, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to update ACL rule %s, rv:%d", m_id.c_str(), status);
}
}

AclRuleCounters AclRule::getCounters()
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -675,7 +693,8 @@ shared_ptr<AclRule> AclRule::makeShared(acl_table_type_t type, AclOrch *acl, Mir
type != ACL_TABLE_MIRROR_DSCP &&
type != ACL_TABLE_DTEL_FLOW_WATCHLIST &&
type != ACL_TABLE_DTEL_DROP_WATCHLIST &&
type != ACL_TABLE_MCLAG)
type != ACL_TABLE_MCLAG &&
type != ACL_TABLE_DROP)
{
throw runtime_error("Unknown table type");
}
Expand Down Expand Up @@ -723,6 +742,10 @@ shared_ptr<AclRule> AclRule::makeShared(acl_table_type_t type, AclOrch *acl, Mir
{
return make_shared<AclRuleMclag>(acl, rule, table, type);
}
else if (type == ACL_TABLE_DROP)
{
return make_shared<AclRulePfcwd>(acl, rule, table, type);
}

throw runtime_error("Wrong combination of table type and action in rule " + rule);
}
Expand Down Expand Up @@ -998,20 +1021,13 @@ void AclRuleL3::update(SubjectType, void *)
// Do nothing
}


AclRulePfcwd::AclRulePfcwd(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) :
AclRuleL3(aclOrch, rule, table, type, createCounter)
{
}

bool AclRulePfcwd::validateAddMatch(string attr_name, string attr_value)
{
if (attr_name != MATCH_TC)
{
SWSS_LOG_ERROR("%s is not supported for the tables of type Pfcwd", attr_name.c_str());
return false;
}

return AclRule::validateAddMatch(attr_name, attr_value);
}

Expand Down Expand Up @@ -1326,7 +1342,7 @@ bool AclTable::create()
vector<int32_t> bpoint_list;

// PFC watch dog ACLs are only applied to port
if (type == ACL_TABLE_PFCWD)
if ((type == ACL_TABLE_PFCWD) || (type == ACL_TABLE_DROP))
{
bpoint_list = { SAI_ACL_BIND_POINT_TYPE_PORT };
}
Expand All @@ -1340,7 +1356,7 @@ bool AclTable::create()
attr.value.s32list.list = bpoint_list.data();
table_attrs.push_back(attr);

if (type == ACL_TABLE_PFCWD)
if ((type == ACL_TABLE_PFCWD) || (type == ACL_TABLE_DROP))
{
attr.id = SAI_ACL_TABLE_ATTR_FIELD_TC;
attr.value.booldata = true;
Expand All @@ -1349,7 +1365,14 @@ bool AclTable::create()
attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE;
attr.value.s32 = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS;
table_attrs.push_back(attr);


if (stage == ACL_STAGE_INGRESS)
{
attr.id = SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS;
attr.value.booldata = true;
table_attrs.push_back(attr);
}

sai_status_t status = sai_acl_api->create_acl_table(&m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data());

if (status == SAI_STATUS_SUCCESS)
Expand Down Expand Up @@ -2919,6 +2942,64 @@ bool AclOrch::removeAclRule(string table_id, string rule_id)
return m_AclTables[table_oid].remove(rule_id);
}

bool AclOrch::updateAclRule(shared_ptr<AclRule> rule, string table_id, string attr_name, void *data, bool oper)
{
SWSS_LOG_ENTER();

sai_object_id_t table_oid = getTableById(table_id);
string attr_value;

if (table_oid == SAI_NULL_OBJECT_ID)
{
SWSS_LOG_ERROR("Failed to update ACL rule in ACL table %s. Table doesn't exist", table_id.c_str());
return false;
}

switch (aclMatchLookup[attr_name])
{
case SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS:
{
sai_object_id_t port_oid = *(sai_object_id_t *)data;
vector<sai_object_id_t> in_ports = rule->getInPorts();

if (oper == RULE_OPER_ADD)
{
in_ports.push_back(port_oid);
}
else
{
for (auto port_iter = in_ports.begin(); port_iter != in_ports.end(); port_iter++)
{
if (*port_iter == port_oid)
{
in_ports.erase(port_iter);
break;
}
}
}

for (const auto& port_iter: in_ports)
{
Port p;
gPortsOrch->getPort(port_iter, p);
attr_value += p.m_alias;
attr_value += ',';
}
attr_value.pop_back();

rule->validateAddMatch(MATCH_IN_PORTS, attr_value);
m_AclTables[table_oid].rules[rule->getId()]->updateInPorts();
}
break;

default:
SWSS_LOG_ERROR("Acl rule update not supported for attr name %s", attr_name.c_str());
break;
}

return true;
}

bool AclOrch::isCombinedMirrorV6Table()
{
return m_isCombinedMirrorV6Table;
Expand Down
19 changes: 17 additions & 2 deletions orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#define TABLE_TYPE_DTEL_DROP_WATCHLIST "DTEL_DROP_WATCHLIST"
#define TABLE_TYPE_MCLAG "MCLAG"
#define TABLE_TYPE_MUX "MUX"
#define TABLE_TYPE_DROP "DROP"

#define RULE_PRIORITY "PRIORITY"
#define MATCH_IN_PORTS "IN_PORTS"
Expand Down Expand Up @@ -103,6 +104,9 @@
#define IP_TYPE_ARP_REPLY "ARP_REPLY"

#define MLNX_MAX_RANGES_COUNT 16
#define INGRESS_TABLE_DROP "IngressTableDrop"
#define RULE_OPER_ADD 0
#define RULE_OPER_DELETE 1

typedef enum
{
Expand All @@ -117,7 +121,8 @@ typedef enum
ACL_TABLE_DTEL_FLOW_WATCHLIST,
ACL_TABLE_DTEL_DROP_WATCHLIST,
ACL_TABLE_MCLAG,
ACL_TABLE_MUX
ACL_TABLE_MUX,
ACL_TABLE_DROP
} acl_table_type_t;

typedef map<string, acl_table_type_t> acl_table_type_lookup_t;
Expand Down Expand Up @@ -196,6 +201,7 @@ class AclRule
virtual bool create();
virtual bool remove();
virtual void update(SubjectType, void *) = 0;
virtual void updateInPorts();
virtual AclRuleCounters getCounters();

string getId()
Expand All @@ -213,6 +219,11 @@ class AclRule
return m_counterOid;
}

vector<sai_object_id_t> getInPorts()
{
return m_inPorts;
}

static shared_ptr<AclRule> makeShared(acl_table_type_t type, AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&);
virtual ~AclRule() {}

Expand Down Expand Up @@ -417,7 +428,6 @@ class AclOrch : public Orch, public Observer
return m_countersTable;
}


// FIXME: Add getters for them? I'd better to add a common directory of orch objects and use it everywhere
MirrorOrch *m_mirrorOrch;
NeighOrch *m_neighOrch;
Expand All @@ -429,6 +439,7 @@ class AclOrch : public Orch, public Observer
bool updateAclTable(AclTable &currentTable, AclTable &newTable);
bool addAclRule(shared_ptr<AclRule> aclRule, string table_id);
bool removeAclRule(string table_id, string rule_id);
bool updateAclRule(shared_ptr<AclRule> aclRule, string table_id, string attr_name, void *data, bool oper);

bool isCombinedMirrorV6Table();
bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const;
Expand All @@ -443,6 +454,10 @@ class AclOrch : public Orch, public Observer
static bool getAclBindPortId(Port& port, sai_object_id_t& port_id);

using Orch::doTask; // Allow access to the basic doTask
map<sai_object_id_t, AclTable> getAclTables()
{
return m_AclTables;
}

private:
SwitchOrch *m_switchOrch;
Expand Down
84 changes: 69 additions & 15 deletions orchagent/pfcactionhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,35 +225,50 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,
{
SWSS_LOG_ENTER();

acl_table_type_t table_type = ACL_TABLE_PFCWD;
acl_table_type_t table_type;
sai_object_id_t table_oid;

// There is one handler instance per queue ID
string queuestr = to_string(queueId);
m_strIngressTable = "IngressTable_PfcWdAclHandler_" + queuestr;
m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr;
m_strRule = "Rule_PfcWdAclHandler_" + queuestr;

// Ingress table/rule creation
table_type = ACL_TABLE_DROP;
m_strIngressTable = INGRESS_TABLE_DROP;
auto found = m_aclTables.find(m_strIngressTable);
if (found == m_aclTables.end())
{
// First time of handling PFC for this queue, create ACL table, and bind
createPfcAclTable(port, m_strIngressTable, true);
shared_ptr<AclRulePfcwd> newRule = make_shared<AclRulePfcwd>(gAclOrch, m_strRule, m_strIngressTable, table_type);
createPfcAclRule(newRule, queueId, m_strIngressTable);
createPfcAclRule(newRule, queueId, m_strIngressTable, port);
}
else
{
// Otherwise just bind ACL table with the port
found->second.bind(port);
table_oid = gAclOrch->getTableById(m_strIngressTable);
map<sai_object_id_t, AclTable> table_map = gAclOrch->getAclTables();
auto rule_iter = table_map[table_oid].rules.find(m_strRule);

if (rule_iter == table_map[table_oid].rules.end())
{
shared_ptr<AclRulePfcwd> newRule = make_shared<AclRulePfcwd>(gAclOrch, m_strRule, m_strIngressTable, table_type);
createPfcAclRule(newRule, queueId, m_strIngressTable, port);
}
else
{
gAclOrch->updateAclRule(rule_iter->second, m_strIngressTable, MATCH_IN_PORTS, &port, RULE_OPER_ADD);
}
}

// Egress table/rule creation
table_type = ACL_TABLE_PFCWD;
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<AclRulePfcwd> newRule = make_shared<AclRulePfcwd>(gAclOrch, m_strRule, m_strEgressTable, table_type);
createPfcAclRule(newRule, queueId, m_strEgressTable);
createPfcAclRule(newRule, queueId, m_strEgressTable, port);
}
else
{
Expand All @@ -266,11 +281,24 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
{
SWSS_LOG_ENTER();

auto found = m_aclTables.find(m_strIngressTable);
found->second.unbind(getPort());
sai_object_id_t table_oid = gAclOrch->getTableById(m_strIngressTable);
map<sai_object_id_t, AclTable> table_map = gAclOrch->getAclTables();
auto rule_iter = table_map[table_oid].rules.find(m_strRule);

found = m_aclTables.find(m_strEgressTable);
found->second.unbind(getPort());
vector<sai_object_id_t> port_set = rule_iter->second->getInPorts();
sai_object_id_t port = getPort();

if ((port_set.size() == 1) && (port_set[0] == port))
{
gAclOrch->removeAclRule(m_strIngressTable, m_strRule);
}
else
{
gAclOrch->updateAclRule(rule_iter->second, m_strIngressTable, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
}

auto found = m_aclTables.find(m_strEgressTable);
found->second.unbind(port);
}

void PfcWdAclHandler::clear()
Expand All @@ -295,14 +323,24 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b
assert(inserted.second);

AclTable& aclTable = inserted.first->second;
aclTable.type = ACL_TABLE_PFCWD;
aclTable.link(port);
aclTable.id = strTable;
aclTable.stage = ingress ? ACL_STAGE_INGRESS : ACL_STAGE_EGRESS;

if (ingress)
{
aclTable.type = ACL_TABLE_DROP;
aclTable.stage = ACL_STAGE_INGRESS;
}
else
{
aclTable.type = ACL_TABLE_PFCWD;
aclTable.stage = ACL_STAGE_EGRESS;
}

gAclOrch->addAclTable(aclTable);
}

void PfcWdAclHandler::createPfcAclRule(shared_ptr<AclRulePfcwd> rule, uint8_t queueId, string strTable)
void PfcWdAclHandler::createPfcAclRule(shared_ptr<AclRulePfcwd> rule, uint8_t queueId, string strTable, sai_object_id_t portOid)
{
SWSS_LOG_ENTER();

Expand All @@ -316,6 +354,22 @@ void PfcWdAclHandler::createPfcAclRule(shared_ptr<AclRulePfcwd> rule, uint8_t qu
attr_value = to_string(queueId);
rule->validateAddMatch(attr_name, attr_value);

// Add MATCH_IN_PORTS as match criteria for ingress table
if (strTable == INGRESS_TABLE_DROP)
{
Port p;
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);
}

attr_name = ACTION_PACKET_ACTION;
attr_value = PACKET_ACTION_DROP;
rule->validateAddAction(attr_name, attr_value);
Expand Down
3 changes: 2 additions & 1 deletion orchagent/pfcactionhandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ class PfcWdAclHandler: public PfcWdLossyHandler
string m_strEgressTable;
string m_strRule;
void createPfcAclTable(sai_object_id_t port, string strTable, bool ingress);
void createPfcAclRule(shared_ptr<AclRulePfcwd> rule, uint8_t queueId, string strTable);
void createPfcAclRule(shared_ptr<AclRulePfcwd> 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);
};

// PFC queue that implements drop action by draining queue with buffer of zero size
Expand Down
Loading

0 comments on commit 30103e3

Please sign in to comment.