Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[syncd] Add pre match logic for acl entry #1240

Merged
merged 2 commits into from
Jun 8, 2023
Merged

[syncd] Add pre match logic for acl entry #1240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7291103
[syncd] Add pre match logic for acl entry
kcudnik May 16, 2023
6c2f490
Fix log
kcudnik May 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions syncd/BestCandidateFinder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3002,6 +3002,13 @@ std::shared_ptr<SaiAttr> BestCandidateFinder::getSaiAttrFromDefaultValue(
return std::make_shared<SaiAttr>(meta.attridname, str_attr_value);
}

case SAI_ATTR_VALUE_TYPE_ACL_FIELD_DATA_UINT8:
case SAI_ATTR_VALUE_TYPE_ACL_FIELD_DATA_UINT16:

// ACL field data don't have default values (disabled)

return nullptr;

default:

SWSS_LOG_ERROR("serialization type %s is not supported yet, FIXME",
Expand Down
52 changes: 51 additions & 1 deletion syncd/ComparisonLogic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2806,6 +2806,53 @@ void ComparisonLogic::cretePreMatchForLagMembers(
}
}

void ComparisonLogic::cretePreMatchForAclEntries(
_In_ const AsicView& cur,
_Inout_ AsicView& tmp,
_Inout_ std::set<std::string>& processed)
{
SWSS_LOG_ENTER();

// match acl entries which have the same priority attribute

auto cAclEntries = cur.getObjectsByObjectType(SAI_OBJECT_TYPE_ACL_ENTRY);
auto tAclEntries = tmp.getObjectsByObjectType(SAI_OBJECT_TYPE_ACL_ENTRY);

for (auto& cAclEntry: cAclEntries)
{
if (processed.find(cAclEntry->m_str_object_id) != processed.end())
continue;

for (auto& tAclEntry: tAclEntries)
{
if (processed.find(tAclEntry->m_str_object_id) != processed.end())
continue;

auto cPrio = cAclEntry->getSaiAttr(SAI_ACL_ENTRY_ATTR_PRIORITY);
auto tPrio = tAclEntry->getSaiAttr(SAI_ACL_ENTRY_ATTR_PRIORITY);

if (cPrio->getStrAttrValue() != tPrio->getStrAttrValue())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rookie question: are ACL priorities guaranteed to be always unique? If not, then we will likely run into same issue again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no they are not guaranteed in general, but sonic make sure that they are, since we want to control the right order of the acl, so having 2 acls with the same priority not guarantee the order of them

{
continue;
}

// at this point current and temporary acl entry share the same
// priority, then we can assume that those objects are the same

SWSS_LOG_NOTICE("pre match Acl Entry: cur: %s, tmp: %s, using prio: %s",
cAclEntry->m_str_object_id.c_str(),
tAclEntry->m_str_object_id.c_str(),
cPrio->getStrAttrValue().c_str());

tmp.m_preMatchMap[tAclEntry->getVid()] = cAclEntry->getVid();

createPreMatchMapForObject(cur, tmp, cAclEntry, tAclEntry, processed);

break;
}
}
}

void ComparisonLogic::createPreMatchMap(
_In_ const AsicView& cur,
_Inout_ AsicView& tmp)
Expand Down Expand Up @@ -2926,6 +2973,8 @@ void ComparisonLogic::createPreMatchMap(
count++;
}

cretePreMatchForAclEntries(cur, tmp, processed);

