Re: [SPDK] Changes to NVMF Target Opts Usage

[Walker, Benjamin <benjamin.walker at intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4916 bytes --]

I'm definitely on board, at least at a high level. We'll work through the
details as they come up.

On Tue, 2018-06-12 at 17:44 +0000, Pai, Madhu wrote:
> Hi John,
>  
> FWIW, I think the below is a very good idea to abstract out the changes for
> multiple transport support.
> 
> Thanks,
> Madhu
>  
> From: SPDK <spdk-bounces(a)lists.01.org> On Behalf Of John Barnard
> Sent: Monday, June 11, 2018 6:49 PM
> To: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: [SPDK] Changes to NVMF Target Opts Usage
>  
> With the addition of addtional transport protocols to the nvmf layer, the use
> of the spdk_nvmf_tgt_opts structure held by the global spdk_nvmf_tgt,
> containing the target configuration parameters (e.g. max_queue_depth,
> max_io_size, etc.) no longer works since each protocol will need to use
> different values for these parameters.  Therefore, I’m proposing, moving most
> of these parameters into the transport layer.  Here are the proposed changes
> for your review: 
> 
>  Create a new structure for the transport opts (spdk_nvmf_transport_opts) and
> move most of the fields in spdk_nvmf_tgt_opts into this structure.
>  Add spdk_nvmf_transport_opts to spdk_nvmf_transport.
>  Add new spdk_nvmf_transport_opts argument to spdk_nvmf_transport_ops:create()
> and spdk_nvmf_transport_create() functions.  This argument also gets passed
> down to the transport specific create function.
>  In spdk_nvmf_transport_create() function, copy spdk_nvmf_transport_opts into
> spdk_nvmf_transport.
>  Modify functions in rdma.c and ctlrlr.c that access spdk_nvmf_tgt for
> target opts to access spdk_nvmf_transport_opts in the transport structure to
> get the transport opts.
> Descriptions of Additional Changes
> 1. Decouple transport creation from adding listeners
>  
> The problem with creating the transport through the listener configuration
> is that Fibre Channel does not necessarily require listeners to be configured
> in order to operate.  The listeners would be obtained from the FC driver
> (physical ports) and/or the management plane (i.e. nport creation).  This
> means it is not possible to get the FC transport created using the current
> implementation.  Therefore, I’m proposing that we decouple transport creation
> from listener configuration.  This means that the transport is created first
> and the listeners are added later.
>  
> To do this a new function, spdk_nvmf_transport_create(), in nvmf.c  would be
> created it would call the renamed transport create function in
> transport.c.  This function would be a direct call (i.e. no
> callback).  Additionally, add another function, spdk_nvmf_add_transport(),
> which takes care of adding the transports to the PGs, which would provide a
> callback when completed. The existing spdk_nvmf_tgt_listen() function will be
> modified to be a direct call to add a listener to the transport. Note: If the
> transport for the specific listener has not been created when this function is
> called, it will be considered an error condition.
>  
> 2. Target App/Conf File Changes
>  
> I understand that the target configuration in the nvmf target app. is
> currently undergoing changes in order to discontinue use of the nvmf.conf
> file, but for now I only have the .conf solution to work with so I will
> reference those changes here.  I want to update the nvmf.conf file and conf
> file parsing by adding new [Transport] sections for each protocol (e.g. RDMA,
> FC, etc.).  These sections contain the protocol specific “transport” opts
> settings.
>  
> Example Conf File Transport Sections
>  
>   [Transport]
>   Type RDMA
>   MaxQueuesPerSession 4
>   MaxQueueDepth 128
>   InCapsuleDataSize 4096
>   MaxIOSize 131072
>  
>   [Transport]
>   Type FC
>   MaxQueuesPerSession 6
>   MaxQueueDepth 256
>   MaxIOSize 1048576
>   MaxAQSize 32
>  
>  
> 3. Setting Default Transport Option Values
>  
> Each of the options has a default value in case they are not set by the
> user.  This is currently done by the spdk_nvmf_tgt_opts_init() function, which
> will no longer work for the transports opts.  I’m proposing adding a new
> function to the spdk_nvmf_transport_ops table to initialize the transport
> options to their default values, which can be called first before customizing
> them (similar to how the target opts init works now, but specif to transport).
>  
> 4. RPC
> The one area that I’m not too familiar with and have no experience is the rpc
> code.  I’m aware that with the decoupling of transport creation and adding
> listeners, the nvmf rcp code will need to be changed, but I will need help
> with this.
>  
> Regards,
> John Barnard
>  
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk