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

wireguard-tools: Add netifd protocol helper #3514

Merged
merged 1 commit into from Nov 15, 2016
Merged

wireguard-tools: Add netifd protocol helper #3514

merged 1 commit into from Nov 15, 2016

Conversation

danrl
Copy link
Contributor

@danrl danrl commented Nov 15, 2016

Maintainer: @zorun
Run tested: x86, latest snapshot
Description: Adds netifd protocol helper for WireGuard, which is used by luci-proto-wireguard.

Signed-off-by: Dan Luedtke mail@danrl.com

Signed-off-by: Dan Luedtke <mail@danrl.com>
@p4yne
Copy link

p4yne commented Nov 15, 2016

Finally someone (@danrl + @zorun) gets wireguard support integrated into openwrt, very good work guys! Really looking forward to a lightweigth VPN alternative in openwrt.

@zorun
Copy link

zorun commented Nov 15, 2016

Looks good to me!

@stintel stintel merged commit 13f9c9d into openwrt:master Nov 15, 2016
@stintel
Copy link
Member

stintel commented Nov 15, 2016

Merged, thanks!

@danrl
Copy link
Contributor Author

danrl commented Nov 15, 2016

Thank you very much!

@danrl danrl deleted the proto-wg branch November 15, 2016 20:32
@zx2c4
Copy link
Contributor

zx2c4 commented Nov 18, 2016

Can you make proto_add_host_dependency work again? What issue were you facing with v6? It works quite well with v4.

@danrl
Copy link
Contributor Author

danrl commented Nov 18, 2016

Can you make proto_add_host_dependency work again? What issue were you facing with v6? It works quite well with v4.

I'd love to, but it doesn't work reliably. I asked at the openwrt and lede mailing lists for advice. No response so far.

@zx2c4
Copy link
Contributor

zx2c4 commented Nov 18, 2016

What goes wrong specifically?

@danrl
Copy link
Contributor Author

danrl commented Nov 18, 2016

I have a weird situation where proto_add_host_dependency() does not like IPv6 addresses. The WAN/WAN6 interface don't have global unicast addresses assigned, but my protocol script calls

proto_add_host_dependency "${config}" "${ip}"

with $ip containing a global unicast address. Interface setup fails. If I use IPv4 instead, interface setup succeeds.

What happened in my tests is, that the interface did not came up, because the proto_add_host_dependency() call failed. If you use a dual stack endpoint in an non-dual stack environment, the interface may not come up (as in: netifd says it is up).

@zx2c4
Copy link
Contributor

zx2c4 commented Nov 18, 2016

So it seems like the solution, then, is for resolveip to use getaddrinfo's AI_ADDRCONFIG flag, so that it only returns v4 IPs if there's an interface with a v4 address and v6 IPs if there's an interface with a v6 address.

@danrl
Copy link
Contributor Author

danrl commented Nov 18, 2016

Maybe better to s/if there is an interface with/if there is a default route for $ip_version/, but yes, that's the basic idea. I wish proto_add_host_dependency would just accept hostnames and do the resolving. Would remove clutter from many scripts, not only wireguard.sh

@zx2c4
Copy link
Contributor

zx2c4 commented Nov 18, 2016

Well here, instead of having to patch resolveip, there's actually a more robust way of doing this: ip route get. Call that on each IP. If there's a route, then you accept it. Since looking up hostnames is already dependent on the default route and dns servers and whatnot being accessible, probably this is okay.

@danrl
Copy link
Contributor Author

danrl commented Nov 18, 2016

I don't think it is a good idea for protocol helper scripts to implement features that are available via netifd.

I will run another test when the code hits the snapshots. Was away from my lab the whole week, so testing capabilites were limited.

@zx2c4
Copy link
Contributor

zx2c4 commented Nov 18, 2016

I don't think it is a good idea for protocol helper scripts to implement features that are available via netifd.

That's fair. I guess if you're not getting any responses on the mailing list about the issue, you could just submit a patch for netifd that fixes it in the way you see being most straightforward. At least on LKML, I've found this to be an effective strategy. Either I did it correctly, and people are happy and merge it, or it's wrong, in which case people start chiming in with a view of how they'd like to see things.

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.

None yet

5 participants