Skip to content

Commit

Permalink
[aclorch] Fix and simplify DTel watchlist tables and entries (sonic-n…
Browse files Browse the repository at this point in the history
…et#2155)

* Fix DTel acl rule creation

The significant rewrite of aclorch when adding ACL_TABLE_TYPE
configuration caused a bug that prevents configuration of any
DTel rules. This is due to use of an incorrect set of enum
mappings while determining which type of AclRule to create.
  • Loading branch information
mickeyspiegel authored Jun 17, 2022
1 parent 59f77ea commit 700492f
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 115 deletions.
133 changes: 33 additions & 100 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1455,23 +1455,14 @@ shared_ptr<AclRule> AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOr
{
return make_shared<AclRulePacket>(acl, rule, table);
}
else if (aclDTelFlowOpTypeLookup.find(action) != aclDTelFlowOpTypeLookup.cend())
else if (aclDTelActionLookup.find(action) != aclDTelActionLookup.cend())
{
if (!dtel)
{
throw runtime_error("DTel feature is not enabled. Watchlists cannot be configured");
}

if (action == ACTION_DTEL_DROP_REPORT_ENABLE ||
action == ACTION_DTEL_TAIL_DROP_REPORT_ENABLE ||
action == ACTION_DTEL_REPORT_ALL_PACKETS)
{
return make_shared<AclRuleDTelDropWatchListEntry>(acl, dtel, rule, table);
}
else
{
return make_shared<AclRuleDTelFlowWatchListEntry>(acl, dtel, rule, table);
}
return make_shared<AclRuleDTelWatchListEntry>(acl, dtel, rule, table);
}
}

Expand Down Expand Up @@ -2447,13 +2438,13 @@ bool AclTable::clear()
return true;
}

AclRuleDTelFlowWatchListEntry::AclRuleDTelFlowWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table) :
AclRuleDTelWatchListEntry::AclRuleDTelWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table) :
AclRule(aclOrch, rule, table),
m_pDTelOrch(dtel)
{
}

bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string attr_val)
bool AclRuleDTelWatchListEntry::validateAddAction(string attr_name, string attr_val)
{
SWSS_LOG_ENTER();

Expand Down Expand Up @@ -2535,7 +2526,7 @@ bool AclRuleDTelFlowWatchListEntry::validateAddAction(string attr_name, string a
return setAction(aclDTelActionLookup[attr_name], actionData);
}

bool AclRuleDTelFlowWatchListEntry::validate()
bool AclRuleDTelWatchListEntry::validate()
{
SWSS_LOG_ENTER();

Expand All @@ -2552,19 +2543,19 @@ bool AclRuleDTelFlowWatchListEntry::validate()
return true;
}

bool AclRuleDTelFlowWatchListEntry::createRule()
bool AclRuleDTelWatchListEntry::createRule()
{
SWSS_LOG_ENTER();

return activate();
}

bool AclRuleDTelFlowWatchListEntry::removeRule()
bool AclRuleDTelWatchListEntry::removeRule()
{
return deactivate();
}

bool AclRuleDTelFlowWatchListEntry::activate()
bool AclRuleDTelWatchListEntry::activate()
{
SWSS_LOG_ENTER();

Expand All @@ -2581,7 +2572,7 @@ bool AclRuleDTelFlowWatchListEntry::activate()
return AclRule::createRule();
}

bool AclRuleDTelFlowWatchListEntry::deactivate()
bool AclRuleDTelWatchListEntry::deactivate()
{
SWSS_LOG_ENTER();

Expand Down Expand Up @@ -2612,7 +2603,7 @@ bool AclRuleDTelFlowWatchListEntry::deactivate()
return true;
}

void AclRuleDTelFlowWatchListEntry::onUpdate(SubjectType type, void *cntx)
void AclRuleDTelWatchListEntry::onUpdate(SubjectType type, void *cntx)
{
sai_acl_action_data_t actionData;
sai_object_id_t session_oid = SAI_NULL_OBJECT_ID;
Expand Down Expand Up @@ -2673,72 +2664,19 @@ void AclRuleDTelFlowWatchListEntry::onUpdate(SubjectType type, void *cntx)
}
}

