Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] Adding support for reloading configuration via systemd
       [not found] <VI1PR02MB52169D6F055314DCD03746EDE6760@VI1PR02MB5216.eurprd02.prod.outlook.com>
@ 2020-07-23 14:10 ` Tomcsanyi, Domonkos
  2020-07-24  9:14   ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Tomcsanyi, Domonkos @ 2020-07-23 14:10 UTC (permalink / raw)
  To: wireguard

Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
---
src/systemd/wg-quick@.service | 1 +
1 file changed, 1 insertion(+)

diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
index a9cbb58..8eb040b 100644
--- a/src/systemd/wg-quick@.service
+++ b/src/systemd/wg-quick@.service
@@ -15,6 +15,7 @@ Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/bin/wg-quick up %i
ExecStop=/usr/bin/wg-quick down %i
+ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
%i)'
Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity

[Install]
--
2.17.1

Not the cleanest solution, but I think it might help a lot of people, so I'm
submitting it.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-23 14:10 ` [PATCH] Adding support for reloading configuration via systemd Tomcsanyi, Domonkos
@ 2020-07-24  9:14   ` Jason A. Donenfeld
  2020-07-24  9:25     ` Garrit Franke
  2020-07-25 12:16     ` Tore Anderson
  0 siblings, 2 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-24  9:14 UTC (permalink / raw)
  To: Tomcsanyi, Domonkos; +Cc: WireGuard mailing list

On Fri, Jul 24, 2020 at 10:30 AM Tomcsanyi, Domonkos <domi@tomcsanyi.net> wrote:
>
> Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
> ---
> src/systemd/wg-quick@.service | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
> index a9cbb58..8eb040b 100644
> --- a/src/systemd/wg-quick@.service
> +++ b/src/systemd/wg-quick@.service
> @@ -15,6 +15,7 @@ Type=oneshot
> RemainAfterExit=yes
> ExecStart=/usr/bin/wg-quick up %i
> ExecStop=/usr/bin/wg-quick down %i
> +ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
> %i)'
> Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
>
> [Install]
> --
> 2.17.1
>
> Not the cleanest solution, but I think it might help a lot of people, so I'm
> submitting it.

This actually doesn't seem too bad to me. Are there cleaner solutions
that I'm not thinking of that I should consider before applying this
patch?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:14   ` Jason A. Donenfeld
@ 2020-07-24  9:25     ` Garrit Franke
  2020-07-24  9:27       ` Garrit Franke
                         ` (2 more replies)
  2020-07-25 12:16     ` Tore Anderson
  1 sibling, 3 replies; 20+ messages in thread
From: Garrit Franke @ 2020-07-24  9:25 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

On Fri, Jul 24, 2020 at 11:14:52AM +0200, Jason A. Donenfeld wrote:
> On Fri, Jul 24, 2020 at 10:30 AM Tomcsanyi, Domonkos <domi@tomcsanyi.net> wrote:
> >
> > Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
> > ---
> > src/systemd/wg-quick@.service | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
> > index a9cbb58..8eb040b 100644
> > --- a/src/systemd/wg-quick@.service
> > +++ b/src/systemd/wg-quick@.service
> > @@ -15,6 +15,7 @@ Type=oneshot
> > RemainAfterExit=yes
> > ExecStart=/usr/bin/wg-quick up %i
> > ExecStop=/usr/bin/wg-quick down %i
> > +ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
> > %i)'
> > Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
> >
> > [Install]
> > --
> > 2.17.1
> >
> > Not the cleanest solution, but I think it might help a lot of people, so I'm
> > submitting it.
> 
> This actually doesn't seem too bad to me. Are there cleaner solutions
> that I'm not thinking of that I should consider before applying this
> patch?

I think it doesn't get cleaner than this one-liner. 
Some time back I submitted a patch that added a restart command to wg-tools. 
We settled on the conclusion that a systemd approach would be much cleaner.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:25     ` Garrit Franke
@ 2020-07-24  9:27       ` Garrit Franke
  2020-07-24  9:29       ` Jason A. Donenfeld
  2020-07-24  9:54       ` Matthias Urlichs
  2 siblings, 0 replies; 20+ messages in thread
From: Garrit Franke @ 2020-07-24  9:27 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

