On Fri, Sep 22, 2023 at 12:27 PM David Gwynne <da...@gwynne.id.au> wrote:
> On Mon, Sep 18, 2023 at 12:47:52PM -0000, Stuart Henderson wrote: > > On 2023-09-17, Andrew Lemin <andrew.le...@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. */ > > > > > > i think this should go in, ok by me. > > implementing txprio and rxprio might be useful too, but requires more > plumbing than i have the energy for now. > Hi David, Just to make sure I understand currently, you mean also copying the priority to/from txprio/rxprio for the VLAN/CoS priorities? Thanks. I plan to test Stuart's patch this weekend and will confirm here, but I'm confident it will work first time knowing him :)