[vslib/vpp] Fix sub-port and LAG sub-port datapath on SONiC-VPP platform - #1907
Open
lunyue-ms wants to merge 1 commit into
Open
[vslib/vpp] Fix sub-port and LAG sub-port datapath on SONiC-VPP platform#1907lunyue-ms wants to merge 1 commit into
lunyue-ms wants to merge 1 commit into
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
lunyue-ms
added a commit
to lunyue-ms/sonic-sairedis
that referenced
this pull request
May 25, 2026
swsslogentercheck.sh in CI flagged three helpers introduced earlier
in this PR for missing the SWSS_LOG_ENTER() entry-trace macro:
vslib/vpp/SwitchVppRif.cpp:
format_bond_hwif() - bond_id / vlan_id -> 'BondEthernet<N>[.<v>]'
format_be_lcp() - bond_id / vlan_id -> 'be<N>[.<v>]'
vpp_warm_arp_for_subnet_peers() - sub-port pre-warm helper
Follow sairedis convention (see vslib/vpp/SwitchVppAcl.cpp helpers) and
add SWSS_LOG_ENTER() at the top of each function body. This is the only
change needed to make the Azure 'Build amd64' and 'BuildAsan amd64'
stages green again on PR sonic-net#1907; no other check failures exist.
Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced May 25, 2026
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
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. |
lunyue-ms
force-pushed
the
lunyue/sub-port-fixes
branch
from
June 2, 2026 09:25
3e30d77 to
d422da2
Compare
lunyue-ms
added a commit
to lunyue-ms/sonic-sairedis
that referenced
this pull request
Jun 2, 2026
swsslogentercheck.sh in CI flagged three helpers introduced earlier
in this PR for missing the SWSS_LOG_ENTER() entry-trace macro:
vslib/vpp/SwitchVppRif.cpp:
format_bond_hwif() - bond_id / vlan_id -> 'BondEthernet<N>[.<v>]'
format_be_lcp() - bond_id / vlan_id -> 'be<N>[.<v>]'
vpp_warm_arp_for_subnet_peers() - sub-port pre-warm helper
Follow sairedis convention (see vslib/vpp/SwitchVppAcl.cpp helpers) and
add SWSS_LOG_ENTER() at the top of each function body. This is the only
change needed to make the Azure 'Build amd64' and 'BuildAsan amd64'
stages green again on PR sonic-net#1907; no other check failures exist.
Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
lunyue-ms
force-pushed
the
lunyue/sub-port-fixes
branch
from
June 9, 2026 02:44
a7dd695 to
ac9bb86
Compare
lunyue-ms
added a commit
to lunyue-ms/sonic-sairedis
that referenced
this pull request
Jun 9, 2026
swsslogentercheck.sh in CI flagged three helpers introduced earlier
in this PR for missing the SWSS_LOG_ENTER() entry-trace macro:
vslib/vpp/SwitchVppRif.cpp:
format_bond_hwif() - bond_id / vlan_id -> 'BondEthernet<N>[.<v>]'
format_be_lcp() - bond_id / vlan_id -> 'be<N>[.<v>]'
vpp_warm_arp_for_subnet_peers() - sub-port pre-warm helper
Follow sairedis convention (see vslib/vpp/SwitchVppAcl.cpp helpers) and
add SWSS_LOG_ENTER() at the top of each function body. This is the only
change needed to make the Azure 'Build amd64' and 'BuildAsan amd64'
stages green again on PR sonic-net#1907; no other check failures exist.
Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| bool ingress_disabled = false; | ||
| bool egress_disabled = false; | ||
|
|
||
| sai_attribute_t attr; |
Contributor
There was a problem hiding this comment.
I suggest we use SaiObject instead of the low level get(). Here is an example.
We build a graph of SaiObject that helps navigating through SaiObject. It is preferred for the new code. Like below code, you can use CHECK_STATUS_QUIET with get_mandatory_attr. The latter has error log.
Contributor
Author
There was a problem hiding this comment.
Yes, will follow up.
| m_lag_members_in_vpp.erase(lag_member_oid); | ||
| } | ||
| } | ||
| m_lag_members_in_vpp.erase(lag_member_oid); |
Contributor
Author
There was a problem hiding this comment.
Ha, need to solve the redundancy.
| const char *dev = name.c_str(); | ||
| const char *hwif_name = tap_to_hwif_name(dev); | ||
|
|
||
| configure_lcp_interface(hwif_name, dev, true); |
Contributor
There was a problem hiding this comment.
why is the code moved after sw_interface_set_mac?
Contributor
Author
There was a problem hiding this comment.
When creating the LCP pair, the hardware MAC will be copied to the tap interface. Then when sw_interface_set_mac() is called, only the hardware MAC will be changed but the tap interface is still the original one. So I adjusted the sequence.
| * adding the IP on the kernel netdev. It is intentionally limited to | ||
| * small subnets to avoid broadcast-style probing on production /24's. | ||
| */ | ||
| static void vpp_warm_arp_for_subnet_peers(const std::string &netdev, |
Contributor
There was a problem hiding this comment.
I feel this is very specifically targeting the sonic-mgmt test (ipv4, small subnet only). It would fail if the test case increases subnet size. We should treat sonic-vpp as a product, not a test vehicle and actually there are C0 platforms using sonic-vpp as data plane. Can we modify the test to be more accommodating to different platforms? And are you sure ASIC buffer the packet, send ARP, wait for ARP resolved then send the buffered packet? This is too much for ASIC. It is typically done by control plane. ASIC punts the packet due to unresolved nh to cpu and cpu can do above steps. Here SONiC is the control plane. Does SONiC support this?
Contributor
Author
There was a problem hiding this comment.
Understood, we should treat sonic-vpp as a product rather than tolerate tests by modifying the device side behavior. Also, ASICs don't buffer. The control plane will handle that. I think that the right way is to modify the test to support the vpp platform. Will remove vpp_warm_arp_for_subnet_peers()
| * vlan-tagged ARP reply enters the parent port and linux-cp punts it | ||
| * to the LCP host tap before VPP's arp-reply / neighbor-learning path | ||
| * has a chance to install the neighbor. The kernel then drops it as | ||
| * an unsolicited reply (arp_accept=0 default), so lcp-sync has no |
Contributor
There was a problem hiding this comment.
would orchagent install the nexthop through SAI? sonic-vpp currently doesn't use lcp-sync. It relies on SAI calls instead. I think this is consistent with hardware platforms.
Contributor
Author
There was a problem hiding this comment.
Yes, this is the correct way. Previouly, I used this way to punt the ARP packet to Linux host side when the lcp-sync was enabled. But with your new design with the new node, we can totally depend on the above path you mentioned. I will revert ths change.
| * "if (is_add)" gate below, not the helper choice. orchagent | ||
| * emits transient ROUTE_ENTRY remove+create cycles for the | ||
| * same connected prefix during bulk batch processing (observed | ||
| * during sub_port PTF tests: bulk C, R, C, R for the same /30 |
Contributor
There was a problem hiding this comment.
I did some experiment. subnet route is created when the interface is up and removed if the interface goes down. But the /32 route stays. Also in the kernel, portchannel sub interface's ip is not removed. Is it possible to use vpp_add_del_intf_ip_addr_norif in the PORT path?
Contributor
Author
There was a problem hiding this comment.
Thanks for the experiment. It seems that vpp_add_del_intf_ip_addr_norif has already been used in the PORT path. I will remove the current ROUTER_INTERFACE part and retest.
1 task
lunyue-ms
force-pushed
the
lunyue/sub-port-fixes
branch
from
July 30, 2026 03:54
9d4eeb9 to
f51be0c
Compare
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
lunyue-ms
added a commit
to lunyue-ms/sonic-platform-vpp
that referenced
this pull request
Jul 30, 2026
…to the kernel A routed packet whose next hop is not yet resolved makes VPP glean and ARP by itself. The ARP reply then arrives at the kernel unsolicited, is discarded (arp_accept=0), and the neighbour never reaches SAI, so the flow never recovers. On a hardware NPU the equivalent packet is punted to the CPU, the kernel ARPs, neighsyncd learns the entry and SAI programs it back into the dataplane -- that is the behaviour this node restores for the VPP platform. Add sonic-ext-glean-redirect on the ip4-drop / ip6-drop feature arcs: - Match GLEAN/ARP adjacencies, rewind to the L2 header, re-push the original VLAN tag when the packet arrived on a sub-interface, and send the packet out of the *ingress* LCP host tap. Using the ingress tap rather than the egress one keeps the kernel reverse-path filter happy, which is what makes the kernel actually emit the ARP request. - Throttle per adjacency so a burst of transit traffic cannot flood the host tap. - Gate on the producing node. The node sits on the ip4-drop / ip6-drop arcs, which ip_drop_or_punt() starts with a hardcoded sw_if_index 0, so it observes every dropped packet on the box, not just those coming from ip4-arp / ip4-glean / ip6-glean. For a packet dropped elsewhere, vnet_buffer(b)->ip.adj_index[VLIB_TX] holds no adjacency index -- ip4-lookup writes dpo0->dpoi_index there for every DPO type, and the drop DPO's index is simply the dpo_proto (0 for IPv4, 1 for IPv6). Those values are not ADJ_INDEX_INVALID, so adj_is_valid() accepts them and adj_get() returns whatever adjacency happens to live in slot 0 or 1; on a live DUT those slots hold real ipv6-glean adjacencies. Resolve ip4-arp, ip4-glean and ip6-glean at main-loop-enter and require b->error to map to one of them before touching the adjacency. The adjacency-type check is kept as a second line of defence, since error slot 0 still resolves to some node and the node gate alone cannot reject the already-RESOLVED case. ip6-discover-neighbor is deliberately excluded. Unlike the IPv4 path, which only reads ip0->dst_address, ip6_neighbor.c rewrites the destination address in place when !is_glean. Redirecting such a packet would hand the kernel a datagram whose destination has been replaced and whose L4 checksum no longer matches. The egress scope is every routed interface, including bond main interfaces and bond sub-interfaces. Physical sub-interfaces, main interfaces and BVI/SVI all suffer the same unsolicited-reply problem: a main interface is only self-healing when the DUT itself initiates the traffic (a BGP peer, say), while a routed transit destination behind any RIF -- physical or PortChannel -- has no such trigger. The redirect target stays correct on a LAG because orig_rx is the receiving physical member, so the packet re-enters the kernel on the member netdev and is delivered up through the team device to the netdev carrying the RIF address. Two datapath fixes are required for LAG sub-ports to work at all: - Put bond members in promiscuous mode. Creating a sub-interface on a plain phy makes VPP flip that phy's device into promiscuous mode, so the NIC stops applying its own-MAC + VLAN receive filter and tagged frames reach dpdk-input. A bond sub-interface hangs off the *bond* hw interface, so the flip lands on a pure software interface with no receive filter, while the members that own the wire keep their default filter and silently drop every VLAN-tagged frame addressed to the LAG. Untagged LAG traffic (LACP, the main PortChannel RIF, BGP) is unaffected, which is why the breakage looked sub-port specific. Measured on a member port, tagged frames reaching dpdk-input: 0 by default, 50 once the member is set promiscuous. A LAG member on a switch ASIC accepts whatever is addressed to the LAG, so this mirrors the hardware behaviour. - Never toggle device-input features on sub-interfaces. VPP starts the device-input arc from the device's own sw_if_index -- virtio calls vnet_feature_start_device_input (vif->sw_if_index, ...) and dpdk uses xd->sw_if_index -- so a sub-interface never gets a device-input dispatch of its own and rides its parent's. Enabling a feature there is a no-op, but disabling is destructive: vnet_config feature strings are interned and shared by every interface with an identical feature set, so removing the feature through the sub-interface rewrites the config the parent still points at, and the parent's input node falls back to the arc's end node, ethernet-input. Creating and then deleting Ethernet64.20 therefore stripped sonic-ext-host-xc off Ethernet64's tap; once that port was later enslaved into a PortChannel and given a sub-port, the kernel's ARP for the sub-port neighbour left PortChannel1.20, crossed the team to the Ethernet64 tap and reached ethernet-input instead of host-xc, where the tagged frame was dropped as "unknown vlan". sonic-ext-aggr-tap-redirect is on the interface-output arc, which is dispatched per sw_if_index, and is unaffected. Also add the sonic_ext_phy_is_bond() / sonic_ext_phy_is_aggregate() device-class helpers that SONiC PR #2440 section 5.2 previously left as a TODO, glean-redirect counters in 'show sonic-ext', and bump VPP_VERSION to 2606-0.4 so downstream sonic-buildimage builds do not silently pull debs that pre-date this patch. Validated on the vms-kvm-vpp-t1-lag KVM testbed together with the matching sairedis fixes (sonic-net/sonic-sairedis#1907), which this change depends on for the SUB_PORT LCP wiring: sub_port_interfaces/test_sub_port_interfaces.py reports 27 passed, 5 skipped, 0 failed, 0 errors with loganalyzer enabled, against 3 passed / 13 failed before the change. Three of the five skips are upstream's unconditional port_in_lag skips that apply to every platform; the other two are sonic-vpp SVI gaps unrelated to sub-ports. BGP sessions stayed established throughout. 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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes several SONiC-VPP vslib/vpp router-interface and LCP (linux-cp) integration issues that break sub-port and LAG sub-port datapath on the sonic-platform-vpp backend, with the goal of restoring sub-port routing PTF coverage (notably test_packet_routed_with_valid_vlan) when used with a companion sonic-platform-vpp change.
Changes:
- Fix PortChannel sub-port IP programming to target the correct VPP sub-interface (e.g.,
BondEthernet<id>.<vlan>), avoidingstd::stoi("1.20") -> 1truncation behavior. - Correct LAG sub-port linux-cp host pairing and make newly created host sub-interfaces administratively UP to avoid silent punt drops.
- Remove legacy
tc mirredredirect setup for LAG LCP, relying on the newersonic_extpunt-via-member path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vslib/vpp/SwitchVppRif.cpp | Fixes PortChannel VLAN parsing for IP programming; adjusts LAG sub-port host pairing; brings host sub-interfaces admin-UP during SUB_PORT RIF creation; keeps create/remove naming symmetric. |
| vslib/vpp/SwitchVppHostif.cpp | Adds vs_set_dev_admin_up() helper to set kernel interface admin state via SIOCGIFFLAGS/SIOCSIFFLAGS. |
| vslib/vpp/SwitchVppFdb.cpp | Drops legacy tc redirect installation for LAG LCP and updates log messaging accordingly. |
| vslib/vpp/SwitchVpp.h | Declares the new vs_set_dev_admin_up() helper. |
11 tasks
lunyue-ms
added a commit
to lunyue-ms/sonic-mgmt
that referenced
this pull request
Jul 30, 2026
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>
lunyue-ms
added a commit
to lunyue-ms/sonic-mgmt
that referenced
this pull request
Jul 31, 2026
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>
Contributor
Author
|
Test result: sub_port_interfaces/test_sub_port_interfaces.py::TestSubPorts::test_packet_routed_with_valid_vlan[port] PASSED [ 3%] =========================== short test summary info ============================ sub_port_interfaces/test_show_subinterface.py::test_subinterface_status[port] PASSED [ 50%] =========================== short test summary info ============================ |
lunyue-ms
force-pushed
the
lunyue/sub-port-fixes
branch
from
July 31, 2026 11:06
f51be0c to
3af2d58
Compare
Collaborator
|
/azp run |
lunyue-ms
added a commit
to lunyue-ms/sonic-platform-vpp
that referenced
this pull request
Jul 31, 2026
…to the kernel A routed packet whose next hop is not yet resolved makes VPP glean and ARP by itself. The ARP reply then arrives at the kernel unsolicited, is discarded (arp_accept=0), and the neighbour never reaches SAI, so the flow never recovers. On a hardware NPU the equivalent packet is punted to the CPU, the kernel ARPs, neighsyncd learns the entry and SAI programs it back into the dataplane -- that is the behaviour this node restores for the VPP platform. Add sonic-ext-glean-redirect on the ip4-drop / ip6-drop feature arcs: - Match GLEAN/ARP adjacencies, rewind to the L2 header, re-push the original VLAN tag when the packet arrived on a sub-interface, and send the packet out of the *ingress* LCP host tap. Using the ingress tap rather than the egress one keeps the kernel reverse-path filter happy, which is what makes the kernel actually emit the ARP request. - Throttle per adjacency so a burst of transit traffic cannot flood the host tap. - Gate on the producing node. The node sits on the ip4-drop / ip6-drop arcs, which ip_drop_or_punt() starts with a hardcoded sw_if_index 0, so it observes every dropped packet on the box, not just those coming from ip4-arp / ip4-glean / ip6-glean. For a packet dropped elsewhere, vnet_buffer(b)->ip.adj_index[VLIB_TX] holds no adjacency index -- ip4-lookup writes dpo0->dpoi_index there for every DPO type, and the drop DPO's index is simply the dpo_proto (0 for IPv4, 1 for IPv6). Those values are not ADJ_INDEX_INVALID, so adj_is_valid() accepts them and adj_get() returns whatever adjacency happens to live in slot 0 or 1; on a live DUT those slots hold real ipv6-glean adjacencies. Resolve ip4-arp, ip4-glean and ip6-glean at main-loop-enter and require b->error to map to one of them before touching the adjacency. The adjacency-type check is kept as a second line of defence, since error slot 0 still resolves to some node and the node gate alone cannot reject the already-RESOLVED case. ip6-discover-neighbor is deliberately excluded. Unlike the IPv4 path, which only reads ip0->dst_address, ip6_neighbor.c rewrites the destination address in place when !is_glean. Redirecting such a packet would hand the kernel a datagram whose destination has been replaced and whose L4 checksum no longer matches. The egress scope is every routed interface, including bond main interfaces and bond sub-interfaces. Physical sub-interfaces, main interfaces and BVI/SVI all suffer the same unsolicited-reply problem: a main interface is only self-healing when the DUT itself initiates the traffic (a BGP peer, say), while a routed transit destination behind any RIF -- physical or PortChannel -- has no such trigger. The redirect target stays correct on a LAG because orig_rx is the receiving physical member, so the packet re-enters the kernel on the member netdev and is delivered up through the team device to the netdev carrying the RIF address. One further datapath fix is required for LAG sub-ports to work at all: - Never toggle device-input features on sub-interfaces. VPP starts the device-input arc from the device's own sw_if_index -- virtio calls vnet_feature_start_device_input (vif->sw_if_index, ...) and dpdk uses xd->sw_if_index -- so a sub-interface never gets a device-input dispatch of its own and rides its parent's. Enabling a feature there is a no-op, but disabling is destructive: vnet_config feature strings are interned and shared by every interface with an identical feature set, so removing the feature through the sub-interface rewrites the config the parent still points at, and the parent's input node falls back to the arc's end node, ethernet-input. Creating and then deleting Ethernet64.20 therefore stripped sonic-ext-host-xc off Ethernet64's tap; once that port was later enslaved into a PortChannel and given a sub-port, the kernel's ARP for the sub-port neighbour left PortChannel1.20, crossed the team to the Ethernet64 tap and reached ethernet-input instead of host-xc, where the tagged frame was dropped as "unknown vlan". sonic-ext-aggr-tap-redirect is on the interface-output arc, which is dispatched per sw_if_index, and is unaffected. Also add the sonic_ext_phy_is_bond() / sonic_ext_phy_is_aggregate() device-class helpers that SONiC PR #2440 section 5.2 previously left as a TODO, glean-redirect counters in 'show sonic-ext', and bump VPP_VERSION to 2606-0.4 so downstream sonic-buildimage builds do not silently pull debs that pre-date this patch. Putting bond members into promiscuous mode -- the other half of making LAG sub-ports work -- is handled on the SAI side instead (sonic-net/sonic-sairedis#1907). VPP's automatic promiscuous flip on sub-interface creation lands on the bond, a pure software interface, while the members that own the wire keep their receive filter and drop every VLAN-tagged frame addressed to the LAG. SAI owns bond membership, so it can re-apply the flag on every enslave rather than only on the first one, which is all a plugin hook on LCP-pair creation can reach. Validated on the vms-kvm-vpp-t1-lag KVM testbed together with the matching sairedis fixes (sonic-net/sonic-sairedis#1907), which this change depends on for the SUB_PORT LCP wiring: sub_port_interfaces/test_sub_port_interfaces.py reports 27 passed, 5 skipped, 0 failed, 0 errors with loganalyzer enabled, against 3 passed / 13 failed before the change. Three of the five skips are upstream's unconditional port_in_lag skips that apply to every platform; the other two are sonic-vpp SVI gaps unrelated to sub-ports. BGP sessions stayed established throughout. 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>
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
vslib/vpp/SwitchVppHostif.cpp:146
- The SIOCSIFFLAGS failure log prints
err(typically -1) instead of the realerrno, so the message isn’t actionable. Logerrno(andstrerror(errno)) here as well.
if (err < 0)
{
SWSS_LOG_ERROR("ioctl SIOCSIFFLAGS %s on %s failed, err %d",
(up ? "UP" : "DOWN"), dev, err);
}
vslib/vpp/SwitchVppHostif.cpp:129
- The error log for SIOCGIFFLAGS prints
err(which will be -1) instead oferrno, so it loses the actual failure reason. Loggingerrno(and ideallystrerror(errno)) makes debugging failures like ENODEV/EPERM much easier.
This issue also appears on line 142 of the same file.
if (err < 0)
{
SWSS_LOG_ERROR("ioctl SIOCGIFFLAGS on %s failed, err %d", dev, err);
close(s);
return err;
lunyue-ms
force-pushed
the
lunyue/sub-port-fixes
branch
from
July 31, 2026 11:29
3af2d58 to
1bd366d
Compare
lunyue-ms
added a commit
to lunyue-ms/sonic-platform-vpp
that referenced
this pull request
Jul 31, 2026
…to the kernel A routed packet whose next hop is not yet resolved makes VPP glean and ARP by itself. The ARP reply then arrives at the kernel unsolicited, is discarded (arp_accept=0), and the neighbour never reaches SAI, so the flow never recovers. On a hardware NPU the equivalent packet is punted to the CPU, the kernel ARPs, neighsyncd learns the entry and SAI programs it back into the dataplane -- that is the behaviour this node restores for the VPP platform. Add sonic-ext-glean-redirect on the ip4-drop / ip6-drop feature arcs: - Match GLEAN/ARP adjacencies, rewind to the L2 header, re-push the original VLAN tag when the packet arrived on a sub-interface, and send the packet out of the *ingress* LCP host tap. Using the ingress tap rather than the egress one keeps the kernel reverse-path filter happy, which is what makes the kernel actually emit the ARP request. - Throttle per adjacency so a burst of transit traffic cannot flood the host tap. - Gate on the producing node. The node sits on the ip4-drop / ip6-drop arcs, which ip_drop_or_punt() starts with a hardcoded sw_if_index 0, so it observes every dropped packet on the box, not just those coming from ip4-arp / ip4-glean / ip6-glean. For a packet dropped elsewhere, vnet_buffer(b)->ip.adj_index[VLIB_TX] holds no adjacency index -- ip4-lookup writes dpo0->dpoi_index there for every DPO type, and the drop DPO's index is simply the dpo_proto (0 for IPv4, 1 for IPv6). Those values are not ADJ_INDEX_INVALID, so adj_is_valid() accepts them and adj_get() returns whatever adjacency happens to live in slot 0 or 1; on a live DUT those slots hold real ipv6-glean adjacencies. Resolve ip4-arp, ip4-glean and ip6-glean at main-loop-enter and require b->error to map to one of them before touching the adjacency. The adjacency-type check is kept as a second line of defence, since error slot 0 still resolves to some node and the node gate alone cannot reject the already-RESOLVED case. ip6-discover-neighbor is deliberately excluded. Unlike the IPv4 path, which only reads ip0->dst_address, ip6_neighbor.c rewrites the destination address in place when !is_glean. Redirecting such a packet would hand the kernel a datagram whose destination has been replaced and whose L4 checksum no longer matches. The egress scope is every routed interface, including bond main interfaces and bond sub-interfaces. Physical sub-interfaces, main interfaces and BVI/SVI all suffer the same unsolicited-reply problem: a main interface is only self-healing when the DUT itself initiates the traffic (a BGP peer, say), while a routed transit destination behind any RIF -- physical or PortChannel -- has no such trigger. The redirect target stays correct on a LAG because orig_rx is the receiving physical member, so the packet re-enters the kernel on the member netdev and is delivered up through the team device to the netdev carrying the RIF address. One further datapath fix is required for LAG sub-ports to work at all: - Never toggle device-input features on sub-interfaces. VPP starts the device-input arc from the device's own sw_if_index -- virtio calls vnet_feature_start_device_input (vif->sw_if_index, ...) and dpdk uses xd->sw_if_index -- so a sub-interface never gets a device-input dispatch of its own and rides its parent's. Enabling a feature there is a no-op, but disabling is destructive: vnet_config feature strings are interned and shared by every interface with an identical feature set, so removing the feature through the sub-interface rewrites the config the parent still points at, and the parent's input node falls back to the arc's end node, ethernet-input. Creating and then deleting Ethernet64.20 therefore stripped sonic-ext-host-xc off Ethernet64's tap; once that port was later enslaved into a PortChannel and given a sub-port, the kernel's ARP for the sub-port neighbour left PortChannel1.20, crossed the team to the Ethernet64 tap and reached ethernet-input instead of host-xc, where the tagged frame was dropped as "unknown vlan". sonic-ext-aggr-tap-redirect is on the interface-output arc, which is dispatched per sw_if_index, and is unaffected. Also add the sonic_ext_phy_is_bond() / sonic_ext_phy_is_aggregate() device-class helpers that SONiC PR #2440 section 5.2 previously left as a TODO, glean-redirect counters in 'show sonic-ext', and bump VPP_VERSION to 2606-0.4 so downstream sonic-buildimage builds do not silently pull debs that pre-date this patch. Putting bond members into promiscuous mode -- the other half of making LAG sub-ports work -- is handled on the SAI side instead (sonic-net/sonic-sairedis#1907). VPP's automatic promiscuous flip on sub-interface creation lands on the bond, a pure software interface, while the members that own the wire keep their receive filter and drop every VLAN-tagged frame addressed to the LAG. SAI owns bond membership, so it can re-apply the flag on every enslave rather than only on the first one, which is all a plugin hook on LCP-pair creation can reach. Validated on the vms-kvm-vpp-t1-lag KVM testbed together with the matching sairedis fixes (sonic-net/sonic-sairedis#1907), which this change depends on for the SUB_PORT LCP wiring: sub_port_interfaces/test_sub_port_interfaces.py reports 27 passed, 5 skipped, 0 failed, 0 errors with loganalyzer enabled, against 3 passed / 13 failed before the change. Three of the five skips are upstream's unconditional port_in_lag skips that apply to every platform; the other two are sonic-vpp SVI gaps unrelated to sub-ports. BGP sessions stayed established throughout. 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>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
5 tasks
Four defects broke L3 sub-interfaces created on a PortChannel
(SAI_ROUTER_INTERFACE_TYPE_SUB_PORT over SAI_OBJECT_TYPE_LAG):
1. The connected-route address landed on the bond main interface
instead of the sub-interface.
vpp_add_del_intf_ip_addr_norif() parsed the bond id with
std::stoi(full_if_name.substr(strlen("PortChannel"))). For
"PortChannel1.20" std::stoi() stops at the '.' and silently yields 1,
so the helper programmed BondEthernet1 instead of BondEthernet1.20.
Every sub-port on the same bond then contended for the parent's
address space and VPP rejected the later ones as overlapping
prefixes. Parse the id from if_name -- the name without the VLAN
suffix, which get_intf_vlanid() has already split out -- and append
".<vlan>" when a VLAN id is present.
2. The linux-cp host tap for the sub-port collided with the kernel's
own 8021q netdev.
vpp_create_router_interface() bound the LCP pair to
"PortChannel<id>.<vlan>", a name the Linux bond/team stack already
owns. Use "be<id>.<vlan>" instead, a VLAN netdev on the be<id> bond
tap, so linux-cp-punt-xc has an unambiguous target.
vpp_remove_router_interface() builds the same name for symmetry.
3. The host tap was left admin-down, silently dropping punted traffic.
lcp-auto-subint and lcp-sync are both disabled (vlan-bvi HLD 3.6), so
nothing brought the freshly created host netdev up. A down netdev
makes the kernel discard the for-us punt, so the sub-port never
answered ARP/ND and never terminated traffic. Bring it up explicitly
from the SAI path through the new vs_set_dev_admin_up() helper.
4. LAG members were left non-promiscuous after being enslaved.
VPP clears a hardware interface's promiscuous flag when the interface
is enslaved into a bond, so a member programmed by
vpp_create_lag_member() only accepted frames addressed to its own
MAC. Traffic forwarded over the port channel carries the common
SONiC router MAC rather than the member's, and port-channel
sub-interface traffic is VLAN tagged against the bond rather than the
member, so neither was accepted. Re-apply
interface_set_promiscuous() right after create_bond_member(), and
again on the re-attach path in vpp_set_lag_member_egress_disable(),
which enslaves the port a second time.
Also drop the be<id> -> PortChannel<id> tc mirred redirect from
vpp_ensure_lag_lcp(). Control-plane punt for a port channel now goes
through the sonic_ext plugin's punt-via-member path (sonic-ext-capture
plus aggr-tap-redirect), which delivers the punted copy to the
originating member tap.
Sub-port IP programming stays entirely on the next_hop=PORT path via
vpp_add_del_intf_ip_addr_norif(); the ROUTER_INTERFACE variant remains
disabled as upstream had it. With the parsing fix above that single
path covers physical and port-channel sub-interfaces alike, and no
duplicate or overlapping-prefix rejection is observed.
Tested on the vms-kvm-vpp-t1-lag KVM testbed with loganalyzer enabled:
sub_port_interfaces/test_sub_port_interfaces.py reports 27 passed,
5 skipped, 0 failed, 0 errors. syslog over the run contains no
overlapping-prefix rejection and no interface_ip_address_add_del
failure. All 16 members of the 8 port channels report
"promiscuous: unicast on" in "show hardware-interfaces"; before this
change only the first member of each bond was promiscuous.
Signed-off-by: Lun Yue <17232861+lunyue-ms@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6c4e1533-56e4-4d1d-8c98-9b000f07a580
lunyue-ms
added a commit
to lunyue-ms/sonic-platform-vpp
that referenced
this pull request
Aug 7, 2026
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>
lunyue-ms
force-pushed
the
lunyue/sub-port-fixes
branch
from
August 7, 2026 15:07
1bd366d to
5377652
Compare
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Fix the VPP SAI datapath for routed sub-interfaces created on PortChannels (
SAI_ROUTER_INTERFACE_TYPE_SUB_PORToverSAI_OBJECT_TYPE_LAG). Changes are confined tovslib/vpp.Type of change
Motivation and implementation
Four defects prevented PortChannel sub-interface traffic from working:
PortChannel<id>.<vlan>, so addresses were programmed on the bond parent. Program them onBondEthernet<id>.<vlan>instead.be<id>.<vlan>and construct the same name during removal.vs_set_dev_admin_up().interface_set_promiscuous()after everycreate_bond_member(), including the egress-disable reattach path.Also remove the obsolete
be<id> -> PortChannel<id>tc redirect; punt traffic now uses sonic-platform-vpp#267's sonic_ext path.Verification
Validated after rebasing onto current master, together with sonic-net/sonic-platform-vpp#267 on
vms-kvm-vpp-t1-lag:docker-syncd-vpp.gzbuild and deployment succeeded.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.Platform impact
Only the VPP virtual-switch backend is changed; other ASIC backends are unaffected.