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: new upstream version #3816

Merged
merged 1 commit into from Jan 11, 2017

Conversation

Projects
None yet
6 participants
@danrl
Copy link
Contributor

commented Jan 11, 2017

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

Maintainer: @zorun & me
Compile tested: x86_64 latest source from yesterday evening

Description: Version bump. Also introduced new variable PKG_HASH for compatibility with anticipated build system changes.

@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2017

I like to get comments on:

  • using a shorter hash (copied from tagged upstream commit) Can't see the benefit right now. Actually, I wondered why it even build without complaining. I expect this to change before the merge but would like to discuss it.
  • Using PKG_HASH. I learned on the mailing list about it's future use and how many packages are already having this variable. I think it is good practice to use it. Comments?

/cc @zx2c4

@hnyman

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

PKG_HASH only works with LEDE, as it has not yet been backported to Openwrt (there is a PR open for that).

So, we need to still use PKG_MD5SUM in this packages feed repo.

PKG_MD5SUM accepts both MD5 and SHA256 hash and uses the hash length to recognize the hash type..
PKG_HASH should only have SHA256 hash, as the goal is to deprecate MD5 usage.

Three working alternatives:

  • PKG_MD5SUM with MD5 hash
  • PKG_MD5SUM with SHA256 hash
  • both PKG_MD5SUM (with MD5 hash) and PKG_HASH (with SHA256 hash)

Please fix your Makefile with one of them.

@lucize

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

please also change the make file to use AUTOLOAD:=$(call AutoProbe,wireguard) (issue #3790)

@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2017

@lucize done. Interesting issue BTW.
@hnyman switching back to PKG_MD5SUM until the openwrt/openwrt#324 is merged.

@hnyman

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

Could you squash the three commits into one, please.

@zorun

This comment has been minimized.

Copy link

commented Jan 11, 2017

@danrl thanks for the update

Don't you need to disable all the new stuff, as mentioned here? https://lists.zx2c4.com/pipermail/wireguard/2017-January/000887.html

@build000

This comment has been minimized.

Copy link

commented Jan 11, 2017

Please add more essential and practical features to this program - most modern routers and practical uses multissid and two versions of the radio (2.4 GHz and 5 GHz - not everyone expects that both radios must be simultaneously turned on or off) - The application is simply too poor in functionality.
More information in my post: #3790 (comment)

@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2017

@build000 I wonder how you are using WireGuard. I can't see how Multi-SSID and WireGuard are conflicting. I use both in my router and it works fine.

The application is simply too poor in functionality.
Elaborate... Code and patches welcome.

If you are using LuCi, this is how you configure wg interfaces: https://www.danrl.com/2016/11/16/openwrt-luci-proto-wireguard.html

@build000

This comment has been minimized.

Copy link

commented Jan 11, 2017

@danrl @zorun and others - very...very sorry.
Something got confused in a beret and with the force of the waterfall itself persuaded that it wireguard luci-app-wifishedule - the same sorry for both of my posts because they relate to entirely different program/plugin in LuCI- again sorry (force self-suggestion).
🥇 ...hehe

@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2017

No problem, we all get confused one time or the other. It's a huge code base :)

@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2017

@zorun: My first thought was: I don't think it hurts "building" the additional files since they do not end up in the package. Also, as I understand, they are not really build but rather come with the repository as it is cloned. Since we do not run make install (if we do, I am not aware of it), we never hit the path in the makefile where these files would be installed, do we?

Here is upstream's Makefile for reference: https://git.zx2c4.com/WireGuard/tree/src/tools/Makefile?h=0.0.20170105
Only the install target makes use of the variables, right?

We could add

MAKE_VARS += WITH_BASHCOMPLETION=no WITH_SYSTEMDUNITS=no WITH_WGQUICK=no

to our Makefile. But it should not have any effect. Correct me if I am wrong.
I also played with this:

	$(MAKE) M="$(PKG_BUILD_DIR)/src/tools" \
		WITH_BASHCOMPLETION=no \
		WITH_SYSTEMDUNITS=no \
		WITH_WGQUICK=no \
		tools

Again, can't see the effect.

However, I came across another thing:

    $(MAKE) $(KERNEL_MAKEOPTS) M="$(PKG_BUILD_DIR)/src" modules

There is no target modules but there is one named module in upstream's Makefile. Shouldn't we use this line instead?

    $(MAKE) $(KERNEL_MAKEOPTS) M="$(PKG_BUILD_DIR)/src" module <-- Note the missing "s"!

See this branch for all the other weird tests I ran, just to see if we hit the path with one of them: https://github.com/danrl/lede-packages/commits/wireguard-test (Caution, ugly :) )

@hnyman I haven't squashed yet to avoid merging to early. I will squash once I am fully satisfied with all the changes. Sorry for not communicating that more clear. I'll ping you once squashed, ok?

@zx2c4

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

Hey,

Y'all aren't using the make install target, so those install flags don't really matter. You don't need to change anything. Just keep copying wg to the right location, like you already do.

With regards to your question about the modules target. Keep it how it is. That's a target in the kernel's makefile, not in the wireguard one. @zorun chose to just call the kernel's makefile directly, instead of going through my makefile's simple shim to do the same. That seems fine to me.

In short, things are fine as is.

Jason

wireguard: version bump
Signed-off-by: Dan Luedtke <mail@danrl.com>
@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2017

@hnyman squashed and ready to merge

Also fixes #3790 to some extend.

@hnyman hnyman merged commit 3b2e6ca into openwrt:master Jan 11, 2017

@danrl danrl deleted the danrl:wireguard branch Jan 11, 2017

@zorun

This comment has been minimized.

Copy link

commented Jan 12, 2017

@danrl Oops, this was just a suggestion that there might be some changes, not that there necessarily were. Sorry to have misled you into trying all that stuff!

For the module/modules target, Jason is right, we are calling the kernel's Makefile directly to build the module.

@hnyman

This comment has been minimized.

Copy link
Contributor

commented Feb 4, 2017

@danrl @zx2c4

Could a mirror for the source download be added to the Makefile?

Apparently the Openwrt buildbot's buildslaves can not reach the download site, as the buildslaves do not fulfill the SSL/TLS requirements of git.zx2c4.com...

"https://git.zx2c4.com/WireGuard/snapshot/"'
--2017-02-04 02:14:24--  https://git.zx2c4.com/WireGuard/snapshot/WireGuard-0.0.20170115.tar.xz
Resolving git.zx2c4.com (git.zx2c4.com)... 2607:5300:61:14f::c05f:545, 192.95.5.69
Connecting to git.zx2c4.com (git.zx2c4.com)|2607:5300:61:14f::c05f:545|:443... connected.
GnuTLS: A TLS fatal alert has been received.
Unable to establish SSL connection.
@danrl

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2017

Oh. Let me check that with upstream. Is buildbot not using the latest GnuTLS? What exactly is causing the problem?

@zx2c4 FYI

@hnyman

This comment has been minimized.

Copy link
Contributor

commented Feb 4, 2017

Is buildbot not using the latest GnuTLS? What exactly is causing the problem?

No idea. I have just noticed that LEDE buildbot builds wireguard ok, but Openwrt buildbot fails constantly. It is older and has older buildslaves probably with older software. I have nothing to do with that buildbot's maintenance. @wigyori might know more. But in general the Openwrt buildbot has been able to load other source packages, so likely git.zx2c4.com uses more tight SSL/TLS restrictions than the usual source download sites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.