Re: [PATCH 05/15] nvme-rdma: Add metadata/T10-PI support

[Max Gurtovoy <maxg@mellanox.com>]

On 11/5/2019 7:58 PM, Christoph Hellwig wrote:
> On Tue, Nov 05, 2019 at 06:20:16PM +0200, Max Gurtovoy wrote:
>> For capable HCAs (e.g. ConnectX-4/ConnectX-5) this will allow end-to-end
>> protection information passthrough and validation for NVMe over RDMA
>> transport. Metadata offload support was implemented over the new RDMA
>> signature verbs API and it is enabled per controller by using nvme-cli.
>>
>> usage example:
>> nvme connect --pi_enable --transport=rdma --traddr=10.0.1.1 --nqn=test-nvme
>>
>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>> Signed-off-by: Israel Rukshin <israelr@mellanox.com>
>> ---
>>   drivers/nvme/host/rdma.c | 346 ++++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 298 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
>> index 05f2dfa..16263b8 100644
>> --- a/drivers/nvme/host/rdma.c
>> +++ b/drivers/nvme/host/rdma.c
>> @@ -48,6 +48,12 @@ struct nvme_rdma_qe {
>>   	u64			dma;
>>   };
>>   
>> +struct nvme_rdma_sgl {
>> +	int			nents;
>> +	struct sg_table		sg_table;
>> +	struct scatterlist	first_sgl[SG_CHUNK_SIZE];
> I think this needs some rework.  Kill the first_sgl pointer, and then
> just set the pointer in the table to address found by pointer
> arithmetics behind the nvme_request in the allocation.
>
> We also have an open todo item to not actually allocate SG_CHUNK_SIZE
> entries, but a much smaller value like in SCSI.

do you mean to use "real" SGL chain in NVMf/RDMA code ?

We can do it as a separate preparation commit. But we need to make sure 
that the performance will not be effected from this change.


>
> Also the whole switch to use struct sg_table should be a separate prep
> patch.
>
>> -	struct sg_table		sg_table;
>> -	struct scatterlist	first_sgl[];
>> +	/* T10-PI support */
>> +	bool			is_protected;
> This is a bit of an odd variable name.  Why not use_pi or something like
> that?
>
> Also all the new code should only be built if CONFIG_BLK_DEV_INTEGRITY
> is set.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme