[vpp][acl] match on ingress interface to support SAI ACL IN_PORTS

[lolyu]

Description of PR

Summary:
Implements SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS for the VPP platform by fanning each ACL entry out into one VPP rule per named ingress port.

A VPP ACL rule has no notion of an ingress interface, so until now the field had nowhere to go: it fell through to the default: case of acl_rule_field_update(), which logs Unhandled ACL entry attribute ID and carries on. The entry was still programmed, just without the restriction, so it applied to every port its table was bound to.

That is the wrong direction to fail in. On a dual ToR the standby DENY entries name only the standby mux ports, so dropping the restriction turns them into a table wide deny and blackholes traffic on the active ports as well.

The matching VPP side is sonic-net/sonic-platform-vpp#278, which adds an in_sw_if_index field to the ACL rule. Design writeup: sonic-net/SONiC#2506.

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Documentation update
• Test improvement

Approach

What is the motivation for this PR?

Fixes: sonic-net/sonic-buildimage#29077

IN_PORTS is how orchagent scopes an ACL entry to a subset of the ports its table is bound to. aclorch restricts it to physical interfaces, and the dual ToR standby DENY entries rely on it.

Silently widening those entries to the whole table is worse than not supporting the field at all, because nothing in the logs distinguishes a rule that was scoped from one that was not.

Work item tracking

• Microsoft ADO (number only): 39466368

How did you do it?

A VPP ACL rule matches one interface, so a SAI entry naming N ports becomes N VPP rules:

• acl_entry_in_ports_get() finds the attribute and resolves it. The cached attribute only carries the list count, its list pointer is still null, so the entry is read again with a buffer of the right size. Each port OID is then resolved to a VPP interface name via vpp_get_hwif_name().
• fill_acl_rules() pops the rules it just generated for the entry and re-adds one copy per resolved interface. This is the same shape as the existing fan-out where a port based entry with no protocol becomes a UDP rule and a TCP rule.
• vpp_acl_rule_t gains in_hwif_name. The SAI layer keeps working in interface names, as it already does for binding, and vpp_acl_add_replace() resolves the name to a sw_if_index when it builds the API message. Empty means any.
• SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS becomes an explicit no-op in acl_rule_field_update(), since it is not a per rule field.

The fanned out rules stay contiguous from ace.vpp_rule_base_index, so num_rules still describes the entry and the counter aggregation in getAclEntryStats() sums over them unchanged.

Failure handling is deliberate, since three different things can leave the interface set empty and they do not mean the same thing:

Case	Behaviour
Field absent or not enabled	Not scoped, one rule as before
Enabled but names no port	Scoped to nothing, no rule emitted
Port list unreadable, or no port resolves	Error, ACL programming fails

The last row matters. On failure the scope of the entry is simply unknown, and installing it unscoped would re-introduce exactly the blackhole this PR is fixing, so fill_acl_rules() fails and CHECK_STATUS_ACLTBLCONFIG leaves the previous ACL in place.

The empty list case is left as "matches nothing" on purpose: no ingress interface can be a member of an empty list, so emitting no rule is the faithful translation.

How did you verify/test it?

Built into docker-syncd-vpp and deployed to a vms-kvm-dual-vpp-t0-1 dual ToR testbed, then checked the programmed rules against the mux state:

• vlab-vpp-03: 18 standby ports, 18 matching ingress scoped deny rules.
• vlab-vpp-04: 6 standby ports, 6 matching ingress scoped deny rules.
• ACL 3 stayed bound to all 32 interfaces in both cases, confirming the scoping comes from the rules and not from the binding.

Rule counts tracking the standby set on two ToRs with different mux distributions is the behaviour this PR is after, and is what regressed before.

Still outstanding: live downstream forwarding under mux toggles (standby → active → standby) has not been run yet, so the dataplane effect is verified by rule inspection rather than by packets.

Any platform specific information?

VPP only.

This depends on sonic-net/sonic-platform-vpp#278 and the two have to merge together. Adding the field changes the acl_add_replace message, and therefore its CRC, so a syncd built against one VPP version cannot program ACLs on the other. That PR also bumps VPP_VERSION to 2606-0.6.

A scale note for reviewers: an entry naming N ports costs N rules, so a table bound to many mux ports grows proportionally. VPP's tuplemerge can additionally fold the fanned out copies into one less specific mask type, and split_partition() has no interface dimension to split them back apart on, so they can end up on a single collision chain. Correctness is unaffected, single_rule_match_5tuple() in the platform PR re-checks the interface on the collision path, but it is worth knowing before scaling the port count up.

