netdev - Re: [PATCH bpf-next 02/12] net: Add BPF_XDP_EGRESS as a bpf_attach

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]

Date:   Fri, 24 Jan 2020 10:49:30 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     David Ahern <dsahern@...il.com>, David Ahern <dsahern@...nel.org>,
        netdev@...r.kernel.org
Cc:     prashantbhole.linux@...il.com, jasowang@...hat.com,
        davem@...emloft.net, jakub.kicinski@...ronome.com,
        jbrouer@...hat.com, mst@...hat.com, toshiaki.makita1@...il.com,
        daniel@...earbox.net, john.fastabend@...il.com, ast@...nel.org,
        kafai@...com, songliubraving@...com, yhs@...com, andriin@...com,
        David Ahern <dahern@...italocean.com>
Subject: Re: [PATCH bpf-next 02/12] net: Add BPF_XDP_EGRESS as a bpf_attach_type

David Ahern <dsahern@...il.com> writes:

> On 1/23/20 4:34 AM, Toke Høiland-Jørgensen wrote:
>> David Ahern <dsahern@...nel.org> writes:
>> 
>>> From: Prashant Bhole <prashantbhole.linux@...il.com>
>>>
>>> Add new bpf_attach_type, BPF_XDP_EGRESS, for BPF programs attached
>>> at the XDP layer, but the egress path.
>>>
>>> Since egress path does not have rx_queue_index and ingress_ifindex set,
>>> update xdp_is_valid_access to block access to these entries in the xdp
>>> context when a program is attached to egress path.
>> 
>> Isn't the whole point of this to be able to use unchanged XDP programs?
>
> See patch 12. Only the userspace code was changed to load the same
> program with the egress attach type set.
>
> The verifier needs to check the egress program does not access Rx only
> entries in xdp_md context. The attach type allows that check.
>
>> But now you're introducing a semantic difference. Since supposedly only
>> point-to-point links are going to be using this attach type, don't they
>> know enough about their peer device to be able to populate those fields
>> with meaningful values, instead of restricting access to them?
>> 
>
> You are conflating use cases. Don't assume point to point or peer devices.
>
> This could be a REDIRECT from eth0 to eth1 and then an EGRESS program on
> eth1 to do something. In the current test scenario it is REDIRECT from
> eth0 to tapN and then on tapN run an egress program (Tx for a tap is
> ingress to the VM).

But why would any hardware driver implement this program type, instead
of the proper TX hook? I thought the whole idea of this "third"
not-quite-TX program type was the virtualisation offload case. If you
just want to be able to run eBPF code in the TX path, why not implement
the proper TX hook straight away?

-Toke