Am Fr., 24. Juli 2020 um 11:25 Uhr schrieb Garrit Franke
<garritfranke@gmail.com>:
>
> On Fri, Jul 24, 2020 at 11:14:52AM +0200, Jason A. Donenfeld wrote:
> > On Fri, Jul 24, 2020 at 10:30 AM Tomcsanyi, Domonkos <domi@tomcsanyi.net> wrote:
> > >
> > > Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
> > > ---
> > > src/systemd/wg-quick@.service | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
> > > index a9cbb58..8eb040b 100644
> > > --- a/src/systemd/wg-quick@.service
> > > +++ b/src/systemd/wg-quick@.service
> > > @@ -15,6 +15,7 @@ Type=oneshot
> > > RemainAfterExit=yes
> > > ExecStart=/usr/bin/wg-quick up %i
> > > ExecStop=/usr/bin/wg-quick down %i
> > > +ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
> > > %i)'
> > > Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
> > >
> > > [Install]
> > > --
> > > 2.17.1
> > >
> > > Not the cleanest solution, but I think it might help a lot of people, so I'm
> > > submitting it.
> >
> > This actually doesn't seem too bad to me. Are there cleaner solutions
> > that I'm not thinking of that I should consider before applying this
> > patch?
>
> I think it doesn't get cleaner than this one-liner.
> Some time back I submitted a patch that added a restart command to wg-tools.
> We settled on the conclusion that a systemd approach would be much cleaner.

Sorry, just a little follow up:
https://lists.zx2c4.com/pipermail/wireguard/2020-June/005549.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:25     ` Garrit Franke
  2020-07-24  9:27       ` Garrit Franke
@ 2020-07-24  9:29       ` Jason A. Donenfeld
  2020-07-24 13:09         ` Tomcsányi, Domonkos
  2020-07-24  9:54       ` Matthias Urlichs
  2 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-24  9:29 UTC (permalink / raw)
  To: Garrit Franke; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

On Fri, Jul 24, 2020 at 11:25 AM Garrit Franke <garritfranke@gmail.com> wrote:
>
> On Fri, Jul 24, 2020 at 11:14:52AM +0200, Jason A. Donenfeld wrote:
> > On Fri, Jul 24, 2020 at 10:30 AM Tomcsanyi, Domonkos <domi@tomcsanyi.net> wrote:
> > >
> > > Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
> > > ---
> > > src/systemd/wg-quick@.service | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
> > > index a9cbb58..8eb040b 100644
> > > --- a/src/systemd/wg-quick@.service
> > > +++ b/src/systemd/wg-quick@.service
> > > @@ -15,6 +15,7 @@ Type=oneshot
> > > RemainAfterExit=yes
> > > ExecStart=/usr/bin/wg-quick up %i
> > > ExecStop=/usr/bin/wg-quick down %i
> > > +ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
> > > %i)'
> > > Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
> > >
> > > [Install]
> > > --
> > > 2.17.1
> > >
> > > Not the cleanest solution, but I think it might help a lot of people, so I'm
> > > submitting it.
> >
> > This actually doesn't seem too bad to me. Are there cleaner solutions
> > that I'm not thinking of that I should consider before applying this
> > patch?
>
> I think it doesn't get cleaner than this one-liner.
> Some time back I submitted a patch that added a restart command to wg-tools.
> We settled on the conclusion that a systemd approach would be much cleaner.

Right, I recall this conversation, and this patch seems to be what we
all had in mind there. So I'm just wondering about the "not the
cleanest" part in the original patch -- if there are other systemd
tricks or something to consider.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:25     ` Garrit Franke
  2020-07-24  9:27       ` Garrit Franke
  2020-07-24  9:29       ` Jason A. Donenfeld
@ 2020-07-24  9:54       ` Matthias Urlichs
  2020-07-24 10:52         ` Stefan Tatschner
  2 siblings, 1 reply; 20+ messages in thread
From: Matthias Urlichs @ 2020-07-24  9:54 UTC (permalink / raw)
  To: wireguard

On 24.07.20 11:25, Garrit Franke wrote:
> /bin/bash -c 

Small systems may not have /bin/bash installed. Having wireguard tools
depend on bash is not a good decision from a system packaging point of view.

I recommend using a small helper script for this – one that limits
itself to POSIX shell features.

-- 
-- Matthias Urlichs


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:54       ` Matthias Urlichs
@ 2020-07-24 10:52         ` Stefan Tatschner
  2020-07-24 11:00           ` Matthias Urlichs
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Tatschner @ 2020-07-24 10:52 UTC (permalink / raw)
  To: Matthias Urlichs, wireguard

