diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index a2f5482a36e4..54a57c47ede5 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -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 }, @@ -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) { @@ -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) @@ -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; @@ -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 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; @@ -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"); @@ -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()) diff --git a/tests/test_acl.py b/tests/test_acl.py index 51f4986079f0..d4b317bf55cf 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -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"} @@ -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", @@ -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 = {