Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-22 Thread Andy Grover 

On 03/07/2013 05:45 PM, Nicholas A. Bellinger wrote:

From: Nicholas Bellinger n...@linux-iscsi.org

Add basic struct iscsit_transport API template to allow iscsi-target for
running with external transport modules using existing iscsi_target_core.h
code.

For all external modules, this calls try_module_get() and module_put()
to obtain + release an external iscsit_transport module reference count.

Also include the iscsi-target symbols necessary in iscsi_transport.h to
allow external transport modules to function.

Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
  drivers/target/iscsi/Makefile |3 +-
  drivers/target/iscsi/iscsi_target_transport.c |   57 ++
  include/target/iscsi/iscsi_transport.h|   77 +
  3 files changed, 136 insertions(+), 1 deletions(-)
  create mode 100644 drivers/target/iscsi/iscsi_target_transport.c
  create mode 100644 include/target/iscsi/iscsi_transport.h

diff --git a/drivers/target/iscsi/Makefile b/drivers/target/iscsi/Makefile
index 5b9a2cf..13a9240 100644
--- a/drivers/target/iscsi/Makefile
+++ b/drivers/target/iscsi/Makefile
@@ -15,6 +15,7 @@ iscsi_target_mod-y += iscsi_target_parameters.o \
iscsi_target_util.o \
iscsi_target.o \
iscsi_target_configfs.o \
-   iscsi_target_stat.o
+   iscsi_target_stat.o \
+   iscsi_target_transport.o

  obj-$(CONFIG_ISCSI_TARGET)+= iscsi_target_mod.o
diff --git a/drivers/target/iscsi/iscsi_target_transport.c 
b/drivers/target/iscsi/iscsi_target_transport.c
new file mode 100644
index 000..4ffd965
--- /dev/null
+++ b/drivers/target/iscsi/iscsi_target_transport.c
@@ -0,0 +1,57 @@
+#include linux/spinlock.h
+#include linux/list.h
+#include target/iscsi/iscsi_transport.h
+
+static LIST_HEAD(g_transport_list);
+static DEFINE_MUTEX(transport_mutex);
+
+struct iscsit_transport *iscsit_get_transport(int type)
+{
+   struct iscsit_transport *t;
+
+   mutex_lock(transport_mutex);
+   list_for_each_entry(t, g_transport_list, t_node) {
+   if (t-transport_type == type) {
+   if (t-owner  !try_module_get(t-owner)) {
+   t = NULL;
+   }
+   mutex_unlock(transport_mutex);
+   return t;
+   }
+   }
+   mutex_unlock(transport_mutex);
+
+   return NULL;
+}
+EXPORT_SYMBOL(iscsit_get_transport);
+
+void iscsit_put_transport(struct iscsit_transport *t)
+{
+   if (t-owner)
+   module_put(t-owner);
+}
+EXPORT_SYMBOL(iscsit_put_transport);
+
+int iscsit_create_transport(struct iscsit_transport *t)
+{
+   INIT_LIST_HEAD(t-t_node);
+
+   mutex_lock(transport_mutex);
+   list_add_tail(t-t_node, g_transport_list);
+   mutex_unlock(transport_mutex);
+
+   printk(Created iSCSI transport: %s\n, t-name);
+
+   return 0;
+}
+EXPORT_SYMBOL(iscsit_create_transport);
+
+void iscsit_destroy_transport(struct iscsit_transport *t)
+{
+   mutex_lock(transport_mutex);
+   list_del(t-t_node);
+   mutex_unlock(transport_mutex);
+
+   printk(Destroyed iSCSI transport: %s\n, t-name);
+}


I don't think create/destroy are the right names to use here - I suggest 
register/unregister or something like that?


Please also make sure to turn those printks in to pr_something() for the 
final version.


-- Andy