SWSS_LOG_NOTICE("preMatch map size: %zu, tmp oid obj: %zu",
tmp.m_preMatchMap.size(),
count);
Expand Down Expand Up @@ -3045,7 +3094,8 @@ void ComparisonLogic::applyViewTransition(
{
// TODO make generic list of root objects (or meta SAI for this)

if (obj.second->getObjectType() == SAI_OBJECT_TYPE_ACL_TABLE_GROUP)
if (obj.second->getObjectType() == SAI_OBJECT_TYPE_ACL_TABLE_GROUP ||
obj.second->getObjectType() == SAI_OBJECT_TYPE_ACL_ENTRY)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change needed? Wouldn't cretePreMatchForAclEntries be sufficient to match right ACLs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, since ACL_ENTRY is leaf object

{
if (temp.m_preMatchMap.find(obj.second->getVid()) == temp.m_preMatchMap.end())
{
Expand Down
5 changes: 5 additions & 0 deletions syncd/ComparisonLogic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ namespace syncd
_Inout_ AsicView& tmp,
_Inout_ std::set<std::string>& processed);

void cretePreMatchForAclEntries(
_In_ const AsicView& cur,
_Inout_ AsicView& tmp,
_Inout_ std::set<std::string>& processed);

sai_object_id_t asic_translate_vid_to_rid(
_In_ const AsicView& current,
_In_ const AsicView& temporary,
Expand Down
27 changes: 27 additions & 0 deletions tests/BCM56850.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,35 @@ sub test_voq_switch_create
play("-z", "redis_sync", "voq_switch_create.rec")
}

sub test_acl_counter_match
{
# this will test scenario where we have 1 acl entry with counter and after
# warm boot 2 acl entries and 2 counters the same which could end up
# matching wrong acl counter and causing fail "object exist" on broadcom
# acl entry set, which will require improve pre match logic for acl entry

fresh_start;

play "acl_counter_match.rec";

open (my $H, "<", "applyview.log") or die "failed to open applyview.log $!";

my $line = <$H>;

close ($H);

chomp$line;

if (not $line =~ /ASIC_OPERATIONS: (\d+)/ or $1 != 2)
{
print color('red') . "expected 2 ASIC_OPERATIONS count on first line, but got: '$line'" . color('reset') . "\n";
exit 1;
}
}

# RUN TESTS

test_acl_counter_match;
test_neighbor_lag;
test_lag_member;
test_remove_port_serdes;
Expand Down
24 changes: 24 additions & 0 deletions tests/BCM56850/acl_counter_match.rec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
2023-04-09.06:05:51.758572|a|INIT_VIEW
2023-04-09.06:05:54.894996|A|SAI_STATUS_SUCCESS
2023-04-09.06:05:54.898126|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true|SAI_SWITCH_ATTR_FDB_EVENT_NOTIFY=0x55e3e3d2db30|SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY=0x55e3e3d2db40|SAI_SWITCH_ATTR_SWITCH_SHUTDOWN_REQUEST_NOTIFY=0x55e3e3d2db60|SAI_SWITCH_ATTR_SRC_MAC_ADDRESS=94:8E:D3:B8:D1:48
2023-04-09.06:06:01.316781|g|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_PORT_LIST=32:oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0
2023-04-09.06:06:01.320955|G|SAI_STATUS_SUCCESS|SAI_SWITCH_ATTR_PORT_LIST=32:oid:0x1000000000002,oid:0x1000000000003,oid:0x1000000000004,oid:0x1000000000005,oid:0x1000000000006,oid:0x1000000000007,oid:0x1000000000008,oid:0x1000000000009,oid:0x100000000000a,oid:0x100000000000b,oid:0x100000000000c,oid:0x100000000000d,oid:0x100000000000e,oid:0x100000000000f,oid:0x1000000000010,oid:0x1000000000011,oid:0x1000000000012,oid:0x1000000000013,oid:0x1000000000014,oid:0x1000000000015,oid:0x1000000000016,oid:0x1000000000017,oid:0x1000000000018,oid:0x1000000000019,oid:0x100000000001a,oid:0x100000000001b,oid:0x100000000001c,oid:0x100000000001d,oid:0x100000000001e,oid:0x100000000001f,oid:0x1000000000020,oid:0x1000000000021
2023-04-09.06:06:06.534280|c|SAI_OBJECT_TYPE_ACL_TABLE:oid:0x700000000069f|SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST=2:SAI_ACL_BIND_POINT_TYPE_PORT,SAI_ACL_BIND_POINT_TYPE_LAG|SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID=true|SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_SRC_IP=true|SAI_ACL_TABLE_ATTR_FIELD_DST_IP=true|SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE=true|SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL=true|SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS=true|SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6=true|SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6=true|SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE=true|SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER=true|SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT=true|SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT=true|SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS=true|SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE=2:SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE,SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE|SAI_ACL_TABLE_ATTR_ACL_STAGE=SAI_ACL_STAGE_INGRESS|SAI_ACL_TABLE_ATTR_FIELD_DSCP=true
2023-04-09.06:13:45.951630|c|SAI_OBJECT_TYPE_ACL_COUNTER:oid:0x90000000008fa|SAI_ACL_COUNTER_ATTR_TABLE_ID=oid:0x700000000069f|SAI_ACL_COUNTER_ATTR_ENABLE_BYTE_COUNT=true|SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT=true
2023-04-09.06:13:42.965856|c|SAI_OBJECT_TYPE_MIRROR_SESSION:oid:0xe0000000008f9|SAI_MIRROR_SESSION_ATTR_MONITOR_PORT=oid:0x1000000000020|SAI_MIRROR_SESSION_ATTR_TYPE=SAI_MIRROR_SESSION_TYPE_ENHANCED_REMOTE|SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE=SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL|SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION=4|SAI_MIRROR_SESSION_ATTR_TOS=32|SAI_MIRROR_SESSION_ATTR_TTL=255|SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS=10.1.0.32|SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS=192.168.8.1|SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS=94:8E:D3:B8:D1:48|SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS=3E:2F:91:12:89:AB|SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE=35006
2023-04-09.06:13:45.974778|c|SAI_OBJECT_TYPE_ACL_ENTRY:oid:0x80000000008fb|SAI_ACL_ENTRY_ATTR_TABLE_ID=oid:0x700000000069f|SAI_ACL_ENTRY_ATTR_PRIORITY=8888|SAI_ACL_ENTRY_ATTR_ADMIN_STATE=true|SAI_ACL_ENTRY_ATTR_ACTION_COUNTER=oid:0x90000000008fa|SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE=2054&mask:0xffff|SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS=1:oid:0xe0000000008f9
2023-04-09.06:06:01.989510|a|APPLY_VIEW
2023-04-09.06:06:01.990574|A|SAI_STATUS_SUCCESS
2023-04-09.06:16:00.119350|a|INIT_VIEW
2023-04-09.06:16:10.264317|A|SAI_STATUS_SUCCESS
2023-04-09.06:16:10.268256|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true|SAI_SWITCH_ATTR_FDB_EVENT_NOTIFY=0x5631fe2d0b30|SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY=0x5631fe2d0b40|SAI_SWITCH_ATTR_SWITCH_SHUTDOWN_REQUEST_NOTIFY=0x5631fe2d0b60|SAI_SWITCH_ATTR_SRC_MAC_ADDRESS=94:8E:D3:B8:D1:48
2023-04-09.06:16:10.314901|g|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_PORT_LIST=32:oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0
2023-04-09.06:16:10.323831|G|SAI_STATUS_SUCCESS|SAI_SWITCH_ATTR_PORT_LIST=32:oid:0x1000000000002,oid:0x1000000000003,oid:0x1000000000004,oid:0x1000000000005,oid:0x1000000000006,oid:0x1000000000007,oid:0x1000000000008,oid:0x1000000000009,oid:0x100000000000a,oid:0x100000000000b,oid:0x100000000000c,oid:0x100000000000d,oid:0x100000000000e,oid:0x100000000000f,oid:0x1000000000010,oid:0x1000000000011,oid:0x1000000000012,oid:0x1000000000013,oid:0x1000000000014,oid:0x1000000000015,oid:0x1000000000016,oid:0x1000000000017,oid:0x1000000000018,oid:0x1000000000019,oid:0x100000000001a,oid:0x100000000001b,oid:0x100000000001c,oid:0x100000000001d,oid:0x100000000001e,oid:0x100000000001f,oid:0x1000000000020,oid:0x1000000000021
2023-04-09.06:16:16.176451|c|SAI_OBJECT_TYPE_ACL_TABLE:oid:0x7000000000ba4|SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST=2:SAI_ACL_BIND_POINT_TYPE_PORT,SAI_ACL_BIND_POINT_TYPE_LAG|SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID=true|SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_SRC_IP=true|SAI_ACL_TABLE_ATTR_FIELD_DST_IP=true|SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE=true|SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL=true|SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS=true|SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6=true|SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6=true|SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE=true|SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE=true|SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER=true|SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT=true|SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT=true|SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS=true|SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE=2:SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE,SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE|SAI_ACL_TABLE_ATTR_ACL_STAGE=SAI_ACL_STAGE_INGRESS|SAI_ACL_TABLE_ATTR_FIELD_DSCP=true
2023-04-09.06:16:19.208992|c|SAI_OBJECT_TYPE_MIRROR_SESSION:oid:0xe000000000be0|SAI_MIRROR_SESSION_ATTR_MONITOR_PORT=oid:0x1000000000020|SAI_MIRROR_SESSION_ATTR_TYPE=SAI_MIRROR_SESSION_TYPE_ENHANCED_REMOTE|SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE=SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL|SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION=4|SAI_MIRROR_SESSION_ATTR_TOS=32|SAI_MIRROR_SESSION_ATTR_TTL=255|SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS=10.1.0.32|SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS=192.168.8.1|SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS=94:8E:D3:B8:D1:48|SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS=3E:2F:91:12:89:AB|SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE=35006
2023-04-09.06:16:19.210586|c|SAI_OBJECT_TYPE_ACL_COUNTER:oid:0x9000000000be1|SAI_ACL_COUNTER_ATTR_TABLE_ID=oid:0x7000000000ba4|SAI_ACL_COUNTER_ATTR_ENABLE_BYTE_COUNT=true|SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT=true
2023-04-09.06:16:19.211547|c|SAI_OBJECT_TYPE_ACL_ENTRY:oid:0x8000000000be2|SAI_ACL_ENTRY_ATTR_TABLE_ID=oid:0x7000000000ba4|SAI_ACL_ENTRY_ATTR_PRIORITY=8887|SAI_ACL_ENTRY_ATTR_ADMIN_STATE=true|SAI_ACL_ENTRY_ATTR_ACTION_COUNTER=oid:0x9000000000be1|SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_TYPE=135&mask:0xff|SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS=1:oid:0xe000000000be0
2023-04-09.06:16:19.212592|c|SAI_OBJECT_TYPE_ACL_COUNTER:oid:0x9000000000be3|SAI_ACL_COUNTER_ATTR_TABLE_ID=oid:0x7000000000ba4|SAI_ACL_COUNTER_ATTR_ENABLE_BYTE_COUNT=true|SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT=true
2023-04-09.06:16:19.213400|c|SAI_OBJECT_TYPE_ACL_ENTRY:oid:0x8000000000be4|SAI_ACL_ENTRY_ATTR_TABLE_ID=oid:0x7000000000ba4|SAI_ACL_ENTRY_ATTR_PRIORITY=8888|SAI_ACL_ENTRY_ATTR_ADMIN_STATE=true|SAI_ACL_ENTRY_ATTR_ACTION_COUNTER=oid:0x9000000000be3|SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE=2054&mask:0xffff|SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS=1:oid:0xe000000000be0
2023-04-09.06:16:19.214593|a|APPLY_VIEW
2023-04-09.06:16:21.919159|A|SAI_STATUS_SUCCESS