[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAAeCc_=ud873LvHHucSK6fzUTOwLoir+CWxcvTn8UuxRgjajiw@mail.gmail.com>
Date: Sat, 24 Aug 2024 19:29:46 +0530
From: Bharat Bhushan <bharatb.linux@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Bharat Bhushan <bbhushan2@...vell.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, sgoutham@...vell.com, gakula@...vell.com,
sbhatta@...vell.com, hkelam@...vell.com, davem@...emloft.net,
edumazet@...gle.com, pabeni@...hat.com, jerinj@...vell.com,
lcherian@...vell.com, richardcochran@...il.com
Subject: Re: [net-next,v6 1/8] octeontx2-pf: map skb data as device writeable
On Fri, Aug 23, 2024 at 4:55 PM Bharat Bhushan <bharatb.linux@...il.com> wrote:
>
> On Thu, Aug 22, 2024 at 8:18 PM Jakub Kicinski <kuba@...nel.org> wrote:
> >
> > On Thu, 22 Aug 2024 09:15:43 +0530 Bharat Bhushan wrote:
> > > On Wed, Aug 21, 2024 at 4:06 AM Jakub Kicinski <kuba@...nel.org> wrote:
> > > > On Mon, 19 Aug 2024 17:53:41 +0530 Bharat Bhushan wrote:
> > > > > Crypto hardware need write permission for in-place encrypt
> > > > > or decrypt operation on skb-data to support IPsec crypto
> > > > > offload. So map this memory as device read-write.
> > > >
> > > > How do you know the fragments are not read only?
> > >
> > > IOMMU permission faults will be reported if the DMA_TO_DEVICE direction flag
> > > is used in dma_map_page_attrs(). This is because iommu creates read only mapping
> > > if the DMA_TO_DEVICE direction flag is used. If the direction flag used in
> > > dma_map_pages() is DMA_BIDIRECTIONAL then iommu creates mapping with
> > > both read and write permission.
> >
> > The other way around, I understand that your code makes the pages
> > writable for the device. What I'm concerned about is that if this
> > code path is fed Tx skbs you will corrupt them. Are these not Tx
> > skbs that you're mapping? Have you fully CoW'd them to make sure
> > they are writable?
>
> This code is mapping skb data for hardware to over-write plain-text with
> cypher-text and update authentication data (in-place encap/auth).
> This patch series doesn't take care of CoWing for skb data. Actually I was
> not aware of that before your comment.
>
> To understand your comment better, If the device writes to shared skb data
> without CoWing then we have an issue. Is that correct?
>
> I do not see any other driver supporting IPsec crypto offload ensuring
> skb data CoWing,
> but there is a possibility that those devices are not doing in-place
> encap and auth (encap
> and auth data written to separate buffer). I do not have clarity about
> this, This skb is set for
> IPSEC crypto offload, Is this the driver which has to ensure that the
> skb is writeable or the
> network stack (xfrm layer) will ensure the same. If it is former then
> add code to call skb_unshare().
> Please suggest.
My understanding after further looking into the code is that it is the
driver responsibility to ensure skb is not in a shared state.
Thanks for pointing this out. Will change code in the next version.
Regards
-Bharat
>
> Thanks
> -Bharat
Powered by blists - more mailing lists