netdev - Re: [PATCH RFC v4 bpf-next 00/11] Add support for XDP in egress path

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87r1ygufgu.fsf@toke.dk>
Date:   Thu, 27 Feb 2020 18:06:57 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     David Ahern <dsahern@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Prashant Bhole <prashantbhole.linux@...il.com>,
        Jason Wang <jasowang@...hat.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Toshiaki Makita <toshiaki.makita1@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH RFC v4 bpf-next 00/11] Add support for XDP in egress path

Alexei Starovoitov <alexei.starovoitov@...il.com> writes:

> On Thu, Feb 27, 2020 at 3:55 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>
>> However, my issue with this encoding is that it is write-only: You can't
>> inspect a BPF program already loaded into the kernel and tell which type
>> it is. So my proposal would be to make it explicit: Expose the
>> expected_attach_type as a new field in bpf_prog_info so userspace can
>> query it, and clearly document it as, essentially, a program subtype
>> that can significantly affect how a program is treated by the kernel.
>
> You had the same request for "freplace" target prog.

Yes, and for the same reason; I'm being consistent here :)

> My answer to both is still the same:
> Please take a look at drgn and the script that Andrey posted.
> All this information is trivial to extract from the kernel
> without introducing new uapi.

I'm sorry, but having this kind of write-only data structure is a
horrible API design; and saying "you can just parse the internal kernel
data structures to see what is going on" is a cop-out. The whole point
of having a stable UAPI is to make it possible for people to write
applications that make use of kernel features with an expectation that
those applications will keep working. XDP is a networking feature;
people building networking applications shouldn't have to chase internal
kernel APIs just to keep their packet processing programs working.

Besides, it's already UAPI - there's a setter for it! If we introduce
this new egress program type that is still going to be a de facto new
program type with different semantics than the RX program, whether we
try to hide the fact or not. Even if we end up completely changing the
internal data structures, we're still going to have to support someone 
loading a program with type==XDP and attach_type == XDP_EGRESS. So why
can't we allow the user to query the state of a previously loaded
program as well?

-Toke