Linux controlplane

[christophefontaine]

Add linux tap interface for each grout interface, except the loopback.

Depends on #393, focuses only on IPv4 tests, will create another PR dedicated to OSPFv3 (IPv6) which requires additional rework about event ordering.

• add & display CP statistics
• create 1 tap for each interface in grout
• sync link state, mtu and speed
• iface type bond
• L3 only interfaces (IPIP): do not create a CP interface
• Rework netlink manual messages to use the internal netlink helper
• proper vrf handling (iface index used as vrf id in FRR)
• smoke tests ISISd
• smoke tests OSPFd

Summary by CodeRabbit

Release Notes

• New Features

  • Added control plane interface with L2/L3 packet redirect and traffic statistics tracking
  • Enabled ISIS and OSPF routing protocol support

• Improvements

  • Enhanced interface index mapping between routing and forwarding components
  • Streamlined IPv4/IPv6 address configuration

• Tests

  • Added ISIS and OSPF integration tests

[christophefontaine]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces a control plane module that implements packet forwarding over TAP devices with integrated L2/L3 graph nodes, along with bidirectional index mapping between Grout and FRR interface namespaces. A new if_map module manages these bidirectional mappings, which are integrated throughout FRR routing and interface handling. Control plane statistics are added to interface stats and exposed via telemetry and CLI. VRF-based netlink address operations are simplified to use interface names directly. FRR daemons (ISIS and OSPF) are enabled, and corresponding smoke test scripts validate ISIS and OSPF routing convergence and neighbor adjacency.

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.94% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	Title 'Linux controlplane' is too vague and generic; it doesn't clearly convey the scope or main changes of this substantial PR.	Consider a more descriptive title that captures the core functionality, such as 'Add Linux TAP-based control plane for FRR interfaces' or 'Implement control plane TAP device management'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 7

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e3e474 and 7e1a12b.

⛔ Files ignored due to path filters (1)

• docs/graph.svg is excluded by !**/*.svg

📒 Files selected for processing (24)

• frr/frr_plugin_install.sh (1 hunks)
• frr/if_grout.c (8 hunks)
• frr/if_map.c (1 hunks)
• frr/if_map.h (1 hunks)
• frr/meson.build (1 hunks)
• frr/rt_grout.c (10 hunks)
• frr/zebra_dplane_grout.c (4 hunks)
• modules/ctlplane/control.c (1 hunks)
• modules/ctlplane/ctlplane.h (1 hunks)
• modules/ctlplane/l2_datapath.c (1 hunks)
• modules/ctlplane/l3_datapath.c (1 hunks)
• modules/ctlplane/meson.build (1 hunks)
• modules/infra/api/gr_infra.h (1 hunks)
• modules/infra/api/stats.c (3 hunks)
• modules/infra/cli/iface.c (2 hunks)
• modules/infra/control/gr_iface.h (2 hunks)
• modules/infra/control/loopback.c (3 hunks)
• modules/ip/control/address.c (3 hunks)
• modules/ip6/control/address.c (2 hunks)
• modules/meson.build (1 hunks)
• smoke/_init.sh (1 hunks)
• smoke/_init_frr.sh (2 hunks)
• smoke/isis_frr_test.sh (1 hunks)
• smoke/ospf_frr_test.sh (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.sh

⚙️ CodeRabbit configuration file

**/*.sh: - Don't bother about unquoted shell variables.

Files:

• smoke/_init_frr.sh
• frr/frr_plugin_install.sh
• smoke/_init.sh
• smoke/isis_frr_test.sh
• smoke/ospf_frr_test.sh

**/*.{c,h}

⚙️ CodeRabbit configuration file

**/*.{c,h}: - gr_vec_*() functions cannot fail. No need to check their return value.

• gr_vec_free(x) always sets x = NULL. There is no risk of double free.
• ec_node_*() functions consume their ec_node arguments. No leaks on error.
• rte_node->ctx is an uint8_t array of size 16, not a pointer.
• Don't suggest to replace assert() with graceful error checking.
• We compile with -std=gnu2x. Unnamed parameters in function definitions are valid.

