Re: OpenBSD Wireguard implementation not copying ToS from inner to outer WG header

Hi Stuart,

I completed some testing using https://speed.cloudflare.com/

I used a long-fat wg pipe (~140ms), as long links benefit a lot from packet ordering optimisations.

The improvement is quite impressive. General application and web latency (esp SSH etc) is noticeably faster from users perspective.

Before (default 7.3 kernel) - image attached;


You can see that the latency extends from ~150ms to as much as 250ms, with the mean being weighted above 205ms.

After with wg Prio copy patch (7.3-stable) - image attached;

You can see now that the mean latency is tightly weighted around 160ms, and is not as impacted by neighbour flows.

As only prio is copied, all traffic still goes into the same single hfsc queue (as expected), but the priority packets are now correctly being picked out of that queue first, providing the latency reductions above.

It would be great to have the queue name (set during firewall ingress before the packet is encrypted), copied as well so the egressing encrypted ACK could go into the 'pri' queue, instead of the default queue, so that bandwidth is reserved. It's easy for the default queue to become filled when all encrypted traffic goes there.

Not sure how to do this. I believe the queue label is only stored in the state.

Thank you very much Stuart - big improvements :)
Regards, Andy.


> On 18 Sep 2023, at 22:59, Stuart Henderson <stu.lists@spacehopper.org> wrote:
>
> On 2023-09-17, Andrew Lemin <andrew.lemin@gmail.com> wrote:
>> I have been testing the Wireguard implementation on OpenBSD and noticed
>> that the ToS field is not being copied from the inner unencrypted header to
>> the outer Wireguard header, resulting in ALL packets going into the same PF
>> Prio / Queue.
>>
>> For example, ACKs (for Wireguard encrypted packets) end up in the first
>> queue (not the priority queue) despite PF rules;
>>
>> queue ext_iface on $extif bandwidth 1000M max 1000M
>> queue pri on $extif parent ext_iface flows 1000 bandwidth 25M min 5M
>> queue data on $extif parent ext_iface flows 1000 bandwidth 100M default
>>
>> match on $extif proto tcp set prio (3, 6) set queue (data, pri)
>>
>> All unencrypted SYNs and ACKs etc correctly go into the 'pri' queue, and
>> payload packets go into 'data' queue.
>> However for Wireguard encrypted packets, _all_ packets (including SYNs and
>> ACKs) go into the 'data' queue.
>>
>> I thought maybe you need to force the ToS/prio/queue values, so I also
>> tried sledgehammer approach;
>> match proto tcp flags A/A set tos lowdelay set prio 7 set queue pri
>> match proto tcp flags S/S set tos lowdelay set prio 7 set queue pri
>>
>> But sadly all encrypted SYNs and ACKs etc still only go into the data queue
>> no matter what.
>> This can be confirmed with wireshark that all ToS bits are lost
>>
>> This results in poor Wireguard performance on OpenBSD.
>
> Here's a naive untested diff that might at least use the prio internally
> in OpenBSD...
>
> Index: if_wg.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 if_wg.c
> --- if_wg.c 3 Aug 2023 09:49:08 -0000 1.29
> +++ if_wg.c 18 Sep 2023 12:47:02 -0000
> @@ -1525,6 +1525,8 @@ wg_encap(struct wg_softc *sc, struct mbu
> */
> mc->m_pkthdr.ph_flowid = m->m_pkthdr.ph_flowid;
>
> + mc->m_pkthdr.pf.prio = m->m_pkthdr.pf.prio;
> +
> res = noise_remote_encrypt(&peer->p_remote, &data->r_idx, &nonce,
> data->buf, plaintext_len);
> nonce = htole64(nonce); /* Wire format is little endian. */
>
>