zebra: kernel_init function optimizations

[sri-mohan1]

these changes are for improving the code maintainability and readability

[greptile-apps]

Greptile Summary

This PR refactors the kernel_init function in zebra/kernel_netlink.c for readability and maintainability by extracting repeated initialization patterns into three small helpers and a convenience macro.

• Introduces FRR_NLGRP_BIT(g) macro to replace repeated ((uint32_t)1 << (RTNLGRP_XXX - 1)) inline expressions, kernel_init_nlsock() to consolidate the five-step socket creation pattern (name, mark uncreated, create, log, insert), and netlink_set_nonblock() / netlink_enable_ext_ack() to de-duplicate the non-blocking and extended-ACK setsockopt calls.
• The socket creation order, group bitmasks, fatal/non-fatal error semantics, and event-loop registration are all preserved relative to the original code; no behavioral change is intended.

Confidence Score: 5/5

Safe to merge — this is a pure refactor with no change in socket creation order, group bitmasks, or fatal/non-fatal error semantics.

All observable behavior in kernel_init is preserved: the same five sockets are created in the same order, the same groups are subscribed, and the same exit paths are taken on failure. The new helpers consolidate repeated patterns without introducing new logic paths.

No files require special attention.

Important Files Changed

Filename	Overview
zebra/kernel_netlink.c	Refactors kernel_init into three helpers (kernel_init_nlsock, netlink_set_nonblock, netlink_enable_ext_ack) and introduces FRR_NLGRP_BIT macro; logic and socket-creation order are preserved.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[kernel_init] --> B[Compute group bitmasks\nFRR_NLGRP_BIT macro]
    B --> C[kernel_init_nlsock\nnetlink-listen\nfatal]
    C -->|fail| X1[frr_exit_with_buffer_flush]
    C -->|ok| D[kernel_init_nlsock\nnetlink-cmd\nfatal]
    D -->|fail| X2[frr_exit_with_buffer_flush]
    D -->|ok| E[kernel_init_nlsock\nnetlink-dp\nfatal]
    E -->|fail| X3[frr_exit_with_buffer_flush]
    E -->|ok| F[kernel_init_nlsock\nnetlink-dp-in\nfatal]
    F -->|fail| X4[frr_exit_with_buffer_flush]
    F -->|ok| G[kernel_init_nlsock\ngeneric-netlink-cmd\nnon-fatal]
    G --> H{SOL_NETLINK?}
    H -->|yes| I[setsockopt BRVLAN]
    I --> J[netlink_enable_ext_ack\ncmd + dp]
    J --> K[EXT_ACK ge_netlink]
    K --> L[CAP_ACK dplane_out]
    H -->|no| M
    L --> M[netlink_set_nonblock\nall sockets]
    M --> N[netlink_recvbuf]
    N --> O[netlink_install_filter]
    O --> P[event_add_read]
    P --> Q[rt_netlink_init\nge_netlink_init]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[kernel_init] --> B[Compute group bitmasks\nFRR_NLGRP_BIT macro]
    B --> C[kernel_init_nlsock\nnetlink-listen\nfatal]
    C -->|fail| X1[frr_exit_with_buffer_flush]
    C -->|ok| D[kernel_init_nlsock\nnetlink-cmd\nfatal]
    D -->|fail| X2[frr_exit_with_buffer_flush]
    D -->|ok| E[kernel_init_nlsock\nnetlink-dp\nfatal]
    E -->|fail| X3[frr_exit_with_buffer_flush]
    E -->|ok| F[kernel_init_nlsock\nnetlink-dp-in\nfatal]
    F -->|fail| X4[frr_exit_with_buffer_flush]
    F -->|ok| G[kernel_init_nlsock\ngeneric-netlink-cmd\nnon-fatal]
    G --> H{SOL_NETLINK?}
    H -->|yes| I[setsockopt BRVLAN]
    I --> J[netlink_enable_ext_ack\ncmd + dp]
    J --> K[EXT_ACK ge_netlink]
    K --> L[CAP_ACK dplane_out]
    H -->|no| M
    L --> M[netlink_set_nonblock\nall sockets]
    M --> N[netlink_recvbuf]
    N --> O[netlink_install_filter]
    O --> P[event_add_read]
    P --> Q[rt_netlink_init\nge_netlink_init]

Loading

_{Reviews (2): Last reviewed commit: "zebra: kernel_init function optimization..." | Re-trigger Greptile}

[sri-mohan1]

Greptile Summary

This PR refactors kernel_init in zebra/kernel_netlink.c by extracting three static helpers (netlink_set_nonblock, kernel_init_nlsock, netlink_enable_ext_ack) and introducing a RTNLGRP_BIT macro to replace the inline bit-shift expressions for multicast group bitmasks. The underlying socket creation and configuration behavior is preserved faithfully.