+EXPORT_SYMBOL(iscsit_destroy_transport);
diff --git a/include/target/iscsi/iscsi_transport.h 
b/include/target/iscsi/iscsi_transport.h
new file mode 100644
index 000..c885376
--- /dev/null
+++ b/include/target/iscsi/iscsi_transport.h
@@ -0,0 +1,77 @@
+#include linux/module.h
+#include linux/list.h
+#include ../../../drivers/target/iscsi/iscsi_target_core.h
+
+struct iscsit_transport {
+#define ISCSIT_TRANSPORT_NAME  16
+   char name[ISCSIT_TRANSPORT_NAME];
+   int transport_type;
+   struct module *owner;
+   struct list_head t_node;
+   int (*iscsit_setup_np)(struct iscsi_np *, struct 
__kernel_sockaddr_storage *);
+   int (*iscsit_accept_np)(struct iscsi_np *, struct iscsi_conn *);
+   void (*iscsit_free_np)(struct iscsi_np *);
+   void (*iscsit_free_conn)(struct iscsi_conn *);
+   struct iscsi_cmd *(*iscsit_alloc_cmd)(struct iscsi_conn *, gfp_t);
+   void (*iscsit_unmap_cmd)(struct iscsi_cmd *, struct iscsi_conn *);
+   void (*iscsit_free_cmd)(struct iscsi_cmd *);
+   int (*iscsit_get_login_rx)(struct iscsi_conn *, struct iscsi_login *);
+   int (*iscsit_put_login_tx)(struct iscsi_conn *, struct iscsi_login *, 
u32);
+   int (*iscsit_immediate_queue)(struct iscsi_conn *, struct iscsi_cmd *, 
int);
+   int (*iscsit_response_queue)(struct iscsi_conn *, struct iscsi_cmd *, 
int);
+};
+
+/*
+ * From iscsi_target_transport.c
+ */
+
+extern int

Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-22 Thread Nicholas A. Bellinger 
On Fri, 2013-03-22 at 10:23 -0700, Andy Grover wrote:
 On 03/07/2013 05:45 PM, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@linux-iscsi.org
 
  Add basic struct iscsit_transport API template to allow iscsi-target for
  running with external transport modules using existing iscsi_target_core.h
  code.
 
  For all external modules, this calls try_module_get() and module_put()
  to obtain + release an external iscsit_transport module reference count.
 
  Also include the iscsi-target symbols necessary in iscsi_transport.h to
  allow external transport modules to function.
 
  Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
  ---
drivers/target/iscsi/Makefile |3 +-
drivers/target/iscsi/iscsi_target_transport.c |   57 ++
include/target/iscsi/iscsi_transport.h|   77 
  +
3 files changed, 136 insertions(+), 1 deletions(-)
create mode 100644 drivers/target/iscsi/iscsi_target_transport.c
create mode 100644 include/target/iscsi/iscsi_transport.h
 
  diff --git a/drivers/target/iscsi/Makefile b/drivers/target/iscsi/Makefile
  index 5b9a2cf..13a9240 100644
  --- a/drivers/target/iscsi/Makefile
  +++ b/drivers/target/iscsi/Makefile
  @@ -15,6 +15,7 @@ iscsi_target_mod-y += 
  iscsi_target_parameters.o \
  iscsi_target_util.o \
  iscsi_target.o \
  iscsi_target_configfs.o \
  -   iscsi_target_stat.o
  +   iscsi_target_stat.o \
  +   iscsi_target_transport.o
 