Files:

• modules/infra/control/loopback.c
• frr/if_map.c
• frr/zebra_dplane_grout.c
• modules/ctlplane/ctlplane.h
• modules/infra/control/gr_iface.h
• modules/ctlplane/l2_datapath.c
• modules/ctlplane/l3_datapath.c
• modules/ip6/control/address.c
• modules/ip/control/address.c
• frr/rt_grout.c
• modules/infra/cli/iface.c
• modules/infra/api/gr_infra.h
• frr/if_map.h
• frr/if_grout.c
• modules/ctlplane/control.c
• modules/infra/api/stats.c

🧠 Learnings (15)

📓 Common learnings

Learnt from: maxime-leroy
Repo: DPDK/grout PR: 372
File: smoke/cross_vrf_forward_test.sh:18-18
Timestamp: 2025-11-05T13:55:26.189Z
Learning: In the DPDK/grout codebase, VRF interfaces (named gr-vrf<id>) are automatically created when an interface is added to a non-existing VRF using port_add. The VRF creation is handled automatically by the event system in vrf_netlink.c, so no explicit VRF interface creation commands are needed in test scripts.

📚 Learning: 2025-11-05T14:28:28.817Z

Learnt from: rjarry
Repo: DPDK/grout PR: 372
File: modules/infra/datapath/loop_input.c:53-56
Timestamp: 2025-11-05T14:28:28.817Z
Learning: In modules/infra/datapath/loop_input.c, the l3_edges array is intentionally indexed using network byte order (rte_be16_t) for EtherType values. Both loopback_input_add_type() registration and loopback_input_process() lookup use network byte order consistently, so no byte order conversion is needed when accessing l3_edges.

Applied to files:

• modules/infra/control/loopback.c
• modules/ctlplane/l2_datapath.c
• modules/ctlplane/l3_datapath.c

📚 Learning: 2025-11-05T13:55:26.189Z

Learnt from: maxime-leroy
Repo: DPDK/grout PR: 372
File: smoke/cross_vrf_forward_test.sh:18-18
Timestamp: 2025-11-05T13:55:26.189Z
Learning: In the DPDK/grout codebase, VRF interfaces (named gr-vrf<id>) are automatically created when an interface is added to a non-existing VRF using port_add. The VRF creation is handled automatically by the event system in vrf_netlink.c, so no explicit VRF interface creation commands are needed in test scripts.

Applied to files:

• frr/zebra_dplane_grout.c
• modules/infra/control/gr_iface.h
• modules/ctlplane/l2_datapath.c
• modules/ctlplane/l3_datapath.c
• smoke/isis_frr_test.sh
• smoke/ospf_frr_test.sh
• modules/ip6/control/address.c
• modules/ip/control/address.c
• frr/rt_grout.c
• frr/if_map.h
• frr/if_grout.c
• modules/ctlplane/control.c

📚 Learning: 2025-11-03T13:28:19.489Z

Learnt from: rjarry
Repo: DPDK/grout PR: 379
File: modules/infra/datapath/port_tx.c:36-46
Timestamp: 2025-11-03T13:28:19.489Z
Learning: In DPDK graph node process callbacks, the return value is only used for statistics and does not affect packet flow or scheduling through the graph. Nodes can return 0 when they haven't processed packets (e.g., when dropping or redirecting due to port state).

Applied to files:

• modules/ctlplane/l2_datapath.c
• modules/ctlplane/l3_datapath.c

📚 Learning: 2025-09-05T07:06:51.554Z

Learnt from: rjarry
Repo: DPDK/grout PR: 294
File: modules/policy/cli/meson.build:4-8
Timestamp: 2025-09-05T07:06:51.554Z
Learning: In this project, cli_src is a global variable that gets populated by individual modules (like modules/policy/cli/meson.build) and is ultimately consumed by an executable() call in the top-level meson.build file. This creates a modular CLI build where each module contributes its CLI sources to the main CLI binary.

Applied to files:

• modules/ctlplane/meson.build
• modules/meson.build