On Fri, 2020-07-24 at 11:54 +0200, Matthias Urlichs wrote:
> I recommend using a small helper script for this – one that limits
> itself to POSIX shell features.

wg-quick itself is in bash:
https://git.zx2c4.com/wireguard-tools/tree/src/wg-quick/linux.bash

So depending on bash should be ok, I guess.

S


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24 10:52         ` Stefan Tatschner
@ 2020-07-24 11:00           ` Matthias Urlichs
  0 siblings, 0 replies; 20+ messages in thread
From: Matthias Urlichs @ 2020-07-24 11:00 UTC (permalink / raw)
  To: Stefan Tatschner, wireguard

On 24.07.20 12:52, Stefan Tatschner wrote:
> wg-quick itself is in bash:

Ah. Thanks, I missed that.

However, IMHO it'd still be a good idea to use a small script -- or to
teach wg-quick how to do this directly. Using "bash -c" in systemd units
is a "you should think about this a bit harder" flag to me.

-- 
-- Matthias Urlichs



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:29       ` Jason A. Donenfeld
@ 2020-07-24 13:09         ` Tomcsányi, Domonkos
  2020-07-24 14:26           ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Tomcsányi, Domonkos @ 2020-07-24 13:09 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Garrit Franke, WireGuard mailing list

On Fri, Jul 24, 2020 at 11:29 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Fri, Jul 24, 2020 at 11:25 AM Garrit Franke <garritfranke@gmail.com> wrote:
> >
> > On Fri, Jul 24, 2020 at 11:14:52AM +0200, Jason A. Donenfeld wrote:
> > > On Fri, Jul 24, 2020 at 10:30 AM Tomcsanyi, Domonkos <domi@tomcsanyi.net> wrote:
> > > >
> > > > Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
> > > > ---
> > > > src/systemd/wg-quick@.service | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
> > > > index a9cbb58..8eb040b 100644
> > > > --- a/src/systemd/wg-quick@.service
> > > > +++ b/src/systemd/wg-quick@.service
> > > > @@ -15,6 +15,7 @@ Type=oneshot
> > > > RemainAfterExit=yes
> > > > ExecStart=/usr/bin/wg-quick up %i
> > > > ExecStop=/usr/bin/wg-quick down %i
> > > > +ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
> > > > %i)'
> > > > Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
> > > >
> > > > [Install]
> > > > --
> > > > 2.17.1
> > > >
> > > > Not the cleanest solution, but I think it might help a lot of people, so I'm
> > > > submitting it.
> > >
> > > This actually doesn't seem too bad to me. Are there cleaner solutions
> > > that I'm not thinking of that I should consider before applying this
> > > patch?
> >
> > I think it doesn't get cleaner than this one-liner.
> > Some time back I submitted a patch that added a restart command to wg-tools.
> > We settled on the conclusion that a systemd approach would be much cleaner.
>
> Right, I recall this conversation, and this patch seems to be what we
> all had in mind there. So I'm just wondering about the "not the
> cleanest" part in the original patch -- if there are other systemd
> tricks or something to consider.


