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.
