On Tue, Jun 11, 2019 at 11:08 PM Lonnie Abelbeck <li...@lonnie.abelbeck.com> wrote: > > On Jun 11, 2019, at 12:28 PM, Jason A. Donenfeld <ja...@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