Thanks for the positive feedback guys. I'm not very much experienced
with systemd and frankly this one liner was the first hit from a
simple Google search, hence my comment about it not being the
best/cleanest solution. It suited my needs and it worked, so I decided
to send it in, because the functionality seemed like something other
sysadmins would appreciate.
If you like it and there is currently no other solution suggested by
the list I'd be very happy and proud to have it merged :).

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24 13:09         ` Tomcsányi, Domonkos
@ 2020-07-24 14:26           ` Jason A. Donenfeld
  2020-07-24 14:46             ` Dominique Martinet
  0 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-24 14:26 UTC (permalink / raw)
  To: Tomcsányi, Domonkos; +Cc: Garrit Franke, WireGuard mailing list

On Fri, Jul 24, 2020 at 3:09 PM Tomcsányi, Domonkos <domi@tomcsanyi.net> wrote:
>
> On Fri, Jul 24, 2020 at 11:29 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > On Fri, Jul 24, 2020 at 11:25 AM Garrit Franke <garritfranke@gmail.com> wrote:
> > >
> > > On Fri, Jul 24, 2020 at 11:14:52AM +0200, Jason A. Donenfeld wrote:
> > > > On Fri, Jul 24, 2020 at 10:30 AM Tomcsanyi, Domonkos <domi@tomcsanyi.net> wrote:
> > > > >
> > > > > Signed-off-by: Domonkos P. Tomcsanyi <domi@tomcsanyi.net>
> > > > > ---
> > > > > src/systemd/wg-quick@.service | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
> > > > > index a9cbb58..8eb040b 100644
> > > > > --- a/src/systemd/wg-quick@.service
> > > > > +++ b/src/systemd/wg-quick@.service
> > > > > @@ -15,6 +15,7 @@ Type=oneshot
> > > > > RemainAfterExit=yes
> > > > > ExecStart=/usr/bin/wg-quick up %i
> > > > > ExecStop=/usr/bin/wg-quick down %i
> > > > > +ExecReload=/bin/bash -c '/usr/bin/wg syncconf %i <(/usr/bin/wg-quick strip
> > > > > %i)'
> > > > > Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
> > > > >
> > > > > [Install]
> > > > > --
> > > > > 2.17.1
> > > > >
> > > > > Not the cleanest solution, but I think it might help a lot of people, so I'm
> > > > > submitting it.
> > > >
> > > > This actually doesn't seem too bad to me. Are there cleaner solutions
> > > > that I'm not thinking of that I should consider before applying this
> > > > patch?
> > >
> > > I think it doesn't get cleaner than this one-liner.
> > > Some time back I submitted a patch that added a restart command to wg-tools.
> > > We settled on the conclusion that a systemd approach would be much cleaner.
> >
> > Right, I recall this conversation, and this patch seems to be what we
> > all had in mind there. So I'm just wondering about the "not the
> > cleanest" part in the original patch -- if there are other systemd
> > tricks or something to consider.
>
>
> Thanks for the positive feedback guys. I'm not very much experienced
> with systemd and frankly this one liner was the first hit from a
> simple Google search, hence my comment about it not being the
> best/cleanest solution. It suited my needs and it worked, so I decided
> to send it in, because the functionality seemed like something other
> sysadmins would appreciate.
> If you like it and there is currently no other solution suggested by
> the list I'd be very happy and proud to have it merged :).

Great, good to know. Made some small adjustments and committed this as:
https://git.zx2c4.com/wireguard-tools/commit/?id=a66219fa107e1bf0a03ebbbc405879c1f0a826c5

Thanks for the patch!

Jason

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24 14:26           ` Jason A. Donenfeld
@ 2020-07-24 14:46             ` Dominique Martinet
  2020-07-24 14:49               ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Dominique Martinet @ 2020-07-24 14:46 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Tomcsányi, Domonkos, Garrit Franke, WireGuard mailing list

Jason A. Donenfeld wrote on Fri, Jul 24, 2020:
> Great, good to know. Made some small adjustments and committed this as:
> https://git.zx2c4.com/wireguard-tools/commit/?id=a66219fa107e1bf0a03ebbbc405879c1f0a826c5

diff --git a/src/systemd/wg-quick@.service
b/src/systemd/wg-quick@.service
index a9cbb58..dbdab44 100644
--- a/src/systemd/wg-quick@.service
+++ b/src/systemd/wg-quick@.service
@@ -15,6 +15,7 @@ Type=oneshot
 RemainAfterExit=yes
 ExecStart=/usr/bin/wg-quick up %i
 ExecStop=/usr/bin/wg-quick down %i
+ExecReload=/bin/bash -c 'exec /usr/bin/wg syncconf %i <(exec /usr/bin/wg-quick strip %i)'

FWIW, bash (and zsh, ksh etc) will optimise the last command call of a
script to not fork, `bash -c 'exec foo'`  is the same as `bash -c 'foo'`

(for some reason it doesn't in the subshell though so that one makes a
difference; you can check with e.g. `strace -f -e clone bash -c ...`)


Simpler shells e.g. dash or busybox ash don't, but they don't support
the pipe substitution syntax either so I guess it doesn't matter here.

-- 
Dominique

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24 14:46             ` Dominique Martinet
@ 2020-07-24 14:49               ` Jason A. Donenfeld
  0 siblings, 0 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-24 14:49 UTC (permalink / raw)
  To: Dominique Martinet
  Cc: Tomcsányi, Domonkos, Garrit Franke, WireGuard mailing list

On Fri, Jul 24, 2020 at 4:46 PM Dominique Martinet
<asmadeus@codewreck.org> wrote:
> FWIW, bash (and zsh, ksh etc) will optimise the last command call of a
> script to not fork, `bash -c 'exec foo'`  is the same as `bash -c 'foo'`
>
> (for some reason it doesn't in the subshell though so that one makes a
> difference; you can check with e.g. `strace -f -e clone bash -c ...`)
>
>
> Simpler shells e.g. dash or busybox ash don't, but they don't support
> the pipe substitution syntax either so I guess it doesn't matter here.

Right, I observed the same thing when I was testing this, but figured
it was better to be explicit in both cases.

thinkpad ~ # strace -f -e clone /bin/bash -c '/usr/bin/wg syncconf
wgnet0 <(/usr/bin/wg-quick strip wgnet0)' 2>&1 | grep clone | wc -l
3
thinkpad ~ # strace -f -e clone /bin/bash -c 'exec /usr/bin/wg
syncconf wgnet0 <(/usr/bin/wg-quick strip wgnet0)' 2>&1 | grep clone |
wc -l
3
thinkpad ~ # strace -f -e clone /bin/bash -c 'exec /usr/bin/wg
syncconf wgnet0 <(exec /usr/bin/wg-quick strip wgnet0)' 2>&1 | grep
clone | wc -l
2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-24  9:14   ` Jason A. Donenfeld
  2020-07-24  9:25     ` Garrit Franke
@ 2020-07-25 12:16     ` Tore Anderson
  2020-07-27 15:51       ` Jason A. Donenfeld
  1 sibling, 1 reply; 20+ messages in thread
From: Tore Anderson @ 2020-07-25 12:16 UTC (permalink / raw)
  To: Jason A. Donenfeld, Tomcsanyi, Domonkos; +Cc: WireGuard mailing list

* Jason A. Donenfeld

> This actually doesn't seem too bad to me. Are there cleaner solutions
> that I'm not thinking of that I should consider before applying this
> patch?

I have no idea why my solution keeps being ignored (even after two
gentle reminders), but perhaps third time's the charm?

In any case, to quote myself from 
https://lists.zx2c4.com/pipermail/wireguard/2020-June/005585.html :

«For what it is worth, I posted a patch that does exactly this back in
March:

https://lists.zx2c4.com/pipermail/wireguard/2020-March/005222.html

Reviews or user tests would be greatly appreciated.

You can also pull from https://github.com/toreanderson/wireguard-tools
if you prefer. The commit in question is here:

https://github.com/toreanderson/wireguard-tools/commit/8305a267ec4259206c0de7f1d3f9cfb8522a3223

There is one bugfix in GitHub compared to the patch I posted to the
list in March - using $REAL_INTERFACE instead of $INTERFACE in wg-
quick/openbsd.bash. I can post the updated patch to the list as well if
you want, just let me know.»

Tore


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-25 12:16     ` Tore Anderson
@ 2020-07-27 15:51       ` Jason A. Donenfeld
  2020-07-27 20:04         ` Tore Anderson
  0 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-27 15:51 UTC (permalink / raw)
  To: Tore Anderson; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

