Re: [PATCH 03/24] ibtrs: core: lib functions shared between client and server modules

[Roman Penyaev <roman.penyaev@profitbricks.com>]

On Tue, Feb 6, 2018 at 5:10 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Tue, Feb 06, 2018 at 01:01:23PM +0100, Roman Penyaev wrote:
>
>> >> +static int ibtrs_ib_dev_init(struct ibtrs_ib_dev *d, struct ib_device
>> >> *dev)
>> >> +{
>> >> +       int err;
>> >> +
>> >> +       d->pd = ib_alloc_pd(dev, IB_PD_UNSAFE_GLOBAL_RKEY);
>> >> +       if (IS_ERR(d->pd))
>> >> +               return PTR_ERR(d->pd);
>> >> +       d->dev = dev;
>> >> +       d->lkey = d->pd->local_dma_lkey;
>> >> +       d->rkey = d->pd->unsafe_global_rkey;
>> >> +
>> >> +       err = ibtrs_query_device(d);
>> >> +       if (unlikely(err))
>> >> +               ib_dealloc_pd(d->pd);
>> >> +
>> >> +       return err;
>> >> +}
>> >
>> >
>> > I must say that this makes me frustrated.. We stopped doing these
>> > sort of things long time ago. No way we can even consider accepting
>> > the unsafe use of the global rkey exposing the entire memory space for
>> > remote access permissions.
>> >
>> > Sorry for being blunt, but this protocol design which makes a concious
>> > decision to expose unconditionally is broken by definition.
>>
>> I suppose we can also afford the same trick which nvme does: provide
>> register_always module argument, can we?  That can be also interesting
>> to measure the performance difference.
>
> I can be firmer than Sagi - new code that has IB_PD_UNSAFE_GLOBAL_RKEY
> at can not be accepted upstream.
>
> Once you get that fixed, you should go and read my past comments on
> how to properly order dma mapping and completions and fix that
> too. Then redo your performance..

Clear.

--
Roman