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 :)

Reply via email to