WireGuard Archive on lore.kernel.org
 help / color / Atom feed
From: "Ivan Labáth" <wg@labo.rs>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	Lonnie Abelbeck <lists@lonnie.abelbeck.com>,
	Luis Ressel <aranea@aixah.de>,
	Steven Honson <steven@honson.id.au>
Cc: WireGuard mailing list <wireguard@lists.zx2c4.com>
Subject: Re: RFC: wg syncpeers wg0 wireguard.conf
Date: Fri, 14 Jun 2019 23:14:39 +0200
Message-ID: <43a193a5-bc33-3c12-37f6-d461ee5ef18a@labo.rs> (raw)
In-Reply-To: <CAHmME9qpp5pkP3RAENJfWjdaL61T7J1C5dDLCM+OM-U_DxRz_Q@mail.gmail.com>

Hi,

walk_remove_by_peer does seem inefficient. Judging by the stack, I guess
it walks the tree looking for addresses from a specific peer. That would
be roughly O(A log A) operations for A addresses, in total O(N A log A)
if removing all of them.

A simple fix would be to know what you want to remove. Then you can find
it fast in the search tree. Does showing peer configuration also scan
the entire tree? If you need to store a list of addresses associated
with a peer (seems like a good idea), memory needed should be no more
and probably less than the tree itself.

I can explain an algorithm, or tell you how to fix it, but I'm not
saying I will code it (yet).


WRT setconf/addconf/syncconf

From a user point of view, I do not see a reason to disconnect peers
which are not modified when changing confguration. It seems excessive
and not quite useful.

After reading wg man page [1], I do have questions about what these
commands do.

1) When setconf-ing an interface without optional parameters, and the
interface was configured with a non-default values, do they remain or
are they reset to defaults?
I guess either would be fine as long as it is consistent. Both interface
and peer configuration.

a) Listen port - if not specified, I would leave as is. It has already
been chosen (perhaps) randomly.

2) If a peer is connected and the configured endpoint changes, does the
endpoint change? Does the peer get disconnected?
If a peer has roamed, you edit another peer and run setconf, it doesn't
seems nice to disconnect the roaming peers. OTOH, if you want change the
address (e.g. to a another valid one), it would be nice if the peers
moved to the new addresses in a reasonable time.

3) Regarding addconf - after reading the man page, I'm not sure what the
intent is, or what does it do.
a) It says only one interface section can be in a file, is changing the
interface configuration permitted? If not, it should probably be called
addpeers.
b) Are multiple definitions of the same peer permitted? If so, what is
the result - can I split the definitions, or are the previous ones
discarded? What about ips, are they summed or overriden?

Regards,
Ivan

[1] https://git.zx2c4.com/WireGuard/about/src/tools/man/wg.8


On 14. 6. 2019 20:09, Jason A. Donenfeld wrote:
> Hey Lonnie,
> 
> If your changes are user-facing, it's probably not a good idea to
> create confusion by introducing distro-specific subcommands.
> 
> I'm leaning toward Steven's suggestion of nixing addconf and making
> setconf behave like syncconf. But two hurdles remain:
> 
> - walk_remove_by_peer is very inefficient. That *must* be to be
> improved for this to be feasible. There's some interesting algorithms
> programming in allowedips.c to be tackled for that. Maybe
> node->peer_list can be used. (CC'ing Ivan in case he wants to put his
> mind to work on that.)
> - A decision needs to be made on consistency: do we want to read back
> the end result and compare it? Or will that kind of looping logic lead
> to other types of DoS or latency spikes?
> 
> Jason
> 
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

      parent 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
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 [this message]

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=43a193a5-bc33-3c12-37f6-d461ee5ef18a@labo.rs \
    --to=wg@labo.rs \
    --cc=Jason@zx2c4.com \
    --cc=aranea@aixah.de \
    --cc=lists@lonnie.abelbeck.com \
    --cc=steven@honson.id.au \
    --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