Skip to content
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.

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

T2102: Add interface to PPPoE-server without restarting sessions #2182

Closed
wants to merge 1 commit into from

Conversation

sever-sever
Copy link
Member

@sever-sever sever-sever commented Aug 28, 2023

Change Summary

Sometimes, it is required to temporarily add new listen interfaces for the PPPoE-server without restarting current sessions/accel-ppp daemon. It is possible from op-mode:

/usr/bin/accel-cmd -p 2001 pppoe interface add "xxx"

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)

Component(s) name

pppoe-server

Proposed changes

How to test

VyOS configuration:

set service pppoe-server authentication local-users username user1 password 'user1'
set service pppoe-server authentication local-users username user2 password 'user2'
set service pppoe-server authentication mode 'local'
set service pppoe-server client-ip-pool name POOL gateway-address '100.64.0.1'
set service pppoe-server client-ip-pool name POOL subnet '100.64.0.0/24'
set service pppoe-server interface eth0

Add temporary listen interface eth1:

vyos@r1:~$ show pppoe-server interfaces 
interface:   connections:    state:
-----------------------------------
     eth0              1    active
vyos@r1:~$ 


vyos@r1:~$ force pppoe-server interface eth1
vyos@r1:~$ 
vyos@r1:~$ show pppoe-server interfaces 
interface:   connections:    state:
-----------------------------------
     eth0              1    active
     eth1              0    active
vyos@r1:~$ 

Checklist:

  • I have read the CONTRIBUTING document
  • [x ] 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

Sometimes it is required to temporary add new listen interfaces
for PPPoE-server without restarting current sessions/accel-ppp.
It is possible from op-mode:

/usr/bin/accel-cmd -p 2001 pppoe interface add "xxx"

$ force pppoe-server interface eth1
$
$ show pppoe-server interfaces
interface:   connections:    state:
-----------------------------------
     eth0              1    active
     eth1              0    active
@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team August 28, 2023 12:05
@sever-sever sever-sever added the equuleus VyOS 1.3 LTS label Aug 28, 2023
Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

There is now a configuration miss-match between running daemon and running config. Why not detect is_node_changed() on the interfaces in config mode and hot-add or hot-remove them on commit instead of restarting the daemon?

We already have several places in the code (get_interface_dict() for dhcp-options as example) that detect CLI changes and set a flag in the config dictionary indicating a daemon restart is inevitable.

@sever-sever
Copy link
Member Author

sever-sever commented Aug 28, 2023

There is now a configuration miss-match between running daemon and running config. Why not detect is_node_changed() on the interfaces in config mode and hot-add or hot-remove them on commit instead of restarting the daemon?

We already have several places in the code (get_interface_dict() for dhcp-options as example) that detect CLI changes and set a flag in the config dictionary indicating a daemon restart is inevitable.

accel-ppp doesn't implement daemon-reload properly, and not all features can be applied without restarting.
He reads the config file during daemon starts/restarts. It is impossible to add an interface without restarting the daemon.

The only case that it implemented without restarting sessions is via accel-cmd as in the example.


That is what I'm talking about

vyos@r1# run show pppoe-server interfaces 
interface:   connections:    state:
-----------------------------------
     eth0              1    active
     eth1              0    active
[edit]
vyos@r1# 
[edit]
vyos@r1# cat /run/accel-pppd/pppoe.conf | grep interface
interface=eth0
interface=eth1
# MY new interfaces here
interface=eth2
interface=eth3
interface=eth4
interface=eth5
[edit]
vyos@r1# 
[edit]
vyos@r1# 
[edit]
vyos@r1# sudo systemctl reload accel-ppp@pppoe.service
[edit]
vyos@r1# 
[edit]
vyos@r1# run show pppoe-server interfaces 
interface:   connections:    state:
-----------------------------------
     eth0              1    active
     eth1              0    active
[edit]
vyos@r1# 

@c-po
Copy link
Member

c-po commented Aug 28, 2023

That is what I'm talking about

That's what I'm talking about, too. Instead of adding an op-mode command that injects interfaces and making the running config and CLI config diverge, we should automate your commands to issue those commands automatically once an interface is added.

All code is there to detect a change on the interface node and run the accel-cmd -p 2001 pppoe interface add "xxx" afterwards and not restart/reload accel-ppp. It's a simple if/else statement in Python.

@sever-sever
Copy link
Member Author

sever-sever commented Aug 28, 2023

That is what I'm talking about

That's what I'm talking about, too. Instead of adding an op-mode command that injects interfaces and making the running config and CLI config diverge, we should automate your commands to issue those commands automatically once an interface is added.

All code is there to detect a change on the interface node and run the accel-cmd -p 2001 pppoe interface add "xxx" afterward and not restart/reload accel-ppp. It's a simple if/else statement in Python.

It is a bad idea, as it can’t be used with interface regex which configured for example user per vlan case
User per vlan must be configured only via the config file. Maybe I misunderstood something.

It is not possible to configure via accel-cmd

set service pppoe-server interface eth0 vlan-range 4-50

It is better don’t use it at all and leave it as raw accel-cmd command which was originally used to get it working.

<children>
<node name="pppoe-server">
<properties>
<help>Set PPPoE-server oprions</help>
Copy link
Member

Choose a reason for hiding this comment

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

Should be spelled: options

@c-po
Copy link
Member

c-po commented Aug 31, 2023

Closed as we will solve the use-case by extending the vyos-documentation to inform the user what he can do.

@c-po c-po closed this Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
equuleus VyOS 1.3 LTS
2 participants