Skip to content

Commit 0c60bdb

Browse files
authored
[ssw][ha] use endpoint IP to for tunnel term acl (#3897)
* This PR updates the VNET orchestration code to use the endpoint IP address instead of the endpoint monitor IP when configuring tunnel termination ACL rules for local endpoints. This ensures that redirect ACL rules properly direct traffic using the actual endpoint IP rather than the monitoring IP. Key changes: Modified tunnel term ACL creation to use endpoint IP (ip) instead of monitor IP (nh_ip) Updated ACL rule to include tunnel termination matching and redirect to next-hop IP instead of interface alias Updated test cases to use separate monitor IPs (9.1.0.3, 9.1.0.4) distinct from endpoint IPs (9.1.0.1, 9.1.0.2)
1 parent 80c742c commit 0c60bdb

File tree

4 files changed

+38
-31
lines changed

4 files changed

+38
-31
lines changed

orchagent/aclorch.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ static acl_rule_attr_lookup_t aclL3ActionLookup =
112112
{ ACTION_DISABLE_TRIM, SAI_ACL_ENTRY_ATTR_ACTION_PACKET_TRIM_DISABLE }
113113
};
114114

115-
static acl_rule_attr_lookup_t aclInnerActionLookup =
115+
static acl_rule_attr_lookup_t aclInnerActionLookup =
116116
{
117117
{ ACTION_INNER_SRC_MAC_REWRITE_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_SET_INNER_SRC_MAC},
118118
};
@@ -2186,8 +2186,8 @@ AclRuleInnerSrcMacRewrite::AclRuleInnerSrcMacRewrite(AclOrch *aclOrch, string ru
21862186
memcpy(actionData.parameter.mac, inner_src_mac_addr.getMac(), sizeof(sai_mac_t));
21872187
action_str = ACTION_INNER_SRC_MAC_REWRITE_ACTION;
21882188
SWSS_LOG_INFO("Converting the Mac address %s to SAI acl action parameter", _attr_value.c_str());
2189-
}
2190-
2189+
}
2190+
21912191
else
21922192
{
21932193
return false;
@@ -2216,7 +2216,7 @@ AclRuleInnerSrcMacRewrite::AclRuleInnerSrcMacRewrite(AclOrch *aclOrch, string ru
22162216

22172217
void AclRuleInnerSrcMacRewrite::onUpdate(SubjectType type, void *cntx)
22182218
{
2219-
//do nothing
2219+
//do nothing
22202220
}
22212221

22222222
AclRuleMirror::AclRuleMirror(AclOrch *aclOrch, MirrorOrch *mirror, string rule, string table) :
@@ -2522,7 +2522,7 @@ void AclRuleUnderlaySetDscp::onUpdate(SubjectType, void *)
25222522
{
25232523
// Do nothing
25242524
}
2525-
2525+
25262526
AclTable::AclTable(AclOrch *pAclOrch, string id) noexcept : m_pAclOrch(pAclOrch), id(id)
25272527
{
25282528

@@ -4419,7 +4419,7 @@ EgressSetDscpTableStatus AclOrch::addEgrSetDscpTable(string table_id, AclTable &
44194419
if (!isAclMetaDataSupported())
44204420
{
44214421
SWSS_LOG_ERROR("Platform does not support MARK_META/MARK_METAV6 tables.");
4422-
return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_SUPPORTED;
4422+
return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_SUPPORTED;
44234423
}
44244424
AclTable egrSetDscpTable(this);
44254425

orchagent/vnetorch.cpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ bool VNetRouteOrch::doRouteTask<VNetVrfObject>(const string& vnet, IpPrefix& ipP
11891189
auto prefixToRemove = ipPrefix;
11901190
if (adv_prefix.to_string() != ipPrefix.to_string())
11911191
{
1192-
prefixToRemove = adv_prefix;
1192+
prefixToRemove = adv_prefix;
11931193
}
11941194
auto prefixSubnet = prefixToRemove.getSubnet();
11951195
if(gRouteOrch && gRouteOrch->isRouteExists(prefixSubnet))
@@ -2433,7 +2433,7 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
24332433
continue;
24342434
}
24352435
// when we add the first nexthop to the route, we dont create a nexthop group, we call the updateTunnelRoute with NHG with one member.
2436-
// when adding the 2nd, 3rd ... members we create each NH using this create_next_hop_group_member call but give it the reference of next_hop_group_id.
2436+
// when adding the 2nd, 3rd ... members we create each NH using this create_next_hop_group_member call but give it the reference of next_hop_group_id.
24372437
// this way we dont have to update the route, the syncd does it by itself. we only call the updateTunnelRoute to add/remove when adding or removing the
24382438
// route fully.
24392439

@@ -2511,7 +2511,7 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
25112511
SWSS_LOG_INFO("Successfully removed existing bgp route for prefix: %s\n", prefixStr.c_str());
25122512
}
25132513
string op = SET_COMMAND;
2514-
SWSS_LOG_INFO("Adding Vnet route for prefix:%s with nexthop group: %s\n", prefixStr.c_str(), nhStr.c_str());
2514+
SWSS_LOG_INFO("Adding Vnet route for prefix:%s with nexthop group: %s\n", prefixStr.c_str(), nhStr.c_str());
25152515

25162516
if (!updateTunnelRoute(vnet, ip_pfx, nexthops, op))
25172517
{
@@ -2520,7 +2520,7 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
25202520
}
25212521
else
25222522
{
2523-
SWSS_LOG_INFO("Successfully created tunnel route in hardware for prefix: %s\n", prefixStr.c_str());
2523+
SWSS_LOG_INFO("Successfully created tunnel route in hardware for prefix: %s\n", prefixStr.c_str());
25242524
}
25252525
}
25262526
}
@@ -2574,7 +2574,7 @@ void VNetRouteOrch::updateVnetTunnel(const BfdUpdate& update)
25742574
{
25752575
for (auto ip_pfx : syncd_nexthop_groups_[vnet][nexthops].tunnel_routes)
25762576
{
2577-
SWSS_LOG_NOTICE("Removing Vnet route for prefix : %s due to no active nexthops.\n",ip_pfx.to_string().c_str());
2577+
SWSS_LOG_NOTICE("Removing Vnet route for prefix : %s due to no active nexthops.\n",ip_pfx.to_string().c_str());
25782578
string op = DEL_COMMAND;
25792579
updateTunnelRoute(vnet, ip_pfx, nexthops, op);
25802580
}
@@ -2651,7 +2651,7 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
26512651
NextHopGroupKey nhg_custom_primary = getActiveNHSet( vnet, primary, prefix);
26522652
NextHopGroupKey nhg_custom_secondary = getActiveNHSet( vnet, secondary, prefix);
26532653
SWSS_LOG_INFO("Primary active(%s), Secondary active (%s), Current active(%s)\n", nhg_custom_primary.to_string().c_str(),
2654-
nhg_custom_secondary.to_string().c_str(), active_nhg.to_string().c_str());
2654+
nhg_custom_secondary.to_string().c_str(), active_nhg.to_string().c_str());
26552655
if (nhg_custom_primary.getSize() > 0)
26562656
{
26572657
if (nhg_custom_primary != active_nhg )
@@ -2731,14 +2731,14 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
27312731
{
27322732
// we need to replace the nhg in the route
27332733
SWSS_LOG_INFO("Replacing nexthop group for prefix: %s, nexthop group: %s\n",
2734-
prefix.to_string().c_str(), nhg_custom.to_string().c_str());
2734+
prefix.to_string().c_str(), nhg_custom.to_string().c_str());
27352735
route_status = update_route(vr_id, pfx, nh_id);
27362736
}
27372737
else
27382738
{
27392739
// we need to readd the route.
27402740
SWSS_LOG_NOTICE("Adding Custom monitored Route with prefix: %s and nexthop group: %s\n",
2741-
prefix.to_string().c_str(), nhg_custom.to_string().c_str());
2741+
prefix.to_string().c_str(), nhg_custom.to_string().c_str());
27422742
auto prefixToUse = prefix;
27432743
if (prefix_to_adv_prefix_.find(prefix) != prefix_to_adv_prefix_.end())
27442744
{
@@ -2817,7 +2817,7 @@ void VNetRouteOrch::updateVnetTunnelCustomMonitor(const MonitorUpdate& update)
28172817
if (nhg_custom.getSize() == 0 && active_nhg_size > 0)
28182818
{
28192819
vrf_obj->removeRoute(prefix);
2820-
SWSS_LOG_NOTICE("Route prefix is no longer active: %s\n", prefix.to_string().c_str());
2820+
SWSS_LOG_NOTICE("Route prefix is no longer active: %s\n", prefix.to_string().c_str());
28212821
removeRouteState(vnet, prefix);
28222822
if (prefix_to_adv_prefix_.find(prefix) != prefix_to_adv_prefix_.end())
28232823
{
@@ -2995,7 +2995,6 @@ bool VNetRouteOrch::handleTunnel(const Request& request)
29952995
IpAddress ip = ip_list[idx_ip];
29962996
bool is_local = isLocalEndpoint(vnet_name, ip);
29972997
bool is_overlay = !is_local;
2998-
IpAddress nh_ip = is_local ? monitor_list[idx_ip] : ip;
29992998
string alias = is_local ? gIntfsOrch->getRouterIntfsAlias(ip) : "";
30002999
MacAddress mac;
30013000
uint32_t vni = 0;
@@ -3015,7 +3014,8 @@ bool VNetRouteOrch::handleTunnel(const Request& request)
30153014

30163015
if (is_local)
30173016
{
3018-
vnet_tunnel_term_acl_->createAclRule(vnet_name, ip_pfx, nh_ip);
3017+
SWSS_LOG_INFO("Attempting to add TUNNEL TERM ACL for local endpoint %s", ip.to_string().c_str());
3018+
vnet_tunnel_term_acl_->createAclRule(vnet_name, ip_pfx, ip);
30193019
}
30203020

30213021
NextHopKey nh(ip, alias, mac, vni, is_overlay);
@@ -3337,6 +3337,7 @@ bool VNetTunnelTermAcl::createAclRule(const string vnet_name, swss::IpPrefix& vi
33373337
if (getAclRule(vnet_name, vip, rule))
33383338
{
33393339
/* If there are more than one local points for the same VIP, we will not create a new rule. */
3340+
SWSS_LOG_NOTICE("ACL rule already exists for VNet %s with VIP %s", vnet_name.c_str(), vip.to_string().c_str());
33403341
return true;
33413342
}
33423343

@@ -3352,10 +3353,14 @@ bool VNetTunnelTermAcl::createAclRule(const string vnet_name, swss::IpPrefix& vi
33523353
vector<FieldValueTuple> fvs = {
33533354
{RULE_PRIORITY, to_string(VNET_TUNNEL_TERM_ACL_BASE_PRIORITY)},
33543355
{MATCH_DST_IP, vip.to_string()},
3356+
{MATCH_TUNNEL_TERM, "true"},
33553357
/* This tunnel term acl is to handle a transient state in DPU failover, so the redirect can't point to a VIP.*/
3356-
{ACTION_REDIRECT_ACTION, alias}
3358+
{ACTION_REDIRECT_ACTION, nh_ip.to_string()}
33573359
};
33583360

3361+
SWSS_LOG_NOTICE("Creating ACL rule %s for VNet %s with VIP %s to redirect to %s",
3362+
rule_name.c_str(), vnet_name.c_str(), vip.to_string().c_str(), nh_ip.to_string().c_str());
3363+
33593364
acl_rule_table_->set(rule_name, fvs);
33603365

33613366
rule.rule_name = rule_name;

tests/dvslib/dvs_acl.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,8 @@ def _check_acl_entry_base(
754754
elif "SAI_ACL_ENTRY_ATTR_ACTION_NO_NAT" in k:
755755
assert action == "DO_NOT_NAT"
756756
assert v == "true"
757+
elif "SAI_ACL_ENTRY_ATTR_FIELD_TUNNEL_TERMINATED" in k:
758+
assert v == "true"
757759
elif k in qualifiers:
758760
assert qualifiers[k](v)
759761
else:

tests/test_vnet.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2685,23 +2685,23 @@ def test_vnet_orch_28(self, dvs, dvs_acl, testlog):
26852685
self.add_neighbor("Ethernet8", "9.1.0.1", "00:01:02:03:04:05")
26862686

26872687
vnet_obj.fetch_exist_entries(dvs)
2688-
create_vnet_routes(dvs, "100.100.1.1/32", vnet_name, '9.1.0.1,9.1.0.2', ep_monitor='9.1.0.1,9.1.0.2', primary ='9.1.0.1', profile="Test_profile", monitoring='custom', adv_prefix='100.100.1.0/24', check_directly_connected=True)
2688+
create_vnet_routes(dvs, "100.100.1.1/32", vnet_name, '9.1.0.1,9.1.0.2', ep_monitor='9.1.0.3,9.1.0.4', primary ='9.1.0.1', profile="Test_profile", monitoring='custom', adv_prefix='100.100.1.0/24', check_directly_connected=True)
26892689

26902690
# verify tunnel term acl
26912691
expected_sai_qualifiers = {
26922692
"SAI_ACL_ENTRY_ATTR_FIELD_DST_IP": dvs_acl.get_simple_qualifier_comparator("100.100.1.1&mask:255.255.255.255")
26932693
}
2694-
intf_id = dvs.asic_db.port_name_map["Ethernet8"]
2695-
dvs_acl.verify_redirect_acl_rule(expected_sai_qualifiers, intf_id, priority="9998")
2694+
nh_id = dvs.get_asic_db().wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", 1)[0]
2695+
dvs_acl.verify_redirect_acl_rule(expected_sai_qualifiers, nh_id, priority="9998")
26962696

26972697
# default monitor session status is down, route should not be programmed in this status
26982698
vnet_obj.check_del_vnet_routes(dvs, vnet_name, ["100.100.1.1/32"])
26992699
check_state_db_routes(dvs, vnet_name, "100.100.1.1/32", [])
27002700
check_remove_routes_advertisement(dvs, "100.100.1.0/24")
27012701

27022702
# Route should be properly configured when all monitor session states go up. Only primary Endpoints should be in use.
2703-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.1', 'up')
2704-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.2', 'up')
2703+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.3', 'up')
2704+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.4', 'up')
27052705
time.sleep(2)
27062706
nhids = get_all_created_entries(asic_db, vnet_obj.ASIC_NEXT_HOP,set())
27072707
tbl_nh = swsscommon.Table(asic_db, vnet_obj.ASIC_NEXT_HOP)
@@ -2723,7 +2723,7 @@ def test_vnet_orch_28(self, dvs, dvs_acl, testlog):
27232723
check_state_db_routes(dvs, vnet_name, "100.100.1.1/32", ['9.1.0.1'])
27242724
check_routes_advertisement(dvs, "100.100.1.0/24", "Test_profile")
27252725

2726-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.2', 'down')
2726+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.4', 'down')
27272727
time.sleep(2)
27282728

27292729
route = get_created_entries(asic_db, vnet_obj.ASIC_ROUTE_ENTRY, vnet_obj.routes, 1)
@@ -2735,8 +2735,8 @@ def test_vnet_orch_28(self, dvs, dvs_acl, testlog):
27352735
check_state_db_routes(dvs, vnet_name, "100.100.1.1/32", ['9.1.0.1'])
27362736
check_routes_advertisement(dvs, "100.100.1.0/24", "Test_profile")
27372737

2738-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.1', 'down')
2739-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.2', 'up')
2738+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.3', 'down')
2739+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.4', 'up')
27402740

27412741
time.sleep(2)
27422742

@@ -2761,7 +2761,7 @@ def test_vnet_orch_28(self, dvs, dvs_acl, testlog):
27612761
check_state_db_routes(dvs, vnet_name, "100.100.1.1/32", ['9.1.0.2'])
27622762
check_routes_advertisement(dvs, "100.100.1.0/24", "Test_profile")
27632763

2764-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.1', 'up')
2764+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.3', 'up')
27652765
time.sleep(2)
27662766

27672767
nhids = get_all_created_entries(asic_db, vnet_obj.ASIC_NEXT_HOP,set())
@@ -2784,8 +2784,8 @@ def test_vnet_orch_28(self, dvs, dvs_acl, testlog):
27842784
check_state_db_routes(dvs, vnet_name, "100.100.1.1/32", ['9.1.0.1'])
27852785
check_routes_advertisement(dvs, "100.100.1.0/24", "Test_profile")
27862786

2787-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.1', 'down')
2788-
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.2', 'down')
2787+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.3', 'down')
2788+
update_monitor_session_state(dvs, '100.100.1.1/32', '9.1.0.4', 'down')
27892789

27902790
time.sleep(2)
27912791

@@ -2800,8 +2800,8 @@ def test_vnet_orch_28(self, dvs, dvs_acl, testlog):
28002800
check_remove_state_db_routes(dvs, vnet_name, "100.100.1.1/32")
28012801
check_remove_routes_advertisement(dvs, "100.100.1.0/24")
28022802

2803-
vnet_obj.check_custom_monitor_deleted(dvs, "100.100.1.1/32", "9.1.0.1")
2804-
vnet_obj.check_custom_monitor_deleted(dvs, "100.100.1.1/32", "9.1.0.2")
2803+
vnet_obj.check_custom_monitor_deleted(dvs, "100.100.1.1/32", "9.1.0.3")
2804+
vnet_obj.check_custom_monitor_deleted(dvs, "100.100.1.1/32", "9.1.0.4")
28052805

28062806
delete_vnet_entry(dvs, vnet_name)
28072807
vnet_obj.check_del_vnet_entry(dvs, vnet_name)

0 commit comments

Comments
 (0)