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.