bool AclRuleDTelFlowWatchListEntry::update(const AclRule& rule)
bool AclRuleDTelWatchListEntry::update(const AclRule& rule)
{
auto dtelDropWathcListRule = dynamic_cast<const AclRuleDTelFlowWatchListEntry*>(&rule);
if (!dtelDropWathcListRule)
auto dtelWatchListRule = dynamic_cast<const AclRuleDTelWatchListEntry*>(&rule);
if (!dtelWatchListRule)
{
SWSS_LOG_ERROR("Cannot update DTEL flow watch list rule with a rule of a different type");
SWSS_LOG_ERROR("Cannot update DTEL watch list rule with a rule of a different type");
return false;
}

SWSS_LOG_ERROR("Updating DTEL flow watch list rule is currently not implemented");
SWSS_LOG_ERROR("Updating DTEL watch list rule is currently not implemented");
return false;
}

AclRuleDTelDropWatchListEntry::AclRuleDTelDropWatchListEntry(AclOrch *aclOrch, DTelOrch *dtel, string rule, string table) :
AclRule(aclOrch, rule, table),
m_pDTelOrch(dtel)
{
}

bool AclRuleDTelDropWatchListEntry::validateAddAction(string attr_name, string attr_val)
{
SWSS_LOG_ENTER();

if (!m_pDTelOrch)
{
return false;
}

sai_acl_action_data_t actionData;
string attr_value = to_upper(attr_val);

if (attr_name != ACTION_DTEL_DROP_REPORT_ENABLE &&
attr_name != ACTION_DTEL_TAIL_DROP_REPORT_ENABLE &&
attr_name != ACTION_DTEL_REPORT_ALL_PACKETS)
{
return false;
}

actionData.parameter.booldata = (attr_value == DTEL_ENABLED) ? true : false;
actionData.enable = (attr_value == DTEL_ENABLED) ? true : false;

return setAction(aclDTelActionLookup[attr_name], actionData);
}

bool AclRuleDTelDropWatchListEntry::validate()
{
SWSS_LOG_ENTER();

if (!m_pDTelOrch)
{
return false;
}

if ((m_rangeConfig.empty() && m_matches.empty()) || m_actions.size() == 0)
{
return false;
}

return true;
}

void AclRuleDTelDropWatchListEntry::onUpdate(SubjectType, void *)
{
// Do nothing
}

AclRange::AclRange(sai_acl_range_type_t type, sai_object_id_t oid, int min, int max):
m_oid(oid), m_refCnt(0), m_min(min), m_max(max), m_type(type)
{
Expand Down Expand Up @@ -4619,11 +4557,10 @@ void AclOrch::createDTelWatchListTables()

AclTableTypeBuilder builder;

AclTable flowWLTable(this, TABLE_TYPE_DTEL_FLOW_WATCHLIST);
AclTable dropWLTable(this, TABLE_TYPE_DTEL_DROP_WATCHLIST);
AclTable dtelWLTable(this, TABLE_TYPE_DTEL_FLOW_WATCHLIST);

flowWLTable.validateAddStage(ACL_STAGE_INGRESS);
flowWLTable.validateAddType(builder
dtelWLTable.validateAddStage(ACL_STAGE_INGRESS);
dtelWLTable.validateAddType(builder
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP))
Expand All @@ -4635,39 +4572,35 @@ void AclOrch::createDTelWatchListTables()
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_INNER_ETHER_TYPE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_INNER_SRC_IP))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_INNER_DST_IP))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DSCP))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER))
.withAction(SAI_ACL_ACTION_TYPE_ACL_DTEL_FLOW_OP)
.withAction(SAI_ACL_ACTION_TYPE_DTEL_INT_SESSION)
.withAction(SAI_ACL_ACTION_TYPE_DTEL_REPORT_ALL_PACKETS)
.withAction(SAI_ACL_ACTION_TYPE_DTEL_FLOW_SAMPLE_PERCENT)
.build()
);
flowWLTable.setDescription("Dataplane Telemetry Flow Watchlist table");