Documentation

Design is written up in sonic-net/SONiC#2506.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR prototypes support for SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS in the VPP-backed VS pipeline by scoping generated VPP ACL rules to a specific ingress interface, while keeping the higher SAI layer working in interface names and resolving to VPP sw_if_index at programming time.

Changes:

• Extend vpp_acl_rule_t with an in_hwif_name field to carry an optional ingress interface name for rule scoping.
• In SwitchVpp::fill_acl_rules(), fan out generated rules per IN_PORTS port and stamp each copy with the resolved VPP interface name.
• In vpp_acl_add_replace(), resolve in_hwif_name to in_sw_if_index (or 0 for “any”) when programming rules into the VPP ACL plugin.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
vslib/vpp/vppxlate/SaiVppXlate.h	Adds in_hwif_name to the VPP ACL rule representation for ingress scoping.
vslib/vpp/vppxlate/SaiVppXlate.c	Resolves in_hwif_name to in_sw_if_index when sending rules to VPP.
vslib/vpp/SwitchVppAcl.cpp	Implements IN_PORTS-driven rule fan-out and adds helper to re-read the IN_PORTS list.
vslib/vpp/SwitchVpp.h	Declares the new IN_PORTS helper and adds needed STL includes.

Suppressed comments (3)

vslib/vpp/SwitchVppAcl.cpp:1052

• acl_entry_in_ports_get() is documented to return true only when the entry is scoped to named ports, but the count == 0 path returns true with an empty hwifs set. That can trigger the fan-out path and drop rules. Return false here so the caller treats the entry as unscoped (or handle empty-set scoping explicitly).

    if (count == 0) {
        SWSS_LOG_WARN("ACL entry %s has an enabled but empty IN_PORTS list",
                      sai_serialize_object_id(ace_oid).c_str());
        return true;
    }

vslib/vpp/SwitchVppAcl.cpp:1082

• The function contract says it returns true only when the entry is scoped to named ports, but this unconditional return true; reports scoped even if all ports failed to resolve to VPP interface names (leaving hwifs empty). Returning !hwifs.empty() keeps the return value consistent with the output set.

    return true;

vslib/vpp/SwitchVppAcl.cpp:1066

• On failure to re-read the IN_PORTS list, acl_entry_in_ports_get() currently returns true, which makes the caller think the entry should be scoped even though no interfaces were produced. Return false on read failure so rules are not accidentally dropped.

    if (get(SAI_OBJECT_TYPE_ACL_ENTRY, ace_oid, 1, &attr) != SAI_STATUS_SUCCESS) {
        SWSS_LOG_ERROR("Failed to read IN_PORTS list of ACL entry %s",
                       sai_serialize_object_id(ace_oid).c_str());
        return true;
    }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[mssonicbld]

/azp run

[lolyu]

