On 05/08/2018 09:29 AM, Jamal Hadi Salim wrote:
On 08/05/18 08:59 AM, Michel Machado wrote:
Overall it looks good to me, just one thing below:
+struct Qdisc_ops gkprio_qdisc_ops __read_mostly = {
+ .id = "gkprio",
+ .priv_size = sizeof(struct gkprio_sched_data),
+ .enqueue = gkprio_enqueue,
+ .dequeue = gkprio_dequeue,
+ .peek = qdisc_peek_dequeued,
+ .init = gkprio_init,
+ .reset = gkprio_reset,
+ .change = gkprio_change,
+ .dump = gkprio_dump,
+ .destroy = gkprio_destroy,
+ .owner = THIS_MODULE,
+};
You probably want to add Qdisc_class_ops here so that you can
dump the stats of each internal queue.
Hi Cong,
In the production scenario we are targeting, this priority queue
must be classless; being classful would only bloat the code for us. I
don't see making this queue classful as a problem per se, but I
suggest leaving it as a future improvement for when someone can come
up with a useful scenario for it.
I am actually struggling with this whole thing.
Have you considered using skb->prio instead of peeking into the packet
header.
Also have you looked at the dsmark qdisc?
As far as I know, skb->priority (skb->prio has been renamed) is unsigned
for packets that come from the network. DSprio, adopting Cong's name
suggestion, is most useful "merging" packets that come from different
network interfaces.
Had we relied on DSmark to mark skb->tc_index with the DS field, we
would have forced anyone using DSprio to use DSmark. This may sound as a
good idea, but DSmark always requires writable socket buffers while
setting skb->tc_index with the DS field of the packet (see
dsmark_enqueue()), what means that the kernel may drop high priority
packets instead of low priority packets due to memory pressure.
[ ]'s
Michel Machado
