Skip to content

Missing show run output#22619

Open
donaldsharp wants to merge 9 commits into
FRRouting:masterfrom
donaldsharp:missing_show_run_output
Open

Missing show run output#22619
donaldsharp wants to merge 9 commits into
FRRouting:masterfrom
donaldsharp:missing_show_run_output

Conversation

@donaldsharp

Copy link
Copy Markdown
Member

See individual commits for what is missing. Code inspection found a bunch of code that was not properly being displayed in a show run.

Add some missing debug commands to the show command.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Found a bit of code where spaces were being used instead of tabs.
Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Current code displays `timers active 0` when a show run is
run.  It should be `timers active disabled`, fix it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
If you enter this:
bfd
 peer 1.2.3.4 bfd-mode sbfd-echo bfd-name FOO multi-hop local-address 1.2.3.4 srv6-source-ipv6 1::1 srv6-encap-data 2::2
 exit

this is displayed:

bfd
 peer 1.2.3.4 bfd-mode sbfd-echo bfd-name FOO local-address 1.2.3.4 srv6-source-ipv6 1::1 srv6-encap-data 2::2
 exit

Fix the missing multihop.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
If you enter this:

router bgp 32939 vrf BLUE
 address-family l2vpn evpn
   advertise-pip

show run does not display `advertise-pip`:
!
router bgp 39239 vrf BLUE
exit
!

Fix it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This was not being properly displayed in a show run.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This command was never displayed when modified, fix it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When configuring ipv6 nhrp authentication, only
ip nhrp authentication is displayed on a show run.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This command is never displayed on a `show run`
Show it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes several missing show running-config outputs across multiple FRR daemons — bfdd, bgpd, eigrpd, nhrpd, pimd, and lib — where configured values were silently omitted from the running config. Each commit targets a distinct missing case discovered via code inspection.

  • bfdd: sBFD-ECHO/INIT sessions now dynamically check the multi-hop yang node instead of hardcoding false/true, so the multihop keyword only appears when actually configured.
  • bgpd: Two missing outputs are added: bgp graceful-restart disable-eor when the flag is set, and advertise-pip for VRF instances where PIP is enabled but no static IP is configured.
  • eigrpd: Active-time value 0 is now correctly rendered as disabled to match the accepted CLI keyword.
  • nhrpd: The AFI (ip/ipv6) is now tracked on the nhrp_interface struct and used when writing the nhrp authentication config line, replacing a hardcoded ip.
  • lib: log filter-text entries are now persisted in running config (previously intentionally excluded with a "may be in the future" comment), and memory release rate is written when non-default. The FRR_MEM_RELEASE_MB_DEFAULT constant is moved to the header so command.c can reference it without tcmalloc conditionals.
  • pimd: debug pim graft and debug pim state-refresh are added to the debug config writer.

Confidence Score: 4/5

Safe to merge — all changes are additive show-run fixes with no behavioral impact on the data plane, and each is well-scoped to its daemon's config writer.

The code changes are narrow and correct: each adds a previously-missing vty_out call or fixes the value passed to an existing one. The one non-obvious change is enabling log filter-text persistence in lib/log_cli.c, which intentionally changes a session-only setting to a persistent one and may surprise operators who relied on the old behavior.

lib/log_cli.c — the log filter-text persistence change is intentional but is a behavioral shift that benefits from explicit documentation.

Important Files Changed

