>> 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

Reply via email to