Misc fixes

[christophefontaine]

Summary by CodeRabbit

• New Features

  • Added core-pinned per-interface counters for RX/TX packets and bytes, exposed in stats, telemetry, and CLI tables.
  • Improved request-type labeling to show two additional request names.

• Bug Fixes

  • Improved IPv6 link-local address generation using Modified EUI-64.
  • Corrected IPv6 link-local prefix to /64.
  • Added defensive null-check in NDP probe handling to avoid crashes.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds four per-interface core-pinned counters (cp_rx_packets, cp_rx_bytes, cp_tx_packets, cp_tx_bytes) across infra and control modules: initialized, aggregated per-core, emitted in telemetry, and shown in CLI. Updates loopback to increment these counters. Adjusts netlink address add/del to use /64 for IPv6 link-local addresses (otherwise /128). Alters IPv6 post-add handling to flip bit 0x02 of MAC first byte before deriving the link-local address. Adds a NULL check in ndp_probe_input_cb to skip processing when L3 nexthop state is missing. Extends zebra request-type string mapping with two entries.

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check	❓ Inconclusive	Title is vague and generic; 'Misc fixes' does not convey meaningful information about the substantial changes across multiple modules including stats tracking, IPv6 handling, and netlink behavior.	Use a more descriptive title that captures the main changes, such as 'Add core-pinned interface statistics and IPv6 address handling improvements'.

✅ Passed checks (1 passed)

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

---

📜 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 0935191 and b186e0b.

📒 Files selected for processing (8)

• frr/zebra_dplane_grout.c (2 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 (1 hunks)
• modules/infra/control/loopback.c (3 hunks)
• modules/infra/control/netlink.c (1 hunks)
• modules/ip6/control/address.c (1 hunks)

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

• modules/infra/cli/iface.c
• modules/infra/control/loopback.c
• modules/infra/api/gr_infra.h
• modules/ip6/control/address.c

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

• frr/zebra_dplane_grout.c
• modules/infra/control/netlink.c
• modules/infra/control/gr_iface.h
• modules/infra/api/stats.c

🧠 Learnings (3)

📓 Common learnings

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.

📚 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/infra/control/netlink.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: The DPDK function rte_ipv6_get_next_ext() only reads 2 bytes from the extension header, so a 2-byte precheck is sufficient before calling it.

Applied to files:

• modules/infra/control/netlink.c

🧬 Code graph analysis (1)

modules/infra/api/stats.c (1)

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

• iface_get_stats (104-106)
• iface (16-22)

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

• GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
• GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
• 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-cross-aarch64
• GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
• GitHub Check: deb
• GitHub Check: rpm

🔇 Additional comments (6)

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

234-243: LGTM! Correct prefix length for IPv6 link-local addresses.

The logic correctly assigns /64 to link-local IPv6 addresses (fe80::/10), which is necessary for on-link communication, while using /128 for other IPv6 addresses. The comment clearly explains the rationale.

frr/zebra_dplane_grout.c (1)

318-333: LGTM! String mappings for new request types.

The additions for GR_INFRA_IFACE_GET and GR_SRV6_TUNSRC_SET follow the existing pattern consistently.

modules/infra/control/gr_iface.h (1)

95-98: LGTM! New control-plane counter fields added.

The four new cp_* counter fields are well-positioned after existing counters and follow the naming convention. The cache-aligned struct ensures proper alignment.

modules/infra/api/stats.c (3)

269-272: LGTM! Initialization of new control-plane counters.

The new cp_* counters are correctly initialized to zero, following the same pattern as existing counters.

---

281-284: LGTM! Per-core aggregation of control-plane counters.

The aggregation logic correctly accumulates cp_* counters across all cores, consistent with the existing pattern for rx/tx counters.

---

397-417: LGTM! Telemetry exposure of control-plane counters.

The telemetry path correctly declares local variables, accumulates per-core stats, and exposes the cp_* counters in the output, following the established pattern.

---

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.}

[rjarry]

We should fix this in DPDK?

[david-marchand]

DPDK states compliance against RFC 2464.

/*                                                                                                  
 * Generate a link-local IPv6 address from an Ethernet address as specified in                      
 * RFC 2464, section 5.                                                                             
 *                                                                                                  
 * @param[out] ip                                                                                   
 *   The link-local IPv6 address to generate.                                                       
 * @param[in] mac                                                                                   
 *   An Ethernet address.                                                                           
 */                                                                                                 
static inline void                                                                                  
rte_ipv6_llocal_from_ethernet(struct rte_ipv6_addr *ip, const struct rte_ether_addr *mac)       

Section 5 points at section 4 of this RFC and reads like:

   The Interface Identifier is then formed from the EUI-64 by
   complementing the "Universal/Local" (U/L) bit, which is the next-to-
   lowest order bit of the first octet of the EUI-64.  Complementing
   this bit will generally change a 0 value to a 1, since an interface's
   built-in address is expected to be from a universally administered
   address space and hence have a globally unique value.  A universally
   administered IEEE 802 address or an EUI-64 is signified by a 0 in the
   U/L bit position, while a globally unique IPv6 Interface Identifier
   is signified by a 1 in the corresponding position.  For further
   discussion on this point, see [[AARCH](https://datatracker.ietf.org/doc/html/rfc2464#ref-AARCH)].

The reference AARCH is a link to the discussion on IPv6 address architecture, which is current RFC 4291.

So it indeed seems like DPDK should be fixed.