netdev - Re: [PATCH RFC v2 01/33] xdp: add frame size to xdp

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:   Thu, 9 Apr 2020 16:27:27 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "akiyano@...zon.com" <akiyano@...zon.com>,
        "willemdebruijn.kernel@...il.com" <willemdebruijn.kernel@...il.com>,
        "borkmann@...earbox.net" <borkmann@...earbox.net>,
        "jeffrey.t.kirsher@...el.com" <jeffrey.t.kirsher@...el.com>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "toke@...hat.com" <toke@...hat.com>,
        "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>,
        "gtzalik@...zon.com" <gtzalik@...zon.com>,
        "dsahern@...il.com" <dsahern@...il.com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "sameehj@...zon.com" <sameehj@...zon.com>,
        "zorik@...zon.com" <zorik@...zon.com>,
        "alexander.duyck@...il.com" <alexander.duyck@...il.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "lorenzo@...nel.org" <lorenzo@...nel.org>
Subject: Re: [PATCH RFC v2 01/33] xdp: add frame size to xdp_buff

On Thu, 9 Apr 2020 23:07:42 +0000 Saeed Mahameed wrote:
> > My concern is that driver may allocate a full page for each frame but
> > only DMA map the amount that can reasonably contain data given the
> > MTU.
> > To save on DMA syncs.
> > 
> > Today that wouldn't be a problem, because XDP_REDIRECT will re-map
> > the
> > page, and XDP_TX has the same MTU.
> 
> I am not worried about dma at all, i am worried about the xdp progs
> which are now allowed to extend packets beyond the mtu and do XDP_TX.
> but as i am thinking about this i just realized that this can already
> happen with xdp_adjust_head()..
> 
> but as you stated above this puts alot of assumptions on how driver
> should dma rx buffs 
> 
> > In this set xdp_data_hard_end is used both to find the end of memory
> > buffer, and end of DMA buffer. Implementation of
> > bpf_xdp_adjust_tail()
> > assumes anything < SKB_DATA_ALIGN(sizeof(struct skb_shared_info))
> > from
> > the end is fair game.
> 
> but why skb_shared_info in particular though ? this assumes someone
> needs this tail for building skbs .. looks weird to me.

Fair, simplifies the internals, I guess.

> > So I was trying to say that we should warn driver authors that the
> > DMA
> > buffer can now grow / move beyond what the driver may expect in
> > XDP_TX.  
> 
> Ack, but can we do it by desing ? i.e instead of having hardcoded
> limits (e.g. SKB_DATA_ALIGN(shinfo)) in bpf_xdp_adjust_tail(), let the
> driver provide this, or any other restrictions, e.g mtu for tx, or
> driver specific memory model restrictions .. 

Right, actually for NFP we need to add the check already - looking at
the code - the DMA mapping does not cover anything beyond the headroom +
MTU.

> > Drivers can either DMA map enough memory, or handle the corner case
> > in
> > a special way.
> > 
> > IDK if that makes sense, we may be talking past each other :)