nexthop: reduce info size

[christophefontaine]

Reduce the size of the info field as we don't have any structure using the whole 128 bytes.
Cap the size of the whole "struct nexthop" to a cacheline.

Summary by CodeRabbit

• Refactor
  • Optimized internal per-item memory layout and added a structural size check to enforce bounds and improve cache alignment. No changes to functionality, APIs, or user interface.

Note: Internal infrastructure optimization only — no direct user-facing impact.

[coderabbitai]

📝 Walkthrough

Walkthrough

In modules/infra/control/gr_nh_control.h, the info field in struct __rte_cache_aligned nexthop was changed from uint8_t info[128] __rte_aligned(alignof(void *)); to uint8_t info[RTE_CACHE_LINE_MIN_SIZE * 2 - sizeof(struct gr_nexthop_base) - sizeof(uint32_t)] __rte_aligned(alignof(void *));. The field's alignment is preserved. A static_assert(sizeof(struct nexthop) <= (RTE_CACHE_LINE_MIN_SIZE * 2)); was added to enforce the structure size bound. Other members and layout remain unchanged.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "nexthop: reduce info size" directly and accurately summarizes the primary change in the pull request. It clearly identifies the component (nexthop) and the main action (reduce info size), which is exactly what the changeset accomplishes by replacing the fixed-size array with a computed-size one. The title is concise, specific, and avoids vague phrasing, making it clear to someone scanning history what the core change addresses.

---

📜 Recent 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 cbe593d and bab65df.

📒 Files selected for processing (1)

• modules/infra/control/gr_nh_control.h (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• modules/infra/control/gr_nh_control.h

---

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: 0

🧹 Nitpick comments (2)

modules/infra/control/gr_nh_control.h (2)

27-29: Round up header size to info’s alignment to avoid padding drift.

The current subtraction ignores padding added to align info (pointer-aligned). If padding appears before info, sizeof(struct nexthop) can exceed the intended cap or you underutilize space. Compute with ceil-to-alignment on the header:

-    uint8_t info
-        [RTE_CACHE_LINE_MIN_SIZE * 2 - sizeof(struct gr_nexthop_base)
-         - sizeof(uint32_t)] __rte_aligned(alignof(void *));
+    uint8_t info[
+        (RTE_CACHE_LINE_MIN_SIZE * 2)
+        - (
+            ( (sizeof(struct gr_nexthop_base) + sizeof(uint32_t) + alignof(void *) - 1)
+              / alignof(void *) ) * alignof(void *)
+          )
+    ] __rte_aligned(alignof(void *));

This keeps the struct strictly within the target size across ABIs even if compiler inserts pre‑info padding.

---

31-31: Optional: strengthen invariant with explicit guard and verify commit message.

The info field size is mathematically guaranteed > 0 by the existing constraints (struct bounded to 128 bytes), but adding an explicit guard improves clarity:

 static_assert(sizeof(struct nexthop) <= (RTE_CACHE_LINE_MIN_SIZE * 2));
+static_assert(MEMBER_SIZE(struct nexthop, info) > 0, "nexthop.info must be > 0");

Clarify cache line wording: The commit claims "single cacheline" but RTE_CACHE_LINE_MIN_SIZE * 2 = 128 bytes, which spans 1 cache line on 128B architectures and 2 on 64B architectures. Verify the commit message matches the actual intent.

Searches found no problematic hardcoded 128-byte assumptions or unsafe casts elsewhere in the codebase.

📜 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 73ffeee and cbe593d.

📒 Files selected for processing (1)

• modules/infra/control/gr_nh_control.h (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{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/gr_nh_control.h

⏰ 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). (7)

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