Thanks for attention at this. But now I don't like this diff. It silently drops stuck packets, so `ph_cookie' linked data could leak. Obviously wg_peer_destroy() and wg_qstart() should be fixed in different way.
On Thu, Oct 05, 2023 at 10:42:25PM +1000, David Gwynne wrote: > > > > On 5 Oct 2023, at 21:50, Vitaliy Makkoveev <m...@openbsd.org> wrote: > > > > Otherwise `if_snd' could still contain packets. If any of them has > > associated data with bumped reference counter, the corresponding > > refcnt_finalize() will sleep forever. I don't think we need to > > transmit these packets when interface became running again. > > there were a bunch of paths where the stack would implicitly take an > interface down and up again around the actual change it was trying to make. > from memory these kinds of things are disruptive anyway, so losing a few more > packets might not be a problem. we do ride this out at the moment though. This is configuration path. In normal condition it followed once or twice. > > > > > I don't like this unlocked if_flags check we have in ifq_start_task(). > > Guess READ_ONCE() is much better to load value. > > there are no inconsistent intermediate values you can read from if_flags. are > you worried the compiler will load the value again and maybe we'll get a > different state the second time? To enforce loading it from memory instead of cache. But may be I'm wrong.