Pushed 6864c79, which came out of review on the HLD (SONiC#2506).

@yue-fred-gao asked what happens if IN_PORTS is used in an egress ACL. It turned out to be a real gap rather than an unsupported-but-harmless one. The ACL plugin fills the 5-tuple slot this scope matches on by direction:

if (is_input)
  out_sw_if_index[ii] = vnet_buffer (b[ii])->sw_if_index[VLIB_RX];
else
  out_sw_if_index[ii] = vnet_buffer (b[ii])->sw_if_index[VLIB_TX];

SAI puts no stage constraint on SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS, and aclBindUnbindPort() binds egress groups for real, so an egress table's entry would have been fanned out and programmed normally — then matched against the egress interface. "Drop what arrives on EthernetX" would have quietly become "drop what leaves on EthernetX", with the dump still printing the interface the operator named.

fill_acl_rules() now resolves the table's stage and fails such an entry with SAI_STATUS_NOT_SUPPORTED, matching the existing treatment of an interface index that does not fit in 16 bits. I considered silently dropping the scope instead and rejected it: that reproduces the exact bug this PR exists to fix — an over-broad rule applying to every bound port.

No in-tree consumer creates this configuration (every SONiC user of IN_PORTS is ingress), so this is a guard rather than a path expected to be taken.

Verified with a full -fsyntax-only type-check of the translation unit against the SAI revision this branch pins and real swss-common headers, including a negative control to confirm the check was actually exercising the new code.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[lolyu]

For anyone looking at the red X: the two Azure failures are expected at this point, and they are the matched-pair dependency this PR describes, showing up as a compile error rather than a runtime one.

Both failing jobs (Build amd64 and BuildAsan amd64) die in the same step, Compile sonic sairedis with coverage enabled:

vpp/vppxlate/SaiVppXlate.c: In function 'vpp_acl_add_replace':
vpp/vppxlate/SaiVppXlate.c:3421:21: error: 'vl_api_acl_rule_t' {aka 'struct _vl_api_acl_rule'}
    has no member named 'in_sw_if_index'
 3421 |             vpp_rule->in_sw_if_index = htonl(in_idx);

vslib builds with -Werror, so this is fatal.

The header comes from the VPP artifact the build environment installs for us:

INFO buildenv_setup.cascade: upstream 'vpp': downloading artifact vpp-bookworm
INFO buildenv_setup.azp_client: resolved sonic-net.sonic-platform-vpp -> build 1208678 (branch master)
...
Unpacking vpp-dev (26.10-rc0~31-g3f9e978d7)

That vpp-dev is built from sonic-platform-vpp master, which does not yet carry 0018-acl-match-on-ingress-interface.patch — that patch is #278, still open. So /usr/include/vpp_plugins/acl/acl.api.h is generated from an acl_rule with six fields and no in_sw_if_index:

vl_api_acl_action_t is_permit;
vl_api_prefix_t     src_prefix;
vl_api_prefix_t     dst_prefix;
vl_api_ip_proto_t   proto;
u8                  tcp_flags_mask;
u8                  tcp_flags_value;

This is the same CRC coupling called out in the PR description and in the HLD: the field has to exist in the .api definition before the SAI layer can assign it.

What unblocks it: merge sonic-platform-vpp#278 first. Once its master build publishes a new vpp-bookworm artifact, this pipeline picks it up automatically — the download uses runVersion: latestFromBranch on refs/heads/master — and a re-run goes green with no change here.

There is no way to make this PR green beforehand. staged_upstreams in build-template.yml only covers artifacts produced earlier in the same pipeline run, so it cannot be pointed at the #278 branch. The alternative — guarding the assignment behind an autoconf member check so it compiles against either header — would be worse: it would silently compile the feature out and produce a syncd that accepts IN_PORTS and ignores it, which is precisely the failure mode this PR exists to remove.

Functionally the pair has been validated together: the combined patch series builds, and the resulting image was exercised on a dual-ToR VPP testbed.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[lolyu]

Thanks — this is a genuinely useful review. All four line references check out against e0de9e71. I've taken item 1, pushed back on the remedies for 2 and 3 (while accepting both findings), and kept 4 as-is for now. Detail below, plus one thing the review led me to that I think matters more than any of the four.

Pushed as a6f0a407.

---

1. Unresolved IN_PORTS fails the whole shared table — fixed

Confirmed end to end, including the shared-table claim: muxorch.cpp:49 sets MUX_ACL_TABLE_NAME = INGRESS_TABLE_DROP and pfcactionhandler.cpp:322 sets m_strIngressTable = INGRESS_TABLE_DROP, so those really are the same table.

One precision correction to the write-up: cleanup_acl_tbl_config() only frees heap and never removes the VPP ACL, so the table doesn't go down — it freezes at its last-good rule set. Arguably worse, because every later add or remove re-walks the bad entry and fails again, and with orchagent treating the retry's ITEM_ALREADY_EXISTS as success the rule is marked ACTIVE throughout. Permanent and silent rather than visible.

Fixed as suggested: hwifs is left empty and the caller emits no rule, reusing the egress path. I extended it to the get() failure at :1136 as well, since the scope is equally unknown there and the alternatives are identical.

2. Partial resolution — finding accepted, remedy declined

Emitting zero rules when any port fails to resolve makes a DENY entry strictly more fail-open:

ports {A, B}, B unresolved	A	B
today	denied	not denied
proposed	not denied	not denied

Same direction for permit-in-default-deny. Partial resolution is closer to intent, and a port absent from VPP can't carry traffic anyway.

That said, the inconsistency you identified was real — and fixing item 1 dissolves it without a separate change. Both paths now follow one rule: emit rules for the ports that resolved, with all-unresolved as the degenerate zero case. That's the consistent policy you asked for, arrived at from the other end.

3. MAX_ACL_ATTRS truncation — finding accepted, remedy declined, and it's bigger than IN_PORTS

Confirmed the mechanism: AttrHash is std::map<std::string, ...> (SwitchState.h:29) keyed by attridname (SwitchStateBase.cpp:639), so the cut is alphabetical as you said. IN_PORTS ranks 107/170 in the full namespace — 62 ACTION_*, 43 FIELD_* and ADMIN_STATE sort ahead of it. (Also worth noting every FIELD_INNER_* sorts before FIELD_IN_PORTS, since 'N' 0x4e < '_' 0x5f.)

Modeling real rule shapes:

shape	IN_PORTS position
mux standby drop	4/12
L3 5-tuple	5/12
L3V4V6 v4+v6 + icmp + range	11/12
+ ACL_IP_TYPE	12/12
+ ACL_USER_META	13/12 — truncated

So your "reachable entries are safe" holds, but by two attributes.

Here's why I don't want the targeted get(): at 21 attributes the same cut also drops PRIORITY, TABLE_ID, SRC_IP, IP_PROTOCOL, L4_* and TCP_FLAGS. And acl_priority_attr_get() returns FAILURE when PRIORITY is missing with its return value ignored at :768, leaving priority 0 and silently misordering the rule.

That's a pre-existing systemic bug, considerably worse than the IN_PORTS symptom. Fixing only IN_PORTS would make it harder to find, since scoping would start working while match fields kept vanishing. I'd rather raise the cap and warn when attrs_count hits it — fixing all of it — in a separate PR, since none of it is caused by this change. Happy to open that; say the word.

4. Egress guard fallback direction — kept, but I'd flip it if you feel strongly

Reasoning: the branch is near-unreachable, so the tiebreaker is which error costs more. On VPP mux_tunnel_egress_acl defaults to disabled (below), so an ingress table is overwhelmingly the likely case, and flipping means an unreadable stage silently stops emitting rules on ordinary ingress tables — making the guard's failure mode worse than not having the guard. My rule of thumb is that a guard which can't make its determination shouldn't change behaviour. Low stakes either way; genuinely happy to flip on your call.

---

The thing your review led me to

Chasing item 4's reachability turned up something that corrects a claim in my own HLD. I had written that no in-tree consumer creates an egress table carrying IN_PORTS. That's wrong — muxorch.cpp:

1485:  string table_name = is_ingress_acl_ ? MUX_ACL_TABLE_NAME : EGRESS_TABLE_DROP;
1551:  acl_table.stage = is_ingress_acl_ ? ACL_STAGE_INGRESS : ACL_STAGE_EGRESS;
...
1566:  // Add MATCH_IN_PORTS as match criteria for ingress table
1567:  attr_name = MATCH_IN_PORTS;          // NOT conditioned on is_ingress_acl_

With SYSTEM_DEFAULTS|mux_tunnel_egress_acl enabled, MuxAclHandler creates an egress DROP table and adds MATCH_IN_PORTS unconditionally — the comment says "for ingress table" but the code doesn't branch. init_cfg.json.j2:211 defaults that key to enabled on mellanox and disabled elsewhere, so VPP is unaffected as shipped, but it's one CONFIG_DB key away, not a hypothetical.

So the egress guard is protecting a real in-tree path. HLD updated (84073d13) with this, the corrected failure-handling semantics, and a new restriction that mux_tunnel_egress_acl must stay disabled on VPP.

The red build is unrelated — cross-repo ordering with sonic-platform-vpp#278, as you noted and as detailed in the comment above.

[Copilot]

🔵 Needs a closer look

acl_entry_in_ports_get() currently returns success on IN_PORTS resolution failures (read failure / no ports resolve), making fill_acl_rules()’s error handling ineffective and potentially leading to silently omitted rules.

Review details

Suppressed comments (2)

vslib/vpp/SwitchVppAcl.cpp:1184

• When IN_PORTS is enabled and non-empty but none of the ports resolve to a VPP interface, this function only logs and still returns success; fill_acl_rules() will then treat the entry as scoped with an empty set and emit no rule, which is an implicit behavioral change. Return a failure status here so the caller can handle the error path consistently with the doxygen contract.

    if (hwifs.empty()) {
        SWSS_LOG_ERROR("None of the %u port(s) named by IN_PORTS of ACL entry %s resolve "
                       "to a VPP interface; no rule is emitted for this entry",
                       count, sai_serialize_object_id(ace_oid).c_str());
    }

vslib/vpp/SwitchVppAcl.cpp:1146

• acl_entry_in_ports_get() currently returns SAI_STATUS_SUCCESS even when re-reading the IN_PORTS list fails, which makes the status != SAI_STATUS_SUCCESS check in fill_acl_rules() ineffective and contradicts the function contract in SwitchVpp.h. Return the failing status (or SAI_STATUS_FAILURE) here so ACL programming can abort rather than silently proceeding with an unknown scope.

    if (get(SAI_OBJECT_TYPE_ACL_ENTRY, ace_oid, 1, &attr) != SAI_STATUS_SUCCESS) {
        /*
         * The scope cannot be determined, so leave hwifs empty and let the
         * caller emit no rule for this entry. See the note below on why that
         * is preferred over failing the table.
         */
        SWSS_LOG_ERROR("Failed to read IN_PORTS list of ACL entry %s; no rule is emitted "
                       "for this entry",
                       sai_serialize_object_id(ace_oid).c_str());
        return SAI_STATUS_SUCCESS;
    }

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[lolyu]

CI update — the in_sw_if_index blocker is cleared

sonic-platform-vpp#278 merged at 07:39Z, so the compile failure this PR was blocked on is gone.

Worth recording why the first post-merge run (build 1210614, 09:34Z) still failed with the same error: it was artifact timing, not a second defect. The platform-vpp master build for merge commit 7fb27b0 did not finish until 11:18:30Z, so that run resolved sonic-net.sonic-platform-vpp -> build 1209689, a pre-merge master build, and installed a vpp-dev without patch 0018.

The current run (1210796) resolves build 1210723 and:

step	before	now
Compile sonic sairedis with coverage enabled (Build amd64)	fail	pass
Compile sonic sairedis with coverage enabled (BuildAsan amd64)	fail	pass
BuildAsan amd64 overall	fail	pass

The one remaining red is FlexCounter.bulkChunksize in unittest/syncd/, which is a known-flaky test tracked by #1978, #1888, #1986 and #1818. It's unrelated to this PR — the change here is confined to vslib/vpp/ — and it passed in the ASan job of this same run.

Also rebased onto master (1205916d, the l2-vlan-filter FDB change) so the branch is current. SwitchVppAcl.cpp is byte-identical across that rebase, so the review threads above are unaffected.

[lolyu]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[aaronber0614]

Thanks, this is a thorough pass and the egress + IN_PORTS catch in MuxOrch is a good one. The item 1 fix looks right: both the read failure and the all unresolved case now cost the entry its rules instead of the table, and partial and total collapse into one policy. Agreed on keeping item 4 as ingress too; that branch is a single attr get of a mandatory on create stage so it is effectively unreachable, and I would rather not silence rules on ordinary ingress tables to harden a path that cannot really be hit.

The one thing I want to land, and I think it is the same follow up you already offered: nothing reprograms an entry once a port it named becomes resolvable. The only caller of AclTblConfig is an ACL add or remove on that table, so an entry that resolves to zero ports or a subset is created and reported done, and the missing scope stays missing until some later unrelated add or remove on the same table happens to rewalk it. That is strictly better than the old freeze, so I am not blocking on it, but it turns a transient resolution miss into a durable one.

That is also why I would not lean on "a port absent from VPP cannot carry traffic" to justify the partial case. Unresolved here is a name to hwif resolution miss, not proof the interface is down. Warm boot restores the object hash but does not repopulate m_hostif_info_map, so a live forwarding port can miss resolution during bring up and, with no reprogram on resolve, stay unscoped afterward. For a standby DROP that is a real fail open window on exactly the port that matters.

So my one ask: fold a reprogram on resolve (or the createAclEntry rollback you mentioned) into the follow up, so a port that appears after the entry gets the scope applied rather than waiting on an unrelated table mutation.

Two smaller notes, neither blocking:

• On the sticky failure write up: I traced the retry and it does not reach ITEM_ALREADY_EXISTS. Orchagent allocates a fresh OID on each retry, leaking the previous RID, so create_internal never hits its same id check and the task stays in PENDING_CREATION rather than being marked ACTIVE. The freeze and the no rollback points still hold, just via a leaked object rather than an active rule.
• Item 3: agree it is bigger than IN_PORTS and belongs in its own PR. The piece I care most about is acl_priority_attr_get returning FAILURE on a truncated PRIORITY with its return ignored at :768, which silently leaves priority 0 and misorders the rule. Please fold that into the same cap raise and warn change. A one line warn when attrs_count hits MAX_ACL_ATTRS in this PR would also be welcome, since the IN_PORTS lookup now depends on the qualifier surviving the dump, but I am fine taking the full fix separately.

Reviewed by AI agent on behalf of aaronber0614.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[lolyu]

All three land. Two of them correct things I had written, so thank you — I've fixed the artifacts rather than just the conversation.

Pushed b51bb24f.

The retry mechanism — you're right, my write-up was wrong

I traced it and confirmed your version. RedisRemoteSaiInterface::create() allocates unconditionally:

// lib/RedisRemoteSaiInterface.cpp:219
*objectId = m_virtualObjectIdManager->allocateNewObjectId(objectType, switchId);

So every retry arrives at create_internal() with a new serialized id, the objectHash.find() duplicate check never fires, and SAI_STATUS_ITEM_ALREADY_EXISTS is unreachable on this path. On the orchagent side the generic failure branch is setAclRuleStatus(..., PENDING_CREATION); it++; — the task stays in m_toSync and is never marked ACTIVE.

So it's a leak per retry, not a rule falsely reported healthy. The freeze and no-rollback conclusions are unchanged, but the mechanism was wrong and it was wrong in the commit message and the HLD, not just in a comment. Both corrected: commit message amended in place (message-only — verified the tree hash is byte-identical), HLD updated in cb6e6267.

The partial-resolution justification — also right to reject it

I had written "a port genuinely absent from VPP cannot carry traffic". That's not what an unresolved port means. vpp_get_hwif_name() resolves via m_port_id_to_tapname, which is populated only in vs_create_hostif_tap_interface(), so resolution tracks hostif creation order, not link state. A forwarding port misses if the ACL entry is programmed first.

One refinement on the warm-boot specifics: SAI_VS_HOSTIF_USE_TAP_DEVICE=true for this platform, so VirtualSwitchSaiInterface does call vs_recreate_hostif_tap_interfaces() on the warm path, which re-runs setPortIdToTapName() for each hostif. m_hostif_info_map does carry the in-tree TODO you're thinking of:

// vslib/SwitchStateBase.cpp:57
// TODO populate m_hostif_info_map - need to be able to remove port after warm boot
// should be auto populated vs_recreate_hostif_tap_interfaces on create_switch

but the map the IN_PORTS lookup actually depends on is repopulated by that path. So warm boot specifically is narrower than stated — the ordering window during bring-up is the real exposure, and your conclusion holds on that alone. I've replaced the bad justification with the ordering argument rather than keeping a claim that only survives on one path.

Reprogram on resolve — agreed, and confirmed

Verified: AclTblConfig() is reached only from createAclEntry(), removeAclEntry() and setAclEntry(). All three are ACL mutations on that table; no port or hostif event re-drives it. So a partial or empty resolution is durable until an unrelated add or remove happens to re-walk the entry.

Taking it as follow-up, and I've recorded it in the HLD as a limitation with both candidate fixes (reprogram on resolve, or roll back in createAclEntry() so the retry re-drives it) rather than leaving it as an undocumented gap. Worth noting the rollback option is now more attractive than when I first raised it, since the retry leaks an object each time.

MAX_ACL_ATTRS warn — done in this PR

Added, since as you say the IN_PORTS lookup now depends on the qualifier surviving the dump:

if (p_ace->attrs_count >= MAX_ACL_ATTRS) {
    SWSS_LOG_WARN("ACL entry %s has at least %u attributes, the most this code reads; "
                  "any beyond that were dropped by name order and are invisible to it",
                  sid.c_str(), MAX_ACL_ATTRS);
}

Modelling the cut over the real SAI attribute names shows the margin is thinner than I'd assumed — the guard fires exactly when something is lost:

entry shape	attrs_count	warns	IN_PORTS lost	PRIORITY lost
mux standby drop	6	no	no	no
L3 five-tuple	11	no	no	no
L3V4V6 + ICMP + range + ACL_IP_TYPE + ACL_USER_META	12 (capped from 21)	yes	yes	yes

And agreed on acl_priority_attr_get — its SAI_STATUS_FAILURE return is discarded at :768, so a truncated PRIORITY silently leaves the rule at 0 and misorders it. That's the worse half of the bug and I'll fold it into the cap-raise PR alongside the assert/warn, not leave it to a follow-up of a follow-up. Also recorded as a limitation in the HLD in the meantime.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[yue-fred-gao]

The MAX_ACL_ATTRS could potentially cause problem. A quick fix is increasing it to 20. It is fine if you want to use a separate PR.