On 2 March 2017 at 02:43, Mike Belopuhov <m...@belopuhov.com> wrote: > On 2 March 2017 at 01:35, David Gwynne <da...@gwynne.id.au> wrote: >> On Wed, Mar 01, 2017 at 10:03:42PM +0100, Mike Belopuhov wrote: >>> Priority queueing is the default policy in OpenBSD and it >>> distributes outgoing packets in 8 lists by priority (0-7) with >>> an aggregate queue depth set by the interface: pseudo interfaces >>> use IFQ_MAXLEN defined equal to 256, hardware device drivers >>> normally size it by their TX ring minus 1 (therefore 127, 255, >>> 511 are common values). >>> >>> Unless a producer generating packets with altered priority is >>> used (such as "set prio" pf directive, PPPoE management frames, >>> ping -T lowdelay, VLAN priority, and so on) all outgoing traffic >>> is sent with a priority of 3 hitting the same list. >>> >>> The drop policy used here is called tail drop because it drops >>> the packet that we're trying to enqueue when there's no more >>> space left on the queue. The obvious downside is that if our >>> queue is full of packets representing low priority traffic, >>> trying to enqueue a packet with a higher priority will still >>> result in a drop. In my opinion, this defeats the purpose of >>> priority queueing. >>> >>> The diff below changes the policy to a head drop from the queue >>> with the lowest priority than the packet we're trying to >>> enqueue. If there's no such queue (e.g. the default case where >>> all traffic has priority of 3) only then the packet is dropped. >>> This ensures that high priority traffic will almost always find >>> the place on the queue and low priority bulk traffic gets a >>> better chance at regulating its throughput. By performing a >>> head drop instead a tail drop we also drop the oldest packet on >>> the queue. This technique is akin to Active Queue Management >>> algorithms. >> >> i agree we should do this. >> > > good! > >> >> the current code has been very careful not to free an mbuf while >> holding the ifq mutex. i would prefer to keep it that way. >> > > i agree. > >> the least worst way to do that would be to return the mbuf to be >> dropped for ifq_enqueue to free. this is complicated because of the >> semantics that ifq_enqueue_try provides, but nothing uses that so >> we can get rid of it to support this. >> >> the diff below makes the ifq enq op return an mbuf to be freed, and >> gets rid of ifq_enqueue_try. that in turn should let you return >> this mbuf here rather than free it directly. >> > > makes sense, but could you please adjust this so that we can return > an mbuf list of rejects. i'm working on a diff that must be able to do > multiple drops at a time.
no, actually dropping one mbuf on enqueue still holds, but i'll need to drop several on dequeue. not sure yet how to properly do that outside of an ifq lock though.
