[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200409162727.5f17d305@kicinski-fedora-PC1C0HJN>
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 :)
Powered by blists - more mailing lists