Hi Tore,

On Sat, Jul 25, 2020 at 2:16 PM Tore Anderson <tore@fud.no> wrote:
> I have no idea why my solution keeps being ignored (even after two
> gentle reminders), but perhaps third time's the charm?
>
> In any case, to quote myself from
> https://lists.zx2c4.com/pipermail/wireguard/2020-June/005585.html :
>
> «For what it is worth, I posted a patch that does exactly this back in
> March:
>
> https://lists.zx2c4.com/pipermail/wireguard/2020-March/005222.html
>
> Reviews or user tests would be greatly appreciated.
>
> You can also pull from https://github.com/toreanderson/wireguard-tools
> if you prefer. The commit in question is here:
>
> https://github.com/toreanderson/wireguard-tools/commit/8305a267ec4259206c0de7f1d3f9cfb8522a3223

But it doesn't sync Address=, DNS=, or any routing particulars. That
seems like a problem if it's to become a bona fide "reload" subcommand
of wg-quick, since it's not doing what it should be. On the other
hand, adding it to the systemd unit seems far enough away from core
code that we can kind of say, "eh, this sort of works," which might be
good enough. If even _that_ causes problems for users too, we'd have
to talk about removing it from the systemd unit. But hopefully it
stays under the radar and people don't have overly high expectations.

