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

Enhancements to accel-ppp for a variety of things #109

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lbjw
Copy link

@lbjw lbjw commented Dec 30, 2019

  • Enhance CLI:
    (a) configurable banner and prompt
    (b) show MTU and MPPE status in 'show sessions'
    (c) a show version function

  • Fix ip pool to leave out network and broadcast number from CIDR range in pool

  • Enhance L2TP code:
    (a) Pass calling-station-id and called-station-id from L2TP if supplied
    (b) Per-client L2TP secrets
    (c) Move client check from every single UDP packet to just L2TP session creation to improve performance

  • Fix RADIUS COA removal of shaper with patch from xebd from forum.

…rom CIDR ranges in IP pool, fix for removing shaper by RADIUS COA and enhancements for L2TP sessions for per client secrets and tiny performance improvements
@lbjw lbjw requested a review from xebd January 25, 2020 02:22
p1 = strstr(ptr, ",l2tp_secret");
if (p1) {
l2tp_secret=opt->val;
*p1 = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi,

Would it be possible not to mutate the input string here? It might be a bit more difficult as the u_parse_endstr bit would need to be tweaked, but that would allow for a more robust and intuitive code : opt is clearly an input argument, and one does not expect its content to be changed by a function called parse_iprange.

Copy link
Author

Choose a reason for hiding this comment

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

I think that's good feedback. One simple way to do it would just to temporarily strdup() the contents. Would that be sufficient?

@Neustradamus
Copy link

Dear @lbjw,

Can you open a PR at "good" place:

Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants