Subject: Re: [RFC PATCH net-next 0/3] TC: Introduce qevents

From: Petr Machata <[email-masked]>
Date: Wed, 03 Jun 2020 12:08:01 +0200

Cong Wang <[email-masked]> writes:

> I imagine we could introduce multiple blocks for a qdisc.

Yes, and that's what the patchset does. If you look at struct
tcf_qevent, it is just some block bookkeeping and an attribute name.

> Currently we have:
>
> struct some_qdisc_data {
>   struct tcf_block *block;
> };
>
> Maybe we can extend it to:
>
> struct some_qdisc_data {
>   struct tcf_block *blocks[3];

Yeah, except not all qdiscs will implement all qevents, so let's instead
make it a handful of fields, like in the patchset.

> };
>
> #define ENQUEUE 0
> #define DEQUEUE 1
> #define DROP 2
>
> static struct tcf_block *foo_tcf_block(struct Qdisc *sch, unsigned long cl,
>                                             struct netlink_ext_ack
> *extack, int position)
> {
>         struct some_qdisc_data *q = qdisc_priv(sch);
>
>         if (cl)
>                 return NULL;
>         return q->block[position];
> }

Interestingly, this is close to my original approach, pre-RFC. But there
needs to be this global list of all existing qevents. On its own, that's
a negative--at least it's an extra uAPI to maintain. What does it bring?

It theoretically allows one to refer to blocks symbolically, through
binding point coordinates (dev D parent P qevent Q) not by indices
(block B). But then one block could be referenced by several different
coordinates, which is confusing. That is the reason TC disallows editing
filters on shared blocks. Qevents should be the same.

What else is there?