Jason

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-27 15:51       ` Jason A. Donenfeld
@ 2020-07-27 20:04         ` Tore Anderson
  2020-07-28  9:03           ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Tore Anderson @ 2020-07-27 20:04 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

* Jason A. Donenfeld

> But it doesn't sync Address=, DNS=, or any routing particulars. That
> seems like a problem if it's to become a bona fide "reload" subcommand
> of wg-quick, since it's not doing what it should be. On the other
> hand, adding it to the systemd unit seems far enough away from core
> code that we can kind of say, "eh, this sort of works," which might be
> good enough. If even _that_ causes problems for users too, we'd have
> to talk about removing it from the systemd unit. But hopefully it
> stays under the radar and people don't have overly high expectations.

Absolutely, a 'wg syncconf' wrapper is unable to fully implement every
conceivable change to the wg-quick config file. That said, 99.9% of my
configuration changes are additions/removal of [Peer] sections that 'wg
syncconf' do handle perfectly. Being able to add and remove individual
VPN users without disrupting the traffic of other unrelated users is a
really big win for me. I would imagine this to ability be highly
desirable for most other VPN server operators as well – even for those
that do not use systemd.

I do use systemd, so I am personally fine with what just got merged. I
do have to wonder, though, if I committed some sort of faux pas and/or
violated some contribution guideline in posting my initial submission,
considering that it was consistently ignored for months even though it
implemented essentially the same thing as what ended up being merged
just now.

Anyway. I would, if you are interested in that, be happy update my
patch to rename the new wg-quick action «syncconf» instead of «reload»,
in order to more clearly indicate that this action will only change the
parameters that 'wg syncconf' can change.

Tore


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-27 20:04         ` Tore Anderson
@ 2020-07-28  9:03           ` Jason A. Donenfeld
  2020-07-28  9:54             ` Tore Anderson
  0 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-28  9:03 UTC (permalink / raw)
  To: Tore Anderson; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

On Mon, Jul 27, 2020 at 10:04 PM Tore Anderson <tore@fud.no> wrote:
> Absolutely, a 'wg syncconf' wrapper is unable to fully implement every
> conceivable change to the wg-quick config file. That said, 99.9% of my
> configuration changes are additions/removal of [Peer] sections that 'wg
> syncconf' do handle perfectly. Being able to add and remove individual
> VPN users without disrupting the traffic of other unrelated users is a
> really big win for me. I would imagine this to ability be highly
> desirable for most other VPN server operators as well – even for those
> that do not use systemd.

But for people shell scripting, can't they just use `wg syncconf
wgnet0 <(wg-quick strip wgnet0)`, so that it's explicit what's
happening?

> I do use systemd, so I am personally fine with what just got merged. I
> do have to wonder, though, if I committed some sort of faux pas and/or
> violated some contribution guideline in posting my initial submission,
> considering that it was consistently ignored for months even though it
> implemented essentially the same thing as what ended up being merged
> just now.

No faux pas, just a bit backlogged in reviews. Then Domonkos' patch
came through, which seemed more straightforwardly mergable.

> Anyway. I would, if you are interested in that, be happy update my
> patch to rename the new wg-quick action «syncconf» instead of «reload»,
> in order to more clearly indicate that this action will only change the
> parameters that 'wg syncconf' can change.

I'm still pretty hesitant for the reasons I outlined in the previous
email. If anything, it'd probably have to be "syncpeers", but even
then, it wouldn't update the routing information that wg-quick(8)
sometimes does. The right thing to do for a `wg-quick reload` command
would be to take into account all of the various other changes, and
mutate them the minimal distance to reflect the updated config file.
But this sounds pretty hard to do in bash. And that makes me worry
about overall mission creep in wg-quick(8). syncconf in wg(8) is
fairly simple, though still a bit verbose, but that's in C:
https://git.zx2c4.com/wireguard-tools/tree/src/setconf.c#n30 . And
there's a very clear way of doing this, whereas there are lots of
weird edge cases when handling routing.

Plus, how hard is it to add `wg syncconf wgnet0 <(wg-quick strip
wgnet0)` to scripts?

Jason

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-28  9:03           ` Jason A. Donenfeld
@ 2020-07-28  9:54             ` Tore Anderson
  2020-07-28 11:55               ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Tore Anderson @ 2020-07-28  9:54 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

* Jason A. Donenfeld

> On Mon, Jul 27, 2020 at 10:04 PM Tore Anderson <tore@fud.no> wrote:
> > Absolutely, a 'wg syncconf' wrapper is unable to fully implement every
> > conceivable change to the wg-quick config file. That said, 99.9% of my
> > configuration changes are additions/removal of [Peer] sections that 'wg
> > syncconf' do handle perfectly. Being able to add and remove individual
> > VPN users without disrupting the traffic of other unrelated users is a
> > really big win for me. I would imagine this to ability be highly
> > desirable for most other VPN server operators as well – even for those
> > that do not use systemd.
> 
> But for people shell scripting, can't they just use `wg syncconf
> wgnet0 <(wg-quick strip wgnet0)`, so that it's explicit what's
> happening?

Of course they can, just as they can opt to not use wg-quick at all.

However, it would be better, in my opinion, if every user do not have
to re-invent the wheel in order to accomplish common tasks, which (I
assume) is the reason why wg-quick – «an extremely simple script for
easily bringing up a WireGuard interface, suitable for a few common use
cases», to quote its manual page – exists in the first place.

> I'm still pretty hesitant for the reasons I outlined in the previous
> email. If anything, it'd probably have to be "syncpeers", but even
> then, it wouldn't update the routing information that wg-quick(8)
> sometimes does.

Fair enough. If you do not want it in wg-quick, I won't insist.

> The right thing to do for a `wg-quick reload` command
> would be to take into account all of the various other changes, and
> mutate them the minimal distance to reflect the updated config file.
> But this sounds pretty hard to do in bash. And that makes me worry
> about overall mission creep in wg-quick(8). syncconf in wg(8) is
> fairly simple, though still a bit verbose, but that's in C:
> https://git.zx2c4.com/wireguard-tools/tree/src/setconf.c#n30 . And
> there's a very clear way of doing this, whereas there are lots of
> weird edge cases when handling routing.

Agreed, this sounds very complex and not worth the trouble.

That said, I do believe that most admins would not be bothered by the
fact that some changes would require a restart (i.e., an wg-quick
up/down cycle). This limitation is common in other pieces of software
too, e.g., one can normally not use 'apachectl graceful' to make Apache
listen on a new port below 1024. However, in spite of it not being
perfect, 'apachectl graceful' remains extremely useful.

> Plus, how hard is it to add `wg syncconf wgnet0 <(wg-quick strip
> wgnet0)` to scripts?

Not hard - it is precisely what my patch did, after all.

Tore


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-28  9:54             ` Tore Anderson
@ 2020-07-28 11:55               ` Jason A. Donenfeld
  2020-07-28 12:17                 ` Tore Anderson
  0 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-28 11:55 UTC (permalink / raw)
  To: Tore Anderson; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