obj-$(CONFIG_ISCSI_TARGET)+= iscsi_target_mod.o
  diff --git a/drivers/target/iscsi/iscsi_target_transport.c 
  b/drivers/target/iscsi/iscsi_target_transport.c
  new file mode 100644
  index 000..4ffd965
  --- /dev/null
  +++ b/drivers/target/iscsi/iscsi_target_transport.c
  @@ -0,0 +1,57 @@
  +#include linux/spinlock.h
  +#include linux/list.h
  +#include target/iscsi/iscsi_transport.h
  +
  +static LIST_HEAD(g_transport_list);
  +static DEFINE_MUTEX(transport_mutex);
  +
  +struct iscsit_transport *iscsit_get_transport(int type)
  +{
  +   struct iscsit_transport *t;
  +
  +   mutex_lock(transport_mutex);
  +   list_for_each_entry(t, g_transport_list, t_node) {
  +   if (t-transport_type == type) {
  +   if (t-owner  !try_module_get(t-owner)) {
  +   t = NULL;
  +   }
  +   mutex_unlock(transport_mutex);
  +   return t;
  +   }
  +   }
  +   mutex_unlock(transport_mutex);
  +
  +   return NULL;
  +}
  +EXPORT_SYMBOL(iscsit_get_transport);
  +
  +void iscsit_put_transport(struct iscsit_transport *t)
  +{
  +   if (t-owner)
  +   module_put(t-owner);
  +}
  +EXPORT_SYMBOL(iscsit_put_transport);
  +
  +int iscsit_create_transport(struct iscsit_transport *t)
  +{
  +   INIT_LIST_HEAD(t-t_node);
  +
  +   mutex_lock(transport_mutex);
  +   list_add_tail(t-t_node, g_transport_list);
  +   mutex_unlock(transport_mutex);
  +
  +   printk(Created iSCSI transport: %s\n, t-name);
  +
  +   return 0;
  +}
  +EXPORT_SYMBOL(iscsit_create_transport);
  +
  +void iscsit_destroy_transport(struct iscsit_transport *t)
  +{
  +   mutex_lock(transport_mutex);
  +   list_del(t-t_node);
  +   mutex_unlock(transport_mutex);
  +
  +   printk(Destroyed iSCSI transport: %s\n, t-name);
  +}
 
 I don't think create/destroy are the right names to use here - I suggest 
 register/unregister or something like that?
 

Fair enough.  Renamed to register/unregister in the forth-coming RFC-v2

 Please also make sure to turn those printks in to pr_something() for the 
 final version.
 

Done.

Thanks,

--nab


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-08 Thread Or Gerlitz 
On Fri, Mar 8, 2013 at 6:14 AM, Roland Dreier rol...@kernel.org wrote:
 Nicholas A. Bellinger n...@linux-iscsi.org wrote:
  +EXPORT_SYMBOL(iscsit_get_transport);

 It's not clear to me why this needs to be exported.  Who would use it
 outside the core iscsi target module?

Yep, as Nic noted, we're adding here an iscsi transport concept e.g in
the same manner Mike did libiscsi back in 2005/6 when the iser
initiator was pushed. This allows for multiple iscsi flavours to use a
common code  for common functionality. In the initiator area initially
there were iscsi tcp and iser, later few iscsi HW offloads were merged
too. Same story here. I think that the point is whether or not these
APIs are needed, since once we agree on that, we need an header file
and exporting of functions. As libiscsi.h resided under include/ it
makes sense to me for this include to be located there too.

Or.

Or.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-08 Thread Roland Dreier 
On Thu, Mar 7, 2013 at 10:02 PM, Nicholas A. Bellinger
n...@linux-iscsi.org wrote:
 Or and I discussed this point in the last status call, and given what
 the initiator did originally (eg: export iscsi_transport) he asked to
 keep it under drivers/infiniband/ulp/isert/ with the extra include bits.

 I'd have a slight preference to move iser-target code under
 drivers/target/iscsi/, and not put anything into include/target/iscsi/
 if there won't be another module that uses it..

 Do you have a preference here..?

It's not really something that matters to me.

 - R.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-08 Thread Nicholas A. Bellinger 
On Fri, 2013-03-08 at 14:36 +0200, Or Gerlitz wrote:
 On Fri, Mar 8, 2013 at 6:14 AM, Roland Dreier rol...@kernel.org wrote:
  Nicholas A. Bellinger n...@linux-iscsi.org wrote:
   +EXPORT_SYMBOL(iscsit_get_transport);
 
  It's not clear to me why this needs to be exported.  Who would use it
  outside the core iscsi target module?
 
 Yep, as Nic noted, we're adding here an iscsi transport concept e.g in
 the same manner Mike did libiscsi back in 2005/6 when the iser
 initiator was pushed. This allows for multiple iscsi flavours to use a
 common code  for common functionality. In the initiator area initially
 there were iscsi tcp and iser, later few iscsi HW offloads were merged
 too. Same story here. I think that the point is whether or not these
 APIs are needed, since once we agree on that, we need an header file
 and exporting of functions. As libiscsi.h resided under include/ it
 makes sense to me for this include to be located there too.

