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

Adding support for EdgeMax clients #473

Closed
wants to merge 2 commits into from
Closed

Conversation

kiratp
Copy link

@kiratp kiratp commented Apr 23, 2017

Known issues

  1. IPV6 is untested, probably broken
  2. multiple clients connecting to the server seems to break routing.

Known issues
1) IPV6 is untested, probably broken
2) multiple clients connecting to the server seems to break routing.
leftsubnet={{ edgemax_lan_subnet }}
auto=route
{% else %}
audto=add

Choose a reason for hiding this comment

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

Spelling

@jackivanov jackivanov self-requested a review April 30, 2017 08:09
config.cfg Outdated
# ALPHA feature: Uncomment below to build EdgeMaxclient configuration. Note that this will break connectivity from other clients
# We recommend that you deploy a sepearate Algo server for each Edgerouter with only one client user defined.
# Set the lan subnet to the IP CIDR notation for the lan behind the router. This MUST not interest with the vpn_network set above
edgemax_support: True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be disabled by default

Typo in client_ipsec.conf
Disabling by default
@dmwyatt
Copy link

dmwyatt commented May 12, 2017

What's the blockers on this PR?

@jackivanov
Copy link
Collaborator

I'm not a fan of such changes (not sure about the others) @dguido
If we are going to support various routers we need to make it more flexible, but not just if-else statements

@defunctio
Copy link
Contributor

I agree with @gunph1ld on this one but for different reasons. The more features we accept from third parties ultimately results in the core team needing to support them in the future. I would also still really like to see a stable release cut.

On another note, I have been working on a refactor and CLI that supports and promotes the creation of third party roles for both providers and feature support. These providers and features can just be installed /upgraded/removed from the CLI as necessary with automatic module dependency resolution and can be maintained by the third party developers. Unfortunately it is not ready for testing at this time.

@dmwyatt
Copy link

dmwyatt commented May 15, 2017

It seems like the ideal long-term solution would be a plugin-type of architecture. What would be cool and fancy is if you could call algo with a git repo argument that pulls in the plugin automatically.

On the one hand, there's security implications to making it easy to pull in third-party plugins. On the other hand, I just don't see how it's possible for core devs to support everything that would be good to support.

Short-term is the idea that algo should just NOT support anything other than what the core dev team wants to support? Maybe a compromise solution would be a section on a wiki detailing the changes needed for EdgeMax or whatever client or router?

Selfishly I'd like to see this PR merged (assuming it works)! The poor guy worked hard on it!

@defunctio
Copy link
Contributor

defunctio commented May 15, 2017

Short-term isn't that we only accept things that we want to support. It's just that I'd love to see a release version cut before introducing new features but I've been aiming for that for awhile. We really need a stable branch so that people interested in using 'bleeding-edge' features can without effecting the rest of the userbase. Ultimately I'd like to see all features refactored as independent roles that can be included optionally.

I'm just one of the developers here but these goals are based on an alpha quality private fork and at this stage it is unclear whether or not this concept will become part of Algo in the future. But the project goals are exactly that. In regards to the security implications, there will be an authorized list of 'trusted' developer teams that are only allowed be installed by default. This can be overridden by the end user after accepting a warning. These will be implemented through ansible-galaxy w/ additional metadata for Algo.

The way i see it this way we can focus on providing a core platform for the community to contribute to via role plugins that does not introduce breaking changes, maintenance and support issues into mainline Algo. I'll probably break this up into something like core, contrib and untrusted.

@ndfred
Copy link

ndfred commented Jul 8, 2017

@kiratp: you don't mention hardware offload support here though you do mention it in #307 and your change discourages HW offload because it is unreliable. You also mention that routing might radomly break as other clients connect to the VPN server. Should these issues be addressed before adding official Algo support for EdgeMax? Are there any related discussions on the community forums to explore these issues?

It is fantastic to see this working at all and I cannot wait to have it on my ERL (and you deserve full credit for getting us that far already!). I wouldn't use it until it is reasonably stable and performant though, and I would assume the same from anyone using Algo VPN.

In the meantime, having a wiki page describing the steps needed to get this working would be cool (should also be possible to have an ad-hoc script that will spit out EdgeMax configuration from reading the existing configuration files if needed, may address the concerns about waiting for a modular architecture for Algo VPN). Happy to help there if we have a good idea about wether the two issues mentioned above are addressable.

@dguido
Copy link
Member

dguido commented Jul 8, 2017

@ndfred: We're not adding official support for EdgeMax devices to Algo. We're refactoring the project into smaller, more modular parts. When that work is complete, you can probably expect to find a module for EdgeMax support. For now, you're on your own if this is the route you want to go.

@kiratp
Copy link
Author

kiratp commented Jul 8, 2017

@ndfred - UBNt thread regarding offload - https://community.ubnt.com/t5/EdgeMAX/IPSec-performance-issue/m-p/1946992#M162999

I plan to redo this PR once the refactor is complete.

@ndfred
Copy link

ndfred commented Jul 9, 2017

@dguido: how far do you think we are from a stable release that would unlock the modularity work (the project's readme doesn't share where we are in the release process)? How bif a deal is Algo modularity vs other features?

@kiratp: super thorough research again on the offload bugs on that thread, would be cool to see Ubiquiti engineers pick it up. About the routing problems you are mentioning in this PR's description, do you have an idea of wether that would be on the ERL or Algo side of things? (assuming Algo since it depends on the number of VPN clients)

Edit: the routing issues seem to come from conflicting / overlapping rules although it is unclear wether that's on the ERL or server side.

@dguido dguido mentioned this pull request Jul 16, 2017
4 tasks
@dguido dguido closed this Jul 16, 2017
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.

7 participants