Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
First test image failed every dualtor server-traffic case — root-caused, not an ACL issueThe image built from the first revision ( The ACL change is working correctlyBoth fan-outs are exactly right. Upper ToR, with only Peer ToR, with the other 23 mux ports standby, has 23 rules — Live 20-packet replay,
That is precisely the intended behaviour: the active ToR forwards, the standby ToR drops the Y-cable copy on the port it is standby for. The real causeVPP trace on the upper ToR — the ACL permits, then the frame dies two nodes later: The bridge domain's BVI carries the switch MAC, not the shared VLAN gateway MAC: So every frame a server sends to its default gateway misses the L2 FIB and is dropped. Isolation test — same packet, same ports, same ACL state, only the destination MAC changed:
The delivered copies arrive with Both symptoms are sonic-net/sonic-swss#4816: FixI originally left
|
6b71afd to
b003690
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
…RTS branches
Build a test image carrying the SAI ACL IN_PORTS support so it can be
deployed to a dual-ToR VPP testbed.
The two changes are a matched pair and cannot be tested apart. Adding
in_sw_if_index to the VPP acl_rule type changes the layout of the
acl_add_replace message and therefore its CRC, so a syncd built against
one side cannot program ACLs on the other.
platform/vpp -> lolyu/acl_match_in_port
sonic-net/sonic-platform-vpp#278
src/sonic-sairedis -> lolyu/vpp_acl_in_port_match
sonic-net/sonic-sairedis#2064
vpp.mk carries VPP_VERSION 2606-0.6 for the new patch series. That deb
is not published, so the download step fails and vppbld/Makefile falls
back to build_locally, which applies the patch series and builds VPP
from source. The image build therefore also verifies that the patch
applies and compiles under the real build flags.
Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
The first test image built from this PR failed every dualtor server
traffic case: servers sent to their gateway and nothing came back.
The ACL work is not at fault. A VPP trace on vlab-vpp-03 shows the
frame is permitted by the ACL and then dropped two nodes later:
acl-plugin-in-ip4-l2 action: 1 (permit), acl 1 rule 0
lsb_of_sw_if_index 22
l2-fwd dst 00:aa:bb:cc:dd:ee result [0xffff..., -1]
error-drop l2-fwd: L2 forward misses
bvi1000 carries the switch MAC 22:ff:ab:76:97:c0 while Vlan1000 in the
kernel carries the shared gateway MAC 00:aa:bb:cc:dd:ee, and the bridge
domain has unknown-unicast flooding set to drop. Every frame a server
sends to its default gateway therefore misses the L2 FIB and is dropped.
That is sonic-net/sonic-swss#4816: IntfsOrch::doTask() creates the RIF
with the VLAN MAC and then immediately overwrites it with gMacAddress.
The reference PR 29062 carried that branch; omitting it here left the
testbed unable to route any server traffic.
Point src/sonic-swss at test_vpp_acl_in_ports_swss, which is the current
buildimage master pointer 93fc008c with the two fix commits cherry-picked
on top. The PR branch itself is 18 commits behind that pointer, so using
it directly would have regressed swss.
Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
Bump platform/vpp to 3f6f3f8, which adds VPP patch 0019 so an ACL deny no longer increments the ingress interface drop counter. The previous test image passed dualtor traffic correctly but failed dualtor_io/test_normal_op.py::test_upstream_standby_rx_drop_check: Ethernet16 on the standby ToR reported RX_DRP +5706 against a threshold of 10, because VPP charged every intentional ACL deny to /if/drops, which surfaces as RX_DRP. VPP_VERSION moves to 2606-0.7 with this bump, so the image build will again build VPP from source rather than fetching a cached deb. Signed-off-by: Longxiang Lyu <lolv@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
8a9c421 to
8835222
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
sonic-platform-vpp: the branch this pointed at carried both the IN_PORTS plugin patch and the ACL policy-drop counter fix. Those have since been split into two independent PRs (sonic-net#278 and sonic-net#280), so no single review branch holds both any more. Point at test_acl_in_ports_combined, which merges them for CI only, with the patches renumbered 0018/0019 and VPP_VERSION at 2606-0.7 - the same combination already validated on the vms-kvm-dual-vpp-t0-1 testbed. Note that merging the two branches conflicts only in patches/series; rules/vpp.mk auto-resolves silently because both sides set 2606-0.6 identically, so the version was bumped to 0.7 by hand. Leaving it at 0.6 would have published a deb under a version minted for one patch series while containing two. sonic-sairedis: pick up the egress-stage guard, so an IN_PORTS entry in an egress table emits no rule instead of being matched against the TX interface. Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The IN_PORTS patch had been renumbered to 0019 when the policy-drop fix was split into its own PR and took 0018. 0018 is the next free slot after master's 0017, and it belongs to the feature PR, so the two have been swapped back: sonic-platform-vpp#278 is 0018 and sonic-net#280 is 0019. Numbers only; the patch contents are unchanged and the two touch disjoint files, so the combined series still applies in either order. VPP_VERSION stays at 2606-0.7 for the CI image. Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
sonic-sairedis: an entry whose IN_PORTS cannot be resolved now emits no rule for that entry instead of failing the whole ACL table. Relevant to this image because IngressTableDrop is shared between MuxOrch and the PFC watchdog, so the old behaviour could freeze the mux drop table on a startup port-ordering race. Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
sonic-platform-vpp#278 has merged, so the hand-merged test_acl_in_ports_combined branch is no longer needed: patch 0018 is on master and sonic-net#280 has been rebased onto it, which leaves that branch carrying both patches at VPP_VERSION 2606-0.7 by itself. Point the submodule at acl_policy_drop_not_if_drop instead. The tree is identical to the combined branch apart from one stray blank line in series, so this changes nothing about what is built, but the image now tracks a branch that is actually under review rather than a throwaway merge that has to be re-created by hand every time either side moves. Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Resolves the src/sonic-sairedis conflict. Both sides moved the pointer from f6344f0b7: master to 1205916dd, this branch to its feature commit. The feature branch has been rebased onto 1205916dd, so its head is a descendant of master's pointer and is the correct resolution - the image keeps the l2-vlan-filter FDB change rather than silently dropping it. Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
sonic-sairedis: warn when an ACL entry's attribute dump hits MAX_ACL_ATTRS, since resolving IN_PORTS now depends on that qualifier surviving a cap that drops attributes by name order. Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Why I did it
Do not merge. This PR exists only to produce a test image; it will not be merged.
SAI ACL
IN_PORTSis not honoured by the VPP dataplane.saivppaccepts the qualifier and reports success, then discards the scope and programs the entry as if it were absent. An entry meant to drop on one port drops on every port the table is bound to.On a dual-ToR testbed this inverts MuxOrch's standby drop rule into a box-wide IPv4 outage: with one port in standby,
acl-index 3(ipv4 deny any -> any) was applied inbound on all 32 front-panel ports.The fix spans two repositories, which is why this PR points at both:
in_sw_if_indexto the VPP ACL rule so a rule can match the ingress interfaceRX_DRPrate#280 is independent of
IN_PORTSand is in review on its own, but the image needs it: without itdualtor_io/test_normal_op.py::test_upstream_standby_rx_drop_checkfails on the very drop rules this feature installs.HLD: sonic-net/SONiC#2506
Work item tracking
How I did it
Repointed the submodules at the PR branches:
test_acl_in_ports_combinedexists only for CI. The twosonic-platform-vppchanges were originally stacked on one branch, but they are independent and are now in review separately, so no single review branch carries both. It merges #278 and #280 with the patches numbered0018(IN_PORTS) and0019(policy drop), andVPP_VERSIONat2606-0.7.That version needs care. Both PR branches set
2606-0.6, and because the edit is textually identical on each side git auto-resolvesrules/vpp.mkwithout reporting a conflict — onlypatches/seriesconflicts. Left alone, the merge would publish a deb under a version minted for one patch series while actually containing two, which is precisely the stale-cache failurerules/vpp.mkwarns about. The bump to0.7is therefore deliberate and manual, and it matches the image already validated on the testbed.The two must move together. Adding
in_sw_if_indexto theacl_ruletype changes the layout of theacl_add_replacemessage and therefore its CRC, which VPP uses to detect API mismatch. A syncd built against one side cannot program ACLs on the other, so neither branch is testable alone.src/sonic-swsscarries an unrelatedintfsorchVLAN RIF MAC fix (sonic-net/sonic-swss#4816) needed for this testbed to come up; it is not part of theIN_PORTSfeature.VPP is rebuilt from source.
rules/vpp.mkcarriesVPP_VERSION = 2606-0.7for the combined patch series. That deb is not published (404on packages.buildkite.com), so thecurl -L -fdownload step fails andvppbld/Makefilefalls back tobuild_locally, which applies the full patch series and builds VPP. The image build therefore also verifies the patch applies cleanly and compiles under the real build flags (-Werror -Wall -O3). Expect the VPP build to add significant time to this job.How to verify it
Deploy the resulting image to a
dualtor-vpptestbed and confirm the drop rule is scoped to the standby ports.The binding is deliberately unchanged — the ACL stays bound to every front-panel port, and the scope now comes from the rules:
in-portis a standby portshow mux statusreportshealthyIN_PORTSregressionPre-fix, the same commands showed
acl-index 3onsw_if_index 1 .. 32with a single unscopeddeny any -> anyrule.Which release branch to backport (provide reason below if selected)
Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:
Tested branch
Test result
Image build pending.
Rule-level state was verified on
vlab-vpp-03(18 standby ports → 18 ingress-scoped deny rules) andvlab-vpp-04(6 → 6), with ACL 3 still bound to all 32 interfaces on both.A
2606-0.7image carrying this same patch pair was deployed to both DUTs anddualtor_io/test_normal_op.py::test_upstream_standby_rx_drop_checkpassed —RX_DRP diff=0against a threshold of 10, on a port with 5,762RX_OK, where it had previously failed at +5706. Denies remain visible at the ACL node (vppctl show errors→11398 acl-plugin-in-ip4-l2 ACL deny packets), so the counter is corrected rather than the drops hidden.Downstream forwarding under mux toggles is what this image is being built to exercise more broadly.
Description for the changelog
N/A — test-only PR, not for merge.
Link to config_db schema for YANG module changes
N/A — no schema change.
IN_PORTSis an existing CONFIG_DBACL_RULEfield.