Skip to content

docs:T8598: Add IPv6 peer-IID secret configuration for calling-sid option - #2135

Open
ritika0313 wants to merge 2 commits into
vyos:circinusfrom
ritika0313:T8598-ipv6-peer-iid-calling-sid-secret-doc
Open

docs:T8598: Add IPv6 peer-IID secret configuration for calling-sid option#2135
ritika0313 wants to merge 2 commits into
vyos:circinusfrom
ritika0313:T8598-ipv6-peer-iid-calling-sid-secret-doc

Conversation

@ritika0313

@ritika0313 ritika0313 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Change Summary

Updated the IPv6 Peer-IID section in the PPoE, L2TP, PPTP and SSTP documentation to include the details about secret-key which is to be configured when calling-sid mode is chosen for ipv-peer-interface-id

Related Task(s)

https://vyos.dev/T8598

Related PR(s)

https://github.com/VyOS-Networks/vyos-1x/pull/2188

Backport

Checklist:

@ritika0313
ritika0313 marked this pull request as draft July 6, 2026 19:04
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: 225e7fc7-3780-4e2b-89db-3a925a0ee32e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Documentation across docs/configuration/service/pppoe-server.md, docs/configuration/vpn/l2tp.md, docs/configuration/vpn/pptp.md, and docs/configuration/vpn/sstp.md clarifies IPv6 interface identifier options, adds ipv6-peer-interface-id-secret details, and wraps reference blocks with vyoslinter directives.

Changes

PPP IPv6 Identifier Documentation

Layer / File(s) Summary
PPPoE server IPv6 identifier docs
docs/configuration/service/pppoe-server.md:9-11, 445-486, 771-774
Clarified ipv6-accept-peer-interface-id, expanded ipv6-interface-id and ipv6-peer-interface-id, and added ipv6-peer-interface-id-secret with calling-sid constraints and example.
L2TP IPv6 identifier docs
docs/configuration/vpn/l2tp.md:364-396, 634-638
Documented deterministic default behavior for ipv6-interface-id, expanded ipv6-peer-interface-id modes, added ipv6-peer-interface-id-secret, and wrapped the reference block with vyoslinter markers.
PPTP IPv6 identifier docs
docs/configuration/vpn/pptp.md:197-219, 281-312, 613-616
Reflowed RADIUS allocation and VRF guidance, clarified IPv6 identifier modes, added ipv6-peer-interface-id-secret, and wrapped dictionary links with vyoslinter markers.
SSTP IPv6 identifier docs
docs/configuration/vpn/sstp.md:14-25, 319-351, 555-556, 716-719
Rewrapped intro/certificate text, corrected IPv6 identifier documentation, added ipv6-peer-interface-id-secret, and added vyoslinter markers around reference links.

Sequence Diagram(s)

Not applicable — documentation-only changes with no executable flow.

Related issues: None specified

Related PRs: None specified

Suggested labels: documentation

Suggested reviewers: None specified

Poem

Docs now name the bits with care,
Fixed or random, peer secrets there,
calling-sid gets its keyed reply,
vyoslinter brackets pass by.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title matches the main change: adding IPv6 peer-IID secret config for calling-sid across docs.
Description check ✅ Passed Description is directly related and accurately summarizes the documentation updates.
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
✨ Simplify code
  • Create PR with simplified code

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

@mergify mergify Bot added the circinus label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ritika0313's task in 1m 59s —— View job


Claude PR Review

  • Read pass1-findings.json and diff
  • Verify vyos-1x XML/conf for ipv6-peer-interface-id-secret
  • Classify pass1 findings (confirm/reject false positives)
  • Post summary

Issues

None. The new ipv6-peer-interface-id-secret documentation accurately reflects the vyos-1x circinus source:

  • XML node exists: .vyos-1x/interface-definitions/include/accel-ppp/ppp-options-ipv6-interface-id.xml.i:48-60 defines ipv6-peer-interface-id-secret with constraint [!-~]{16,128} — matches the doc's "16 to 128 printable non-whitespace ASCII characters".
  • "Required when calling-sid" claim verified: conf_mode/service_pppoe-server.py:194-196, conf_mode/vpn_l2tp.py:86-87, conf_mode/vpn_pptp.py:84-85, conf_mode/vpn_sstp.py:99-100 all raise ConfigError('ppp-options ipv6-peer-interface-id calling-sid requires ipv6-peer-interface-id-secret') when the mode is calling-sid and the secret is absent.
  • Rendering path verified: data/templates/accel-ppp/ppp-options.j2:33-35 emits ipv6-peer-intf-id-secret= to accel-ppp only when the secret is defined.
  • Behavioral wording ("combined with the calling-station-id to derive a stable, non-predictable IID") is a fair paraphrase of the XML help text ("Secret key for generating calling-sid based IPv6 peer interface identifier (IID)").
  • The four files (pppoe-server.md, l2tp.md, pptp.md, sstp.md) are consistent with each other and all four have the corresponding conf_mode validation.

Needs Verification

None.

Pass 1 findings — dispositions

  • All eight x:x:x:x findings (pppoe-server:448/458, l2tp:362/370, pptp:277/285, sstp:316/324) — FALSE POSITIVE. The Pass 1 check only compares against <completionHelp>, but x:x:x:x is a <valueHelp> format entry in the XML and is accepted by the <constraint> regex (random|((\d+){1,4}:){3}(\d+){1,4}) (and the analogous regex for the peer variant). Documenting x:x:x:x as a value form is correct.
  • vpn ipsec interface at l2tp.md:68OUT OF SCOPE / pre-existing. This line is not part of this PR's diff (this PR modifies lines ~360-397 only). Not a blocker for merge, but worth a separate look — vpn ipsec interface is not present in the current circinus vyos-1x reference DB. Consider a follow-up PR to audit that section.

