Skip to content

Commit

Permalink
Revert "[ACL] Write ACL table/rule creation status into STATE_DB (son…
Browse files Browse the repository at this point in the history 
…ic-net#2662)"

This reverts commit 9d38fbc.
  • Loading branch information
@keboliu
keboliu committed Apr 1, 2023
1 parent 7a25a60 commit e0aa24f
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 305 deletions.
100 changes: 0 additions & 100 deletions orchagent/aclorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,14 +375,6 @@ static map<sai_acl_counter_attr_t, sai_acl_counter_attr_t> aclCounterLookup =
{SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT, SAI_ACL_COUNTER_ATTR_PACKETS},
};

static map<AclObjectStatus, string> aclObjectStatusLookup =
{
{AclObjectStatus::ACTIVE, "Active"},
{AclObjectStatus::INACTIVE, "Inactive"},
{AclObjectStatus::PENDING_CREATION, "Pending creation"},
{AclObjectStatus::PENDING_REMOVAL, "Pending removal"}
};

static sai_acl_table_attr_t AclEntryFieldToAclTableField(sai_acl_entry_attr_t attr)
{
if (!IS_ATTR_ID_IN_RANGE(attr, ACL_ENTRY, FIELD))
Expand Down Expand Up @@ -2908,10 +2900,6 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
{
SWSS_LOG_ENTER();

// Clear ACL_TABLE and ACL_RULE status from STATE_DB
removeAllAclTableStatus();
removeAllAclRuleStatus();

// TODO: Query SAI to get mirror table capabilities
// Right now, verified platforms that support mirroring IPv6 packets are
// Broadcom and Mellanox. Virtual switch is also supported for testing
Expand Down Expand Up @@ -3419,8 +3407,6 @@ AclOrch::AclOrch(vector<TableConnector>& connectors, DBConnector* stateDb, Switc
PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch, DTelOrch *dtelOrch) :
Orch(connectors),
m_aclStageCapabilityTable(stateDb, STATE_ACL_STAGE_CAPABILITY_TABLE_NAME),
m_aclTableStateTable(stateDb, STATE_ACL_TABLE_TABLE_NAME),
m_aclRuleStateTable(stateDb, STATE_ACL_RULE_TABLE_NAME),
m_switchOrch(switchOrch),
m_mirrorOrch(mirrorOrch),
m_neighOrch(neighOrch),
Expand Down Expand Up @@ -4244,8 +4230,6 @@ void AclOrch::doAclTableTask(Consumer &consumer)
{
SWSS_LOG_NOTICE("Successfully updated existing ACL table %s",
table_id.c_str());
// Mark ACL table as ACTIVE
setAclTableStatus(table_id, AclObjectStatus::ACTIVE);
it = consumer.m_toSync.erase(it);
}
else
Expand All @@ -4258,41 +4242,24 @@ void AclOrch::doAclTableTask(Consumer &consumer)
else
{
if (addAclTable(newTable))
{
// Mark ACL table as ACTIVE
setAclTableStatus(table_id, AclObjectStatus::ACTIVE);
it = consumer.m_toSync.erase(it);
}
else
{
setAclTableStatus(table_id, AclObjectStatus::PENDING_CREATION);
it++;
}
}
}
else
{
it = consumer.m_toSync.erase(it);
// Mark the ACL table as inactive if the configuration is invalid
setAclTableStatus(table_id, AclObjectStatus::INACTIVE);
SWSS_LOG_ERROR("Failed to create ACL table %s, invalid configuration",
table_id.c_str());
}
}
else if (op == DEL_COMMAND)
{
if (removeAclTable(table_id))
{
// Remove ACL table status from STATE_DB
removeAclTableStatus(table_id);
it = consumer.m_toSync.erase(it);
}
else
{
// Set the status of ACL_TABLE to pending removal if removeAclTable returns error
setAclTableStatus(table_id, AclObjectStatus::PENDING_REMOVAL);
it++;
}
}
else
{
Expand Down Expand Up @@ -4432,37 +4399,22 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
if (bAllAttributesOk && newRule->validate())
{
if (addAclRule(newRule, table_id))
{
setAclRuleStatus(table_id, rule_id, AclObjectStatus::ACTIVE);
it = consumer.m_toSync.erase(it);
}
else
{
setAclRuleStatus(table_id, rule_id, AclObjectStatus::PENDING_CREATION);
it++;
}
}
else
{
it = consumer.m_toSync.erase(it);
// Mark the rule inactive if the configuration is invalid
setAclRuleStatus(table_id, rule_id, AclObjectStatus::INACTIVE);
SWSS_LOG_ERROR("Failed to create ACL rule. Rule configuration is invalid");
}
}
else if (op == DEL_COMMAND)
{
if (removeAclRule(table_id, rule_id))
{
removeAclRuleStatus(table_id, rule_id);
it = consumer.m_toSync.erase(it);
}
else
{
// Mark pending removal status if removeAclRule returns error
setAclRuleStatus(table_id, rule_id, AclObjectStatus::PENDING_REMOVAL);
it++;
}
}
else
{
Expand Down Expand Up @@ -4818,55 +4770,3 @@ bool AclOrch::getAclBindPortId(Port &port, sai_object_id_t &port_id)

return true;
}

// Set the status of ACL table in STATE_DB
void AclOrch::setAclTableStatus(string table_name, AclObjectStatus status)
{
vector<FieldValueTuple> fvVector;
fvVector.emplace_back("status", aclObjectStatusLookup[status]);
m_aclTableStateTable.set(table_name, fvVector);
}

