On 5/14/2020 7:41 PM, santosh.shilimkar@xxxxxxxxxx wrote:
On 5/14/20 3:23 PM, Sagi Grimberg wrote:
+Santosh
You probably meant to copy the RDS maintainer? Not sure if this
should have
also been sent to netdev@xxxxxxxxxxxxxxx.
Thanks Aron.
On May 14, 2020, at 7:02 AM, Max Gurtovoy <maxg@xxxxxxxxxxxx> 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.
Good point. We aren't using the fmr_pools.
It's not only the fmr_pools. The FMR API operates on a page granularity,
so a sub-page registration, or a non-page-aligned one, ends up exposing
data outside the buffer. This is done silently, and is a significant
vulnerability for most upper layers.
Tom.
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...
There are couple of layers but you can see the FRWR code inside,
net/rds/ib_frmr.c. The MR allocation as well as free/invalidation
is managed from user-land instead of ULP data path. There are
couple of cases where some use_once semantics does MR invalidation
within kernel but thats only because userland indicated that MR
key can be invalidated after issued RDMA ops is complete.
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.
That will surely help if its an option. RDS don't use fmr_pools so no
issues there.
Regards,
Santosh
