[Cake] WireGuard Queuing, Bufferbloat, Performance, Latency, and related issues

[Toke Høiland-Jørgensen]

"Jason A. Donenfeld" <Jason at zx2c4.com> writes:

> Thanks a lot for your responses. This is steering me in the right direction (I
> hope!). Responses are inline below.
>
> Regards,
> Jason
>
> On Sat, Oct 1, 2016 at 1:51 PM, Toke Høiland-Jørgensen <toke at toke.dk> wrote:
>> Looking at your queueing structure description, I'd say the reusable FQ
>> stuff is a pretty good fit. There's a lot of conceptual overlap between
>> the mac80211 concept of having a queue per station, and yours of having
>> a queue per peer. Have a look at include/net/fq.h and
>> include/net/fq_impl.h - and see net/mac80211/tx.c for a usage example
>> (grep for 'txq'). This can be further enhanced by using CoDel as the
>> dequeue function (again, see mac80211 for an example).
>
> Thanks, I'll have a look at these. I had skimmed them earlier, and it wasn't
> immediately clear how this API worked, but I'll give it a deep read.
>
> Part of where I think we're divergent in our understanding is that in
> actuality WireGuard has two queues:

I assumed that there probably was, but was not sure where. Thanks for
clearing this up. I'll take a step back and try to describe this on the
conceptual level:

First, the reason we want better queueing is to deal with the case where
wireguard is a bottleneck. If it's not a bottleneck, no queue will
build, so there's no queueing latency to reduce. In this case the
bottleneck happens when we're waiting for the crypto step to finish,
i.e. when your queue (2) starts to fill. So we want to limit the time
packets spent in the total queueing system, i.e. from they enter through
xmit() and until they are released by send_off_bundle().

The simple way to do this is to just apply CoDel to the whole system.
I.e. timestamp packets when they appear in xmit() and apply CoDel before
sending them off in send_off_bundle(). You'd have to make sure the
timestamp is preserved through the encryption step (keep it in the cb,
for instance), but other than that it's only a couple of handfuls of LOC
you need to add to achieve this.


However, the FQ part of FQ-CoDel gives substantial benefits on top of
plain CoDel, namely fairness between (5-tuple) flows, and lower latency
to sparse flows (because they get priority). Since you have a strict
FIFO encryption step in the middle (an re-order-sensitivity after
encryption is completed), it's difficult to apply FQ across all of
wireguard. However, it can be applied to the peer queues, as I
mentioned. This would have the benefit that when a peer is backlogged
and waiting for space on the crypto queue, packets arriving to it will
get the benefit of the FQ when transmission resumes.


BTW, when reading the code (which is very readable!), I wondered about a
couple of things:

1. When the crypto queue is full, you stick all the unencrypted packets
   on the peer queues. What mechanism resumes the transmission of those
   packets, other than a new packet arriving to the same peer? And what
   happens if several peers are backlogged at the same time? Will their
   order they resume transmission depend on which peer happens to get a
   new packet once the crypto queue has space?

2. There are several places where you walk the queue with a manual
   for-loop. Is there any reason why you're not using the
   skb_queue_walk_* macros? In particular, in some places you are
   storing the pointer in the beginning of a loop in case it goes away;
   that seems to be what skb_queue_walk_safe() is meant for?



-Toke