Skip to content

Commit

Permalink
[ACL] Match TCP protocol while matching TCP_FLAG (#1854)
Browse files Browse the repository at this point in the history 
* Match TCP protocol while matching TCP_FLAG

Signed-off-by: bingwang <wang.bing@microsoft.com>
  • Loading branch information
@bingwang-ms
bingwang-ms authored Aug 20, 2021
1 parent ed867b1 commit 756471a
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 26 deletions.
60 changes: 47 additions & 13 deletions orchagent/aclorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ extern CrmOrch *gCrmOrch;
#define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID
#define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID

const int TCP_PROTOCOL_NUM = 6; // TCP protocol number

acl_rule_attr_lookup_t aclMatchLookup =
{
{ MATCH_IN_PORTS, SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS },
Expand Down Expand Up @@ -645,7 +647,7 @@ void AclRule::updateInPorts()
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)
{
Expand Down Expand Up @@ -1378,14 +1380,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 @@ -2985,11 +2987,11 @@ AclRule* AclOrch::getAclRule(string table_id, string rule_id)
bool AclOrch::updateAclRule(string table_id, string rule_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)
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;
Expand All @@ -3002,29 +3004,29 @@ bool AclOrch::updateAclRule(string table_id, string rule_id, string attr_name, v
return false;
}

switch (aclMatchLookup[attr_name])
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_it->second->getInPorts();

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

for (const auto& port_iter: in_ports)
{
Port p;
Expand Down Expand Up @@ -3277,14 +3279,22 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
it = consumer.m_toSync.erase(it);
return;
}

bool bHasTCPFlag = false;
bool bHasIPProtocol = false;
for (const auto& itr : kfvFieldsValues(t))
{
string attr_name = to_upper(fvField(itr));
string attr_value = fvValue(itr);

SWSS_LOG_INFO("ATTRIBUTE: %s %s", attr_name.c_str(), attr_value.c_str());

if (attr_name == MATCH_TCP_FLAGS)
{
bHasTCPFlag = true;
}
if (attr_name == MATCH_IP_PROTOCOL || attr_name == MATCH_NEXT_HEADER)
{
bHasIPProtocol = true;
}
if (newRule->validateAddPriority(attr_name, attr_value))
{
SWSS_LOG_INFO("Added priority attribute");
Expand All @@ -3304,6 +3314,30 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
break;
}
}
// If acl rule is to match TCP_FLAGS, and IP_PROTOCOL(NEXT_HEADER) is not set
// we set IP_PROTOCOL(NEXT_HEADER) to 6 to match TCP explicitly
if (bHasTCPFlag && !bHasIPProtocol)
{
string attr_name;
if (type == ACL_TABLE_MIRRORV6 || type == ACL_TABLE_L3V6)
{
attr_name = MATCH_NEXT_HEADER;
}
else
{
attr_name = MATCH_IP_PROTOCOL;

}
string attr_value = std::to_string(TCP_PROTOCOL_NUM);
if (newRule->validateAddMatch(attr_name, attr_value))
{
SWSS_LOG_INFO("Automatically added match attribute '%s : %s'", attr_name.c_str(), attr_value.c_str());
}
else
{
SWSS_LOG_ERROR("Failed to add attribute '%s : %s'", attr_name.c_str(), attr_value.c_str());
}
}

// validate and create ACL rule
if (bAllAttributesOk && newRule->validate())
Expand Down
48 changes: 35 additions & 13 deletions tests/test_acl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,23 @@ def test_AclRuleIpProtocol(self, dvs_acl, l3_acl_table):
dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME)
dvs_acl.verify_no_acl_rules()

def test_AclRuleTCPProtocolAppendedForTCPFlags(self, dvs_acl, l3_acl_table):
"""
Verify TCP Protocol number (6) will be appended for ACL rules matching TCP_FLAGS
"""
config_qualifiers = {"TCP_FLAGS": "0x07/0x3f"}
expected_sai_qualifiers = {
"SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS":
dvs_acl.get_simple_qualifier_comparator("7&mask:0x3f"),
"SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL":
dvs_acl.get_simple_qualifier_comparator("6&mask:0xff")
}
dvs_acl.create_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, config_qualifiers)
dvs_acl.verify_acl_rule(expected_sai_qualifiers)

dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME)
dvs_acl.verify_no_acl_rules()

def test_AclRuleNextHeader(self, dvs_acl, l3_acl_table):
config_qualifiers = {"NEXT_HEADER": "6"}

Expand All @@ -98,6 +115,24 @@ def test_AclRuleNextHeader(self, dvs_acl, l3_acl_table):
dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME)
dvs_acl.verify_no_acl_rules()

def test_V6AclRuleNextHeaderAppendedForTCPFlags(self, dvs_acl, l3v6_acl_table):
"""
Verify next heder (6) will be appended for IPv6 ACL rules matching TCP_FLAGS
"""
config_qualifiers = {"TCP_FLAGS": "0x07/0x3f"}
expected_sai_qualifiers = {
"SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS":
dvs_acl.get_simple_qualifier_comparator("7&mask:0x3f"),
"SAI_ACL_ENTRY_ATTR_FIELD_IPV6_NEXT_HEADER":
dvs_acl.get_simple_qualifier_comparator("6&mask:0xff")
}

dvs_acl.create_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME, config_qualifiers)
dvs_acl.verify_acl_rule(expected_sai_qualifiers)

dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME)
dvs_acl.verify_no_acl_rules()

def test_AclRuleInOutPorts(self, dvs_acl, l3_acl_table):
config_qualifiers = {
"IN_PORTS": "Ethernet0,Ethernet4",
Expand Down Expand Up @@ -263,19 +298,6 @@ def test_V6AclRuleL4DstPort(self, dvs_acl, l3v6_acl_table):
dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME)
dvs_acl.verify_no_acl_rules()

def test_V6AclRuleTCPFlags(self, dvs_acl, l3v6_acl_table):
config_qualifiers = {"TCP_FLAGS": "0x07/0x3f"}
expected_sai_qualifiers = {
"SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS":
dvs_acl.get_simple_qualifier_comparator("7&mask:0x3f")
}

dvs_acl.create_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME, config_qualifiers)
dvs_acl.verify_acl_rule(expected_sai_qualifiers)

dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME)
dvs_acl.verify_no_acl_rules()

def test_V6AclRuleL4SrcPortRange(self, dvs_acl, l3v6_acl_table):
config_qualifiers = {"L4_SRC_PORT_RANGE": "1-100"}
expected_sai_qualifiers = {
Expand Down

0 comments on commit 756471a

Please sign in to comment.