Filename Overview
nhrpd/nhrp_vty.c Tracks the AFI on nhrp_interface and uses it when writing the authentication config line; IS_VALID_AFI guard correctly falls back to AFI_IP for zero-initialized structs.
nhrpd/nhrpd.h Adds afi_t auth_afi field to nhrp_interface; the struct is XCALLOC-ed so the field is zero-initialized to AFI_UNSPEC, which IS_VALID_AFI handles correctly.
lib/libfrr.c Removes FRR_MEM_RELEASE_MB_DEFAULT from inside the HAVE_TCMALLOC guard and adds frr_mem_release_rate_get() in both tcmalloc and stub paths; correct symmetric implementation.
lib/libfrr.h Moves FRR_MEM_RELEASE_MB_DEFAULT to the header and declares frr_mem_release_rate_get(), making the constant and accessor available to all translation units unconditionally.
lib/log_cli.c Enables log filter-text persistence in running config; the YANG node is properly defined as a leaf-list (not config false), but this is a behavioral change from the previous intentionally-excluded state.
lib/command.c Adds memory release rate to config write when non-default; correctly calls the new frr_mem_release_rate_get() which works for both tcmalloc and non-tcmalloc builds.
bgpd/bgp_evpn_vty.c Adds missing advertise-pip output for the case where advertise_pip is true but no static IP is configured; the two if blocks are correctly mutually exclusive.
bgpd/bgp_vty.c Adds missing bgp graceful-restart disable-eor config line; straightforward flag-check addition in the correct location.
bfdd/bfdd_cli.c Replaces hardcoded mhop values with dynamic yang_dnode_exists checks for both sbfd-echo and sbfd-init, correctly reflecting the actual configured topology.
eigrpd/eigrp_cli.c Fixes active-time display: YANG stores 0 for disabled, and the CLI keyword disabled round-trips correctly via nb_cli_enqueue_change.
pimd/pim_vty.c Adds missing debug pim graft and state-refresh config writes and fixes PIM_DEBUG_MLAG block indentation; straightforward additions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[show running-config] --> B{config_write_host}
    B --> C[memory release rate\n NEW: write if non-default]
    A --> D{bgp_config_write}
    D --> E[bgp graceful-restart disable-eor\n NEW: write if flag set]
    D --> F{bgp_config_write_evpn_info}
    F --> G[advertise-pip / no advertise-pip\n NEW: else advertise-pip\nwhen pip_ip_static == INADDR_ANY]
    A --> H{interface_config_write - nhrpd}
    H --> I[auth_token present?]
    I -->|yes| J[IS_VALID_AFI auth_afi?\n NEW: use auth_afi, fallback AFI_IP]
    J --> K[write: ip/ipv6 nhrp authentication]
    A --> L{frr_logging yang nodes}
    L --> M[filter-text\n NEW: now included in cli_show]
    A --> N{pim_debug_config_write}
    N --> O[debug pim graft\n NEW]
    N --> P[debug pim state-refresh\n NEW]
    A --> Q{eigrp_cli_show_active_time}
    Q -->|timer == 0| R[write: timers active-time disabled\n NEW]
    Q -->|timer != 0| S[write: timers active-time N]
    A --> T{bfd_cli_show_sbfd_peer}
    T --> U[yang_dnode_exists multi-hop?\n NEW: dynamic instead of hardcoded]
    U -->|yes| V[print multihop keyword]
    U -->|no| W[omit multihop keyword]
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[show running-config] --> B{config_write_host}
    B --> C[memory release rate\n NEW: write if non-default]
    A --> D{bgp_config_write}
    D --> E[bgp graceful-restart disable-eor\n NEW: write if flag set]
    D --> F{bgp_config_write_evpn_info}
    F --> G[advertise-pip / no advertise-pip\n NEW: else advertise-pip\nwhen pip_ip_static == INADDR_ANY]
    A --> H{interface_config_write - nhrpd}
    H --> I[auth_token present?]
    I -->|yes| J[IS_VALID_AFI auth_afi?\n NEW: use auth_afi, fallback AFI_IP]
    J --> K[write: ip/ipv6 nhrp authentication]
    A --> L{frr_logging yang nodes}
    L --> M[filter-text\n NEW: now included in cli_show]
    A --> N{pim_debug_config_write}
    N --> O[debug pim graft\n NEW]
    N --> P[debug pim state-refresh\n NEW]
    A --> Q{eigrp_cli_show_active_time}
    Q -->|timer == 0| R[write: timers active-time disabled\n NEW]
    Q -->|timer != 0| S[write: timers active-time N]
    A --> T{bfd_cli_show_sbfd_peer}
    T --> U[yang_dnode_exists multi-hop?\n NEW: dynamic instead of hardcoded]
    U -->|yes| V[print multihop keyword]
    U -->|no| W[omit multihop keyword]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
lib/log_cli.c:523
**Behavioral change: log filter-text now persists across restarts**

The old code carried an explicit comment: `"This is not saved, but may be the future."` Enabling the `cli_show` callback here means `log filter-text` entries will now appear in running-config and be re-applied when the config is loaded on daemon restart. Previously these were session-only/ephemeral. The YANG leaf-list is properly defined as a config node so there is no technical blocker, but operators who relied on filter-text being temporary (e.g. short-lived debug filters) will now find those entries surviving restarts. Worth calling out in the commit message or release notes so users are not surprised.

Reviews (1): Last reviewed commit: "lib: Fix `log filter-text X` to display" | Re-trigger Greptile

Comment thread lib/log_cli.c
{ .xpath = "/frr-logging:logging/filtered-file/filename", .cbs.cli_show = logging_filtered_file_filename_cli_write },
/* This is not saved, but may be the future. */
/* { .xpath = "/frr-logging:logging/filter-text", .cbs.cli_show = logging_filter_text_cli_write }, */
{ .xpath = "/frr-logging:logging/filter-text", .cbs.cli_show = logging_filter_text_cli_write },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Behavioral change: log filter-text now persists across restarts

The old code carried an explicit comment: "This is not saved, but may be the future." Enabling the cli_show callback here means log filter-text entries will now appear in running-config and be re-applied when the config is loaded on daemon restart. Previously these were session-only/ephemeral. The YANG leaf-list is properly defined as a config node so there is no technical blocker, but operators who relied on filter-text being temporary (e.g. short-lived debug filters) will now find those entries surviving restarts. Worth calling out in the commit message or release notes so users are not surprised.

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/log_cli.c
Line: 523

Comment:
**Behavioral change: log filter-text now persists across restarts**

The old code carried an explicit comment: `"This is not saved, but may be the future."` Enabling the `cli_show` callback here means `log filter-text` entries will now appear in running-config and be re-applied when the config is loaded on daemon restart. Previously these were session-only/ephemeral. The YANG leaf-list is properly defined as a config node so there is no technical blocker, but operators who relied on filter-text being temporary (e.g. short-lived debug filters) will now find those entries surviving restarts. Worth calling out in the commit message or release notes so users are not surprised.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread lib/command.c
else if (!host.motd)
vty_out(vty, "no banner motd\n");

if (frr_mem_release_rate_get() != FRR_MEM_RELEASE_MB_DEFAULT)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a nit, maybe: the new api is going to report a non-zero rate in builds that don't have support at all. that seems a little confusing? do you think that's worth thinking about, or is it too minor to matter?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that it works that way at all. I do not have the memory stuff compiled in at all and it does not show up in my show run

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm - the api returns the "default" value, which is 1MB, right? I get it that the test condition avoids emitting config in that case, but the api returns what it returns?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is returnning FRR_MEM_RELEASE_MB_DEFAULT in the no compile case as such the if statement won't be true. So no display?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rereading this I am not sure what we are arguing about. Can you help me understand ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants