On 04/23/15 18:13, Alexei Starovoitov wrote:
On 4/23/15 1:45 PM, Jamal Hadi Salim wrote:

then it is worse mess than I thought :(
Why call it _qdisc_ then? and have special and convoluted handling for
it in qdisc_create, qdisc_graft and other places?

Convinience for tooling and using existing abstractions is the
historical. You can attach qdiscs to netdevs.
You can then use all sorts of well understood tools.

 > Are you planning to queue things on ingress?

I thought that was the whole purpose of ingress qdisc.
why then we have dev->ingress_queue?


So you are planning to add queues? If you are that is a different
discussion (and the use case needs some clarity).

If queueing was never a goal, may be we should kill ingress qdisc
and replace it with a simple shim that only does cls/act.
The code overall will get much simpler and faster.


Attaching to the device was considered "simpler" based on the tooling
thought process. On whether we should queue on ingress - it was not
considered useful if the packets were going to the host i.e we want to
proceed to completion likely queueing to some socket layer further
upstream.
For packets being forwarded we already had egress qdiscs which had
queues so it didnt seem to make sense to enqueue on ingress for such
use cases.
There was a use case later where multiple ingress ports had to be shared
and ifb was born - which is pseudo temporary enqueueing on ingress.


Feels like falling into rabbit hole.

The fact that qdiscs dealt with these codes directly
allows for specialized handling. Moving them to a generic function
seems to defeat that purpose. So I am siding with Cong on this.

that's not what patch 1 is doing. It is still doing specialized
handling... but in light of what you said above, it looks like much
bigger cleanup is needed. We'll continue arguing when I refactor
this set and resubmit when net-next reopens.


Sorry - i was refereing to the patch where you agregated things after
the qdisc invokes a classifier.

cheers,
jamal
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to