On Tue, Jul 28, 2020 at 11:54 AM Tore Anderson <tore@fud.no> wrote:
>
> * Jason A. Donenfeld
>
> > On Mon, Jul 27, 2020 at 10:04 PM Tore Anderson <tore@fud.no> wrote:
> > > Absolutely, a 'wg syncconf' wrapper is unable to fully implement every
> > > conceivable change to the wg-quick config file. That said, 99.9% of my
> > > configuration changes are additions/removal of [Peer] sections that 'wg
> > > syncconf' do handle perfectly. Being able to add and remove individual
> > > VPN users without disrupting the traffic of other unrelated users is a
> > > really big win for me. I would imagine this to ability be highly
> > > desirable for most other VPN server operators as well – even for those
> > > that do not use systemd.
> >
> > But for people shell scripting, can't they just use `wg syncconf
> > wgnet0 <(wg-quick strip wgnet0)`, so that it's explicit what's
> > happening?
>
> Of course they can, just as they can opt to not use wg-quick at all.
>
> However, it would be better, in my opinion, if every user do not have
> to re-invent the wheel in order to accomplish common tasks, which (I
> assume) is the reason why wg-quick – «an extremely simple script for
> easily bringing up a WireGuard interface, suitable for a few common use
> cases», to quote its manual page – exists in the first place.
>
> > I'm still pretty hesitant for the reasons I outlined in the previous
> > email. If anything, it'd probably have to be "syncpeers", but even
> > then, it wouldn't update the routing information that wg-quick(8)
> > sometimes does.
>
> Fair enough. If you do not want it in wg-quick, I won't insist.
>
> > The right thing to do for a `wg-quick reload` command
> > would be to take into account all of the various other changes, and
> > mutate them the minimal distance to reflect the updated config file.
> > But this sounds pretty hard to do in bash. And that makes me worry
> > about overall mission creep in wg-quick(8). syncconf in wg(8) is
> > fairly simple, though still a bit verbose, but that's in C:
> > https://git.zx2c4.com/wireguard-tools/tree/src/setconf.c#n30 . And
> > there's a very clear way of doing this, whereas there are lots of
> > weird edge cases when handling routing.
>
> Agreed, this sounds very complex and not worth the trouble.
>
> That said, I do believe that most admins would not be bothered by the
> fact that some changes would require a restart (i.e., an wg-quick
> up/down cycle). This limitation is common in other pieces of software
> too, e.g., one can normally not use 'apachectl graceful' to make Apache
> listen on a new port below 1024. However, in spite of it not being
> perfect, 'apachectl graceful' remains extremely useful.
>
> > Plus, how hard is it to add `wg syncconf wgnet0 <(wg-quick strip
> > wgnet0)` to scripts?
>
> Not hard - it is precisely what my patch did, after all.

By the way, I just made this change:
https://git.zx2c4.com/wireguard-tools/commit/?id=37ed947239f4b3ef161c85428d9267eb23a69d69
I had assumed it was syncconf all along, until I checked. That makes
sense; Luis added `strip` before I added `syncconf`.

Jason

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-28 11:55               ` Jason A. Donenfeld
@ 2020-07-28 12:17                 ` Tore Anderson
  2020-07-28 12:17                   ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Tore Anderson @ 2020-07-28 12:17 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

* Jason A. Donenfeld

> By the way, I just made this change:
> https://git.zx2c4.com/wireguard-tools/commit/?id=37ed947239f4b3ef161c85428d9267eb23a69d69
> I had assumed it was syncconf all along, until I checked. That makes
> sense; Luis added `strip` before I added `syncconf`.

That is useful, thank you.

However, I believe this change invalidates the «note that the above
command will add and update peers but will not remove peers» caveat.

Tore


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Adding support for reloading configuration via systemd
  2020-07-28 12:17                 ` Tore Anderson
