From dbe8cbd047c416f44659c0ffc569a175edf16c2f Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Fri, 7 Aug 2026 13:34:14 +0000 Subject: [PATCH 1/2] [vpp] Fix IPv6 local permit rule family mismatch make_local_permit left src_prefix unset for IPv6 local addresses. The xlate defaults an unset source prefix to IPv4, so an IPv6 dst produced a rule with src=IPv4 / dst=IPv6. VPP rejects mismatched-family ACL rules with VNET_API_ERROR_INVALID_SRC_ADDRESS (-57), which failed the entire acl_add_replace. As a result only the very first (IPv4-only) refresh committed and every later rebuild -- including the ones that would add the SoC loopback permit -- was rejected, so control traffic to those addresses kept hitting the mux drop. Set a same-family wildcard source (0.0.0.0/0 or ::/0) so every rule is family-consistent and accepted. Signed-off-by: Longxiang Lyu --- vslib/vpp/SwitchVpp.h | 34 ++++++++++ vslib/vpp/SwitchVppAcl.cpp | 128 +++++++++++++++++++++++++++++++++++++ vslib/vpp/SwitchVppRif.cpp | 9 ++- 3 files changed, 169 insertions(+), 2 deletions(-) diff --git a/vslib/vpp/SwitchVpp.h b/vslib/vpp/SwitchVpp.h index 8a3b2ed8e0..cc4a9c1f2b 100644 --- a/vslib/vpp/SwitchVpp.h +++ b/vslib/vpp/SwitchVpp.h @@ -13,8 +13,11 @@ #include "vppxlate/SaiVppXlate.h" #include "vppxlate/SaiRouteStats.h" +#include "swss/ipaddress.h" + #include #include +#include #include #include #include @@ -846,6 +849,9 @@ namespace saivs bool m_acl_default_created = false; uint32_t m_sflow_sample_rate = 0; + std::set m_local_ips; // Local RIF addresses + std::set m_local_deny_tables; // ACL tables that contain a deny rule + protected: // VPP sai_status_t createAclEntry( @@ -868,6 +874,34 @@ namespace saivs sai_status_t AclTblConfig( _In_ sai_object_id_t tbl_oid); + /** + * @brief Records or drops a local host address as router + * interface IPs are added/removed, and refreshes any ACL table that + * has a deny rule so its prepended local permits stay in sync. + * + * @param[in] prefix VPP interface prefix just programmed; only the + * host address is used (matched later as /32 or /128). + * @param[in] is_add True when the address was added, false on remove. + */ + void trackLocalIp( + _In_ const vpp_ip_route_t *prefix, + _In_ bool is_add); + + /** + * @brief If the assembled rule list contains any deny (drop) rule + * -- e.g. the dual-ToR mux drop -- prepend permit rules for the + * switch's own local addresses so local (for-us) traffic bypasses the + * deny and is punted at ip4-local. + * + * @param[in] tbl_oid ACL table being (re)configured. + * @param[in,out] acl_rules Ordered rule list; local permits are + * spliced onto the front when a deny rule is present. + * @return Number of permit rules prepended (0 if none). + */ + size_t injectLocalPermits( + _In_ sai_object_id_t tbl_oid, + _Inout_ std::list &acl_rules); + sai_status_t AclTblRemove( _In_ sai_object_id_t tbl_oid); diff --git a/vslib/vpp/SwitchVppAcl.cpp b/vslib/vpp/SwitchVppAcl.cpp index 6104aa43e9..d9606a59d0 100644 --- a/vslib/vpp/SwitchVppAcl.cpp +++ b/vslib/vpp/SwitchVppAcl.cpp @@ -21,11 +21,48 @@ #include #include #include +#include using namespace saivs; #define DEFAULT_PERMIT_RULES 2 +static bool acl_rule_is_deny(const vpp_acl_rule_t &r) +{ + return r.action == VPP_ACL_ACTION_API_DENY; +} + +static vpp_acl_rule_t make_local_permit(const swss::IpAddress &ip) +{ + vpp_acl_rule_t rule = {}; + + rule.action = VPP_ACL_ACTION_API_PERMIT; + + // VPP requires src and dst prefixes of an ACL rule to share the same address + // family, so set a same-family wildcard source (0.0.0.0/0 or ::/0). Leaving + // src unset defaults it to IPv4 in the xlate, which for an IPv6 dst yields a + // family mismatch that VPP rejects with VNET_API_ERROR_INVALID_SRC_ADDRESS. + if (ip.isV4()) { + rule.src_prefix.sa_family = AF_INET; // src 0.0.0.0/0 + rule.src_prefix_mask.sa_family = AF_INET; + + rule.dst_prefix.sa_family = AF_INET; + rule.dst_prefix.addr.ip4.sin_addr.s_addr = ip.getV4Addr(); + rule.dst_prefix_mask.sa_family = AF_INET; + rule.dst_prefix_mask.addr.ip4.sin_addr.s_addr = 0xFFFFFFFF; // /32 + } else { + rule.src_prefix.sa_family = AF_INET6; // src ::/0 + rule.src_prefix_mask.sa_family = AF_INET6; + + rule.dst_prefix.sa_family = AF_INET6; + memcpy(rule.dst_prefix.addr.ip6.sin6_addr.s6_addr, ip.getV6Addr(), 16); + rule.dst_prefix_mask.sa_family = AF_INET6; + memset(rule.dst_prefix_mask.addr.ip6.sin6_addr.s6_addr, 0xFF, 16); // /128 + } + + return rule; +} + static sai_status_t acl_ip_field_to_vpp_acl( _In_ sai_acl_entry_attr_t attr_id, _In_ const sai_attribute_value_t *value, @@ -1163,6 +1200,90 @@ sai_status_t SwitchVpp::tunterm_acl_delete(sai_object_id_t tbl_oid, bool table_d return status; } +void SwitchVpp::trackLocalIp( + _In_ const vpp_ip_route_t *prefix, + _In_ bool is_add) +{ + SWSS_LOG_ENTER(); + + if (prefix == NULL) { + return; + } + + swss::ip_addr_t a; + memset(&a, 0, sizeof(a)); + + if (prefix->prefix_addr.sa_family == AF_INET) { + a.family = AF_INET; + a.ip_addr.ipv4_addr = prefix->prefix_addr.addr.ip4.sin_addr.s_addr; + } else if (prefix->prefix_addr.sa_family == AF_INET6) { + a.family = AF_INET6; + memcpy(a.ip_addr.ipv6_addr, prefix->prefix_addr.addr.ip6.sin6_addr.s6_addr, 16); + } else { + return; + } + + swss::IpAddress ip(a); + bool changed = is_add ? m_local_ips.insert(ip).second + : (m_local_ips.erase(ip) > 0); + + if (!changed || m_local_deny_tables.empty()) { + return; + } + + std::vector tables_to_refresh(m_local_deny_tables.begin(), + m_local_deny_tables.end()); + for (auto tbl_oid : tables_to_refresh) { + SWSS_LOG_NOTICE("Local address %s %s, refreshing ACL table %s (has deny rule)", + ip.to_string().c_str(), + is_add ? "added" : "removed", + sai_serialize_object_id(tbl_oid).c_str()); + AclTblConfig(tbl_oid); + } +} + +size_t SwitchVpp::injectLocalPermits( + _In_ sai_object_id_t tbl_oid, + _Inout_ std::list &acl_rules) +{ + SWSS_LOG_ENTER(); + + bool has_deny = false; + for (const auto &rule : acl_rules) { + if (acl_rule_is_deny(rule)) { + has_deny = true; + break; + } + } + + if (!has_deny) { + m_local_deny_tables.erase(tbl_oid); + return 0; + } + + m_local_deny_tables.insert(tbl_oid); + + // Prepend a /32 (or /128) permit for every local address. Rules within one + // VPP ACL are evaluated first-match-wins, and a permit ahead of the deny (in + // the same ACL) also wins over a deny in any later-bound ACL, so local + // (for-us) traffic reaches ip4-local for punt while everything else still + // hits the table's rules unchanged. + std::list permits; + for (const auto &ip : m_local_ips) { + permits.push_back(make_local_permit(ip)); + } + + size_t n_permits = permits.size(); + if (n_permits) { + acl_rules.splice(acl_rules.begin(), permits); + } + + SWSS_LOG_INFO("Local permit injection: table %s has a deny rule, prepended %zu local permit(s)", + sai_serialize_object_id(tbl_oid).c_str(), n_permits); + + return n_permits; +} + sai_status_t SwitchVpp::AclTblConfig( _In_ sai_object_id_t tbl_oid) { @@ -1198,6 +1319,13 @@ sai_status_t SwitchVpp::AclTblConfig( SWSS_LOG_INFO("Generated %ld regular ACL rules and %ld tunterm ACL rules", acl_rules.size(), tunterm_acl_rules.size()); + size_t n_local = injectLocalPermits(tbl_oid, acl_rules); + if (n_local) { + for (auto &ace : ordered_aces) { + ace.vpp_rule_base_index += (uint32_t) n_local; + } + } + // Create and populate regular ACL if we have rules if (!acl_rules.empty()) { auto tbl_sid = sai_serialize_object_id(tbl_oid); diff --git a/vslib/vpp/SwitchVppRif.cpp b/vslib/vpp/SwitchVppRif.cpp index 6283370878..310b9acdaa 100644 --- a/vslib/vpp/SwitchVppRif.cpp +++ b/vslib/vpp/SwitchVppRif.cpp @@ -955,6 +955,7 @@ sai_status_t SwitchVpp::vpp_add_del_intf_ip_addr ( if (ret == 0) { + trackLocalIp(&vpp_ip_prefix, is_add); return SAI_STATUS_SUCCESS; } else { @@ -1184,6 +1185,7 @@ sai_status_t SwitchVpp::vpp_add_del_intf_ip_addr_norif ( if (ret == 0) { + trackLocalIp(&vpp_ip_prefix, is_add); if (is_add) { m_tunnel_mgr_ipip.retry_pending_unnumbered(vpp_ip_prefix.prefix_addr); } @@ -1346,8 +1348,11 @@ sai_status_t SwitchVpp::vpp_interface_ip_address_update ( int ret = interface_ip_address_add_del(vppIfname, &ip_route, is_add); if (ret != 0) { SWSS_LOG_ERROR("interface_ip_address_add returned error"); - } else if (is_add) { - m_tunnel_mgr_ipip.retry_pending_unnumbered(ip_route.prefix_addr); + } else { + trackLocalIp(&ip_route, is_add); + if (is_add) { + m_tunnel_mgr_ipip.retry_pending_unnumbered(ip_route.prefix_addr); + } } return SAI_STATUS_SUCCESS; From 54d14c7f2b2802d5df4f2e89842ae4e2553c9463 Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Fri, 7 Aug 2026 13:53:22 +0000 Subject: [PATCH 2/2] [vpp] Do not shift tunterm ACE base index on local-permit inject Signed-off-by: Longxiang Lyu --- vslib/vpp/SwitchVppAcl.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/vslib/vpp/SwitchVppAcl.cpp b/vslib/vpp/SwitchVppAcl.cpp index d9606a59d0..874c42fb01 100644 --- a/vslib/vpp/SwitchVppAcl.cpp +++ b/vslib/vpp/SwitchVppAcl.cpp @@ -1319,10 +1319,16 @@ sai_status_t SwitchVpp::AclTblConfig( SWSS_LOG_INFO("Generated %ld regular ACL rules and %ld tunterm ACL rules", acl_rules.size(), tunterm_acl_rules.size()); + // Prepending local permits shifts every regular ACE forward in the VPP ACL, + // so offset the recorded rule base index to keep ACL counters mapped. Tunterm + // ACE base indices are relative to the separate tunnel-termination ACL, which + // does not gain these leading rules, so they must not be shifted. size_t n_local = injectLocalPermits(tbl_oid, acl_rules); if (n_local) { for (auto &ace : ordered_aces) { - ace.vpp_rule_base_index += (uint32_t) n_local; + if (!ace.is_tunterm) { + ace.vpp_rule_base_index += (uint32_t) n_local; + } } }