dropWLTable.validateAddStage(ACL_STAGE_INGRESS);
dropWLTable.validateAddType(builder
.withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH)
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DST_IP))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT))
.withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL))
.withAction(SAI_ACL_ACTION_TYPE_DTEL_DROP_REPORT_ENABLE)
.withAction(SAI_ACL_ACTION_TYPE_DTEL_TAIL_DROP_REPORT_ENABLE)
.withAction(SAI_ACL_ACTION_TYPE_DTEL_REPORT_ALL_PACKETS)
.withAction(SAI_ACL_ACTION_TYPE_DTEL_FLOW_SAMPLE_PERCENT)
.build()
);
dropWLTable.setDescription("Dataplane Telemetry Drop Watchlist table");
dtelWLTable.setDescription("Dataplane Telemetry Watchlist table");

addAclTable(flowWLTable);
addAclTable(dropWLTable);
addAclTable(dtelWLTable);
}

void AclOrch::deleteDTelWatchListTables()
{
SWSS_LOG_ENTER();

removeAclTable(TABLE_TYPE_DTEL_FLOW_WATCHLIST);
removeAclTable(TABLE_TYPE_DTEL_DROP_WATCHLIST);
}

void AclOrch::registerFlexCounter(const AclRule& rule)
Expand Down
15 changes: 2 additions & 13 deletions orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,10 @@ class AclRuleMirror: public AclRule
MirrorOrch *m_pMirrorOrch {nullptr};
};

class AclRuleDTelFlowWatchListEntry: public AclRule
class AclRuleDTelWatchListEntry: public AclRule
{
public:
AclRuleDTelFlowWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table);
AclRuleDTelWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table);
bool validateAddAction(string attr_name, string attr_value);
bool validate();
bool createRule();
Expand All @@ -360,17 +360,6 @@ class AclRuleDTelFlowWatchListEntry: public AclRule
bool INT_session_valid;
};

class AclRuleDTelDropWatchListEntry: public AclRule
{
public:
AclRuleDTelDropWatchListEntry(AclOrch *m_pAclOrch, DTelOrch *m_pDTelOrch, string rule, string table);
bool validateAddAction(string attr_name, string attr_value);
bool validate();
void onUpdate(SubjectType, void *) override;
protected:
DTelOrch *m_pDTelOrch;
};