@ 2020-07-28 12:17                   ` Jason A. Donenfeld
  0 siblings, 0 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2020-07-28 12:17 UTC (permalink / raw)
  To: Tore Anderson; +Cc: Tomcsanyi, Domonkos, WireGuard mailing list

On Tue, Jul 28, 2020 at 2:17 PM Tore Anderson <tore@fud.no> wrote:
>
> * Jason A. Donenfeld
>
> > By the way, I just made this change:
> > https://git.zx2c4.com/wireguard-tools/commit/?id=37ed947239f4b3ef161c85428d9267eb23a69d69
> > I had assumed it was syncconf all along, until I checked. That makes
> > sense; Luis added `strip` before I added `syncconf`.
>
> That is useful, thank you.
>
> However, I believe this change invalidates the «note that the above
> command will add and update peers but will not remove peers» caveat.

Thanks for pointing that out. Fixing.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-07-28 12:18 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <VI1PR02MB52169D6F055314DCD03746EDE6760@VI1PR02MB5216.eurprd02.prod.outlook.com>
2020-07-23 14:10 ` [PATCH] Adding support for reloading configuration via systemd Tomcsanyi, Domonkos
2020-07-24  9:14   ` Jason A. Donenfeld
2020-07-24  9:25     ` Garrit Franke
2020-07-24  9:27       ` Garrit Franke
2020-07-24  9:29       ` Jason A. Donenfeld
2020-07-24 13:09         ` Tomcsányi, Domonkos
2020-07-24 14:26           ` Jason A. Donenfeld
2020-07-24 14:46             ` Dominique Martinet
2020-07-24 14:49               ` Jason A. Donenfeld
2020-07-24  9:54       ` Matthias Urlichs
2020-07-24 10:52         ` Stefan Tatschner
2020-07-24 11:00           ` Matthias Urlichs
2020-07-25 12:16     ` Tore Anderson
2020-07-27 15:51       ` Jason A. Donenfeld
2020-07-27 20:04         ` Tore Anderson
2020-07-28  9:03           ` Jason A. Donenfeld
2020-07-28  9:54             ` Tore Anderson
2020-07-28 11:55               ` Jason A. Donenfeld
2020-07-28 12:17                 ` Tore Anderson
2020-07-28 12:17                   ` Jason A. Donenfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).