• RTNLGRP_BIT(g) centralizes the (uint32_t)1 << (g-1) pattern with a comment explaining the 1-based RTNLGRP convention and the < 32 limit.
• kernel_init_nlsock consolidates the five-step socket init pattern (name format, set sock=-1, create, log failure, hash-insert), with a warn_only flag to distinguish the optional ge_netlink_cmd socket from the four critical ones.
• netlink_enable_ext_ack deduplicates the setsockopt(SOL_NETLINK, NETLINK_EXT_ACK, ...) calls for the command and dataplane-out sockets; the generic-netlink socket's distinct flog_err path is intentionally kept inline.
• netlink_set_nonblock unifies the fcntl(F_SETFL, O_NONBLOCK) calls under flog_err_sys, which is more appropriate than the original mix of flog_err_sys/flog_err.

Confidence Score: 3/5

The refactoring is logically correct, but kernel_init_nlsock passes a non-literal name_fmt to snprintf, which will fire the unconditionally-enabled -Wformat-nonliteral build flag and break CI where --enable-werror is active.

The socket initialization logic and group bitmask computation are faithfully preserved. The one concrete defect is in kernel_init_nlsock: snprintf(nl->name, sizeof(nl->name), name_fmt, ns_id) uses a runtime const char * as the format argument. FRR's configure.ac unconditionally adds -Wformat-nonliteral to the compiler flags, producing a warning on every developer build and a hard error on CI that runs --enable-werror. The fix is small, but the build breakage makes it worth resolving before merge.

zebra/kernel_netlink.c — specifically kernel_init_nlsock and its snprintf call with the non-literal name_fmt parameter.

Important Files Changed

Filename Overview
zebra/kernel_netlink.c Refactors kernel_init by extracting three helpers and introducing RTNLGRP_BIT; functional behavior is preserved, but kernel_init_nlsock passes a non-literal format string to snprintf, which will fire the unconditionally-enabled -Wformat-nonliteral build flag.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[kernel_init] --> B[Compute groups / dplane_groups / ext_groups bitmasks]
    B --> C[kernel_init_nlsock: netlink-listen\ngroups + ext_groups, fatal]
    C -->|fail| X1[frr_exit_with_buffer_flush]
    C -->|ok| D[kernel_init_nlsock: netlink-cmd\nno groups, fatal]
    D -->|fail| X2[frr_exit_with_buffer_flush]
    D -->|ok| E[kernel_init_nlsock: netlink-dp\nno groups, fatal]
    E -->|fail| X3[frr_exit_with_buffer_flush]
    E -->|ok| F[kernel_init_nlsock: netlink-dp-in\ndplane_groups, fatal]
    F -->|fail| X4[frr_exit_with_buffer_flush]
    F -->|ok| G[kernel_init_nlsock: generic-netlink-cmd\nno groups, warn-only]
    G --> H{SOL_NETLINK defined?}
    H -->|yes| I[setsockopt RTNLGRP_BRVLAN on dp-in]
    I --> J[netlink_enable_ext_ack on cmd + dp-out]
    J --> K{ge_netlink_cmd sock >= 0?}
    K -->|yes| L[setsockopt NETLINK_EXT_ACK on ge_netlink_cmd]
    L --> M[setsockopt NETLINK_CAP_ACK on dp-out]
    K -->|no| M
    M --> N[endif SOL_NETLINK]
    H -->|no| N
    N --> O[netlink_set_nonblock on all sockets]
    O --> P{rcvbufsize set?}
    P -->|yes| Q[netlink_recvbuf on all sockets]
    P -->|no| R[netlink_install_filter on listen + dp-in]
    Q --> R
    R --> S[event_add_read for kernel_read]
    S --> T[rt_netlink_init + ge_netlink_init]

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
zebra/kernel_netlink.c:1661
**Non-literal format string triggers `-Wformat-nonliteral`**

`configure.ac` unconditionally enables `-Wformat-nonliteral`, so passing `name_fmt` (a `const char *` parameter, not a string literal) as the format argument to `snprintf` will produce a compiler warning in every FRR build, and a hard build failure on CI where `--enable-werror` is used. Either annotate the parameter with `__attribute__((format(printf, 2, 0)))` / `PRINTFRR(2, 0)`, or replace the single `snprintf` with a call that makes the format an inline literal.

### Issue 2 of 2
zebra/kernel_netlink.c:81-84
Double blank line after the macro definition — FRR style uses a single blank line between top-level declarations.

```suggestion
#define RTNLGRP_BIT(g) ((uint32_t)1 << ((g) - 1))

static const struct message nlmsg_str[] = {

Reviews (1): Last reviewed commit: ["zebra: kernel_init function optimization..."](https://github.com/frrouting/frr/commit/8a3b37630f043587c31cd5760dd15a4bb0f24ed7) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=37595639)

Corrected the reported items.

[ramanjiee]

Changes are fine, improving the readability and the code is optimized.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[sri-mohan1]

Review comments addressed and Rebased onto latest master.
Kindly review further and take it forward.

[mjstapp]

Thanks

[mjstapp]

@greptileai review

[mjstapp]

asked greptile to check again - let's see what it says