Volatile in Wintun

Volatile in Wintun

[Lev Stipakov]

[-- Attachment #1.1: Type: text/plain, Size: 1261 bytes --]

Hi,

I am working on Wintun support for OpenVPN..

I am a bit unsure about the purpose of volatile in Wintun.

According to https://en.cppreference.com/w/cpp/atomic/memory_order,
volatile ensures that accesses are not reordered within the thread of
execution, but order is not guaranteed to be observed by another thread.

On the other hand, Visual Studio generates memory barriers while accessing
volatile objects and targeting non-ARM architectures
https://docs.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=vs-2019#microsoft-specific.
However, it also says that "for architectures other than ARM we strongly
recommend that you specify /volatile:iso, and use explicit synchronization
primitives and compiler intrinsics when you are dealing with memory that is
shared across threads."

A few question I would like to ask:

- Is guarantee that accesses are not reordered within the thread of
execution is enough?

- Do we rely on Microsoft-specific behavior of volatile for non-ARM
architecture?

- Could reordering performed in ARM be a problem? If yes, shouldn't we
(wintun and client app) use memory fences to synchronize accesses between
threads? In openvpn3 we use C++11 so we were thinking about
std::atomic_long with acquire/release semantics.

-- 
-Lev

[-- Attachment #1.2: Type: text/html, Size: 1807 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

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

Re: Volatile in Wintun

[Jason A. Donenfeld]

Generally volatile is used to prevent the compiler from reordering
reads and writes, but we don't assume that volatile implies a memory
barrier. We could probably switch to /volatile:iso.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: Volatile in Wintun

[Lev Stipakov]

[-- Attachment #1.1: Type: text/plain, Size: 345 bytes --]

Hi Jason,

we don't assume that volatile implies a memory
> barrier. We could probably switch to /volatile:iso.
>

Does it mean that hardware reordering like writes reordered after reads
on x86/amd64 and everything else on ARM are not considered to be a problem?

Should barriers generated by WaitForSingleObject / SetEvent be enough?

-- 
-Lev

[-- Attachment #1.2: Type: text/html, Size: 731 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

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

Re: Volatile in Wintun

[Jason A. Donenfeld]

Hi Lev,

I spent a while squinting at assembly diffs and reviewing how the
linux kernel does things. Voila:
https://git.zx2c4.com/wintun/commit/?id=c072f4f3c4b540c7cfebec563b0fb5d7ba932b6f

> Does it mean that hardware reordering

I'm mostly concerned here with the compiler reordering accesses,
actually. With regards to hardware memory barriers, I haven't seen
places yet where we'd need an explicit smp_mb() or similar that's not
already provided by the various locks. But let me know if your
analysis concludes differently.

Jason
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard