* [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 related [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: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 related [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: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: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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.