Control input cleanup - #676
Open
rjarry wants to merge 12 commits into
Open
Conversation
rjarry
force-pushed
the
control-input-cleanup
branch
from
August 3, 2026 10:41
acae4a7 to
85d28ac
Compare
This comment was marked as resolved.
This comment was marked as resolved.
rjarry
marked this pull request as draft
August 3, 2026 10:56
rjarry
force-pushed
the
control-input-cleanup
branch
from
August 3, 2026 12:57
85d28ac to
9fdbc53
Compare
rjarry
marked this pull request as ready for review
August 3, 2026 15:35
rjarry
force-pushed
the
control-input-cleanup
branch
from
August 4, 2026 09:37
9fdbc53 to
a1d485e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/infra/control/loopback.c`:
- Around line 268-281: Initialize ioctl_sock and iface->cp_fd to -1 before
descriptor creation in the surrounding setup flow, then update the cleanup
checks in the err path to close each descriptor when its value is >= 0. Preserve
the existing resource-release order and avoid reading uninitialized descriptor
values when socket() or open() fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 713283b0-c686-4130-82f4-fcf5c77437b4
📒 Files selected for processing (32)
modules/dhcp/control/client.cmodules/dhcp/control/dhcp.hmodules/dhcp/control/packet.cmodules/infra/control/ctlplane.cmodules/infra/control/iface.hmodules/infra/control/iface_test.cmodules/infra/control/loopback.cmodules/infra/control/loopback.hmodules/infra/control/vrf.cmodules/infra/control/vrf.hmodules/infra/datapath/control_input.cmodules/infra/datapath/control_input.hmodules/infra/datapath/eth.hmodules/infra/datapath/eth_output.cmodules/infra/datapath/iface_output.cmodules/infra/datapath/lacp_output.cmodules/infra/datapath/loop_input.cmodules/infra/datapath/loop_output.cmodules/infra/datapath/rxtx.hmodules/ip/control/nexthop.cmodules/ip/datapath/arp_output_reply.cmodules/ip/datapath/arp_output_request.cmodules/ip/datapath/icmp_local_send.cmodules/ip/datapath/ip4_datapath.hmodules/ip/datapath/ip_output.cmodules/ip6/control/nexthop.cmodules/ip6/control/router_advert.cmodules/ip6/datapath/icmp6_local_send.cmodules/ip6/datapath/ip6_datapath.hmodules/ip6/datapath/ip6_output.cmodules/ip6/datapath/ndp_na_output.cmodules/ip6/datapath/ndp_ns_output.c
💤 Files with no reviewable changes (2)
- modules/infra/control/vrf.h
- modules/dhcp/control/dhcp.h
Loopback interfaces store their TUN fd and libevent handle in iface_info_loopback, while ctlplane interfaces use iface->cp_fd and iface->cp_ev for the same purpose. Unify both by using the fields already on struct iface, which makes iface_info_loopback empty and removes it. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Instead of two static mempools in ctlplane and loopback, allocate a per-interface mempool and store it in the generic iface structure. Use the mempool to fetch mbufs for packets received from control plane representor and loopback interfaces. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Use iface->pool instead of the module-global dhcp_mp pool. Remove dhcp_get_mempool() since DHCP packet building already has the iface available. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Allocate the mbuf in lacp_send_pdu and write the PDU directly into the packet data. The LACP PDU is too large to fit in mbuf private data so it goes into the packet buffer itself. The downstream graph node reads it with rte_pktmbuf_mtod. This removes the malloc/free of the intermediate lacp_output_data struct. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Allocate the mbuf in icmp_local_send and store request parameters in icmp_send_mbuf_data instead of a calloc'd ctl_to_stack struct. The downstream graph node reads from mbuf private data instead of control_input_mbuf_data. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Same approach as the IPv4 ICMP change. Replace the calloc'd ctl_to_stack struct with icmp6_send_mbuf_data in mbuf private data. The iface_id field is replaced by the inherited iface pointer in the private data base struct. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Allocate the mbuf in arp_output_request_solicit from iface->pool and store the nexthop pointer in l3_mbuf_data instead of passing a raw pointer through the control_input ring. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Use iface->pool instead of ra_ctx.mp. The ra_ctx struct is now empty, remove it. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Same approach as ARP. Allocate the mbuf in nh6_solicit and store the nexthop pointer in l3_mbuf_data. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Allocate the mbuf in nh6_advertise from iface->pool and store context in ndp_na_mbuf_data instead of a malloc'd advertise_context struct. Signed-off-by: Robin Jarry <rjarry@redhat.com>
All callers now pre-allocate mbufs before calling post_to_stack. Remove the data_is_mbuf flag, the control_input_mbuf_data private data type, and the per-node mempool. The control_input node always treats the enqueued data as an mbuf. Signed-off-by: Robin Jarry <rjarry@redhat.com>
Each datapath node that can receive mbufs from the control plane now registers its own edge and exposes a *_send() function. Callers use these typed helpers instead of holding a static edge variable and calling post_to_stack directly. This removes the ip_nexthop and ip6_nexthop modules which only existed to register control_input edges at init time. Signed-off-by: Robin Jarry <rjarry@redhat.com>
rjarry
force-pushed
the
control-input-cleanup
branch
from
August 4, 2026 10:30
a1d485e to
3507674
Compare
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.
Depends on #672
Summary
struct iface.rte_edge_tregistration and(edge, mbuf)queue entries.