Skip to content

Commit 7297d14

Browse files
StephenWangGoogleStephenWangGoogle
authored
Merge pull request sonic-net#3894 from divyagayathri-hcl/acl_vrf_id_oid
[P4Orch] Update ACL VRF ID to use oid instead of u16.
2 parents 1244a7d + d7fc953 commit 7297d14

File tree

4 files changed

+100
-51
lines changed

4 files changed

+100
-51
lines changed

‎orchagent/p4orch/acl_rule_manager.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,6 @@ ReturnCode AclRuleManager::setMatchValue(const acl_entry_attr_union_t attr_name,
860860
case SAI_ACL_ENTRY_ATTR_FIELD_IP_IDENTIFICATION:
861861
case SAI_ACL_ENTRY_ATTR_FIELD_OUTER_VLAN_ID:
862862
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_VLAN_ID:
863-
case SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID:
864863
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_ETHER_TYPE:
865864
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_SRC_PORT:
866865
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_DST_PORT: {
@@ -1030,6 +1029,20 @@ ReturnCode AclRuleManager::setMatchValue(const acl_entry_attr_union_t attr_name,
10301029
value->aclfield.mask.u32 = 0xFFFFFFFF;
10311030
break;
10321031
}
1032+
case SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID: {
1033+
const std::vector<std::string>& value_and_mask =
1034+
tokenize(attr_value, kDataMaskDelimiter);
1035+
const std::string vrf_id = trim(value_and_mask[0]);
1036+
if (vrf_id.empty() || !m_vrfOrch->isVRFexists(vrf_id)) {
1037+
return ReturnCode(StatusCode::SWSS_RC_NOT_FOUND)
1038+
<< "VRF ID " << QuotedVar(vrf_id) << " was not found.";
1039+
}
1040+
value->aclfield.data.oid = m_vrfOrch->getVRFid(vrf_id);
1041+
if (value_and_mask.size() > 1) {
1042+
SWSS_LOG_INFO("Mask ignored for VRF ID.");
1043+
}
1044+
break;
1045+
}
10331046
case SAI_ACL_ENTRY_ATTR_FIELD_IPMC_NPU_META_DST_HIT:
10341047
{
10351048
const std::vector<std::string>& value_and_mask =

‎orchagent/p4orch/acl_util.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,6 @@ bool isDiffMatchFieldValue(const acl_entry_attr_union_t attr_name, const sai_att
929929
case SAI_ACL_ENTRY_ATTR_FIELD_IP_IDENTIFICATION:
930930
case SAI_ACL_ENTRY_ATTR_FIELD_OUTER_VLAN_ID:
931931
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_VLAN_ID:
932-
case SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID:
933932
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_ETHER_TYPE:
934933
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_SRC_PORT:
935934
case SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_DST_PORT: {
@@ -955,6 +954,9 @@ bool isDiffMatchFieldValue(const acl_entry_attr_union_t attr_name, const sai_att
955954
return memcmp(value.aclfield.data.mac, old_value.aclfield.data.mac, sizeof(sai_mac_t)) ||
956955
memcmp(value.aclfield.mask.mac, old_value.aclfield.mask.mac, sizeof(sai_mac_t));
957956
}
957+
case SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID: {
958+
return value.aclfield.data.oid != old_value.aclfield.data.oid;
959+
}
958960
case SAI_ACL_ENTRY_ATTR_FIELD_IPMC_NPU_META_DST_HIT:
959961
{
960962
return value.aclfield.data.booldata != old_value.aclfield.data.booldata;

‎orchagent/p4orch/acl_util.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ static const acl_table_attr_format_lookup_t aclMatchTableAttrFormatLookup = {
540540
{SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER, Format::HEX_STRING},
541541
{SAI_ACL_TABLE_ATTR_FIELD_ROUTE_DST_USER_META, Format::HEX_STRING},
542542
{SAI_ACL_TABLE_ATTR_FIELD_ACL_USER_META, Format::HEX_STRING},
543-
{SAI_ACL_TABLE_ATTR_FIELD_VRF_ID, Format::HEX_STRING},
543+
{SAI_ACL_TABLE_ATTR_FIELD_VRF_ID, Format::STRING},
544544
{SAI_ACL_TABLE_ATTR_FIELD_IPMC_NPU_META_DST_HIT, Format::HEX_STRING},
545545
};
546546

‎orchagent/p4orch/tests/acl_manager_test.cpp‎

Lines changed: 82 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,7 @@ P4AclTableDefinitionAppDbEntry getDefaultAclTableDefAppDbEntry()
637637
app_db_entry.match_field_lookup["inner_vlan_id"] = BuildMatchFieldJsonStrKindSaiField(P4_MATCH_INNER_VLAN_ID);
638638
app_db_entry.match_field_lookup["inner_vlan_cfi"] = BuildMatchFieldJsonStrKindSaiField(P4_MATCH_INNER_VLAN_CFI);
639639
app_db_entry.match_field_lookup["vrf_id"] =
640-
BuildMatchFieldJsonStrKindSaiField(P4_MATCH_VRF_ID, P4_FORMAT_HEX_STRING,
641-
/*bitwidth=*/16);
640+
BuildMatchFieldJsonStrKindSaiField(P4_MATCH_VRF_ID, P4_FORMAT_STRING);
642641
app_db_entry.match_field_lookup["ipmc_table_hit"] =
643642
BuildMatchFieldJsonStrKindSaiField(P4_MATCH_IPMC_TABLE_HIT,
644643
P4_FORMAT_HEX_STRING, /*bitwidth=*/1);
@@ -2721,6 +2720,14 @@ TEST_F(AclManagerTest, CreateAclRuleWithInvalidSaiMatchFails)
27212720
app_db_entry.match_fvs.erase("arp_tpa");
27222721
acl_table->udf_group_attr_index_lookup = saved_udf_group_attr_index_lookup;
27232722

2723+
// ACL rule has invalid VRF ID.
2724+
app_db_entry.match_fvs["vrf_id"] = "invalid";
2725+
acl_rule_key =
2726+
KeyGenerator::generateAclRuleKey(app_db_entry.match_fvs, "100");
2727+
EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND,
2728+
ProcessAddRuleRequest(acl_rule_key, app_db_entry));
2729+
app_db_entry.match_fvs.erase("vrf_id");
2730+
27242731
// ACL rule has undefined match field
27252732
app_db_entry.match_fvs["undefined"] = "1";
27262733
acl_rule_key = KeyGenerator::generateAclRuleKey(app_db_entry.match_fvs, "100");
@@ -2800,7 +2807,7 @@ TEST_F(AclManagerTest, AclRuleWithValidMatchFields)
28002807
app_db_entry.match_fvs["inner_vlan_pri"] = "200";
28012808
app_db_entry.match_fvs["inner_vlan_id"] = "200";
28022809
app_db_entry.match_fvs["inner_vlan_cfi"] = "200";
2803-
app_db_entry.match_fvs["vrf_id"] = "0x777";
2810+
app_db_entry.match_fvs["vrf_id"] = gVrfName;
28042811
app_db_entry.match_fvs["ipmc_table_hit"] = "0x1";
28052812

28062813
const auto &acl_rule_key = KeyGenerator::generateAclRuleKey(app_db_entry.match_fvs, "100");
@@ -2898,8 +2905,9 @@ TEST_F(AclManagerTest, AclRuleWithValidMatchFields)
28982905
EXPECT_EQ(SAI_ACL_IP_FRAG_HEAD, acl_rule->match_fvs[SAI_ACL_ENTRY_ATTR_FIELD_ACL_IP_FRAG].aclfield.data.u32);
28992906
EXPECT_EQ(SAI_PACKET_VLAN_SINGLE_OUTER_TAG,
29002907
acl_rule->match_fvs[SAI_ACL_ENTRY_ATTR_FIELD_PACKET_VLAN].aclfield.data.u32);
2901-
EXPECT_EQ(0x777, acl_rule->match_fvs[SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID].aclfield.data.u16);
2902-
EXPECT_EQ(0xFFFF, acl_rule->match_fvs[SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID].aclfield.mask.u16);
2908+
EXPECT_EQ(
2909+
gVrfOid,
2910+
acl_rule->match_fvs[SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID].aclfield.data.oid);
29032911
EXPECT_EQ(true,
29042912
acl_rule->match_fvs[SAI_ACL_ENTRY_ATTR_FIELD_IPMC_NPU_META_DST_HIT]
29052913
.aclfield.data.booldata);
@@ -5155,12 +5163,13 @@ TEST_F(AclManagerTest, AclRuleVerifyStateTest)
51555163
attributes.push_back(swss::FieldValueTuple{"meter/pir", "200"});
51565164
attributes.push_back(swss::FieldValueTuple{"meter/pburst", "200"});
51575165
attributes.push_back(swss::FieldValueTuple{"controller_metadata", "..."});
5158-
const auto &acl_rule_json_key = "{\"match/ether_type\":\"0x0800\",\"match/"
5159-
"ipv6_dst\":\"fdf8:f53b:82e4::53 & "
5160-
"fdf8:f53b:82e4::53\",\"match/arp_tpa\": \"0xff112231\", "
5161-
"\"match/in_ports\": \"Ethernet1,Ethernet2\", \"match/out_ports\": "
5162-
"\"Ethernet4,Ethernet5\", \"priority\":15,\"match/ipmc_table_hit\":"
5163-
"\"0x1\"}";
5166+
const auto& acl_rule_json_key =
5167+
"{\"match/ether_type\":\"0x0800\",\"match/"
5168+
"ipv6_dst\":\"fdf8:f53b:82e4::53 & "
5169+
"fdf8:f53b:82e4::53\",\"match/arp_tpa\": \"0xff112231\", "
5170+
"\"match/in_ports\": \"Ethernet1,Ethernet2\", \"match/out_ports\": "
5171+
"\"Ethernet4,Ethernet5\", \"priority\":15,\"match/ipmc_table_hit\":"
5172+
"\"0x1\",\"match/vrf_id\":\"b4-traffic\"}";
51645173
const auto &rule_tuple_key = std::string(kAclIngressTableName) + kTableKeyDelimiter + acl_rule_json_key;
51655174
EnqueueRuleTuple(std::string(kAclIngressTableName),
51665175
swss::KeyOpFieldsValuesTuple({rule_tuple_key, SET_COMMAND, attributes}));
@@ -5182,21 +5191,37 @@ TEST_F(AclManagerTest, AclRuleVerifyStateTest)
51825191
table.set(
51835192
"SAI_OBJECT_TYPE_ACL_ENTRY:oid:0x3e9",
51845193
std::vector<swss::FieldValueTuple>{
5185-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_TABLE_ID", "oid:0x7000000000606"},
5194+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_TABLE_ID",
5195+
"oid:0x7000000000606"},
51865196
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_PRIORITY", "15"},
51875197
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ADMIN_STATE", "true"},
5188-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_DST_IPV6", "fdf8:f53b:82e4::53&mask:fdf8:f53b:82e4::53"},
5189-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE", "2048&mask:0xffff"},
5190-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_ACL_IP_TYPE",
5191-
"SAI_ACL_IP_TYPE_ANY&mask:0xffffffffffffffff"},
5192-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_USER_DEFINED_FIELD_GROUP_MIN", "2:255,17&mask:2:0xff,0xff"},
5193-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_IPMC_NPU_META_DST_HIT", "true"},
5194-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_USER_DEFINED_FIELD_GROUP_1", "2:34,49&mask:2:0xff,0xff"},
5195-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS", "2:oid:0x112233,oid:0x1fed3"},
5196-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORTS", "2:oid:0x9988,oid:0x56789abcdef"},
5197-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS", "1:oid:0x2329"},
5198-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ACTION_SET_POLICER", "oid:0x7d1"},
5199-
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ACTION_COUNTER", "oid:0xbb9"}});
5198+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_DST_IPV6",
5199+
"fdf8:f53b:82e4::53&mask:fdf8:f53b:82e4::53"},
5200+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE",
5201+
"2048&mask:0xffff"},
5202+
swss::FieldValueTuple{
5203+
"SAI_ACL_ENTRY_ATTR_FIELD_ACL_IP_TYPE",
5204+
"SAI_ACL_IP_TYPE_ANY&mask:0xffffffffffffffff"},
5205+
swss::FieldValueTuple{
5206+
"SAI_ACL_ENTRY_ATTR_USER_DEFINED_FIELD_GROUP_MIN",
5207+
"2:255,17&mask:2:0xff,0xff"},
5208+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_VRF_ID",
5209+
"oid:0x6f"},
5210+
swss::FieldValueTuple{
5211+
"SAI_ACL_ENTRY_ATTR_FIELD_IPMC_NPU_META_DST_HIT", "true"},
5212+
swss::FieldValueTuple{
5213+
"SAI_ACL_ENTRY_ATTR_USER_DEFINED_FIELD_GROUP_1",
5214+
"2:34,49&mask:2:0xff,0xff"},
5215+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS",
5216+
"2:oid:0x112233,oid:0x1fed3"},
5217+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORTS",
5218+
"2:oid:0x9988,oid:0x56789abcdef"},
5219+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS",
5220+
"1:oid:0x2329"},
5221+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ACTION_SET_POLICER",
5222+
"oid:0x7d1"},
5223+
swss::FieldValueTuple{"SAI_ACL_ENTRY_ATTR_ACTION_COUNTER",
5224+
"oid:0xbb9"}});
52005225
table.set("SAI_OBJECT_TYPE_ACL_COUNTER:oid:0xbb9",
52015226
std::vector<swss::FieldValueTuple>{
52025227
swss::FieldValueTuple{"SAI_ACL_COUNTER_ATTR_TABLE_ID", "oid:0x7000000000606"},
@@ -5221,38 +5246,47 @@ TEST_F(AclManagerTest, AclRuleVerifyStateTest)
52215246
EXPECT_FALSE(VerifyRuleState(std::string(APP_P4RT_TABLE_NAME) + ":invalid", attributes).empty());
52225247
EXPECT_FALSE(VerifyRuleState(std::string(APP_P4RT_TABLE_NAME) + ":invalid:invalid", attributes).empty());
52235248
EXPECT_FALSE(VerifyRuleState(std::string(APP_P4RT_TABLE_NAME) + ":ACL_PUNT_TABLE:invalid", attributes).empty());
5224-
EXPECT_FALSE(VerifyRuleState(std::string(APP_P4RT_TABLE_NAME) +
5225-
":ACL_PUNT_TABLE:{\"match/ether_type\":\"0x0800\",\"match/"
5226-
"ipv6_dst\":\"fdf8:f53b:82e4::53 & "
5227-
"fdf8:f53b:82e4::53\",\"priority\":0,\"match/ipmc_table_hit\":"
5228-
"\"0x1\"}",
5229-
attributes)
5230-
.empty());
5231-
EXPECT_FALSE(VerifyRuleState(std::string(APP_P4RT_TABLE_NAME) +
5232-
":ACL_PUNT_TABLE:{\"match/ether_type\":\"0x0800\",\"match/"
5233-
"ipv6_dst\":\"127.0.0.1/24\",\"priority\":15,"
5234-
"\"match/ipmc_table_hit\":\"0x1\"}",
5235-
attributes)
5236-
.empty());
5249+
EXPECT_FALSE(
5250+
VerifyRuleState(
5251+
std::string(APP_P4RT_TABLE_NAME) +
5252+
":ACL_PUNT_TABLE:{\"match/ether_type\":\"0x0800\",\"match/"
5253+
"ipv6_dst\":\"fdf8:f53b:82e4::53 & "
5254+
"fdf8:f53b:82e4::53\",\"priority\":0,\"match/ipmc_table_hit\":"
5255+
"\"0x1\",\"match/vrf_id\":\"b4-traffic\"}",
5256+
attributes)
5257+
.empty());
5258+
EXPECT_FALSE(
5259+
VerifyRuleState(
5260+
std::string(APP_P4RT_TABLE_NAME) +
5261+
":ACL_PUNT_TABLE:{\"match/ether_type\":\"0x0800\",\"match/"
5262+
"ipv6_dst\":\"127.0.0.1/24\",\"priority\":15,"
5263+
"\"match/ipmc_table_hit\":\"0x1\",\"match/"
5264+
"vrf_id\":\"b4-traffic\"}",
5265+
attributes)
5266+
.empty());
52375267

52385268
// Verification should fail if entry does not exist.
5239-
EXPECT_FALSE(VerifyRuleState(std::string(APP_P4RT_TABLE_NAME) +
5240-
":ACL_PUNT_TABLE:{\"match/ether_type\":\"0x0800\",\"match/"
5241-
"ipv6_dst\":\"fdf8:f53b:82e4::54 & "
5242-
"fdf8:f53b:82e4::54\",\"priority\":15,\"match/ipmc_table_hit\":"
5243-
"\"0x1\"}",
5244-
attributes)
5245-
.empty());
5269+
EXPECT_FALSE(
5270+
VerifyRuleState(
5271+
std::string(APP_P4RT_TABLE_NAME) +
5272+
":ACL_PUNT_TABLE:{\"match/ether_type\":\"0x0800\",\"match/"
5273+
"ipv6_dst\":\"fdf8:f53b:82e4::54 & "
5274+
"fdf8:f53b:82e4::54\",\"priority\":15,\"match/ipmc_table_hit\":"
5275+
"\"0x1\",\"match/vrf_id\":\"b4-traffic\"}",
5276+
attributes)
5277+
.empty());
52465278

52475279
// Verification should fail with invalid attribute.
52485280
EXPECT_FALSE(VerifyTableState(db_key, std::vector<swss::FieldValueTuple>{{kAction, "invalid"}}).empty());
52495281

52505282
auto *acl_table = GetAclTable(kAclIngressTableName);
52515283
EXPECT_NE(acl_table, nullptr);
5252-
const auto &acl_rule_key = "match/arp_tpa=0xff112231:match/ether_type=0x0800:match/"
5253-
"in_ports=Ethernet1,Ethernet2:match/ipmc_table_hit=0x1:"
5254-
"match/ipv6_dst=fdf8:f53b:82e4::53 & "
5255-
"fdf8:f53b:82e4::53:match/out_ports=Ethernet4,Ethernet5:priority=15";
5284+
const auto& acl_rule_key =
5285+
"match/arp_tpa=0xff112231:match/ether_type=0x0800:match/"
5286+
"in_ports=Ethernet1,Ethernet2:match/ipmc_table_hit=0x1:"
5287+
"match/ipv6_dst=fdf8:f53b:82e4::53 & "
5288+
"fdf8:f53b:82e4::53:match/out_ports=Ethernet4,Ethernet5:match/"
5289+
"vrf_id=b4-traffic:priority=15";
52565290
auto *acl_rule = GetAclRule(kAclIngressTableName, acl_rule_key);
52575291
ASSERT_NE(acl_rule, nullptr);
52585292

0 commit comments

Comments
 (0)