Skip to content

Commit e6d9790

Browse files
prsunnyprsunny
authored
[MUX/PFCWD] Use in_ports for acls instead of seperate ACL table (#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
1 parent 1951365 commit e6d9790

File tree

6 files changed

+168
-33
lines changed

6 files changed

+168
-33
lines changed

‎orchagent/aclorch.cpp

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,7 +2961,26 @@ bool AclOrch::removeAclRule(string table_id, string rule_id)
29612961
return m_AclTables[table_oid].remove(rule_id);
29622962
}
29632963

2964-
bool AclOrch::updateAclRule(shared_ptr<AclRule> rule, string table_id, string attr_name, void *data, bool oper)
2964+
AclRule* AclOrch::getAclRule(string table_id, string rule_id)
2965+
{
2966+
sai_object_id_t table_oid = getTableById(table_id);
2967+
if (table_oid == SAI_NULL_OBJECT_ID)
2968+
{
2969+
SWSS_LOG_INFO("Table %s does not exist", table_id.c_str());
2970+
return nullptr;
2971+
}
2972+
2973+
const auto& rule_it = m_AclTables[table_oid].rules.find(rule_id);
2974+
if (rule_it == m_AclTables[table_oid].rules.end())
2975+
{
2976+
SWSS_LOG_INFO("Rule %s doesn't exist", rule_id.c_str());
2977+
return nullptr;
2978+
}
2979+
2980+
return rule_it->second.get();
2981+
}
2982+
2983+
bool AclOrch::updateAclRule(string table_id, string rule_id, string attr_name, void *data, bool oper)
29652984
{
29662985
SWSS_LOG_ENTER();
29672986

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

2996+
auto rule_it = m_AclTables[table_oid].rules.find(rule_id);
2997+
if (rule_it == m_AclTables[table_oid].rules.end())
2998+
{
2999+
SWSS_LOG_ERROR("Failed to update ACL rule in ACL table %s. Rule doesn't exist", rule_id.c_str());
3000+
return false;
3001+
}
3002+
29773003
switch (aclMatchLookup[attr_name])
29783004
{
29793005
case SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS:
29803006
{
29813007
sai_object_id_t port_oid = *(sai_object_id_t *)data;
2982-
vector<sai_object_id_t> in_ports = rule->getInPorts();
3008+
vector<sai_object_id_t> in_ports = rule_it->second->getInPorts();
29833009

29843010
if (oper == RULE_OPER_ADD)
29853011
{
@@ -3004,10 +3030,14 @@ bool AclOrch::updateAclRule(shared_ptr<AclRule> rule, string table_id, string at
30043030
attr_value += p.m_alias;
30053031
attr_value += ',';
30063032
}
3007-
attr_value.pop_back();
30083033

3009-
rule->validateAddMatch(MATCH_IN_PORTS, attr_value);
3010-
m_AclTables[table_oid].rules[rule->getId()]->updateInPorts();
3034+
if (!attr_value.empty())
3035+
{
3036+
attr_value.pop_back();
3037+
}
3038+
3039+
rule_it->second->validateAddMatch(MATCH_IN_PORTS, attr_value);
3040+
rule_it->second->updateInPorts();
30113041
}
30123042
break;
30133043

‎orchagent/aclorch.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,8 @@ class AclOrch : public Orch, public Observer
440440
bool updateAclTable(AclTable &currentTable, AclTable &newTable);
441441
bool addAclRule(shared_ptr<AclRule> aclRule, string table_id);
442442
bool removeAclRule(string table_id, string rule_id);
443-
bool updateAclRule(shared_ptr<AclRule> aclRule, string table_id, string attr_name, void *data, bool oper);
443+
bool updateAclRule(string table_id, string rule_id, string attr_name, void *data, bool oper);
444+
AclRule* getAclRule(string table_id, string rule_id);
444445

445446
bool isCombinedMirrorV6Table();
446447
bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const;

‎orchagent/muxorch.cpp

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ extern sai_router_interface_api_t* sai_router_intfs_api;
4040

4141
/* Constants */
4242
#define MUX_TUNNEL "MuxTunnel0"
43-
#define MUX_ACL_TABLE_NAME "mux_acl_table";
44-
#define MUX_ACL_RULE_NAME "mux_acl_rule";
43+
#define MUX_ACL_TABLE_NAME INGRESS_TABLE_DROP
44+
#define MUX_ACL_RULE_NAME "mux_acl_rule"
4545
#define MUX_HW_STATE_UNKNOWN "unknown"
4646
#define MUX_HW_STATE_ERROR "error"
4747

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

205+
attr.id = SAI_TUNNEL_ATTR_LOOPBACK_PACKET_ACTION;
206+
attr.value.s32 = SAI_PACKET_ACTION_DROP;
207+
tunnel_attrs.push_back(attr);
208+
205209
if (p_src_ip != nullptr)
206210
{
207211
attr.id = SAI_TUNNEL_ATTR_ENCAP_SRC_IP;
@@ -343,7 +347,7 @@ bool MuxCable::stateActive()
343347
return false;
344348
}
345349

346-
if (!aclHandler(port.m_port_id, false))
350+
if (!aclHandler(port.m_port_id, mux_name_, false))
347351
{
348352
SWSS_LOG_INFO("Remove ACL drop rule failed for %s", mux_name_.c_str());
349353
return false;
@@ -373,7 +377,7 @@ bool MuxCable::stateStandby()
373377
return false;
374378
}
375379

376-
if (!aclHandler(port.m_port_id))
380+
if (!aclHandler(port.m_port_id, mux_name_))
377381
{
378382
SWSS_LOG_INFO("Add ACL drop rule failed for %s", mux_name_.c_str());
379383
return false;
@@ -416,6 +420,7 @@ void MuxCable::setState(string new_state)
416420
}
417421

418422
st_chg_in_progress_ = false;
423+
st_chg_failed_ = false;
419424
SWSS_LOG_INFO("Changed state to %s", new_state.c_str());
420425

421426
return;
@@ -429,11 +434,11 @@ string MuxCable::getState()
429434
return (muxStateValToString.at(state_));
430435
}
431436

432-
bool MuxCable::aclHandler(sai_object_id_t port, bool add)
437+
bool MuxCable::aclHandler(sai_object_id_t port, string alias, bool add)
433438
{
434439
if (add)
435440
{
436-
acl_handler_ = make_shared<MuxAclHandler>(port);
441+
acl_handler_ = make_shared<MuxAclHandler>(port, alias);
437442
}
438443
else
439444
{
@@ -685,16 +690,18 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey)
685690

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

688-
MuxAclHandler::MuxAclHandler(sai_object_id_t port)
693+
MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias)
689694
{
690695
SWSS_LOG_ENTER();
691696

692697
// There is one handler instance per MUX port
693-
acl_table_type_t table_type = ACL_TABLE_MUX;
698+
acl_table_type_t table_type = ACL_TABLE_DROP;
694699
string table_name = MUX_ACL_TABLE_NAME;
695700
string rule_name = MUX_ACL_RULE_NAME;
696701

697702
port_ = port;
703+
alias_ = alias;
704+
698705
auto found = acl_table_.find(table_name);
699706
if (found == acl_table_.end())
700707
{
@@ -709,20 +716,44 @@ MuxAclHandler::MuxAclHandler(sai_object_id_t port)
709716
else
710717
{
711718
SWSS_LOG_NOTICE("Binding port %" PRIx64 "", port);
712-
// Otherwise just bind ACL table with the port
713-
found->second.bind(port);
719+
720+
AclRule* rule = gAclOrch->getAclRule(table_name, rule_name);
721+
if (rule == nullptr)
722+
{
723+
shared_ptr<AclRuleMux> newRule =
724+
make_shared<AclRuleMux>(gAclOrch, rule_name, table_name, table_type);
725+
createMuxAclRule(newRule, table_name);
726+
}
727+
else
728+
{
729+
gAclOrch->updateAclRule(table_name, rule_name, MATCH_IN_PORTS, &port, RULE_OPER_ADD);
730+
}
714731
}
715732
}
716733

717734
MuxAclHandler::~MuxAclHandler(void)
718735
{
719736
SWSS_LOG_ENTER();
720737
string table_name = MUX_ACL_TABLE_NAME;
738+
string rule_name = MUX_ACL_RULE_NAME;
721739

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

724-
auto found = acl_table_.find(table_name);
725-
found->second.unbind(port_);
742+
AclRule* rule = gAclOrch->getAclRule(table_name, rule_name);
743+
if (rule == nullptr)
744+
{
745+
SWSS_LOG_THROW("ACL Rule does not exist for port %s, rule %s", alias_.c_str(), rule_name.c_str());
746+
}
747+
748+
vector<sai_object_id_t> port_set = rule->getInPorts();
749+
if ((port_set.size() == 1) && (port_set[0] == port_))
750+
{
751+
gAclOrch->removeAclRule(table_name, rule_name);
752+
}
753+
else
754+
{
755+
gAclOrch->updateAclRule(table_name, rule_name, MATCH_IN_PORTS, &port_, RULE_OPER_DELETE);
756+
}
726757
}
727758

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

738769
AclTable& acl_table = inserted.first->second;
739-
acl_table.type = ACL_TABLE_MUX;
770+
771+
sai_object_id_t table_oid = gAclOrch->getTableById(strTable);
772+
if (table_oid != SAI_NULL_OBJECT_ID)
773+
{
774+
// DROP ACL table is already created
775+
SWSS_LOG_NOTICE("ACL table %s exists, reuse the same", strTable.c_str());
776+
acl_table = *(gAclOrch->getTableByOid(table_oid));
777+
return;
778+
}
779+
780+
acl_table.type = ACL_TABLE_DROP;
740781
acl_table.id = strTable;
741782
acl_table.link(port);
742783
acl_table.stage = ACL_STAGE_INGRESS;
@@ -753,6 +794,11 @@ void MuxAclHandler::createMuxAclRule(shared_ptr<AclRuleMux> rule, string strTabl
753794
attr_value = "999";
754795
rule->validateAddPriority(attr_name, attr_value);
755796

797+
// Add MATCH_IN_PORTS as match criteria for ingress table
798+
attr_name = MATCH_IN_PORTS;
799+
attr_value = alias_;
800+
rule->validateAddMatch(attr_name, attr_value);
801+
756802
attr_name = ACTION_PACKET_ACTION;
757803
attr_value = PACKET_ACTION_DROP;
758804
rule->validateAddAction(attr_name, attr_value);

‎orchagent/muxorch.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class MuxStateOrch;
3838
class MuxAclHandler
3939
{
4040
public:
41-
MuxAclHandler(sai_object_id_t port);
41+
MuxAclHandler(sai_object_id_t port, string alias);
4242
~MuxAclHandler(void);
4343

4444
private:
@@ -48,6 +48,7 @@ class MuxAclHandler
4848
// class shared dict: ACL table name -> ACL table
4949
static std::map<std::string, AclTable> acl_table_;
5050
sai_object_id_t port_ = SAI_NULL_OBJECT_ID;
51+
string alias_;
5152
};
5253

5354
// IP to nexthop index mapping
@@ -101,7 +102,7 @@ class MuxCable
101102
bool stateInitActive();
102103
bool stateStandby();
103104

104-
bool aclHandler(sai_object_id_t, bool add = true);
105+
bool aclHandler(sai_object_id_t port, string alias, bool add = true);
105106
bool nbrHandler(bool enable, bool update_routes = true);
106107

107108
string mux_name_;

‎orchagent/pfcactionhandler.cpp

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,
226226
SWSS_LOG_ENTER();
227227

228228
acl_table_type_t table_type;
229-
sai_object_id_t table_oid;
230229

231230
string queuestr = to_string(queueId);
232231
m_strRule = "Rule_PfcWdAclHandler_" + queuestr;
@@ -244,18 +243,15 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,
244243
}
245244
else
246245
{
247-
table_oid = gAclOrch->getTableById(m_strIngressTable);
248-
map<sai_object_id_t, AclTable> table_map = gAclOrch->getAclTables();
249-
auto rule_iter = table_map[table_oid].rules.find(m_strRule);
250-
251-
if (rule_iter == table_map[table_oid].rules.end())
246+
AclRule* rule = gAclOrch->getAclRule(m_strIngressTable, m_strRule);
247+
if (rule == nullptr)
252248
{
253249
shared_ptr<AclRulePfcwd> newRule = make_shared<AclRulePfcwd>(gAclOrch, m_strRule, m_strIngressTable, table_type);
254250
createPfcAclRule(newRule, queueId, m_strIngressTable, port);
255251
}
256252
else
257253
{
258-
gAclOrch->updateAclRule(rule_iter->second, m_strIngressTable, MATCH_IN_PORTS, &port, RULE_OPER_ADD);
254+
gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_ADD);
259255
}
260256
}
261257

@@ -281,11 +277,13 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
281277
{
282278
SWSS_LOG_ENTER();
283279

284-
sai_object_id_t table_oid = gAclOrch->getTableById(m_strIngressTable);
285-
map<sai_object_id_t, AclTable> table_map = gAclOrch->getAclTables();
286-
auto rule_iter = table_map[table_oid].rules.find(m_strRule);
280+
AclRule* rule = gAclOrch->getAclRule(m_strIngressTable, m_strRule);
281+
if (rule == nullptr)
282+
{
283+
SWSS_LOG_THROW("ACL Rule does not exist for rule %s", m_strRule.c_str());
284+
}
287285

288-
vector<sai_object_id_t> port_set = rule_iter->second->getInPorts();
286+
vector<sai_object_id_t> port_set = rule->getInPorts();
289287
sai_object_id_t port = getPort();
290288

291289
if ((port_set.size() == 1) && (port_set[0] == port))
@@ -294,7 +292,7 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
294292
}
295293
else
296294
{
297-
gAclOrch->updateAclRule(rule_iter->second, m_strIngressTable, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
295+
gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
298296
}
299297

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

325323
AclTable& aclTable = inserted.first->second;
324+
325+
sai_object_id_t table_oid = gAclOrch->getTableById(strTable);
326+
if (ingress && table_oid != SAI_NULL_OBJECT_ID)
327+
{
328+
// DROP ACL table is already created
329+
SWSS_LOG_NOTICE("ACL table %s exists, reuse the same", strTable.c_str());
330+
aclTable = *(gAclOrch->getTableByOid(table_oid));
331+
return;
332+
}
333+
326334
aclTable.link(port);
327335
aclTable.id = strTable;
328336

0 commit comments

Comments
 (0)