>> And there’s also the problem that we might not need to drop packets as >> large as the incoming packet in order to fit the latter into the queue >> - so this corrected correction may be *negative* (the queue is longer >> than before) - but qdisc_tree_reduce_backlog() only takes an unsigned >> parameter here. > > That's a very minor detail. > > If the code does : > > reduce_backlog(unsigned quantity) > { > q->backlog -= quantity; > } > > Then the fact that @quantity is signed or unsigned is irrelevant.
Not so - unless you are very sure that q->backlog is the same size as quantity. In an increasingly 64-bit world, that is by no means assured in the future. I don’t like relying on wraparound behaviour without making that assumption explicit, which is precisely what the signed types in C are for. >> IMHO the NET_XMIT_CN semantics are broken. It might be better to drop >> support for it, since it should rarely be triggered. > > What exact part is broken ? > Semantic looks good to me. It’s broken in that a negative correction may be required in the first place. It places additional burden on every producer of the CN signal who isn’t a tail-dropper. I can only assume that the behaviour was designed with only tail-drop in mind - and as we both know, that is not the only option any more. It appears to me that there are four things than enqueue() may want to tell its caller: 1: That the packet was enqueued successfully (NET_XMIT_SUCCESS). 2: That the packet was enqueued successfully, but some other relevant packet had to be dropped due to link congestion. 3: That the packet was *not* enqueued, and this was due to link congestion. This is potentially useful for tail-dropping queues, including AQMs. 4: That the packet was *not* enqueued, for some reason other than link congestion; the “error case". Currently NET_XMIT_CN appears to cover case 3, whereas Cake normally wants cases 1 & 2 (and will only signal case 4 if a GSO aggregate couldn’t be segmented). For the time being, I have changed my development branch of Cake to always signal case 1 except for the error case. - Jonathan Morton _______________________________________________ Cake mailing list Cake@lists.bufferbloat.net https://lists.bufferbloat.net/listinfo/cake