[SPDK] Changes to NVMF Target Opts Usage

[SPDK] Changes to NVMF Target Opts Usage

[John Barnard]

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

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

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 18314 bytes --]

Re: [SPDK] Changes to NVMF Target Opts Usage

[Pai, Madhu]

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

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


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 18485 bytes --]

Re: [SPDK] Changes to NVMF Target Opts Usage

[Walker, Benjamin]

[-- 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

Re: [SPDK] Changes to NVMF Target Opts Usage

[John Barnard]

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

Hi Ben, et. al.,

I've uploaded a RFC patch to gerrithub with the changes need to support
multiple transport opts (416570).

-John

On Wed, Jun 13, 2018 at 12:35 PM, Walker, Benjamin <
benjamin.walker(a)intel.com> wrote:

> 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
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 6723 bytes --]