Subject: Re: [RFC 1/3] tc: fix return values of ingress qdisc

From: Alexei Starovoitov <[email-masked]>
Date: Wed, 22 Apr 2015 15:04:03 -0700

On 4/21/15 9:59 PM, Cong Wang wrote:
> On Tue, Apr 21, 2015 at 12:27 PM, Alexei Starovoitov <[email-masked]> wrote:
>> ingress qdisc should return NET_XMIT_* values just like all other qdiscs.
>>
>
> XMIT already means egress...

may be then it should be renamed as well.
from include/linux/netdevice.h:
/* qdisc ->enqueue() return codes. */
#define NET_XMIT_SUCCESS        0x00
...

the point is that qdisc->enqeue() must return NET_XMIT_* values.
ingress qdisc is violating this and therefore should be fixed.

>> Since it's invoked via qdisc_enqueue_root() (which suppose to return
>> only NET_XMIT_* values as well), it was working by accident,
>> since TC_ACT_* values fit within NET_XMIT_MASK.
>>
>
> Why not just add a BUILD_BUG_ON() to capture this?

ingress qdisc returning TC_ACT_* values is an obvious layering
violation. I'm puzzled why it's been this way for so long.
Adding BUILD_BUG_ON is not an option.