netdev - Re: [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set

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 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:   Mon, 07 Dec 2020 13:03:28 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     alardam@...il.com, magnus.karlsson@...el.com,
        bjorn.topel@...el.com, andrii.nakryiko@...il.com, kuba@...nel.org,
        ast@...nel.org, netdev@...r.kernel.org, davem@...emloft.net,
        john.fastabend@...il.com, hawk@...nel.org,
        jonathan.lemon@...il.com, bpf@...r.kernel.org,
        jeffrey.t.kirsher@...el.com, maciejromanfijalkowski@...il.com,
        intel-wired-lan@...ts.osuosl.org,
        Marek Majtyka <marekx.majtyka@...el.com>
Subject: Re: [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set

Daniel Borkmann <daniel@...earbox.net> writes:

> On 12/4/20 6:20 PM, Toke Høiland-Jørgensen wrote:
>> Daniel Borkmann <daniel@...earbox.net> writes:
> [...]
>>> We tried to standardize on a minimum guaranteed amount, but unfortunately not
>>> everyone seems to implement it, but I think it would be very useful to query
>>> this from application side, for example, consider that an app inserts a BPF
>>> prog at XDP doing custom encap shortly before XDP_TX so it would be useful to
>>> know which of the different encaps it implements are realistically possible on
>>> the underlying XDP supported dev.
>> 
>> How many distinct values are there in reality? Enough to express this in
>> a few flags (XDP_HEADROOM_128, XDP_HEADROOM_192, etc?), or does it need
>> an additional field to get the exact value? If we implement the latter
>> we also run the risk of people actually implementing all sorts of weird
>> values, whereas if we constrain it to a few distinct values it's easier
>> to push back against adding new values (as it'll be obvious from the
>> addition of new flags).
>
> It's not everywhere straight forward to determine unfortunately, see also [0,1]
> as some data points where Jesper looked into in the past, so in some cases it
> might differ depending on the build/runtime config..
>
>    [0] https://lore.kernel.org/bpf/158945314698.97035.5286827951225578467.stgit@firesoul/
>    [1] https://lore.kernel.org/bpf/158945346494.97035.12809400414566061815.stgit@firesoul/

Right, well in that case maybe we should just expose the actual headroom
as a separate netlink attribute? Although I suppose that would require
another round of driver changes since Jesper's patch you linked above
only puts this into xdp_buff at XDP program runtime.

Jesper, WDYT?

-Toke