Stats

Validated 4 MyST files. Skipped 0 RST files awaiting MyST migration. Files reviewed: 4. Commands checked: 5 (ipv6-accept-peer-interface-id, ipv6-interface-id, ipv6-peer-interface-id, ipv6-peer-interface-id-secret, template rendering). Branch reviewed: circinus (vyos-1x: circinus).


Verdict: LGTM for the documented feature — the new ipv6-peer-interface-id-secret command, its constraint, and the "required with calling-sid" semantics all match the circinus vyos-1x source.
· Branch

@ritika0313

Copy link
Copy Markdown
Contributor Author

@copilot review

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ritika0313's task in 2m 3s —— View job


Claude PR Review

  • Read pass1-findings.json and diff
  • Verify vyos-1x XML/conf for ipv6-peer-interface-id-secret
  • Verify conf_mode validation for calling-sid requires secret
  • Classify pass1 findings (confirm/reject false positives)
  • Post summary

Issues

None. Re-verified after the latest sync (linter-fix commit f5aaa1e). The four MyST files still match the circinus vyos-1x source:

  • XML node exists: .vyos-1x/interface-definitions/include/accel-ppp/ppp-options-ipv6-interface-id.xml.i:48-60 defines ipv6-peer-interface-id-secret with constraint [!-~]{16,128} and constraintErrorMessage "Secret must be 16 to 128 printable non-whitespace ASCII characters" — matches the doc wording exactly.
  • "Required when calling-sid" claim verified: conf_mode/service_pppoe-server.py:196, conf_mode/vpn_l2tp.py:87, conf_mode/vpn_pptp.py:85, conf_mode/vpn_sstp.py:100 all raise ConfigError('ppp-options ipv6-peer-interface-id calling-sid requires ipv6-peer-interface-id-secret').
  • Rendering path verified: data/templates/accel-ppp/ppp-options.j2:34 emits ipv6-peer-intf-id-secret= to accel-ppp only when the secret is defined.
  • Length bounds match: iid_secret_min_len = 16, iid_secret_max_len = 128 in all four conf_mode files.
  • Migration scripts present: migration-scripts/{sstp/6-to-7, pppoe-server/12-to-13, l2tp/9-to-10, pptp/5-to-6} all reference ipv6-peer-interface-id-secret, confirming this is a first-class node in circinus.
  • The linter-fix commit (f5aaa1e) correctly wraps external reference-link definitions with % stop_vyoslinter / % start_vyoslinter — a valid MyST marker per scripts/doc-linter.py:118-119. Line-wrap changes are cosmetic and safe.

Needs Verification

None.

Pass 1 findings — dispositions

  • All eight x:x:x:x findings (pppoe-server:449/459, l2tp:362/370, pptp:279/287, sstp:317/325) — FALSE POSITIVE. The Pass 1 check only compares against <completionHelp>, but x:x:x:x is a <valueHelp> <format> entry in the XML and is accepted by the <constraint> regex (random|((\d+){1,4}:){3}(\d+){1,4}) (and analogously (random|calling-sid|ipv4-addr|((\d+){1,4}:){3}(\d+){1,4}) for the peer variant). Documenting x:x:x:x is correct.
  • vpn ipsec interface at l2tp.md:68OUT OF SCOPE / pre-existing. This line is not part of this PR's diff (this PR modifies lines ~362–398 and end-of-file linter markers only). The finding is real (vpn ipsec interface is not present in the current circinus vpn_ipsec.xml.in), but it belongs to a separate follow-up.

Stats

Validated 4 MyST files. Skipped 0 RST files awaiting MyST migration. Files reviewed: 4. Commands checked: 4 (ipv6-accept-peer-interface-id, ipv6-interface-id, ipv6-peer-interface-id, ipv6-peer-interface-id-secret) plus template rendering and linter markers. Branch reviewed: circinus (vyos-1x: circinus).


Verdict: LGTM for the documented feature. The ipv6-peer-interface-id-secret command, its 16–128 printable-ASCII constraint, and the "required with calling-sid" semantics all match the circinus vyos-1x source across all four PPP-based services (PPPoE, L2TP, PPTP, SSTP).
· Branch

@ritika0313
ritika0313 marked this pull request as ready for review July 6, 2026 20:11
@ritika0313

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@sever-sever
sever-sever requested a review from dmbaturin July 7, 2026 13:52

VyOS utilizes [accel-ppp](https://accel-ppp.org/) to provide PPPoE server functionality. It can
be used with local authentication or a connected RADIUS server.
VyOS utilizes [accel-ppp](https://accel-ppp.org/) to provide PPPoE server

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
VyOS utilizes [accel-ppp](https://accel-ppp.org/) to provide PPPoE server
VyOS utilizes [accel-ppp-ng](https://accel-ppp.org/) to provide PPPoE server


VyOS utilizes [accel-ppp](https://accel-ppp.org/) to provide SSTP server functionality. We support both
local and RADIUS authentication.
VyOS utilizes [accel-ppp](https://accel-ppp.org/) to provide SSTP server

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
VyOS utilizes [accel-ppp](https://accel-ppp.org/) to provide SSTP server
VyOS utilizes [accel-ppp-ng](https://accel-ppp.org/) to provide SSTP server

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.

2 participants