All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagi@grimberg.me>
To: santosh.shilimkar@oracle.com,
	Aron Silverton <aron.silverton@oracle.com>,
	Max Gurtovoy <maxg@mellanox.com>
Cc: bvanassche@acm.org, Jason Gunthorpe <jgg@mellanox.com>,
	linux-rdma@vger.kernel.org, dledford@redhat.com, leon@kernel.org,
	israelr@mellanox.com, shlomin@mellanox.com
Subject: Re: [PATCH 0/8 v1] Remove FMR support from RDMA drivers
Date: Thu, 14 May 2020 15:23:19 -0700	[thread overview]
Message-ID: <479add48-6fdb-f925-c3b9-699c6aa4cfbf@grimberg.me> (raw)
In-Reply-To: <5c48f60b-23b7-da64-6f37-f52de7bb625d@oracle.com>


>> +Santosh
>>
>> You probably meant to copy the RDS maintainer? Not sure if this should 
>> have
>> also been sent to netdev@vger.kernel.org.
>>
> Thanks Aron.
> 
>>
>>> On May 14, 2020, at 7:02 AM, Max Gurtovoy <maxg@mellanox.com> wrote:
>>>
>>> This series removes the support for FMR mode to register memory. This 
>>> ancient
>>> mode is unsafe and not maintained/tested in the last few years. It 
>>> also doesn't
>>> have any reasonable advantage over other memory registration methods 
>>> such as
>>> FRWR (that is implemented in all the recent RDMA adapters). This 
>>> series should
>>> be reviewed and approved by the maintainer of the effected drivers and I
>>> suggest to test it as well.
>>>
> I know the security issue has been brought up before and this plan of 
> removal of FMR support was on the cards

Actually, what is unsafe is not necessarily fmrs, but rather the
fmr_pool interface. So Max, you can keep fmr if rds wants it, but
we can get rid of fmr pools.

> but on RDS at least on CX3 we
> got more throughput with FMR vs FRWR. And the reasons are well
> understood as well why its the case.

Looking at the rds code, it seems that rds doesn't do any fast
registration at all, rkeys are long lived and are only invalidated (or
unmaped) when need recycling or when a socket is torn down...

So I'm not clear exactly about the model here, but seems to me
its almost like rds needs something like phys_mr, which is static for
all of its lifetime. It seems that fmrs just create a hassle for
rds, unless I'm missing something...

Having said that, it surely isn't the most secure behavior...
At least its not the global dma rkey...

> Is it possible to keep core support still around so that HCA's which
> supports FMR, ULPs can still can leverage it if they want.
>  From RDS perspective, if the HCA like CX3 doesn't support both modes,
> code prefers FMR vs FRWR and hence the question.

Max can start by removing fmr_pools, fmrs can stay as there is nothing
fundamentally wrong with them. And apparently there are still users.

  parent reply	other threads:[~2020-05-14 22:23 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-14 12:02 [PATCH 0/8 v1] Remove FMR support from RDMA drivers Max Gurtovoy
2020-05-14 12:02 ` [PATCH 1/8] RDMA/mlx4: remove FMR support for memory registration Max Gurtovoy
2020-05-14 12:02 ` [PATCH 2/8] RDMA/rds: " Max Gurtovoy
2020-05-14 12:03 ` [PATCH 3/8] RDMA/mthca: " Max Gurtovoy
2020-05-14 12:03 ` [PATCH 4/8] RDMA/rdmavt: remove FMR " Max Gurtovoy
2020-05-14 12:03 ` [PATCH 5/8] RDMA/iser: Remove support for " Max Gurtovoy
2020-05-14 12:03 ` [PATCH 6/8] RDMA/srp: remove " Max Gurtovoy
2020-05-14 14:02   ` Bart Van Assche
2020-05-14 12:03 ` [PATCH 7/8] RDMA/core: remove FMR pool API Max Gurtovoy
2020-05-14 12:03 ` [PATCH 8/8] RDMA/core: remove FMR device ops Max Gurtovoy
2020-05-14 15:13 ` [PATCH 0/8 v1] Remove FMR support from RDMA drivers Aron Silverton
2020-05-14 18:18   ` santosh.shilimkar
2020-05-14 19:42     ` Max Gurtovoy
2020-05-14 22:23     ` Sagi Grimberg [this message]
2020-05-14 23:41       ` santosh.shilimkar
2020-05-15 16:52         ` Tom Talpey
2020-05-15 18:59           ` Sagi Grimberg
2020-05-17 10:51             ` Max Gurtovoy
2020-05-18 16:34               ` santosh.shilimkar
2020-05-15  0:37       ` Max Gurtovoy
2020-05-14 16:00 ` Gal Pressman
2020-05-17 10:37   ` Max Gurtovoy
2020-05-18 15:20 ` Dennis Dalessandro
2020-05-18 18:10   ` Jason Gunthorpe
2020-05-19 13:43     ` Dennis Dalessandro
2020-05-19 13:53       ` Jason Gunthorpe
2020-05-19 14:19         ` Leon Romanovsky
2020-05-19 14:26           ` Dennis Dalessandro
2020-05-19 14:30             ` Jason Gunthorpe
2020-05-19 14:37               ` Dennis Dalessandro
2020-05-23 22:08                 ` Jason Gunthorpe
2020-05-24  1:27                   ` Tom Talpey
2020-05-26 15:38                   ` Dennis Dalessandro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=479add48-6fdb-f925-c3b9-699c6aa4cfbf@grimberg.me \
    --to=sagi@grimberg.me \
    --cc=aron.silverton@oracle.com \
    --cc=bvanassche@acm.org \
    --cc=dledford@redhat.com \
    --cc=israelr@mellanox.com \
    --cc=jgg@mellanox.com \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=maxg@mellanox.com \
    --cc=santosh.shilimkar@oracle.com \
    --cc=shlomin@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.