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:   Tue, 08 Dec 2020 21:50:25 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Jesper Dangaard Brouer <jbrouer@...hat.com>,
        David Ahern <dsahern@...il.com>
Cc:     John Fastabend <john.fastabend@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        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,
        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

Toke Høiland-Jørgensen wrote:
> Jesper Dangaard Brouer <jbrouer@...hat.com> writes:
> 
> > On Mon, 7 Dec 2020 18:01:00 -0700
> > David Ahern <dsahern@...il.com> wrote:
> >
> >> On 12/7/20 1:52 PM, John Fastabend wrote:
> >> >>
> >> >> I think we need to keep XDP_TX action separate, because I think that
> >> >> there are use-cases where the we want to disable XDP_TX due to end-user
> >> >> policy or hardware limitations.  
> >> > 
> >> > How about we discover this at load time though. 
> >
> > Nitpick at XDP "attach" time. The general disconnect between BPF and
> > XDP is that BPF can verify at "load" time (as kernel knows what it
> > support) while XDP can have different support/features per driver, and
> > cannot do this until attachment time. (See later issue with tail calls).
> > (All other BPF-hooks don't have this issue)
> >
> >> > Meaning if the program
> >> > doesn't use XDP_TX then the hardware can skip resource allocations for
> >> > it. I think we could have verifier or extra pass discover the use of
> >> > XDP_TX and then pass a bit down to driver to enable/disable TX caps.
> >> >   
> >> 
> >> This was discussed in the context of virtio_net some months back - it is
> >> hard to impossible to know a program will not return XDP_TX (e.g., value
> >> comes from a map).
> >
> > It is hard, and sometimes not possible.  For maps the workaround is
> > that BPF-programmer adds a bound check on values from the map. If not
> > doing that the verifier have to assume all possible return codes are
> > used by BPF-prog.
> >
> > The real nemesis is program tail calls, that can be added dynamically
> > after the XDP program is attached.  It is at attachment time that
> > changing the NIC resources is possible.  So, for program tail calls the
> > verifier have to assume all possible return codes are used by BPF-prog.
> 
> We actually had someone working on a scheme for how to express this for
> programs some months ago, but unfortunately that stalled out (Jesper
> already knows this, but FYI to the rest of you). In any case, I view
> this as a "next step". Just exposing the feature bits to userspace will
> help users today, and as a side effect, this also makes drivers declare
> what they support, which we can then incorporate into the core code to,
> e.g., reject attachment of programs that won't work anyway. But let's
> do this in increments and not make the perfect the enemy of the good
> here.
> 
> > BPF now have function calls and function replace right(?)  How does
> > this affect this detection of possible return codes?
> 
> It does have the same issue as tail calls, in that the return code of
> the function being replaced can obviously change. However, the verifier
> knows the target of a replace, so it can propagate any constraints put
> upon the caller if we implement it that way.

OK I'm convinced its not possible to tell at attach time if a program
will use XDP_TX or not in general. And in fact for most real programs it
likely will not be knowable. At least most programs I look at these days
use either tail calls or function calls so seems like a dead end.

Also above somewhere it was pointed out that XDP_REDIRECT would want
the queues and it seems even more challenging to sort that out.