From: Toke Høiland-Jørgensen <t...@toke.dk> Date: Mon, 14 May 2018 11:08:28 +0200
> David Miller <da...@davemloft.net> writes: > >> From: Toke Høiland-Jørgensen <t...@toke.dk> >> Date: Tue, 08 May 2018 16:34:19 +0200 >> >>> +struct cake_flow { >>> + /* this stuff is all needed per-flow at dequeue time */ >>> + struct sk_buff *head; >>> + struct sk_buff *tail; >> >> Please do not invent your own SKB list handling mechanism. > > We didn't invent it, we inherited it from fq_codel. I was actually about > to fix that, but then I noticed it was still around in fq_codel, and so > let it be. I can certainly fix it anyway, but, erm, why is it acceptable > in fq_codel but not in cake? struct sk_buff_head is not that new, is it? I guess one argument has to do with the amount of memory consumed by this per-flow or per-queue information, right? Because the skb queue head has a qlen and a spinlock regardless of whether they are used or not. Furthermore, if you use the __skb_insert() et al. helpers, even though it won't use the lock it will adjust the qlen counter. And that's useless work since you have no use for the qlen value. Taken together, it seems that what you and fq_codel are doing is not such a bad idea after all. So please leave it alone. On-and-off again, I've looked into converting skbs to using list_head but it's a non-trivial set of work. All over the tree the different layers use the next/prev pointers in different ways. Some use it for a doubly linked list. Some use it for a singly linked list. Some encode state in the prev pointer. You name it, it's out there. I'll try to get back to that task because obviously it'll be useful to have code like cake and fq_codel use common helpers instead of custom stuff. Thanks.