docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA - #2195
docs: T9200: note no-ipv6-auto-ra doesn't stop already-active RA#2195ruben-herold wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
|
| 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.
Comment @coderabbitai help to get the list of available commands.
|
Claude finished @ruben-herold's task in 2m 36s —— View job Claude PR Review
IssuesNone from this PR. The change is a 9-line Verification against
So the VyOS surface described ( Formatting checks (per
Nits (INFO, non-blocking)
Needs VerificationNone. Pre-existing findings (not from this PR)
Stats
LGTM — small, well-scoped, factually accurate behavioral caveat. |
There was a problem hiding this comment.
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
📒 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.mdonly and never use the oldmd-prefix for new pages.
Use{cfgcmd},{opcmd}, and{cmdincludemd}fenced directives in MyST pages for VyOS command coverage; do not replace them with plaintextorbashfences.
Use MyST ATX headings (#,##,###, etc.) in canonical pages; the RST heading hierarchy does not apply to.mdsources.
In MyST pages, prefer single backticks for inline code; double backticks are reserved for embedded RST contexts.
Use% stop_vyoslinterand% start_vyoslintercomment 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-aswas 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-raas allowingbgpdto request Zebra-sent IPv6 RAs, with thenoform 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-rollingwhere interfaces disappeared fromshow ipv6 nd ra-interfacesafter 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.
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.
|
Claude finished @ruben-herold's task in 1m 51s —— View job Claude PR Review
IssuesNone from this PR. The change is a 10-line
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
Formatting:
Nits (INFO, non-blocking)
Needs VerificationNone. Both branches of the remediation ( Pre-existing findings (not from this PR)
Stats
LGTM — the update cleanly addresses the CodeRabbit feedback and remains factually accurate. |
|
@ruben-herold this pull request has conflicts. Please resolve them before we can evaluate the pull request. |
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.
33c7f5a to
9f43de7
Compare
|
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. |
Change Summary
no-ipv6-auto-raonly prevents future RA initiation; it doesn'tretroactively silence router advertisements already sent on an
established session. Confirmed on VyOS 1.5.1: neither a soft nor a
hard
clear/reset bgpon the neighbor stopped it. Per FRR'sbgp_zebra_terminate_radv()/upstream PRFRRouting/frr#19487,
that only runs when the neighbor's
remote-asis removed and re-added.Related Task(s)
Related PR(s)
None.
Backport
None.
Checklist: