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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87r1uw31nt.fsf@mellanox.com>
Date:   Wed, 03 Jun 2020 12:08:01 +0200
From:   Petr Machata <petrm@...lanox.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Jiri Pirko <jiri@...nulli.us>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [RFC PATCH net-next 0/3] TC: Introduce qevents

Cong Wang <xiyou.wangcong@...il.com> 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?