📚 Learning: 2025-09-09T09:22:31.596Z

Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:97-101
Timestamp: 2025-09-09T09:22:31.596Z
Learning: In SRv6 IPv6 extension header parsing, when is_ipv6_ext[proto] is true, rte_ipv6_get_next_ext() will not return a negative value, making error checking unnecessary. An assert can be used as a defensive measure for future DPDK compatibility.

Applied to files:

• modules/ip6/control/address.c
• frr/rt_grout.c

📚 Learning: 2025-09-22T09:21:51.749Z

Learnt from: rjarry
Repo: DPDK/grout PR: 312
File: modules/ip/control/address.c:102-110
Timestamp: 2025-09-22T09:21:51.749Z
Learning: The rib4_insert() function in the IP module takes ownership of the nexthop reference and automatically calls nexthop_decref(nh) on failure paths, so callers don't need to manually decrement the reference count when rib4_insert fails.

Applied to files:

• modules/ip6/control/address.c
• frr/rt_grout.c

📚 Learning: 2025-09-22T09:21:51.749Z

Learnt from: rjarry
Repo: DPDK/grout PR: 312
File: modules/ip/control/address.c:102-110
Timestamp: 2025-09-22T09:21:51.749Z
Learning: The rib4_insert() function in modules/ip/control/route.c takes ownership of the nexthop reference by calling nexthop_incref(nh) at the beginning and properly calls nexthop_decref(nh) on any failure paths via the fail: label, so callers don't need to manually decrement the reference count when rib4_insert fails.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-09-22T09:21:51.749Z

Learnt from: rjarry
Repo: DPDK/grout PR: 312
File: modules/ip/control/address.c:102-110
Timestamp: 2025-09-22T09:21:51.749Z
Learning: The rib4_insert() function takes ownership of the nexthop reference and automatically calls nexthop_decref(nh) on failure paths, so callers don't need to manually decrement the reference count when rib4_insert fails.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-09-25T07:52:17.403Z

Learnt from: rjarry
Repo: DPDK/grout PR: 312
File: frr/rt_grout.c:355-361
Timestamp: 2025-09-25T07:52:17.403Z
Learning: The nexthop_new() function in FRR's zebra codebase cannot fail and return NULL - it will abort() the process if memory allocation fails, so null checks after calling nexthop_new() are unnecessary.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-10-21T15:42:43.874Z

Learnt from: rjarry
Repo: DPDK/grout PR: 350
File: modules/ip/control/address.c:214-216
Timestamp: 2025-10-21T15:42:43.874Z
Learning: In C code compiled with `-std=gnu2x`, the gr_vec_foreach macro supports inline variable declarations (e.g., `gr_vec_foreach (struct nexthop *nh, vector)`). This is valid C2x syntax and does not require pre-declaring the loop variable.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-09-09T07:08:40.398Z

Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:106-111
Timestamp: 2025-09-09T07:08:40.398Z
Learning: For SRv6 header validation, the correct check for last_entry bounds is `srh->last_entry > (srh->hdrlen / 2) - 1` where hdr_len represents the length in 8-byte units excluding the first 8 bytes, and each segment is 16 bytes, so hdr_len/2 gives the maximum number of segments.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-09-09T06:02:47.703Z

Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:88-122
Timestamp: 2025-09-09T06:02:47.703Z
Learning: When parsing IPv6 routing headers in SRv6 processing, bounds checking for the full extension header length and validation of the routing header structure is needed before accessing fields like hdr_len, last_entry, segments_left, and type.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-09-09T07:36:34.893Z

Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:88-122
Timestamp: 2025-09-09T07:36:34.893Z
Learning: In the SRv6 routing header structure, the flag and tag fields are used for per-flow metadata and Group-Based Policy use cases. Only 5 bytes are needed to access the basic SRv6 fields: next_hdr, hdr_len, type, segments_left, and last_entry.

Applied to files:

• frr/rt_grout.c

📚 Learning: 2025-08-27T15:33:22.299Z

