WireGuard Archive on lore.kernel.org
 help / color / Atom feed
From: Kalin KOZHUHAROV <me.kalin@gmail.com>
To: Lonnie Abelbeck <lists@lonnie.abelbeck.com>
Cc: Luis Ressel <aranea@aixah.de>,
	WireGuard mailing list <wireguard@lists.zx2c4.com>
Subject: Re: RFC: wg syncpeers wg0 wireguard.conf
Date: Tue, 11 Jun 2019 23:41:38 +0200
Message-ID: <CAKXLc7ci2nr+x7Xg_s0zZ66rDJXdsEjY55YVqG_O5=vPb5GNow@mail.gmail.com> (raw)
In-Reply-To: <F0312C89-85A4-4D69-9B94-A970A4681616@lonnie.abelbeck.com>

On Tue, Jun 11, 2019 at 11:08 PM Lonnie Abelbeck
<lists@lonnie.abelbeck.com> wrote:
> > On Jun 11, 2019, at 12:28 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > One of the things that always goes wrong with "sync" algorithms in
> > software -- and the commit above at the moment is no exception -- is
> > that they're kind of racey. In order to synchronize, we have to read
> > the current state, compare it, and then set our new state. But in
> > between, the state could have changed out from underneath us. One
> > strategy for this is to just do nothing and put some notice in the man
> > page. Another strategy is to read back the result at the end, compare
> > it, and loop like this until we reach the stable state. This then
> > requires implementing some equality function.
>
> If "wg" does not offer "syncconf", users will be hacking together their own sync solution and it will no doubt be more racey than your tight code.
>
+1

> > The other thing I was wondering is: aside from performance and races
> > as described above, why not just make this the functionality of
> > `setconf`? Then there's be no need to introduce a new subcommand. In
> > otherwords, the idea would be to make `setconf` not destroy existing
> > peers if we're going to be re-adding them again.
>
> I vote to keep "setconf" as is, with the addition of the "syncconf" subcommand.
> This keeps "setconf" faster, and unchanged, typically used for initial configuration.
> Then "syncconf" would typically be used for followup live updates.
>
I guess you've seen Cisco (an other) network devices having running
and the startup config. I think this is quite similar idea here.
While I understand the need to sync, looking at the code it is more of
an `updateconf` (i.e. file -> memory) operation, while I'd expect sync
to be 2-way sync where startup/saved/disk/file/whatever config is
equal to the running/current/memory/state/whatever config by some
automagic algorithm.

Looking from a high place, a bit tired and before going to bed, these
are my thoughts:

AFAIR, the way to save config is `wg showconf wg0 >wg0.conf` (running
-> startup)...
Then why is `wg setconf` requiring a file, i.e. why not `wg setconf
wg0 <wg0.conf` ??  (to be able to use easily sudo?) If nothing
special, I'd rather see `wg setconf` read from STDIN.
We have also `wg addconf` ... hmm

I'd suggest drop addconf altogether and rename the proposed syncconf
to `updateconf` (other software uses reload, but it is not clear in
this usecase).
Do we need the reverse of `wg setconf` without the redirect like `wg
saveconf wg0 wg0.conf` ? I don't think so.
Yes, I am sure it "will break userspace", but it is better to do it
now than later.

Cheers,
Kalin.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-09 19:59 Lonnie Abelbeck
2019-06-10 12:34 ` Rene 'Renne' Bartsch, B.Sc. Informatics
2019-06-11 17:28 ` Jason A. Donenfeld
2019-06-11 21:06   ` Lonnie Abelbeck
2019-06-11 21:41     ` Kalin KOZHUHAROV [this message]
2019-06-12  0:22   ` Steven Honson
2019-06-12  0:25     ` Marc Fawzi
2019-06-14 18:01       ` Jason A. Donenfeld
2019-06-16 19:43         ` Marc Fawzi
2019-06-13 23:15   ` Lonnie Abelbeck
2019-06-14 18:09   ` Jason A. Donenfeld
2019-06-14 20:48     ` Lonnie Abelbeck
2019-06-14 21:14     ` Ivan Labáth

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKXLc7ci2nr+x7Xg_s0zZ66rDJXdsEjY55YVqG_O5=vPb5GNow@mail.gmail.com' \
    --to=me.kalin@gmail.com \
    --cc=aranea@aixah.de \
    --cc=lists@lonnie.abelbeck.com \
    --cc=wireguard@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

WireGuard Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/wireguard/0 wireguard/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 wireguard wireguard/ https://lore.kernel.org/wireguard \
		wireguard@lists.zx2c4.com zx2c4-wireguard@archiver.kernel.org
	public-inbox-index wireguard


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.zx2c4.lists.wireguard


AGPL code for this site: git clone https://public-inbox.org/ public-inbox