[sonic_ext] Add glean-redirect node: punt unresolved-nexthop transit to the kernel - #267
[sonic_ext] Add glean-redirect node: punt unresolved-nexthop transit to the kernel#267lunyue-ms wants to merge 1 commit into
Conversation
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR enhances the SONiC-VPP sonic_ext plugin to restore hardware-like behavior for unresolved next-hop transit traffic by punting such packets to the Linux kernel (so the kernel performs ARP/ND and neighsyncd can program SAI/VPP), and also fixes LAG sub-port datapath issues around promiscuous mode and feature toggling on sub-interfaces.
Changes:
- Add a new
sonic-ext-glean-redirectfeature node onip4-drop/ip6-dropto redirect unresolved glean/ARP-adjacency drops to the ingress host tap with per-adjacency throttling. - Add bond detection + set bond member ports promiscuous to ensure VLAN-tagged LAG sub-port traffic reaches
dpdk-input. - Prevent toggling
device-inputfeatures on sub-interfaces to avoid corrupting shared feature configs; add newshow sonic-extcounter; bumpVPP_VERSION.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vppbld/plugins/sonic_ext/sonic_ext.h | Adds glean-redirect toggle/counter plus bond helper APIs. |
| vppbld/plugins/sonic_ext/sonic_ext.c | Adds sub-interface guard for device-input feature toggles; implements bond detection and member promisc enabling; enables glean-redirect globally on drop arcs. |
| vppbld/plugins/sonic_ext/glean_redirect_node.c | New drop-arc feature node implementing ingress-tap redirect for unresolved nexthops with throttling and safety gating. |
| vppbld/plugins/sonic_ext/CMakeLists.txt | Builds the new glean redirect node. |
| vppbld/plugins/sonic_ext/cli.c | Exposes glean-redirect summary counter via show sonic-ext. |
| rules/vpp.mk | Bumps VPP package version suffix to 2606-0.4. |
| /* | ||
| * Should this ip4-drop / ip6-drop packet be punted to the kernel to | ||
| * (re)arm neighbour resolution? Yes iff its VLIB_TX adjacency is an | ||
| * unresolved connected (glean) or incomplete (arp) adjacency and the | ||
| * egress interface of that adjacency is not an aggregate. | ||
| * |
| _inc (DISABLED, n_disabled); | ||
| #undef _inc | ||
|
|
||
| sem->glean_redirects += n_redirected; |
The sub-port data plane on sonic-vpp is now functional, so stop skipping
these modules and start running them in the t1-lag-vpp test set.
What changed
------------
1. .azure-pipelines/pr_test_scripts.yaml
The t1-lag-vpp test set had no sub_port_interfaces entry at all, so the
modules were never collected by Elastictest. Add the same two scripts the
t1-lag test set already runs.
2. tests_mark_conditions_sonic_vpp.yaml
test_show_subinterface.py was skipped as a whole module, and
test_sub_port_interfaces.py had all eight class-level cases marked
"Failed/Errored: To be included", for asic_type == vpp. Those cases pass
now, so drop the blanket skips and keep only the two variants that hit a
genuine, unrelated sonic-vpp gap:
test_routing_between_sub_ports_and_port[port-svi-*]
test_routing_between_sub_ports_and_port[port_in_lag-svi-*]
Both fail in the SVI/BVI path, not in the sub-port path: the linux-cp tap
for the Vlan999 SVI cannot be resolved (interface_set_state), FDB events
learned on the BVI cannot be mapped back to a SAI port, and
vpp_fdbentry_flush rejects a LAG bridge port. The -l3 variants of the same
test pass, which isolates the failure to SVI support.
test_sub_port_l2_forwarding.py is left untouched. It carries
pytestmark = [pytest.mark.topology("t0")], so it can never run on a t1
testbed regardless of the conditional marks, and it is not added to the
t1-lag-vpp test set.
Verification
------------
Run on the vms-kvm-vpp-t1-lag KVM testbed:
test_sub_port_interfaces.py 32 collected: 27 passed, 5 skipped, 0 failed
test_show_subinterface.py 2 collected: 1 passed, 1 skipped, 0 failed
test_show_subinterface.py was run twice to confirm stability; both runs gave
the same result in 278s. Its remaining skip is [port_in_lag], which the
platform-agnostic tests_mark_conditions.yaml skips with reason "Not supported
port type" on every platform.
Of the 5 skips in test_sub_port_interfaces.py, 2 are the SVI variants above
and 3 come from that same platform-agnostic file.
Depends on
----------
- sonic-net/sonic-sairedis#1907 (sub-port RIF/IP programming for LAG members)
- sonic-net/sonic-platform-vpp#267 (glean redirect node for sub-port ARP)
Without both of those in the image under test, several of the newly enabled
cases will fail.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6c4e1533-56e4-4d1d-8c98-9b000f07a580
Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
The sub-port data plane on sonic-vpp is now functional, so stop skipping
these modules and start running them in the t1-lag-vpp test set.
What changed
------------
1. .azure-pipelines/pr_test_scripts.yaml
The t1-lag-vpp test set had no sub_port_interfaces entry at all, so the
modules were never collected by Elastictest. Add the same two scripts the
t1-lag test set already runs.
2. tests_mark_conditions_sonic_vpp.yaml
test_show_subinterface.py was skipped as a whole module, and
test_sub_port_interfaces.py had all eight class-level cases marked
"Failed/Errored: To be included", for asic_type == vpp. Every one of those
cases passes now, so drop the vpp-specific skips entirely.
test_sub_port_l2_forwarding.py is left untouched. It carries
pytestmark = [pytest.mark.topology("t0")], so it can never run on a t1
testbed regardless of the conditional marks, and it is not added to the
t1-lag-vpp test set.
Verification
------------
Run on the vms-kvm-vpp-t1-lag KVM testbed:
test_sub_port_interfaces.py 32 collected: 29 passed, 3 skipped, 0 failed
test_show_subinterface.py 2 collected: 1 passed, 1 skipped, 0 failed
The 4 remaining skips all come from the platform-agnostic
tests_mark_conditions.yaml and apply to every platform, not just vpp:
test_untagged_packet_not_routed[port_in_lag] "Not supported port type"
test_routing_between_sub_ports_unaffected_by_sub_ports_removal[port_in_lag-same-*] same
test_routing_between_sub_ports_unaffected_by_sub_ports_removal[port_in_lag-different-*] same
test_show_subinterface.py::test_subinterface_status[port_in_lag] same
The two SVI variants of test_routing_between_sub_ports_and_port were run
three times on their own before being unskipped (313s, 401s, and 451s for
both together); all runs passed. test_show_subinterface.py was likewise run
twice with identical results.
Depends on
----------
- sonic-net/sonic-sairedis#1907 (sub-port RIF/IP programming for LAG members)
- sonic-net/sonic-platform-vpp#267 (glean redirect node for sub-port ARP)
Without both of those in the image under test, several of the newly enabled
cases will fail.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6c4e1533-56e4-4d1d-8c98-9b000f07a580
Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
|
Hi @yue-fred-gao, this PR follows the design principle you raised in #237 — that the VPP datapath should match how other hardware platforms behave. The main change is a new glean-redirect node: when a transit packet hits an unresolved next-hop, VPP's own ip4-glean sends the ARP request and drops the packet, so the reply arriving at the kernel is unsolicited and gets discarded ( arp_accept=0 ) — the neighbour is never learned and never reaches SAI. The node punts that packet to the ingress host tap instead, so the kernel routes it and sends its own ARP, mirroring what an NPU does on an unresolved next-hop. |
9c9d9e8 to
38a5cf7
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
vppbld/plugins/sonic_ext/glean_redirect_node.c:189
- The function comment says redirecting is conditioned on the egress interface not being an aggregate, but the implementation does not check that (and the surrounding comment block explicitly says bonds/BVI are in scope). This mismatch makes it hard to reason about the intended policy; please update the comment (or add the missing aggregate check if that was the intent).
* Should this ip4-drop / ip6-drop packet be punted to the kernel to
* (re)arm neighbour resolution? Yes iff its VLIB_TX adjacency is an
* unresolved connected (glean) or incomplete (arp) adjacency and the
* egress interface of that adjacency is not an aggregate.
vppbld/plugins/sonic_ext/sonic_ext.c:21
<vnet/ethernet/ethernet.h>is included but not used anywhere in this file, which adds unnecessary build coupling and can slow compilation. Please drop the include unless it's required by code outside this diff.
#include <vnet/ethernet/ethernet.h>
38a5cf7 to
677f951
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Add sonic-ext-glean-redirect on the IPv4 and IPv6 drop arcs so unresolved transit nexthops are punted through the ingress LCP host tap. Preserve the ingress interface and VLAN metadata, validate the producing glean/ARP node, and throttle redirects per adjacency. Avoid toggling device-input features on sub-interfaces because they share their parent device feature configuration. This prevents physical sub-port teardown from stripping host-xc from a later LAG member. Keep the bond and aggregate support provided by merged PR sonic-net#257 unchanged. Add glean redirect counters and bump the VPP package version for downstream builds. Validated with sonic-net/sonic-sairedis#1907 on the vms-kvm-vpp-t1-lag testbed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea640ff9-0a8c-4eb3-a41e-b316db32e773 Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
677f951 to
9d27570
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Why I did it
When a transit next hop is unresolved, VPP generates ARP itself. The reply reaches Linux unsolicited and can be discarded, so
neighsyncdnever learns the neighbor and SAI never programs it back into VPP. Hardware platforms instead punt the original packet to Linux, allowing the kernel to own neighbor resolution.This change restores that behavior for sonic-platform-vpp and fixes a device-input feature issue exposed by LAG sub-interfaces.
How I did it
sonic-ext-glean-redirectto the IPv4 and IPv6 drop arcs. It recognizes packets produced by the glean/ARP nodes, restores ingress and VLAN metadata, and redirects a throttled copy through the ingress LCP host tap.sonic-ext-host-xcfrom a LAG member.show sonic-ext.2606-0.5. Published2606-0.4packages did not contain this node, causing downstream builds to download a stale plugin.How to verify it
Validated with sonic-net/sonic-sairedis#1907 on
vms-kvm-vpp-t1-lag:docker-syncd-vpp.gzbuilt successfully from the rebased sources.v2606-0.5+b1sonic1; the glean node was active andshow sonic-extexposed its counter.test_sub_port_interfaces.py: 29 passed, 3 skipped. Two teardown errors were only existing VPP loganalyzer messages during SVI cleanup; all dataplane cases, YANG validation, core/config checks, and post-test sanity checks passed.Notes for reviewers