Subject: Re: [PATCH 2/2 net-next] net: move qdisc ingress filtering code where it belongs

From: Daniel Borkmann <[email-masked]>
Date: Sun, 10 May 2015 23:44:15 +0200

On 05/10/2015 11:31 PM, Daniel Borkmann wrote:
> On 05/10/2015 09:50 PM, Pablo Neira Ayuso wrote:
> ...
>> The numbers show that the existing approach and your approach results
>> in less performance for everyone that don't need to filter from
>> ingress. We have to move ingress to where it belongs.
>
> Your cleanup in patch 1 is okay, thanks for spotting it Pablo.
>
> I agree with you on the qdisc_enqueue_root(), it's not needed, which I
> removed in my set as well. Please note that my set doesn't introduce a
> regression, it improves ingress performance however.
>
> If there's no ingress user than that code path is simply *nop*'ed out.
> If there's one ingress present on one device but not on others, it also
> doesn't make anything slower to the current state. And you can also always
> compile out CONFIG_NET_CLS_ACT (which we actually could make more fine
> grained), if you really care.

But I am still wondering, does your machine have static_key support?
If nothing is enabled, the code runs through a straight-line code path,
it's a nop that is there.

> A next possible step would be to get rid of the ingress netdev queue so
> we can also reduce memory overhead. The only thing that is needed is
> the classifier list, which is then being invoked, we all have stated
> that many times previously.
>
> My other concern is, if we export qdisc_ingress_hook function pointer,
> out of tree modules can simply do rcu_assign_pointer(qdisc_ingress_hook,
> my_own_handler) to transparently implement their own hook, hm.