netdev - Re: [PATCH bpf-next v4 2/3] libbpf: add low level TC-BPF API

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: <20210427231915.4tz4fs6hmthatufm@apollo>
Date:   Wed, 28 Apr 2021 04:49:15 +0530
From:   Kumar Kartikeya Dwivedi <memxor@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        bpf@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 2/3] libbpf: add low level TC-BPF API

On Wed, Apr 28, 2021 at 04:44:23AM IST, Daniel Borkmann wrote:
> On 4/28/21 12:51 AM, Toke Høiland-Jørgensen wrote:
> > Daniel Borkmann <daniel@...earbox.net> writes:
> > > On 4/28/21 12:36 AM, Toke Høiland-Jørgensen wrote:
> > > > Daniel Borkmann <daniel@...earbox.net> writes:
> > > [...]
> > > > > Small addendum:
> > > > >
> > > > >        DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = 42, .which = BPF_TC_INGRESS|BPF_TC_EGRESS);
> > > > >
> > > > >        err = bpf_tc_hook_create(&hook);
> > > > >        [...]
> > > > >
> > > > > ... is also possible, of course, and then both bpf_tc_hook_{create,destroy}() are symmetric.
> > > >
> > > > It should be allowed, but it wouldn't actually make any difference which
> > > > combination of TC_INGRESS and TC_EGRESS you specify, as long as one of
> > > > them is set, right? I.e., we just attach the clsact qdisc in both
> > > > cases...
> > >
> > > Yes, that is correct, for the bpf_tc_hook_create() whether you pass in BPF_TC_INGRESS,
> > > BPF_TC_EGRESS or BPF_TC_INGRESS|BPF_TC_EGRESS, you'll end up creating clsact qdisc in
> > > either of the three cases. Only the bpf_tc_hook_destroy() differs
> > > between all of them.
> >
> > Right, just checking. Other than that, I like your proposal; it loses
> > the "automatic removal of qdisc if we added it" feature, but that's
> > probably OK: less magic is good. And as long as bpf_tc_hook_create()
> > returns EEXIST if the qdisc already exists, the caller can do the same
> > thing if they want.
>
> Yes exactly. Less magic the better, especially given this has global effect.
>

Everything sounds good. I'll do a resend.

> Thanks,
> Daniel

--
Kartikeya