Skip to content

docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA - #2195

Open
ruben-herold wants to merge 2 commits into
vyos:rollingfrom
ruben-herold:T9200-bgp-no-ipv6-auto-ra-caveat
Open

docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA#2195
ruben-herold wants to merge 2 commits into
vyos:rollingfrom
ruben-herold:T9200-bgp-no-ipv6-auto-ra-caveat

Conversation

@ruben-herold

Copy link
Copy Markdown
Contributor

Change Summary

no-ipv6-auto-ra only prevents future RA initiation; it doesn't
retroactively silence router advertisements already sent on an
established session. Confirmed on VyOS 1.5.1: neither a soft nor a
hard clear/reset bgp on the neighbor stopped it. Per FRR's
bgp_zebra_terminate_radv()/upstream PR
FRRouting/frr#19487,
that only runs when the neighbor's remote-as is removed and re-added.

Related Task(s)

Related PR(s)

None.

Backport

None.

Checklist:

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d839613-d7e4-4423-a1b3-8b0beda3eea4

📥 Commits

Reviewing files that changed from the base of the PR and between 33c7f5a and 9f43de7.

📒 Files selected for processing (1)
  • docs/configuration/protocols/bgp.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ansible/ansible (manual)
💤 Files with no reviewable changes (1)
  • docs/configuration/protocols/bgp.md
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Mergify Merge Queue
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
⚠️ CI failures not shown inline (4)