So my main concern with putting iscsit_transport definitions into
include/target/iscsi/ is the number of dependencies required from
iscsi_target_core.h.

Currently with iscsi_cmd embedded into isert_cmd, this will require most
existing iscsi_target_core.h definitions to be pushed out into
include/target/iscsi/.  This also includes some namespace conflicts
between libiscsi.h and iscsi_target_core.h, but those can be solved
easily enough.

I'm still leaning towards just keeping iscsi_transport.h definitions
local to drivers/target/iscsi, if there is really not going to be other
drivers aside from ib_isert that end up using it.  If there was interest
in traditional iscsi-target HW offloads using this interface then the
story would be different, but every HW offload that I've seen thus far
using LIO is based on out-of-tree NIC code to start.

--nab







 

 
 Or.
 
 Or.
 --
 To unsubscribe from this list: send the line unsubscribe target-devel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-07 Thread Nicholas A. Bellinger 
From: Nicholas Bellinger n...@linux-iscsi.org

Add basic struct iscsit_transport API template to allow iscsi-target for
running with external transport modules using existing iscsi_target_core.h
code.

For all external modules, this calls try_module_get() and module_put()
to obtain + release an external iscsit_transport module reference count.

Also include the iscsi-target symbols necessary in iscsi_transport.h to
allow external transport modules to function.

Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
 drivers/target/iscsi/Makefile |3 +-
 drivers/target/iscsi/iscsi_target_transport.c |   57 ++
 include/target/iscsi/iscsi_transport.h|   77 +
 3 files changed, 136 insertions(+), 1 deletions(-)
 create mode 100644 drivers/target/iscsi/iscsi_target_transport.c
 create mode 100644 include/target/iscsi/iscsi_transport.h

diff --git a/drivers/target/iscsi/Makefile b/drivers/target/iscsi/Makefile
index 5b9a2cf..13a9240 100644
--- a/drivers/target/iscsi/Makefile
+++ b/drivers/target/iscsi/Makefile
@@ -15,6 +15,7 @@ iscsi_target_mod-y += iscsi_target_parameters.o \
iscsi_target_util.o \
iscsi_target.o \
iscsi_target_configfs.o \
-   iscsi_target_stat.o
+   iscsi_target_stat.o \
+   iscsi_target_transport.o
 
 obj-$(CONFIG_ISCSI_TARGET) += iscsi_target_mod.o
