[Cake] Possible BUG - parent backlog incorrectly updated in case of NET_XMIT_CN
chromatix99 at gmail.com
Sun Jun 12 13:59:48 EDT 2016
>> 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
More information about the Cake