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

[Cong Wang <xiyou.wangcong@gmail.com>]

On Mon, Jun 1, 2020 at 11:05 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Mon, Jun 01, 2020 at 09:50:23PM CEST, xiyou.wangcong@gmail.com wrote:
> >On Mon, Jun 1, 2020 at 6:40 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >> The first command just says "early drop position should be processed by
> >> block 10"
> >>
> >> The second command just adds a filter to the block 10.
> >
> >This is exactly why it looks odd to me, because it _reads_ like
> >'tc qdisc' creates the block to hold tc filters... I think tc filters should
> >create whatever placeholder for themselves.
>
> That is how it is done already today. We have the block object. The
> instances are created separatelly (clsact for example), user may specify
> the block id to identify the block. Then the user may use this block id
> to add/remove filters directly to/from the block.
>
> What you propose with "position" on the other hand look unnatural for
> me. It would slice the exising blocks in some way. Currently, the block
> has 1 clearly defined entrypoint. With "positions", all of the sudden
> there would be many of them? I can't really imagine how that is supposed
> to work :/

I imagine we could introduce multiple blocks for a qdisc.

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];
};

#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];
}


Just some scratches on my mind.

Thanks.