diff --git a/drivers/target/iscsi/iscsi_target_transport.c 
b/drivers/target/iscsi/iscsi_target_transport.c
new file mode 100644
index 000..4ffd965
--- /dev/null
+++ b/drivers/target/iscsi/iscsi_target_transport.c
@@ -0,0 +1,57 @@
+#include linux/spinlock.h
+#include linux/list.h
+#include target/iscsi/iscsi_transport.h
+
+static LIST_HEAD(g_transport_list);
+static DEFINE_MUTEX(transport_mutex);
+
+struct iscsit_transport *iscsit_get_transport(int type)
+{
+   struct iscsit_transport *t;
+
+   mutex_lock(transport_mutex);
+   list_for_each_entry(t, g_transport_list, t_node) {
+   if (t-transport_type == type) {
+   if (t-owner  !try_module_get(t-owner)) {
+   t = NULL;
+   }
+   mutex_unlock(transport_mutex);
+   return t;
+   }
+   }
+   mutex_unlock(transport_mutex);
+
+   return NULL;
+}
+EXPORT_SYMBOL(iscsit_get_transport);
+
+void iscsit_put_transport(struct iscsit_transport *t)
+{
+   if (t-owner)
+   module_put(t-owner);
+}
+EXPORT_SYMBOL(iscsit_put_transport);
+
+int iscsit_create_transport(struct iscsit_transport *t)
+{
+   INIT_LIST_HEAD(t-t_node);
+
+   mutex_lock(transport_mutex);
+   list_add_tail(t-t_node, g_transport_list);
+   mutex_unlock(transport_mutex);
+
+   printk(Created iSCSI transport: %s\n, t-name);
+
+   return 0;
+}
+EXPORT_SYMBOL(iscsit_create_transport);
+
+void iscsit_destroy_transport(struct iscsit_transport *t)
+{
+   mutex_lock(transport_mutex);
+   list_del(t-t_node);
+   mutex_unlock(transport_mutex);
+
+   printk(Destroyed iSCSI transport: %s\n, t-name);
+}
+EXPORT_SYMBOL(iscsit_destroy_transport);
diff --git a/include/target/iscsi/iscsi_transport.h 
b/include/target/iscsi/iscsi_transport.h
new file mode 100644
index 000..c885376
--- /dev/null
+++ b/include/target/iscsi/iscsi_transport.h
@@ -0,0 +1,77 @@
+#include linux/module.h
+#include linux/list.h
+#include ../../../drivers/target/iscsi/iscsi_target_core.h
+
+struct iscsit_transport {
+#define ISCSIT_TRANSPORT_NAME  16
+   char name[ISCSIT_TRANSPORT_NAME];
+   int transport_type;
+   struct module *owner;
+   struct list_head t_node;
+   int (*iscsit_setup_np)(struct iscsi_np *, struct 
__kernel_sockaddr_storage *);
+   int (*iscsit_accept_np)(struct iscsi_np *, struct iscsi_conn *);
+   void (*iscsit_free_np)(struct iscsi_np *);
+   void (*iscsit_free_conn)(struct iscsi_conn *);
+   struct iscsi_cmd *(*iscsit_alloc_cmd)(struct iscsi_conn *, gfp_t);
+   void (*iscsit_unmap_cmd)(struct iscsi_cmd *, struct iscsi_conn *);
+   void (*iscsit_free_cmd)(struct iscsi_cmd *);
+   int (*iscsit_get_login_rx)(struct iscsi_conn *, struct iscsi_login *);
+   int (*iscsit_put_login_tx)(struct iscsi_conn *, struct iscsi_login *, 
u32);
+   int (*iscsit_immediate_queue)(struct iscsi_conn *, struct iscsi_cmd *, 
int);
+   int (*iscsit_response_queue)(struct iscsi_conn *, struct iscsi_cmd *, 
int);
+};
+
+/*
+ * From iscsi_target_transport.c
+ */
+
+extern int iscsit_create_transport(struct iscsit_transport *);
+extern void iscsit_destroy_transport(struct iscsit_transport *);
+extern struct iscsit_transport *iscsit_get_transport(int);
+extern void iscsit_put_transport(struct iscsit_transport *);
+
+/*
+ * From iscsi_target.c
+ */
+extern int

Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-07 Thread Roland Dreier 
On Thu, Mar 7, 2013 at 5:45 PM, Nicholas A. Bellinger
n...@linux-iscsi.org wrote:
 +EXPORT_SYMBOL(iscsit_get_transport);

It's not clear to me why this needs to be exported.  Who would use it
outside the core iscsi target module?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 01/11] iscsi-target: Add iscsit_transport API template

2013-03-07 Thread Nicholas A. Bellinger 
On Thu, 2013-03-07 at 20:14 -0800, Roland Dreier wrote:
 On Thu, Mar 7, 2013 at 5:45 PM, Nicholas A. Bellinger
 n...@linux-iscsi.org wrote:
  +EXPORT_SYMBOL(iscsit_get_transport);
 
 It's not clear to me why this needs to be exported.  Who would use it
 outside the core iscsi target module?

Oversight on my part.  Dropping the unnecessary export of
iscsit_get_transport() and iscsit_put_transport() from iscsi-target code
now..

Also, realistically, I don't expect module code beyond ib_isert.ko to
ever use the definitions in include/target/iscsi/ either.

Or and I discussed this point in the last status call, and given what
the initiator did originally (eg: export iscsi_transport) he asked to
keep it under drivers/infiniband/ulp/isert/ with the extra include bits.

I'd have a slight preference to move iser-target code under
drivers/target/iscsi/, and not put anything into include/target/iscsi/
if there won't be another module that uses it..

Do you have a preference here..?

--nab






--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html