Skip to content

T8010: Added the vyos_vpp plugin to the accel-pppd's template #4910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dmbaturin merged 1 commit into from
Dec 23, 2025
Merged

T8010: Added the vyos_vpp plugin to the accel-pppd's template #4910

dmbaturin merged 1 commit into from
Dec 23, 2025
+1 −0

Conversation

@AndriiFullroot
Copy link
Contributor

@AndriiFullroot AndriiFullroot commented Dec 19, 2025 •
edited
Loading

Change summary

Added the vyos_vpp plugin for accel-ppp-ng.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T8010

Related PR(s)

How to test / Smoketest result

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@github-actions
Copy link

github-actions bot commented Dec 19, 2025 •
edited
Loading

👍
No issues in PR Title / Commit Title

@github-actions
Copy link

github-actions bot commented Dec 19, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@AndriiFullroot
Copy link
Contributor Author

AndriiFullroot commented Dec 19, 2025

I have read the CLA Document and I hereby sign the CLA

vyosbot added a commit to vyos/vyos-cla-signatures that referenced this pull request Dec 19, 2025
@vyosbot
@AndriiFullroot has signed the CLA in vyos/vyos-1x#4910
6f19428
@AndriiFullroot AndriiFullroot marked this pull request as ready for review December 19, 2025 13:39
sever-sever
sever-sever approved these changes Dec 19, 2025
View reviewed changes
Copy link
Member

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

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

Add vyos_vpp plugin for the PPPoE server
It depends on accel-ppp/accel-ppp-ng#19

@github-actions
Copy link

github-actions bot commented Dec 19, 2025

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests VPP 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • Config tests VPP 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

c-po
c-po reviewed Dec 20, 2025
View reviewed changes
data/templates/accel-ppp/pppoe.config.j2
sigchld
pppd_compat
{% endif %}
vyos_vpp
Copy link
Member

@c-po c-po Dec 20, 2025

Choose a reason for hiding this comment

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

I do wonder if it is a good idea to add this plugin by default. I would make this dynamic based on the fact if VPP service is enabled, or not on the CLI.

Copy link
Contributor Author

@AndriiFullroot AndriiFullroot Dec 22, 2025

Choose a reason for hiding this comment

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

It was my initial idea, the issue is that the option "vpp-cp" is "per-interface" and there is no option to check if vpp-cp is set without iterating through.
Vyacheslav and I discussed that, and we agreed to enable the plugin by default for now.

Copy link
Member

@dmbaturin dmbaturin Dec 23, 2025

Choose a reason for hiding this comment

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

Loading the plugin has no implications or negative effects, so I'd go with this now.

@dmbaturin dmbaturin added the bp/circinus Create automatic backport for circinus label Dec 23, 2025
dmbaturin
dmbaturin approved these changes Dec 23, 2025
View reviewed changes
Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

Loading the plugin has no implications or negative effects, so I'd go with this now.

@dmbaturin dmbaturin merged commit 51036f9 into vyos:current Dec 23, 2025
21 of 22 checks passed
@vyosbot vyosbot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@c-po c-po c-po left review comments

@dmbaturin dmbaturin dmbaturin approved these changes

@sever-sever sever-sever sever-sever approved these changes

@zdc zdc Awaiting requested review from zdc

Assignees

@AndriiFullroot AndriiFullroot

Labels

bp/circinus Create automatic backport for circinus current mirror-completed

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AndriiFullroot @dmbaturin @sever-sever @c-po @vyosbot