Learnt from: rjarry
Repo: DPDK/grout PR: 305
File: modules/ip6/control/route.c:407-413
Timestamp: 2025-08-27T15:33:22.299Z
Learning: DPDK rte_rib6_get_nxt() with RTE_RIB6_GET_NXT_ALL flag does not yield the default route ::/0 if configured. The explicit rte_rib6_lookup_exact() call for the default route is necessary to ensure complete route enumeration.

Applied to files:

• frr/rt_grout.c

🪛 Shellcheck (0.11.0)

smoke/isis_frr_test.sh

[warning] 22-22: Quote this to prevent word splitting.

(SC2046)

---

[warning] 73-73: Remove surrounding $() to avoid executing output (or use eval if intentional).

(SC2091)

---

[warning] 83-83: Remove surrounding $() to avoid executing output (or use eval if intentional).

(SC2091)

---

[warning] 83-83: This command substitution will be empty because the command's output gets redirected away.

(SC2327)

---

[error] 83-83: This redirection takes output away from the command substitution.

(SC2328)

smoke/ospf_frr_test.sh

[warning] 20-20: Quote this to prevent word splitting.

(SC2046)

---

[warning] 77-77: Remove surrounding $() to avoid executing output (or use eval if intentional).

(SC2091)

---

[warning] 77-77: This command substitution will be empty because the command's output gets redirected away.

(SC2327)

---

[error] 77-77: This redirection takes output away from the command substitution.

(SC2328)

---

[warning] 86-86: Remove surrounding $() to avoid executing output (or use eval if intentional).

(SC2091)

---

[warning] 86-86: This command substitution will be empty because the command's output gets redirected away.

(SC2327)

---

[error] 86-86: This redirection takes output away from the command substitution.

(SC2328)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
• GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
• GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)

🔇 Additional comments (18)

modules/ip6/control/address.c (2)

247-248: LGTM: Simplified netlink address operation.

The direct use of iface->name aligns with the control plane design where each interface has a corresponding TAP. Error handling appropriately logs warnings without blocking the operation.

---

333-334: LGTM: Consistent with address addition change.

The simplified netlink operation correctly mirrors the change in iface6_addr_add, maintaining consistency in how addresses are managed on TAP interfaces.

frr/meson.build (1)

23-23: LGTM!

The addition of if_map.c to the build sources correctly integrates the new ifindex mapping module.

modules/ip/control/address.c (2)

66-133: Simplified VRF handling looks good.

The removal of VRF-based interface lookup in favor of direct interface name usage simplifies the address management flow. The iface pointer is validated before use, making the netlink calls safe.

---

139-178: Consistent simplification in delete path.

The same VRF indirection removal is correctly applied to the address deletion path.

frr/if_map.h (1)

15-16: Verify the type asymmetry in conversion functions.

The conversion functions use different types asymmetrically:

• ifindex_grout_to_frr takes int16_t and returns ifindex_t
• ifindex_frr_to_grout takes ifindex_t and returns uint16_t

Confirm this reflects the actual type usage in Grout (signed vs unsigned int16) and FRR (ifindex_t), and that the signedness difference is intentional.

modules/meson.build (1)

11-11: LGTM!

The ctlplane submodule is correctly integrated into the build.

modules/ctlplane/ctlplane.h (1)

1-8: LGTM!

Clean API declaration for the control plane transmit function.

modules/ctlplane/meson.build (1)

1-10: LGTM!

Standard build configuration for the new ctlplane module.

frr/frr_plugin_install.sh (1)

12-13: LGTM!

The ISIS and OSPF daemon enablement follows the existing pattern and correctly updates the FRR configuration.

smoke/_init_frr.sh (2)

249-256: LGTM!

ISIS and OSPF daemons are correctly enabled in the test FRR configuration with appropriate logging options.

---

291-305: LGTM!

The startup verification loops for ISIS and OSPF daemons follow the same pattern as the existing BGP verification, with consistent timeout and polling intervals.

frr/zebra_dplane_grout.c (1)

6-6: LGTM! Clean integration of the interface mapping system.