class AclTable
{
public:
Expand Down
1 change: 0 additions & 1 deletion orchagent/acltable.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ extern "C" {
#define TABLE_TYPE_PFCWD "PFCWD"
#define TABLE_TYPE_CTRLPLANE "CTRLPLANE"
#define TABLE_TYPE_DTEL_FLOW_WATCHLIST "DTEL_FLOW_WATCHLIST"
#define TABLE_TYPE_DTEL_DROP_WATCHLIST "DTEL_DROP_WATCHLIST"
#define TABLE_TYPE_MCLAG "MCLAG"
#define TABLE_TYPE_MUX "MUX"
#define TABLE_TYPE_DROP "DROP"
Expand Down
106 changes: 105 additions & 1 deletion tests/test_dtel.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,111 @@ def test_DtelQueueReportAttribs(self, dvs, testlog):
assert False

tbl._del("Ethernet0|0")


def test_DtelFlowWatchlist(self, dvs, testlog):
self.db = swsscommon.DBConnector(4, dvs.redis_sock, 0)
self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
self.table = "DTEL_FLOW_WATCHLIST"

fields_1=[("PRIORITY", "30"),
("ETHER_TYPE", "0x800"),
("L4_DST_PORT", "1674"),
("FLOW_OP", "POSTCARD"),
("REPORT_ALL_PACKETS", "FALSE"),
("DROP_REPORT_ENABLE", "TRUE"),
("TAIL_DROP_REPORT_ENABLE", "TRUE")]
fields_2=[("PRIORITY", "40"),
("ETHER_TYPE", "0x800"),
("L4_DST_PORT", "1674"),
("FLOW_OP", "POSTCARD"),
("REPORT_ALL_PACKETS", "TRUE"),
("DROP_REPORT_ENABLE", "FALSE"),
("TAIL_DROP_REPORT_ENABLE", "FALSE")]
fields_3=[("PRIORITY", "50"),
("ETHER_TYPE", "0x800"),
("L4_DST_PORT", "1674"),
("FLOW_OP", "POSTCARD"),
("REPORT_ALL_PACKETS", "TRUE")]
fields_4=[("PRIORITY", "60"),
("ETHER_TYPE", "0x800"),
("L4_DST_PORT", "1674"),
("REPORT_ALL_PACKETS", "TRUE"),
("DROP_REPORT_ENABLE", "TRUE"),
("TAIL_DROP_REPORT_ENABLE", "TRUE")]
fields_5=[("PRIORITY", "70"),
("ETHER_TYPE", "0x800"),
("L4_DST_PORT", "1674"),
("FLOW_OP", "NOP"),
("REPORT_ALL_PACKETS", "FALSE"),
("DROP_REPORT_ENABLE", "TRUE"),
("TAIL_DROP_REPORT_ENABLE", "TRUE")]
listfield = [fields_1, fields_2, fields_3, fields_4, fields_5]

for field in listfield:
k = listfield.index(field)
rule = "RULE-" + str(k)
self._create_dtel_acl_rule(self.table, rule, field)
self._check_dtel_acl_rule(dvs, rule)
self._remove_dtel_acl_rule(self.table, rule)

def _create_dtel_acl_rule(self, table, rule, field):
tbl = swsscommon.Table(self.db, "ACL_RULE")
fvs = swsscommon.FieldValuePairs(field)
tbl.set(table + "|" + rule, fvs)
time.sleep(1)

def _remove_dtel_acl_rule(self, table, rule):
tbl = swsscommon.Table(self.db, "ACL_RULE")
tbl._del(table + "|" + rule)
time.sleep(1)

def _check_dtel_acl_rule(self, dvs, rule):
time.sleep(1)
atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY")
keys = atbl.getKeys()
acl_entry = [k for k in keys if k not in dvs.asicdb.default_acl_entries]
assert len(acl_entry) != 0
(status, fvs) = atbl.get(acl_entry[0])
value = dict(fvs)
assert status

if rule == "RULE-0":
assert value["SAI_ACL_ENTRY_ATTR_PRIORITY"] == "30"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE"] == "2048&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT"] == "1674&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_ACL_DTEL_FLOW_OP"] == "SAI_ACL_DTEL_FLOW_OP_POSTCARD"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_REPORT_ALL_PACKETS"] == "disabled"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_DROP_REPORT_ENABLE"] == "true"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_TAIL_DROP_REPORT_ENABLE"] == "true"
elif rule == "RULE-1":
assert value["SAI_ACL_ENTRY_ATTR_PRIORITY"] == "40"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE"] == "2048&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT"] == "1674&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_ACL_DTEL_FLOW_OP"] == "SAI_ACL_DTEL_FLOW_OP_POSTCARD"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_REPORT_ALL_PACKETS"] == "true"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_DROP_REPORT_ENABLE"] == "disabled"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_TAIL_DROP_REPORT_ENABLE"] == "disabled"
elif rule == "RULE-2":
assert value["SAI_ACL_ENTRY_ATTR_PRIORITY"] == "50"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE"] == "2048&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT"] == "1674&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_ACL_DTEL_FLOW_OP"] == "SAI_ACL_DTEL_FLOW_OP_POSTCARD"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_REPORT_ALL_PACKETS"] == "true"
elif rule == "RULE-3":
assert value["SAI_ACL_ENTRY_ATTR_PRIORITY"] == "60"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE"] == "2048&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT"] == "1674&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_REPORT_ALL_PACKETS"] == "true"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_DROP_REPORT_ENABLE"] == "true"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_TAIL_DROP_REPORT_ENABLE"] == "true"
elif rule == "RULE-4":
assert value["SAI_ACL_ENTRY_ATTR_PRIORITY"] == "70"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE"] == "2048&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT"] == "1674&mask:0xffff"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_ACL_DTEL_FLOW_OP"] == "SAI_ACL_DTEL_FLOW_OP_NOP"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_REPORT_ALL_PACKETS"] == "disabled"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_DROP_REPORT_ENABLE"] == "true"
assert value["SAI_ACL_ENTRY_ATTR_ACTION_DTEL_TAIL_DROP_REPORT_ENABLE"] == "true"

def test_DtelEventAttribs(self, dvs, testlog):

Expand Down

0 comments on commit 700492f

Please sign in to comment.