// Remove the status record of given ACL table from STATE_DB
void AclOrch::removeAclTableStatus(string table_name)
{
m_aclTableStateTable.del(table_name);
}

// Set the status of ACL rule in STATE_DB
void AclOrch::setAclRuleStatus(string table_name, string rule_name, AclObjectStatus status)
{
vector<FieldValueTuple> fvVector;
fvVector.emplace_back("status", aclObjectStatusLookup[status]);
m_aclRuleStateTable.set(table_name + string("|") + rule_name, fvVector);
}

// Remove the status record of given ACL rule from STATE_DB
void AclOrch::removeAclRuleStatus(string table_name, string rule_name)
{
m_aclRuleStateTable.del(table_name + string("|") + rule_name);
}

// Remove all ACL table status from STATE_DB
void AclOrch::removeAllAclTableStatus()
{
vector<string> keys;
m_aclTableStateTable.getKeys(keys);

for (auto key : keys)
{
m_aclTableStateTable.del(key);
}
}

// Remove all ACL rule status from STATE_DB
void AclOrch::removeAllAclRuleStatus()
{
vector<string> keys;
m_aclRuleStateTable.getKeys(keys);
for (auto key : keys)
{
m_aclRuleStateTable.del(key);
}
}

20 changes: 0 additions & 20 deletions orchagent/aclorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,6 @@

#define ACL_COUNTER_FLEX_COUNTER_GROUP "ACL_STAT_COUNTER"

enum AclObjectStatus
{
ACTIVE = 0,
INACTIVE,
PENDING_CREATION,
PENDING_REMOVAL
};

struct AclActionCapabilities
{
set<sai_acl_action_type_t> actionList;
Expand Down Expand Up @@ -556,15 +548,6 @@ class AclOrch : public Orch, public Observer

string generateAclRuleIdentifierInCountersDb(const AclRule& rule) const;

void setAclTableStatus(string table_name, AclObjectStatus status);
void setAclRuleStatus(string table_name, string rule_name, AclObjectStatus status);

void removeAclTableStatus(string table_name);
void removeAclRuleStatus(string table_name, string rule_name);

void removeAllAclTableStatus();
void removeAllAclRuleStatus();

map<sai_object_id_t, AclTable> m_AclTables;
// TODO: Move all ACL tables into one map: name -> instance
map<string, AclTable> m_ctrlAclTables;
Expand All @@ -575,9 +558,6 @@ class AclOrch : public Orch, public Observer

Table m_aclStageCapabilityTable;

Table m_aclTableStateTable;
Table m_aclRuleStateTable;

map<acl_stage_type_t, string> m_mirrorTableId;
map<acl_stage_type_t, string> m_mirrorV6TableId;

Expand Down
45 changes: 1 addition & 44 deletions tests/dvslib/dvs_acl.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Utilities for interacting with ACLs when writing VS tests."""
from typing import Callable, Dict, List
from swsscommon import swsscommon


class DVSAcl:
"""Manage ACL tables and rules on the virtual switch."""
Expand All @@ -18,9 +18,6 @@ class DVSAcl:
ADB_ACL_GROUP_MEMBER_TABLE_NAME = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP_MEMBER"
ADB_ACL_COUNTER_TABLE_NAME = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_COUNTER"

STATE_DB_ACL_TABLE_TABLE_NAME = "ACL_TABLE_TABLE"
STATE_DB_ACL_RULE_TABLE_NAME = "ACL_RULE_TABLE"

ADB_ACL_STAGE_LOOKUP = {
"ingress": "SAI_ACL_STAGE_INGRESS",
"egress": "SAI_ACL_STAGE_EGRESS"
Expand Down Expand Up @@ -720,43 +717,3 @@ def _check_acl_entry_counters_map(self, acl_entry_oid: str):
rule_to_counter_map = self.counters_db.get_entry("ACL_COUNTER_RULE_MAP", "")
counter_to_rule_map = {v: k for k, v in rule_to_counter_map.items()}
assert counter_oid in counter_to_rule_map

def verify_acl_table_status(
self,
acl_table_name,
expected_status
) -> None:
"""Verify that the STATE_DB status of ACL table is as expected.
Args:
acl_table_name: The name of ACL table to check
expected_status: The expected status in STATE_DB
"""
if expected_status:
fvs = self.state_db.wait_for_entry(self.STATE_DB_ACL_TABLE_TABLE_NAME, acl_table_name)
assert len(fvs) > 0
assert (fvs['status'] == expected_status)
else:
self.state_db.wait_for_deleted_entry(self.STATE_DB_ACL_TABLE_TABLE_NAME, acl_table_name)

def verify_acl_rule_status(
self,
acl_table_name,
acl_rule_name,
expected_status
) -> None:
"""Verify that the STATE_DB status of ACL rule is as expected.
Args:
acl_table_name: The name of ACL table to check
acl_rule_name: The name of ACL rule to check
expected_status: The expected status in STATE_DB
"""
key = acl_table_name + "|" + acl_rule_name
if expected_status:
fvs = self.state_db.wait_for_entry(self.STATE_DB_ACL_RULE_TABLE_NAME, key)
assert len(fvs) > 0
assert (fvs['status'] == expected_status)
else:
self.state_db.wait_for_deleted_entry(self.STATE_DB_ACL_TABLE_TABLE_NAME, key)

Loading

0 comments on commit e0aa24f

Please sign in to comment.