GitHub Actions: AI Validation / prepare: docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m# Fetch the base branch explicitly by refname to avoid ambiguity with�[0m
 �[36;1m# same-named tags (e.g., a `rolling` tag), then diff against FETCH_HEAD.�[0m
 �[36;1mgit fetch --no-tags --depth=1 origin "refs/heads/rolling"�[0m
 �[36;1mBASE="FETCH_HEAD"�[0m
 �[36;1m# --diff-filter=ACMRT excludes Deleted entries so the bundling�[0m
 �[36;1m# loop below (`git show HEAD:<path>`) doesn't try to extract�[0m
 �[36;1m# blobs for files that no longer exist in the merge ref.�[0m
 �[36;1m# Deletions still appear in diff-md.patch (full diff) but not�[0m
 �[36;1m# in changed-md.txt (which drives the bundling step).�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.md'  > changed-md.z�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.rst' > changed-rst.z�[0m
 �[36;1m# Reject paths containing line-disrupting control bytes (LF, CR,�[0m
 �[36;1m# other 0x01-0x1F + 0x7F) before generating the newline-delimited�[0m
 �[36;1m# *.txt manifests. NUL itself can't appear in a git pathname�[0m
 �[36;1m# (it's the on-disk tree-entry terminator), so it stays out of�[0m
 �[36;1m# the rejection class and remains the legitimate record delimiter�[0m
 �[36;1m# for `git diff -z` — `grep -z` honors that contract.�[0m
 �[36;1m#�[0m
 �[36;1m# POSIX filesystems generally allow LF/CR in filenames and git�[0m
 �[36;1m# stores them fine; the hazard is purely in our line-delimited�[0m
 �[36;1m# downstream tooling. Without this guard, `tr '\0' '\n'` on a�[0m
 �[36;1m# path like `docs/foo\nbar.md` would split it into two logical�[0m
 �[36;1m# lines — downstream consumers reading line-by-line would miss�[0m
 �[36;1m# validation coverage on the real file (or worse, act on a�[0m
 �[36;1m# synthetic path). Fail fast at this seam.�[0m
 �[36;1m#�[0m
 �[36;1m# An earlier `tr -d '\0\n\r' | grep [\x00-\x1F\x7F]` form�[0m
 �[36;1m# stripped the very bytes it was m...

GitHub Actions: AI Validation / validate: docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA

Conclusion: failure

View job details

##[group]Removing auth
 Removing SSH command configuration
 [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
 Removing HTTP extra header
 [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
 Removing includeIf entries pointing to credentials config files
 [command]/usr/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 includeif.gitdir:/github/workspace/reviewer/.git.path
 includeif.gitdir:/github/workspace/reviewer/.git/worktrees/*.path
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 /home/runner/work/_temp/git-credentials-345f5830-529d-4dd2-a15b-e8517d4fa5b7.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path /home/runner/work/_temp/git-credentials-345f5830-529d-4dd2-a15b-e8517d4fa5b7.config
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 /home/runner/work/_temp/git-credentials-345f5830-529d-4dd2-a15b-e8517d4fa5b7.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path /home/runne...

GitHub Actions: AI Validation / 0_validate.txt: docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA

Conclusion: failure

View job details

##[group]Removing auth
 Removing SSH command configuration
 [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
 Removing HTTP extra header
 [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
 [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
 Removing includeIf entries pointing to credentials config files
 [command]/usr/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 includeif.gitdir:/github/workspace/reviewer/.git.path
 includeif.gitdir:/github/workspace/reviewer/.git/worktrees/*.path
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path
 /home/runner/work/_temp/git-credentials-345f5830-529d-4dd2-a15b-e8517d4fa5b7.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git.path /home/runner/work/_temp/git-credentials-345f5830-529d-4dd2-a15b-e8517d4fa5b7.config
 [command]/usr/bin/git config --local --get-all includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path
 /home/runner/work/_temp/git-credentials-345f5830-529d-4dd2-a15b-e8517d4fa5b7.config
 [command]/usr/bin/git config --local --unset includeif.gitdir:/home/runner/work/vyos-documentation/vyos-documentation/reviewer/.git/worktrees/*.path /home/runne...

GitHub Actions: AI Validation / 1_prepare.txt: docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m# Fetch the base branch explicitly by refname to avoid ambiguity with�[0m
 �[36;1m# same-named tags (e.g., a `rolling` tag), then diff against FETCH_HEAD.�[0m
 �[36;1mgit fetch --no-tags --depth=1 origin "refs/heads/rolling"�[0m
 �[36;1mBASE="FETCH_HEAD"�[0m
 �[36;1m# --diff-filter=ACMRT excludes Deleted entries so the bundling�[0m
 �[36;1m# loop below (`git show HEAD:<path>`) doesn't try to extract�[0m
 �[36;1m# blobs for files that no longer exist in the merge ref.�[0m
 �[36;1m# Deletions still appear in diff-md.patch (full diff) but not�[0m
 �[36;1m# in changed-md.txt (which drives the bundling step).�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.md'  > changed-md.z�[0m
 �[36;1mgit diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- ':(glob)docs/**/*.rst' > changed-rst.z�[0m
 �[36;1m# Reject paths containing line-disrupting control bytes (LF, CR,�[0m
 �[36;1m# other 0x01-0x1F + 0x7F) before generating the newline-delimited�[0m
 �[36;1m# *.txt manifests. NUL itself can't appear in a git pathname�[0m
 �[36;1m# (it's the on-disk tree-entry terminator), so it stays out of�[0m
 �[36;1m# the rejection class and remains the legitimate record delimiter�[0m
 �[36;1m# for `git diff -z` — `grep -z` honors that contract.�[0m
 �[36;1m#�[0m
 �[36;1m# POSIX filesystems generally allow LF/CR in filenames and git�[0m
 �[36;1m# stores them fine; the hazard is purely in our line-delimited�[0m
 �[36;1m# downstream tooling. Without this guard, `tr '\0' '\n'` on a�[0m
 �[36;1m# path like `docs/foo\nbar.md` would split it into two logical�[0m
 �[36;1m# lines — downstream consumers reading line-by-line would miss�[0m
 �[36;1m# validation coverage on the real file (or worse, act on a�[0m
 �[36;1m# synthetic path). Fail fast at this seam.�[0m
 �[36;1m#�[0m
 �[36;1m# An earlier `tr -d '\0\n\r' | grep [\x00-\x1F\x7F]` form�[0m
 �[36;1m# stripped the very bytes it was m...
🧰 Additional context used
🔍 Remote MCP vyos.dev

Additional review context

  • T9200 is an open, normal-priority bug for VyOS 1.5.1. Its reproduction confirms that enabling no-ipv6-auto-ra leaves existing RA active; neither soft nor hard neighbor resets stop it. The task attributes termination to removing and re-adding remote-as.
  • T9200 links this documentation PR directly. A comment also notes that the behavior may warrant an upstream FRR fix, but the current task specifically proposes documenting the limitation.
  • T7531, the original feature task, is resolved. It defines the option as disabling BGP-triggered IPv6 RA, including cases such as unnumbered peers and extended-nexthop; it does not document retroactive behavior.
  • T7531 contains a successful rolling-build test after “setting and resetting BGP,” but does not identify the reset procedure, so it does not contradict T9200’s more specific finding.
  • The task search found no additional related tasks beyond T9200 and T7531.

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Clarified that no-ipv6-auto-ra affects only BGP sessions established after activation.
    • Documented that soft and hard BGP resets do not stop router advertisements on existing sessions.
    • Added instructions for stopping advertisements on active sessions by removing and re-adding remote-as or restarting the routing process.
    • Noted that restarting the routing daemon clears all BGP sessions.

Walkthrough

The BGP documentation explains when no-ipv6-auto-ra takes effect and how to stop advertisements on existing sessions.

Changes

BGP IPv6 router advertisement behavior

Layer / File(s) Summary
Document session behavior and remediation
docs/configuration/protocols/bgp.md:1510-1520
Documents that no-ipv6-auto-ra affects sessions established after activation. It also documents the required remote-as removal and re-addition, or a bgpd/frr process restart, for active sessions.

Merge Risk: ⚪ Minimal · up to 9f43d

This documentation-only change clarifies the behavior of no-ipv6-auto-ra without changing runtime behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the documentation change: existing router advertisements are not stopped by no-ipv6-auto-ra.
Description check ✅ Passed The description accurately explains the documented behavior, validation results, FRR behavior, and related task.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
✨ Simplify code
  • Create PR with simplified code

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

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ruben-herold's task in 2m 36s —— View job


Claude PR Review

  • Read CLAUDE.md and PR context
  • Read pass1-findings.json
  • Review the diff in bgp.md
  • Cross-reference with vyos-1x source (no-ipv6-auto-ra)
  • Post review summary

Issues

None from this PR. The change is a 9-line :::{note} block appended to the existing no-ipv6-auto-ra cfgcmd at docs/configuration/protocols/bgp.md:718-725.

Verification against .vyos-1x@rolling:

  • CLI path set protocols bgp parameters no-ipv6-auto-ra exists — interface-definitions/include/bgp/protocol-common-config.xml.i:1617 (<leafNode name="no-ipv6-auto-ra">, <valueless/>).
  • Config-mode logic maps this to FRR's no bgp ipv6-auto-radata/templates/frr/bgpd.frr.j2:668-670.
  • Smoketest confirms — smoketest/scripts/cli/test_protocols_bgp.py:367,423.

So the VyOS surface described (no-ipv6-auto-ra → FRR no bgp ipv6-auto-ra) is factually correct. The behavioral caveat about bgp_zebra_terminate_radv() running only on remote-as remove/re-add is an FRR-internal detail — attributed in the PR body to FRR PR #19487 and confirmed empirically by the author on 1.5.1. That is a reasonable source and I have no basis to dispute it.

Formatting checks (per AGENTS.md conventions):

  • 80-char line limit — longest added line is 73 chars.
  • MyST :::{note} block is well-formed and correctly closed.
  • Wording is consistent with the surrounding cfgcmd block.

Nits (INFO, non-blocking)

  • Line 721: active on an established session - neither a soft uses an ASCII hyphen as a sentence dash. Consider (em dash) or restructuring, e.g. session; neither a soft nor a hard …. Other places in this file mix both styles, so this is a preference.
  • "after the setting takes effect" is accurate but VyOS-idiomatic phrasing would be "after commit". Minor.

Needs Verification

None.

Pre-existing findings (not from this PR)

pass1-findings.json lists ~30 ERROR/WARNING entries against docs/configuration/protocols/bgp.md for CLI paths like protocols bgp neighbor address-family <sub> and protocols bgp address-family <sub>. These are pre-existing lines untouched by this PR and look like the reference-DB extractor not walking address-family/ipv{4,6}-unicast subtrees (rather than real doc bugs). Not this PR's responsibility — mentioning only so the pass-1 output isn't misread as a regression here.

Stats

  • Validated 1 MyST file (docs/configuration/protocols/bgp.md).
  • Skipped 0 RST files awaiting MyST migration.
  • Commands checked: 1 (set protocols bgp parameters no-ipv6-auto-ra).
  • Branch reviewed: rolling (vyos-1x: rolling).

LGTM — small, well-scoped, factually accurate behavioral caveat.
· branch T9200-bgp-no-ipv6-auto-ra-caveat

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/configuration/protocols/bgp.md`:
- Around line 723-724: Update the BGP remediation guidance to keep removing and
re-adding remote-as for the affected neighbor as the primary option. Reframe
restarting bgpd/frr as a process-wide, planned-maintenance fallback, explicitly
noting that it impacts every BGP session and should be used only after
confirming supported VyOS/FRR versions stop existing RAs.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 64fe6369-bdd7-4d04-b90c-b79a0863dde4

📥 Commits

Reviewing files that changed from the base of the PR and between 1942c7d and 35f826d.

📒 Files selected for processing (1)
  • docs/configuration/protocols/bgp.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ansible/ansible (manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: validate
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Canonical docs pages must be written as MyST Markdown (.md); edit existing pages in .md only and never use the old md- prefix for new pages.
Use {cfgcmd}, {opcmd}, and {cmdincludemd} fenced directives in MyST pages for VyOS command coverage; do not replace them with plain text or bash fences.
Use MyST ATX headings (#, ##, ###, etc.) in canonical pages; the RST heading hierarchy does not apply to .md sources.
In MyST pages, prefer single backticks for inline code; double backticks are reserved for embedded RST contexts.
Use % stop_vyoslinter and % start_vyoslinter comment markers in top-level MyST content to suppress real IPs or other allowed long-line exceptions, and keep them paired.
In MyST pages, write TODO markers as {todo} fenced directives.

Files:

  • docs/configuration/protocols/bgp.md
docs/{_include/*.txt,**/*.md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep documentation lines within 80 characters unless the content is inside a code block or fenced/preformatted block.

Files:

  • docs/configuration/protocols/bgp.md
🧠 Learnings (13)
📚 Learning: 2026-05-06T20:48:49.689Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:71-71
Timestamp: 2026-05-06T20:48:49.689Z
Learning: In vyos/vyos-documentation, the 80-character line-length rule documented under Source conventions / Formatting applies only to documentation source files located under docs/ (e.g., docs/**/*.rst and docs/**/*.md). The rule is enforced by the vyoslinter (doc-linter.py from vyos/.github) when reviewing changed files via lint-doc.yml, and only for files within docs/**. Do not suggest hard-wrapping CLAUDE.md (repo-root documentation) because GitHub renders and reflows content. For CLAUDE.md, reviews should not enforce the 80-char wrapping; apply the rule only to files matching **/docs/**/*.{rst,md}.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:48:57.970Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:91-93
Timestamp: 2026-05-06T20:48:57.970Z
Learning: The 80-character line limit applies only to documentation sources under docs/** (RST/MD). Do not enforce this limit on repo-root Markdown files like CLAUDE.md or README.md. The vyoslinter (doc-linter.py, run via lint-doc.yml from vyos/.github) lints only changed files within docs/**; root files are excluded. GitHub renders root Markdown with viewport-width reflow, so hard-wrapping these files reduces readability without tooling benefit.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:48:50.446Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:64-64
Timestamp: 2026-05-06T20:48:50.446Z
Learning: Enforce the 80-character line-length limit only for documentation source files under docs/ (docs/**/*.md and docs/**/*.rst rendered by Sphinx and linted by vyoslinter via lint-doc.yml). Do not flag line-length issues in repository-root Markdown files such as CLAUDE.md or README.md, which GitHub renders with viewport-width reflow. This applies to all files within docs/ that are part of the documentation source.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:48:54.578Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:80-84
Timestamp: 2026-05-06T20:48:54.578Z
Learning: Enforce the 80-character line-length limit only for documentation source files under docs/ (docs/**/*.rst and docs/**/*.md). Do not flag repo-root files like CLAUDE.md or README.md, since they are rendered by GitHub and not subject to this rule. The doc-linter (doc-linter.py via lint-doc.yml) only lints docs/**, so CI checks won't flag root files for line length.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:49:00.044Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:108-108
Timestamp: 2026-05-06T20:49:00.044Z
Learning: Limit the 80-character line length check and vyoslinter (doc-linter.py) enforcement to documentation source files under docs/**/*.{rst,md}. Do not apply or flag line-length issues in repo-root files like CLAUDE.md or README.md, which are rendered directly by GitHub and are not linted by lint-doc.yml. This pattern narrows checks to Sphinx source docs and prevents false positives in non-doc files.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:48:53.302Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:79-79
Timestamp: 2026-05-06T20:48:53.302Z
Learning: Limit line length to 80 characters only for documentation sources under the docs directory (docs/**/*.rst and docs/**/*.md). This is enforced by the vyoslinter doc-linter.py (from the vyos/.github repo) via lint-doc.yml on changed files under docs/**. Do not flag line-length violations in repository-root Markdown files like CLAUDE.md or README.md, as they are rendered by GitHub and reflow in the UI.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:49:10.359Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:142-142
Timestamp: 2026-05-06T20:49:10.359Z
Learning: In vyos/vyos-documentation, the 80-character line limit and vyoslinter enforcement apply only to documentation source files under docs/**/*.rst and docs/**/*.md that Sphinx renders. Repo-root files such as CLAUDE.md and README.md are outside the linter's scope (lint-doc.yml runs on docs/**) and are rendered by GitHub with automatic paragraph reflow — do not flag line-length violations in these files.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-06T20:49:15.361Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1902
File: CLAUDE.md:163-163
Timestamp: 2026-05-06T20:49:15.361Z
Learning: In vyos/vyos-documentation, enforce the 80-character line-length limit (Source conventions / Formatting) only for Sphinx documentation source files under docs/**/*.rst and docs/**/*.md. The lint-doc.yml workflow runs the doc-linter (doc-linter.py) and checks only docs/** changed files. Files in the repository root (e.g., CLAUDE.md, README.md) are rendered by GitHub and are not subject to this rule; do not flag line-length violations in those files.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-08T07:01:22.978Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1878
File: docs/troubleshooting/connectivity.rst:0-0
Timestamp: 2026-05-08T07:01:22.978Z
Learning: For the VyOS documentation (MyST-based docs), MyST directive opener lines must keep the entire directive arguments on a single line. This includes MyST fenced-directive openers like ```{opcmd} ... ``` and the RST-equivalent form .. opcmd:: ... when ported/used in MyST. Because the MyST parser does not support wrapped/continued directive arguments across multiple lines, do not raise/keep review warnings suggesting line wrapping for these directive opener lines due to line-length (even if they exceed 80 characters).

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-08T07:01:22.978Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 1878
File: docs/troubleshooting/connectivity.rst:0-0
Timestamp: 2026-05-08T07:01:22.978Z
Learning: In vyos/vyos-documentation, do not raise line-length (>80 chars) review findings for MyST directive opener lines (the directive “opener” that uses MyST directive syntax such as `{cfgcmd}` / `{opcmd}` fence/openers). CI does not enforce the 80-character limit for these specific opener lines, and existing documentation contains longer opener lines that pass lint.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-13T22:16:06.198Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 2021
File: docs/automation/terraform/terraformvyos.md:14-14
Timestamp: 2026-05-13T22:16:06.198Z
Learning: In the vyos/vyos-documentation repo, when a PR is a byte-for-byte documentation port of an existing file from the rolling branch to a release branch (e.g., circinus, sagitta), keep the port content identical to the production-tested rolling source. For these ports, do not raise new review findings for documentation issues that are already present in the rolling source (for example, markdownlint MD059 like non-descriptive link text such as `[link]`/`[install]`). Instead, defer those existing issues to a rolling-side cleanup PR (e.g., `#2024`) and then backport the cleanup via Mergify.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-05-13T22:43:41.056Z
Learnt from: andamasov
Repo: vyos/vyos-documentation PR: 2024
File: docs/automation/terraform/terraformvyos.md:0-0
Timestamp: 2026-05-13T22:43:41.056Z
Learning: In docs/**/*.md, for Markdown reference definition lines of the form `[label]: <URL>`, if the line cannot be shortened to <= 80 characters (because the URL itself is near/at the limit), suppress the vyoslinter warning by wrapping only that reference definition with a `% stop_vyoslinter` / `% start_vyoslinter` block. If the reference definition can fit within 80 characters, leave it outside any suppression block.

Applied to files:

  • docs/configuration/protocols/bgp.md
📚 Learning: 2026-06-05T19:21:44.474Z
Learnt from: LiudmylaNad
Repo: vyos/vyos-documentation PR: 2066
File: docs/configuration/protocols/traffic-engineering.md:0-0
Timestamp: 2026-06-05T19:21:44.474Z
Learning: In the vyos/vyos-documentation MyST documentation pages, when writing CLI example invocations directly under a `{cfgcmd}` directive, use `none` fenced code blocks for those examples. Do not change these example blocks to `{opcmd}` or `{cfgcmd}`—`{opcmd}` is reserved for operational-mode commands, and the surrounding `{cfgcmd}` directive already documents the target command. Plain `none` blocks for these CLI examples are intentional and correct.

Applied to files:

  • docs/configuration/protocols/bgp.md
🔍 Remote MCP Context7, vyos.dev

Relevant review context

  • Keyword search found T9200, titled exactly “docs: BGP no-ipv6-auto-ra doesn't retroactively stop active RA”. It is Open, priority Requires assessment, and targets VyOS 1.5.1. Its description reports that neither soft nor hard neighbor clears stopped active RA; removing/re-adding remote-as was identified as the effective termination path.
  • The predecessor feature task, T7531 “Add FRR no-ipv6-auto-ra option”, states that the option disables FRR’s automatic IPv6 RA behavior, used for unnumbered peers and extended-nexthop scenarios. It is Resolved.
  • Upstream FRR documentation describes bgp ipv6-auto-ra as allowing bgpd to request Zebra-sent IPv6 RAs, with the no form disabling that behavior, but does not document the retroactive behavior or reset limitation.
  • T7531 discussion records a successful test on VyOS 2025.07.01-0813-rolling where interfaces disappeared from show ipv6 nd ra-interfaces after setting and resetting BGP; this is less specific than T9200’s later 1.5.1 reproduction and should not be treated as contradicting the documented caveat without clarifying what “resetting BGP” meant.

Comment thread docs/configuration/protocols/bgp.md Outdated
ruben-herold added a commit to ruben-herold/vyos-documentation that referenced this pull request Aug 11, 2026
remote-as re-add only affects the one neighbor; a bgpd/frr restart
clears every BGP session on the router. The note listed them as
equivalent options - reframe the restart as a maintenance-window
fallback instead.

Flagged by CodeRabbit review on PR vyos#2195.
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ruben-herold's task in 1m 51s —— View job


Claude PR Review

  • Read PR context and pass1-findings.json
  • Review updated diff in bgp.md (commit 33c7f5a addressing CodeRabbit feedback)
  • Cross-reference with .vyos-1x source
  • Post review summary

Issues

None from this PR. The change is a 10-line :::{note} block appended to the existing no-ipv6-auto-ra cfgcmd at docs/configuration/protocols/bgp.md:718-727. Commit 33c7f5a addresses the CodeRabbit review comment by:

  • Keeping the neighbor-scoped remote-as remove/re-add as the primary remediation (line 722-724).
  • Reframing bgpd/frr restart as a fallback, explicitly noting it drops every BGP session on the router — not just the affected one — and to reserve it for planned maintenance (line 724-726).

This directly matches CodeRabbit's ask ("keep remote-as remove/re-add as primary; process restart as planned-maintenance fallback that impacts every BGP session").

Verification against .vyos-1x@rolling:

  • CLI path set protocols bgp parameters no-ipv6-auto-rainterface-definitions/include/bgp/protocol-common-config.xml.i:1617 (<leafNode name="no-ipv6-auto-ra">, <valueless/>) ✓
  • Smoketest coverage — smoketest/scripts/cli/test_protocols_bgp.py:367

Formatting:

  • 80-char line limit — longest added line is 73 chars. ✓
  • MyST :::{note} block is well-formed and correctly closed. ✓
  • Wording is consistent with the surrounding cfgcmd block. ✓
  • Pass 1 reports no findings on lines 718-727. ✓

Nits (INFO, non-blocking)

  • Lines 721 and 725 use ASCII hyphens as sentence dashes (session - neither, one - reserve). Em dashes () or semicolons would read cleaner, but this file already mixes both styles, so it's a preference.

Needs Verification

None. Both branches of the remediation (remote-as remove/re-add, and bgpd/frr restart) are consistent with FRR's bgp_zebra_terminate_radv() semantics as documented in FRRouting/frr#19487 and the author's empirical confirmation on VyOS 1.5.1.

Pre-existing findings (not from this PR)

pass1-findings.json lists ~30 ERROR/WARNING entries against docs/configuration/protocols/bgp.md for CLI paths like protocols bgp neighbor address-family <sub> — pre-existing lines untouched by this PR and consistent with the reference-DB extractor not walking address-family/ipv{4,6}-unicast subtrees. Not this PR's responsibility.

Stats

  • Validated 1 MyST file (docs/configuration/protocols/bgp.md).
  • Skipped 0 RST files awaiting MyST migration.
  • Commands checked: 1 (set protocols bgp parameters no-ipv6-auto-ra).
  • Branch reviewed: rolling (vyos-1x: rolling).

LGTM — the update cleanly addresses the CodeRabbit feedback and remains factually accurate.
· branch T9200-bgp-no-ipv6-auto-ra-caveat

@mergify

mergify Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

@ruben-herold this pull request has conflicts. Please resolve them before we can evaluate the pull request.

@mergify mergify Bot added the conflicts label Aug 14, 2026
no-ipv6-auto-ra only prevents future RA initiation; it doesn't
retroactively silence router advertisements already sent on an
established session. Confirmed on VyOS 1.5.1: neither a soft nor a
hard clear/reset bgp on the neighbor stopped it. Per FRR's
bgp_zebra_terminate_radv()/upstream PR FRRouting/frr#19487, that only
runs when the neighbor's remote-as is removed and re-added.
remote-as re-add only affects the one neighbor; a bgpd/frr restart
clears every BGP session on the router. The note listed them as
equivalent options - reframe the restart as a maintenance-window
fallback instead.

Flagged by CodeRabbit review on PR vyos#2195.
@ruben-herold
ruben-herold force-pushed the T9200-bgp-no-ipv6-auto-ra-caveat branch from 33c7f5a to 9f43de7 Compare August 14, 2026 15:26
@ruben-herold

Copy link
Copy Markdown
Contributor Author

Rebased onto current rolling to resolve the merge conflict (an unrelated upstream doc rewrite of this same section had landed in the meantime); content merged so both the improved base wording and the already-active-RA caveat are kept. No conflicts now.

@mergify mergify Bot added conflicts and removed conflicts labels Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant