WireGuard Archive on lore.kernel.org
 help / color / Atom feed
* Volatile in Wintun
@ 2019-08-19  7:26 Lev Stipakov
  2019-08-19 18:15 ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Lev Stipakov @ 2019-08-19  7:26 UTC (permalink / raw)
  To: wireguard

[-- 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 --]

<div dir="ltr">Hi,<div><br></div><div><div>I am working on Wintun support for OpenVPN..</div><div><br></div><div>I am a bit unsure about the purpose of volatile in Wintun.</div><div><br></div><div>According to <a href="https://en.cppreference.com/w/cpp/atomic/memory_order" target="_blank">https://en.cppreference.com/w/cpp/atomic/memory_order</a>, volatile ensures that accesses are not reordered within the thread of execution, but order is not guaranteed to be observed by another thread.</div><div><br></div><div>On the other hand, Visual Studio generates memory barriers while accessing volatile objects and targeting non-ARM architectures <a href="https://docs.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=vs-2019#microsoft-specific" target="_blank">https://docs.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=vs-2019#microsoft-specific</a>. However, it also says that &quot;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.&quot;</div><div><br></div><div>A few question I would like to ask:</div><div><br></div><div>- Is guarantee that accesses are not reordered within the thread of execution is enough?</div><div><br></div><div>- Do we rely on Microsoft-specific behavior of volatile for non-ARM architecture?</div><div><br></div><div>- Could reordering performed in ARM be a problem? If yes, shouldn&#39;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.</div></div><div><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">-Lev</div></div></div>

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

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Volatile in Wintun
  2019-08-19  7:26 Volatile in Wintun Lev Stipakov
@ 2019-08-19 18:15 ` Jason A. Donenfeld
  2019-08-21 13:14   ` Lev Stipakov
  0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-08-19 18:15 UTC (permalink / raw)
  To: Lev Stipakov; +Cc: WireGuard mailing list

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Volatile in Wintun
  2019-08-19 18:15 ` Jason A. Donenfeld
@ 2019-08-21 13:14   ` Lev Stipakov
  2019-08-24 12:41     ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Lev Stipakov @ 2019-08-21 13:14 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

[-- 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 --]

<div dir="ltr"><div dir="ltr">Hi Jason,<div><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">we don&#39;t assume that volatile implies a memory<br>
barrier. We could probably switch to /volatile:iso.<br>
</blockquote></div><div><br></div>Does it mean that hardware reordering like writes reordered after reads<div>on x86/amd64 and everything else on ARM are not considered to be a problem?</div><div><br></div><div>Should barriers generated by WaitForSingleObject / SetEvent be enough?<br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail-m_5749885894089564801gmail_signature">-Lev</div>
</div></div>

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

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Volatile in Wintun
  2019-08-21 13:14   ` Lev Stipakov
@ 2019-08-24 12:41     ` Jason A. Donenfeld
  0 siblings, 0 replies; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-08-24 12:41 UTC (permalink / raw)
  To: Lev Stipakov; +Cc: WireGuard mailing list

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19  7:26 Volatile in Wintun Lev Stipakov
2019-08-19 18:15 ` Jason A. Donenfeld
2019-08-21 13:14   ` Lev Stipakov
2019-08-24 12:41     ` Jason A. Donenfeld

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