From 0eb22e52eb8a065bc51b7f00827db2fe3f9720d0 Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Sat, 29 Aug 2026 08:02:49 +0000 Subject: [PATCH 1/2] [vpp][acl] match on ingress interface to support SAI ACL IN_PORTS SAI models the dual-ToR mux drop as one ACL table bound to every port, with SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS naming only the standby ports. VPP ACL rules had no ingress-interface qualifier, so the VPP SAI layer had to drop IN_PORTS silently and the deny ended up applying to every bound port instead of just the ones the entry named. Add an in_sw_if_index field to the ACL rule so a single rule can be scoped to one receive interface, letting the SAI layer expand one ACE into one rule per named port: 0: ipv4 deny src 0.0.0.0/0 dst 0.0.0.0/0 proto 0 ... in-port bobm1 1: ipv4 deny src 0.0.0.0/0 dst 0.0.0.0/0 proto 0 ... in-port bobm10 The match itself is close to free. fa_5tuple_t already carries the receive interface in l4.lsb_of_sw_if_index, inside the hashed part of the key, so a scoped rule only has to stop masking that field: mask->l4.lsb_of_sw_if_index = 0xffff; hi->match.l4.lsb_of_sw_if_index = r->in_sw_if_index & 0xffff; in_sw_if_index 0 means "any interface", the same convention proto = 0 already uses for L4. sw_if_index 0 is local0 and never a data-plane port, so no separate valid flag is needed. Only the low 16 bits are matched, since that is what the packet-side field carries. All three match paths check the interface, not just the hash mask: - single_acl_match_5tuple(): non-first fragments bypass the hash and fall back to linear matching, so without this a scoped rule would match fragments received on any interface. - single_rule_match_5tuple(): this is the hash collision check, and it must not assume the interface is in the key. tuplemerge reuses any existing less-specific mask type, so an ACL carrying a wildcard rule applied to the same lookup context ahead of a scoped one can fold the scoped rule into a mask that leaves lsb_of_sw_if_index masked off. Checking here keeps correctness independent of ACL bind order. Also wire the field through the API type, the API/rule conversion, the "in-port" CLI parser and the ACL display. Note the API wire type changes, so the acl_add_replace CRC changes and VPP and sonic-sairedis must be rebuilt together. rules/vpp.mk: VPP_VERSION 2606-0.5 -> 2606-0.6, since the patch series changed and the version is the buildkite deb cache key. Signed-off-by: Longxiang Lyu --- rules/vpp.mk | 2 +- .../0018-acl-match-on-ingress-interface.patch | 152 ++++++++++++++++++ vppbld/patches/series | 4 + 3 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 vppbld/patches/0018-acl-match-on-ingress-interface.patch diff --git a/rules/vpp.mk b/rules/vpp.mk index 1d59ed0c..739c82c6 100644 --- a/rules/vpp.mk +++ b/rules/vpp.mk @@ -7,7 +7,7 @@ VPP_VERSION_BASE = 2606 # https://packages.buildkite.com/sonic-vpp/vpp; if the suffix isn't bumped, # downstream sonic-buildimage builds will silently pull stale debs that # pre-date the new patch series and end up with VPP/SAI CRC drift. -VPP_VERSION = $(VPP_VERSION_BASE)-0.5 +VPP_VERSION = $(VPP_VERSION_BASE)-0.6 VPP_VERSION_SONIC = $(VPP_VERSION)+b1sonic1 VPP_SRC_PATH = platform/vpp/vppbld diff --git a/vppbld/patches/0018-acl-match-on-ingress-interface.patch b/vppbld/patches/0018-acl-match-on-ingress-interface.patch new file mode 100644 index 00000000..5e31f291 --- /dev/null +++ b/vppbld/patches/0018-acl-match-on-ingress-interface.patch @@ -0,0 +1,152 @@ +diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c +index 6724f503b..9b84ce0a6 100644 +--- a/src/plugins/acl/acl.c ++++ b/src/plugins/acl/acl.c +@@ -177,6 +177,11 @@ acl_print_acl_x (acl_vector_print_func_t vpr, vlib_main_t * vm, + format (out0, " tcpflags %d mask %d", r->tcp_flags_value, + r->tcp_flags_mask); + } ++ if (r->in_sw_if_index) ++ { ++ out0 = format (out0, " in-port %U", format_vnet_sw_if_index_name, ++ vnet_get_main (), r->in_sw_if_index); ++ } + out0 = format (out0, "\n"); + vpr (vm, out0); + } +@@ -376,6 +381,7 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], + r->dst_port_or_code_last = ntohs (rules[i].dstport_or_icmpcode_last); + r->tcp_flags_value = rules[i].tcp_flags_value; + r->tcp_flags_mask = rules[i].tcp_flags_mask; ++ r->in_sw_if_index = ntohl (rules[i].in_sw_if_index); + } + + if (~0 == *acl_list_index) +@@ -1912,6 +1918,7 @@ copy_acl_rule_to_api_rule (vl_api_acl_rule_t * api_rule, acl_rule_t * r) + api_rule->dstport_or_icmpcode_last = htons (r->dst_port_or_code_last); + api_rule->tcp_flags_mask = r->tcp_flags_mask; + api_rule->tcp_flags_value = r->tcp_flags_value; ++ api_rule->in_sw_if_index = htonl (r->in_sw_if_index); + } + + static void +@@ -2861,6 +2868,7 @@ acl_set_aclplugin_acl_fn (vlib_main_t * vm, + u32 port2 = 0; + u32 action = 0; + u32 tcpflags, tcpmask; ++ u32 in_sw_if_index = 0; + ip_prefix_t src, dst; + u8 *tag = 0; + +@@ -2909,6 +2917,12 @@ acl_set_aclplugin_acl_fn (vlib_main_t * vm, + vec_validate_acl_rules (rules, rule_idx); + ip_prefix_encode2 (&dst, &rules[rule_idx].dst_prefix); + } ++ else if (unformat (line_input, "in-port %U", unformat_vnet_sw_interface, ++ vnet_get_main (), &in_sw_if_index)) ++ { ++ vec_validate_acl_rules (rules, rule_idx); ++ rules[rule_idx].in_sw_if_index = htonl (in_sw_if_index); ++ } + else if (unformat (line_input, "sport %d-%d", &port1, &port2)) + { + vec_validate_acl_rules (rules, rule_idx); +diff --git a/src/plugins/acl/acl_types.api b/src/plugins/acl/acl_types.api +index b8a9482f0..1d7834f9d 100644 +--- a/src/plugins/acl/acl_types.api ++++ b/src/plugins/acl/acl_types.api +@@ -69,6 +69,16 @@ typedef acl_rule + */ + u8 tcp_flags_mask; + u8 tcp_flags_value; ++/* ++ * Match only packets received on this ingress interface. ++ * 0 => match any interface. sw_if_index 0 is local0, which is never a ++ * data-plane port, so it is used as the "any" value rather than spending ++ * a separate valid flag - the same convention proto = 0 uses for L4. ++ * ++ * Only the low 16 bits are matched, so this cannot distinguish interfaces ++ * whose indices differ only above bit 15. ++ */ ++ u32 in_sw_if_index; + }; + + +diff --git a/src/plugins/acl/hash_lookup.c b/src/plugins/acl/hash_lookup.c +index 3f6fce060..2ce865665 100644 +--- a/src/plugins/acl/hash_lookup.c ++++ b/src/plugins/acl/hash_lookup.c +@@ -971,6 +971,19 @@ make_mask_and_match_from_rule(fa_5tuple_t *mask, acl_rule_t *r, hash_ace_info_t + + mask->pkt.is_ip6 = 1; + hi->match.pkt.is_ip6 = r->is_ipv6; ++ ++ /* ++ * Ingress interface match. lsb_of_sw_if_index is already filled from ++ * VLIB_RX for every packet on the input arc and already lives inside the ++ * hashed part of the key, so restricting a rule to an interface only ++ * requires unmasking it - no extra per-packet work. ++ */ ++ if (r->in_sw_if_index != 0) ++ { ++ mask->l4.lsb_of_sw_if_index = 0xffff; ++ hi->match.l4.lsb_of_sw_if_index = r->in_sw_if_index & 0xffff; ++ } ++ + if (r->is_ipv6) { + make_ip6_address_mask(&mask->ip6_addr[0], r->src_prefixlen); + hi->match.ip6_addr[0] = r->src.ip6; +diff --git a/src/plugins/acl/public_inlines.h b/src/plugins/acl/public_inlines.h +index a70e88025..2710843de 100644 +--- a/src/plugins/acl/public_inlines.h ++++ b/src/plugins/acl/public_inlines.h +@@ -302,6 +302,16 @@ single_acl_match_5tuple (acl_main_t * am, u32 acl_index, fa_5tuple_t * pkt_5tupl + { + continue; + } ++ /* ++ * Ingress interface match. Also checked here and not only in the hash ++ * path: non-first fragments fall back to linear matching, so a rule ++ * restricted to an interface would otherwise match on any of them. ++ */ ++ if (r->in_sw_if_index != 0 && ++ pkt_5tuple->l4.lsb_of_sw_if_index != (r->in_sw_if_index & 0xffff)) ++ { ++ continue; ++ } + if (is_ip6) { + if (!fa_acl_match_ip6_addr + (&pkt_5tuple->ip6_addr[1], &r->dst.ip6, r->dst_prefixlen)) +@@ -472,6 +482,19 @@ single_rule_match_5tuple (acl_rule_t * r, int is_ip6, fa_5tuple_t * pkt_5tuple) + return 0; + } + ++ /* ++ * Ingress interface match. This is the collision check of the hash path, so ++ * it must not rely on the interface being part of the hash key: tuplemerge ++ * may fold this rule into an existing, less specific mask type that leaves ++ * lsb_of_sw_if_index masked off, which would let the rule match packets ++ * received on any interface. ++ */ ++ if (r->in_sw_if_index != 0 && ++ pkt_5tuple->l4.lsb_of_sw_if_index != (r->in_sw_if_index & 0xffff)) ++ { ++ return 0; ++ } ++ + if (is_ip6) + { + if (!fa_acl_match_ip6_addr +diff --git a/src/plugins/acl/types.h b/src/plugins/acl/types.h +index 3999ee649..92412a9dd 100644 +--- a/src/plugins/acl/types.h ++++ b/src/plugins/acl/types.h +@@ -24,6 +24,8 @@ typedef struct + u16 dst_port_or_code_last; + u8 tcp_flags_value; + u8 tcp_flags_mask; ++ /* Ingress interface to match, 0 for any. See acl_types.api. */ ++ u32 in_sw_if_index; + } acl_rule_t; + + diff --git a/vppbld/patches/series b/vppbld/patches/series index 43f96210..80f09fc0 100644 --- a/vppbld/patches/series +++ b/vppbld/patches/series @@ -35,3 +35,7 @@ # 17. VXLAN VNET source-independent ("decap-any") decap: match local dst+vni # ignoring outer src (RIOT / secondary-VTEP VNET decap) + l2_bvi helper 0017-sonic-vxlan-vnet-source-independent-decap.patch +# 18. acl: match on ingress interface (in_sw_if_index) - lets a single ACL +# rule be scoped to specific ports, so SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS +# no longer has to be silently dropped by the SAI layer. +0018-acl-match-on-ingress-interface.patch From 5bc79f73467cfb0f4eb7b9cf18265c76c80e6dc6 Mon Sep 17 00:00:00 2001 From: Longxiang Lyu Date: Sat, 29 Aug 2026 08:55:14 +0000 Subject: [PATCH 2/2] [vpp][acl] reject an in_sw_if_index that does not fit in 16 bits The 5-tuple carries the ingress interface in lsb_of_sw_if_index, a u16 packed into the L4 key, so only the low 16 bits of in_sw_if_index take part in the match. An index above 0xffff would therefore be programmed as a different interface than the one named, and both the CLI output and the API dump would keep reporting the interface the user asked for, giving no indication that the rule matches somewhere else. Reject such an index in acl_add_list() instead, next to the existing prefix and port range checks, so it fails at configuration time on both the binary API and the CLI path rather than silently mismatching. This is a limit of the key layout rather than of this validation: widening the field would mean growing fa_session_l4_key_t beyond the u64 it packs into. The interface indices SONiC creates stay far below 0xffff, so the check is not expected to fire in practice. Signed-off-by: Longxiang Lyu --- .../0018-acl-match-on-ingress-interface.patch | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/vppbld/patches/0018-acl-match-on-ingress-interface.patch b/vppbld/patches/0018-acl-match-on-ingress-interface.patch index 5e31f291..b77e51a3 100644 --- a/vppbld/patches/0018-acl-match-on-ingress-interface.patch +++ b/vppbld/patches/0018-acl-match-on-ingress-interface.patch @@ -1,5 +1,5 @@ diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c -index 6724f503b..9b84ce0a6 100644 +index 6724f503b..f0bdfdf28 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -177,6 +177,11 @@ acl_print_acl_x (acl_vector_print_func_t vpr, vlib_main_t * vm, @@ -14,7 +14,21 @@ index 6724f503b..9b84ce0a6 100644 out0 = format (out0, "\n"); vpr (vm, out0); } -@@ -376,6 +381,7 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], +@@ -334,6 +339,13 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], + if (ntohs (rules[i].dstport_or_icmpcode_first) > + ntohs (rules[i].dstport_or_icmpcode_last)) + return VNET_API_ERROR_INVALID_VALUE_2; ++ /* ++ * The 5-tuple only carries the low 16 bits of the ingress interface, ++ * so an index that does not fit would silently be matched as a ++ * different interface. Refuse it instead of matching the wrong one. ++ */ ++ if (ntohl (rules[i].in_sw_if_index) > 0xffff) ++ return VNET_API_ERROR_INVALID_SW_IF_INDEX; + } + + if (*acl_list_index != ~0) +@@ -376,6 +388,7 @@ acl_add_list (u32 count, vl_api_acl_rule_t rules[], r->dst_port_or_code_last = ntohs (rules[i].dstport_or_icmpcode_last); r->tcp_flags_value = rules[i].tcp_flags_value; r->tcp_flags_mask = rules[i].tcp_flags_mask; @@ -22,7 +36,7 @@ index 6724f503b..9b84ce0a6 100644 } if (~0 == *acl_list_index) -@@ -1912,6 +1918,7 @@ copy_acl_rule_to_api_rule (vl_api_acl_rule_t * api_rule, acl_rule_t * r) +@@ -1912,6 +1925,7 @@ copy_acl_rule_to_api_rule (vl_api_acl_rule_t * api_rule, acl_rule_t * r) api_rule->dstport_or_icmpcode_last = htons (r->dst_port_or_code_last); api_rule->tcp_flags_mask = r->tcp_flags_mask; api_rule->tcp_flags_value = r->tcp_flags_value; @@ -30,7 +44,7 @@ index 6724f503b..9b84ce0a6 100644 } static void -@@ -2861,6 +2868,7 @@ acl_set_aclplugin_acl_fn (vlib_main_t * vm, +@@ -2861,6 +2875,7 @@ acl_set_aclplugin_acl_fn (vlib_main_t * vm, u32 port2 = 0; u32 action = 0; u32 tcpflags, tcpmask; @@ -38,7 +52,7 @@ index 6724f503b..9b84ce0a6 100644 ip_prefix_t src, dst; u8 *tag = 0; -@@ -2909,6 +2917,12 @@ acl_set_aclplugin_acl_fn (vlib_main_t * vm, +@@ -2909,6 +2924,12 @@ acl_set_aclplugin_acl_fn (vlib_main_t * vm, vec_validate_acl_rules (rules, rule_idx); ip_prefix_encode2 (&dst, &rules[rule_idx].dst_prefix); } @@ -52,7 +66,7 @@ index 6724f503b..9b84ce0a6 100644 { vec_validate_acl_rules (rules, rule_idx); diff --git a/src/plugins/acl/acl_types.api b/src/plugins/acl/acl_types.api -index b8a9482f0..1d7834f9d 100644 +index b8a9482f0..a239f3795 100644 --- a/src/plugins/acl/acl_types.api +++ b/src/plugins/acl/acl_types.api @@ -69,6 +69,16 @@ typedef acl_rule @@ -65,8 +79,8 @@ index b8a9482f0..1d7834f9d 100644 + * data-plane port, so it is used as the "any" value rather than spending + * a separate valid flag - the same convention proto = 0 uses for L4. + * -+ * Only the low 16 bits are matched, so this cannot distinguish interfaces -+ * whose indices differ only above bit 15. ++ * Only the low 16 bits are matched, so an index that does not fit in 16 ++ * bits is rejected rather than matched as a different interface. + */ + u32 in_sw_if_index; };