netdev - Re: [PATCH bpf-next v2 0/3] Add XDP support for bpf_load_hdr

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: <43bfb0fe-5476-c62c-51f2-a83da9fef659@iogearbox.net>
Date:   Thu, 7 Oct 2021 23:25:29 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Joanne Koong <joannekoong@...com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        bpf@...r.kernel.org
Cc:     kafai@...com, netdev@...r.kernel.org, Kernel-team@...com
Subject: Re: [PATCH bpf-next v2 0/3] Add XDP support for bpf_load_hdr_opt

On 10/7/21 10:57 PM, Joanne Koong wrote:
> On 10/7/21 7:41 AM, Toke Høiland-Jørgensen wrote:
>> Joanne Koong <joannekoong@...com> writes:
>>
>>> Currently, bpf_sockops programs have been using bpf_load_hdr_opt() to
>>> parse the tcp header option. It will be useful to allow other bpf prog
>>> types to have a similar way of handling tcp hdr options.
>>>
>>> This series adds XDP support for bpf_load_hdr_opt(). At a high level,
>>> these patches are:
>> Why is this needed? Why not just parse the header directly in XDP?
> Parsing a variable number of TCP options is challenging for the verifier.
> Some programs are using #pragma unroll as a temporary workaround
> (https://github.com/xdp-project/bpf-examples/blob/master/pping/pping_kern.c#L95)
> I believe Christian Deacon also recently posted about this on the xdp mailing list
> with a link to his bpf fail logs in https://github.com/gamemann/XDP-TCP-Header-Options
> which showcases some of the difficulties involved
> 
>> Seems
>> a bit arbitrary to add a helper for this particular type of packet
>> payload parsing to this particular program type. I.e., what about other
>> headers (IP options?)?
> The current use case needs so far have been for parsing tcp headers, but
> in the future, when there are needs for parsing other types, they
> can be supported as well through bpf_load_hdr_opt.
> 
>> Are we going to have a whole bunch of
>> special-purpose parsing helpers to pick out protocol data from packets?
> 
> I think bpf_load_hdr_opt is generic enough to support parsing
> any kind of protocol data (as specified through flags) in the packets

I tend to agree with Toke here that this is not generic. What has been tried
to improve the verifier instead before submitting the series? It would be much
more preferable to improve the developer experience with regards to a generic
solution, so that other/similar problems can be tackled in one go as well such
as IP options, extension headers, etc.

>> Also, why only enable this for XDP (and not, say the TC hook as well)?
> The plan is to also support this in tc as well (this will be in a separate
> patchset)