Skip to content

Commit

Permalink
[MUX/PFCWD] Use in_ports for acls instead of seperate ACL table (soni…
Browse files Browse the repository at this point in the history 
…c-net#1670)

*Use ACL_TABLE_DROP for both PFC_WD and MUX
*Use MATCH_IN_PORTS instead of binding port to ACL table and program single ACL rule
  • Loading branch information
@prsunny
prsunny authored Mar 17, 2021
1 parent 1951365 commit e6d9790
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 33 deletions.
40 changes: 35 additions & 5 deletions orchagent/aclorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2961,7 +2961,26 @@ 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)
AclRule* AclOrch::getAclRule(string table_id, string rule_id)
{
sai_object_id_t table_oid = getTableById(table_id);
if (table_oid == SAI_NULL_OBJECT_ID)
{
SWSS_LOG_INFO("Table %s does not exist", table_id.c_str());
return nullptr;
}

const auto& rule_it = m_AclTables[table_oid].rules.find(rule_id);
if (rule_it == m_AclTables[table_oid].rules.end())
{
SWSS_LOG_INFO("Rule %s doesn't exist", rule_id.c_str());
return nullptr;
}

return rule_it->second.get();
}

bool AclOrch::updateAclRule(string table_id, string rule_id, string attr_name, void *data, bool oper)
{
SWSS_LOG_ENTER();

Expand All @@ -2974,12 +2993,19 @@ bool AclOrch::updateAclRule(shared_ptr<AclRule> rule, string table_id, string at
return false;
}

auto rule_it = m_AclTables[table_oid].rules.find(rule_id);
if (rule_it == m_AclTables[table_oid].rules.end())
{
SWSS_LOG_ERROR("Failed to update ACL rule in ACL table %s. Rule doesn't exist", rule_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();
vector<sai_object_id_t> in_ports = rule_it->second->getInPorts();

if (oper == RULE_OPER_ADD)
{
Expand All @@ -3004,10 +3030,14 @@ bool AclOrch::updateAclRule(shared_ptr<AclRule> rule, string table_id, string at
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();
if (!attr_value.empty())
{
attr_value.pop_back();
}

rule_it->second->validateAddMatch(MATCH_IN_PORTS, attr_value);
rule_it->second->updateInPorts();
}
break;

Expand Down
3 changes: 2 additions & 1 deletion orchagent/aclorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,8 @@ 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 updateAclRule(string table_id, string rule_id, string attr_name, void *data, bool oper);
AclRule* getAclRule(string table_id, string rule_id);

bool isCombinedMirrorV6Table();
bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const;
Expand Down
72 changes: 59 additions & 13 deletions orchagent/muxorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ extern sai_router_interface_api_t* sai_router_intfs_api;

/* Constants */
#define MUX_TUNNEL "MuxTunnel0"
#define MUX_ACL_TABLE_NAME "mux_acl_table";
#define MUX_ACL_RULE_NAME "mux_acl_rule";
#define MUX_ACL_TABLE_NAME INGRESS_TABLE_DROP
#define MUX_ACL_RULE_NAME "mux_acl_rule"
#define MUX_HW_STATE_UNKNOWN "unknown"
#define MUX_HW_STATE_ERROR "error"

Expand Down Expand Up @@ -202,6 +202,10 @@ static sai_object_id_t create_tunnel(const IpAddress* p_dst_ip, const IpAddress*
attr.value.s32 = SAI_TUNNEL_TTL_MODE_PIPE_MODEL;
tunnel_attrs.push_back(attr);

attr.id = SAI_TUNNEL_ATTR_LOOPBACK_PACKET_ACTION;
attr.value.s32 = SAI_PACKET_ACTION_DROP;
tunnel_attrs.push_back(attr);

if (p_src_ip != nullptr)
{
attr.id = SAI_TUNNEL_ATTR_ENCAP_SRC_IP;
Expand Down Expand Up @@ -343,7 +347,7 @@ bool MuxCable::stateActive()
return false;
}

if (!aclHandler(port.m_port_id, false))
if (!aclHandler(port.m_port_id, mux_name_, false))
{
SWSS_LOG_INFO("Remove ACL drop rule failed for %s", mux_name_.c_str());
return false;
Expand Down Expand Up @@ -373,7 +377,7 @@ bool MuxCable::stateStandby()
return false;
}

if (!aclHandler(port.m_port_id))
if (!aclHandler(port.m_port_id, mux_name_))
{
SWSS_LOG_INFO("Add ACL drop rule failed for %s", mux_name_.c_str());
return false;
Expand Down Expand Up @@ -416,6 +420,7 @@ void MuxCable::setState(string new_state)
}

st_chg_in_progress_ = false;
st_chg_failed_ = false;
SWSS_LOG_INFO("Changed state to %s", new_state.c_str());

return;
Expand All @@ -429,11 +434,11 @@ string MuxCable::getState()
return (muxStateValToString.at(state_));
}

bool MuxCable::aclHandler(sai_object_id_t port, bool add)
bool MuxCable::aclHandler(sai_object_id_t port, string alias, bool add)
{
if (add)
{
acl_handler_ = make_shared<MuxAclHandler>(port);
acl_handler_ = make_shared<MuxAclHandler>(port, alias);
}
else
{
Expand Down Expand Up @@ -685,16 +690,18 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey)

std::map<std::string, AclTable> MuxAclHandler::acl_table_;

MuxAclHandler::MuxAclHandler(sai_object_id_t port)
MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias)
{
SWSS_LOG_ENTER();

// There is one handler instance per MUX port
acl_table_type_t table_type = ACL_TABLE_MUX;
acl_table_type_t table_type = ACL_TABLE_DROP;
string table_name = MUX_ACL_TABLE_NAME;
string rule_name = MUX_ACL_RULE_NAME;

port_ = port;
alias_ = alias;

auto found = acl_table_.find(table_name);
if (found == acl_table_.end())
{
Expand All @@ -709,20 +716,44 @@ MuxAclHandler::MuxAclHandler(sai_object_id_t port)
else
{
SWSS_LOG_NOTICE("Binding port %" PRIx64 "", port);
// Otherwise just bind ACL table with the port
found->second.bind(port);

AclRule* rule = gAclOrch->getAclRule(table_name, rule_name);
if (rule == nullptr)
{
shared_ptr<AclRuleMux> newRule =
make_shared<AclRuleMux>(gAclOrch, rule_name, table_name, table_type);
createMuxAclRule(newRule, table_name);
}
else
{
gAclOrch->updateAclRule(table_name, rule_name, MATCH_IN_PORTS, &port, RULE_OPER_ADD);
}
}
}

MuxAclHandler::~MuxAclHandler(void)
{
SWSS_LOG_ENTER();
string table_name = MUX_ACL_TABLE_NAME;
string rule_name = MUX_ACL_RULE_NAME;

SWSS_LOG_NOTICE("Un-Binding port %" PRIx64 "", port_);

auto found = acl_table_.find(table_name);
found->second.unbind(port_);
AclRule* rule = gAclOrch->getAclRule(table_name, rule_name);
if (rule == nullptr)
{
SWSS_LOG_THROW("ACL Rule does not exist for port %s, rule %s", alias_.c_str(), rule_name.c_str());
}

vector<sai_object_id_t> port_set = rule->getInPorts();
if ((port_set.size() == 1) && (port_set[0] == port_))
{
gAclOrch->removeAclRule(table_name, rule_name);
}
else
{
gAclOrch->updateAclRule(table_name, rule_name, MATCH_IN_PORTS, &port_, RULE_OPER_DELETE);
}
}

void MuxAclHandler::createMuxAclTable(sai_object_id_t port, string strTable)
Expand All @@ -736,7 +767,17 @@ void MuxAclHandler::createMuxAclTable(sai_object_id_t port, string strTable)
assert(inserted.second);

AclTable& acl_table = inserted.first->second;
acl_table.type = ACL_TABLE_MUX;

sai_object_id_t table_oid = gAclOrch->getTableById(strTable);
if (table_oid != SAI_NULL_OBJECT_ID)
{
// DROP ACL table is already created
SWSS_LOG_NOTICE("ACL table %s exists, reuse the same", strTable.c_str());
acl_table = *(gAclOrch->getTableByOid(table_oid));
return;
}

acl_table.type = ACL_TABLE_DROP;
acl_table.id = strTable;
acl_table.link(port);
acl_table.stage = ACL_STAGE_INGRESS;
Expand All @@ -753,6 +794,11 @@ void MuxAclHandler::createMuxAclRule(shared_ptr<AclRuleMux> rule, string strTabl
attr_value = "999";
rule->validateAddPriority(attr_name, attr_value);

// Add MATCH_IN_PORTS as match criteria for ingress table
attr_name = MATCH_IN_PORTS;
attr_value = 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
5 changes: 3 additions & 2 deletions orchagent/muxorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class MuxStateOrch;
class MuxAclHandler
{
public:
MuxAclHandler(sai_object_id_t port);
MuxAclHandler(sai_object_id_t port, string alias);
~MuxAclHandler(void);

private:
Expand All @@ -48,6 +48,7 @@ class MuxAclHandler
// class shared dict: ACL table name -> ACL table
static std::map<std::string, AclTable> acl_table_;
sai_object_id_t port_ = SAI_NULL_OBJECT_ID;
string alias_;
};

// IP to nexthop index mapping
Expand Down Expand Up @@ -101,7 +102,7 @@ class MuxCable
bool stateInitActive();
bool stateStandby();

bool aclHandler(sai_object_id_t, bool add = true);
bool aclHandler(sai_object_id_t port, string alias, bool add = true);
bool nbrHandler(bool enable, bool update_routes = true);

string mux_name_;
Expand Down
32 changes: 20 additions & 12 deletions orchagent/pfcactionhandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,
SWSS_LOG_ENTER();

acl_table_type_t table_type;
sai_object_id_t table_oid;

string queuestr = to_string(queueId);
m_strRule = "Rule_PfcWdAclHandler_" + queuestr;
Expand All @@ -244,18 +243,15 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,
}
else
{
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())
AclRule* rule = gAclOrch->getAclRule(m_strIngressTable, m_strRule);
if (rule == nullptr)
{
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);
gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_ADD);
}
}

Expand All @@ -281,11 +277,13 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
{
SWSS_LOG_ENTER();

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);
AclRule* rule = gAclOrch->getAclRule(m_strIngressTable, m_strRule);
if (rule == nullptr)
{
SWSS_LOG_THROW("ACL Rule does not exist for rule %s", m_strRule.c_str());
}

vector<sai_object_id_t> port_set = rule_iter->second->getInPorts();
vector<sai_object_id_t> port_set = rule->getInPorts();
sai_object_id_t port = getPort();

if ((port_set.size() == 1) && (port_set[0] == port))
Expand All @@ -294,7 +292,7 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
}
else
{
gAclOrch->updateAclRule(rule_iter->second, m_strIngressTable, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
}

auto found = m_aclTables.find(m_strEgressTable);
Expand Down Expand Up @@ -323,6 +321,16 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b
assert(inserted.second);

AclTable& aclTable = inserted.first->second;

sai_object_id_t table_oid = gAclOrch->getTableById(strTable);
if (ingress && table_oid != SAI_NULL_OBJECT_ID)
{
// DROP ACL table is already created
SWSS_LOG_NOTICE("ACL table %s exists, reuse the same", strTable.c_str());
aclTable = *(gAclOrch->getTableByOid(table_oid));
return;
}

aclTable.link(port);
aclTable.id = strTable;

Expand Down
Loading

0 comments on commit e6d9790

Please sign in to comment.