The includes, switch cases, and initialization call properly integrate the new bidirectional ifindex mapping. The placement in zd_grout_module_init ensures mappings are ready before any translation operations.

Also applies to: 9-9, 320-321, 334-335, 740-740

smoke/_init.sh (1)

226-227: LGTM! Daemon configuration follows existing patterns.

The ISIS and OSPF daemon enablement and options are consistent with the existing BGP/Zebra configuration style.

Also applies to: 231-232

modules/infra/cli/iface.c (1)

327-330: LGTM! Control-plane stats properly integrated into CLI output.

The new columns follow the existing pattern, indices are correct (7-10 after the existing 0-6), and the field names match the structure definitions.

Also applies to: 353-356

modules/infra/control/loopback.c (1)

56-56: LGTM! Control-plane statistics properly tracked.

The stats updates are correctly placed after successful TX/RX operations and use the appropriate per-lcore stats accessor.

Also applies to: 101-103, 113-116, 164-167

modules/infra/api/gr_infra.h (1)

243-246: LGTM! Control-plane stat fields added cleanly.

The new fields follow the existing naming convention and are appended to maintain compatibility.

frr/rt_grout.c (1)

4-4: LGTM! Interface index translations properly applied throughout.

The remaining translations consistently use ifindex_grout_to_frr when reading from grout structures and ifindex_frr_to_grout when writing to grout APIs, maintaining correct namespace conversions across the FRR/grout boundary.

Also applies to: 194-195, 271-271, 294-295, 467-467, 671-672, 707-709, 713-715, 719-721, 725-727, 834-834, 853-863

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Remove double translation of vrf_id.

Line 317 translates gr_nh->vrf_id from grout to FRR namespace. Line 319 then translates the already-translated vrf_id again. Since the comment states "table_id always equals vrf_id" (in FRR namespace), the second translation is incorrect.

 	uint32_t vrf_id = ifindex_grout_to_frr(gr_nh->vrf_id);
 	// Grout has no per‑VRF routing tables; table_id always equals vrf_id
-	uint32_t tableid = ifindex_grout_to_frr(vrf_id);
+	uint32_t tableid = vrf_id;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	uint32_t vrf_id = ifindex_grout_to_frr(gr_nh->vrf_id);
	// Grout has no per‑VRF routing tables; table_id always equals vrf_id
	uint32_t tableid = vrf_id;
	uint32_t tableid = ifindex_grout_to_frr(vrf_id);
	uint32_t vrf_id = ifindex_grout_to_frr(gr_nh->vrf_id);
	// Grout has no per‑VRF routing tables; table_id always equals vrf_id
	uint32_t tableid = vrf_id;

🤖 Prompt for AI Agents

In frr/rt_grout.c around lines 317 to 319, the code calls ifindex_grout_to_frr
twice: first on gr_nh->vrf_id (correct) and then again on the already-translated
vrf_id to produce tableid; remove the second translation so tableid is assigned
directly from the already-converted vrf_id. Update the assignment to set tableid
= vrf_id and remove the redundant ifindex_grout_to_frr call.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Guard bond active_member bounds
active_member > n_members lets active_member == n_members slip through, so the subsequent array access can run past the members array (and n_members == 0 is also unsafe). Tighten the guard accordingly.

-	if (b->active_member > b->n_members)
+	if (b->n_members == 0 || b->active_member >= b->n_members)
 		goto err;

🤖 Prompt for AI Agents

In modules/ctlplane/control.c around lines 194-200, the bond active_member
bounds check is off-by-one and doesn't guard zero members; replace the current
check with one that fails when there are no members or when active_member is out
of range (i.e. use if (b->n_members == 0 || b->active_member >= b->n_members)
goto err;), so the subsequent array access into b->members[b->active_member] is
always within bounds.

[rjarry]

This pattern isn't used anywhere. We use old school comments.

Are you sure it is in the C23 standard? I had understood it was C++ syntax allowed for C in gcc.

[christophefontaine]

It is in C23, yes:
https://en.cppreference.com/w/c/language/attributes/fallthrough