[RFC PATCH v4 0/5] Add new fcoe_sysfs based control interfaces to libfcoe, bnx2fc and fcoe

[RFC PATCH v4 0/5] Add new fcoe_sysfs based control interfaces to libfcoe, bnx2fc and fcoe

[Robert Love]

v4:

@Neil:
	Policy is now:

	'mode' attribute:
	       input: "Fabric" "VN2VN" (case insensitive)
	       output: "Fabric" "VN2VN"

	'enabled' attribute:
	       input: "1" "0"
	       output: "1" "0"

@Mark:
	Initial patch now optimizes enum-to-string memory usage and
	string retreival

--

v3:

This series applies to the v3.6 kernel.

@Bart: Fixed bus_create_file -> bus_attrs
       Removed sscanf with non-NULL buffer, only checking for '1' or '0' now
       Removed unnecessary whitespace change

@Bhanu: Incorporated check in _bnx2fc_create (thanks for the code)

Additional changes: Added check for 'enabled' in reset in fcoe.c
	   	    Made mode strncmp case insensitive so user can
		    # echo "vn2vn" > /sys/.../ctlr_0/mode # or
		    # echo "VN2VN" > /sys/.../ctlr_0/mode # or even
		    # echo "FaBRiC" > /sys/.../ctlr_0/mode

--

v2:

The following series adds /sys/bus/fcoe based control interfaces to
libfcoe. A fcoe_sysfs infrastructure was added to the kernel a few
cycles ago, this series builds on that work. The patches deprecate
the old create, vn2vn_create, destroy, enable and disable interfaces
that exist in /sys/module/libfcoe/parameters/.

Another goal of this series is to change the initialization sequence for
a FCoE device. The result of this series is that interfaces created using
libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
starting steps-

1) Create/alloc the FCoE Controller
   - Allocate kernel memory and create per-instance sysfs devices
   - The FCoE Controller is created disabled, no discovery or login
     until it is enabled.

   # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_create

2) Configure the FCoE Controller
   - Change mode, set ddp_min (future), set dcb_required (future), etc...

   # echo 2 > /sys/bus/fcoe/ctlr_0/mode #sets mode to VN2VN

3) Enable the FCoE Controller
   - Begins discovery and login in 'Fabric' mode. or
   - Begins FIP FCID claim process, discovery and login in 'VN2VN' mode

4) Destroy the FCoE Controller
   - Logout and free all memory

   # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_destroy

This series converts both fcoe.ko and bnx2fc.ko to use the new
fcoe_sysfs based interfaces. The legacy interfaces can remain in kernel
for a kernel cycle or two before being removed.

I'm looking for feedback on the use of /sys/bus/fcoe/ctlr_create and
/sys/bus/fcoe/ctlr_destroy and that those interfaces take an ifname.
I modeled it after the bonding interface, but I'm not sure if that's
acceptible.

Incorpoated v1 feedback:

@Chris:

I spent a lot of time looking into FCF selection.

I implemented a POC series where I made the 'enabled' attribute
of a fcoe_fcf_device (i.e. /sys/bus/fcoe/devices/fcf_X) writable. The
fcoe_ctlr_device (i.e. /sys/bus/fcoe/devices/ctlr_X) has a
'selection_strategy' attribute that would allow the user to toggle between
AUTO (current kernel selection algorithm) and USER (user writes to FCF's
'selection' attribute).

I also wrote a little libudev based application that listened for fcoe_fcf_device
sysfs add/remove events. My plan was to have fcoemon monitor the discovery
of FCFs and then have it write to the 'selected' attribute of the FCF the user
had chosen.

Working on this series convinced me that any FCF selection needs further
consideration. First of all, it's fairly complicated to update the fcoe_ctlr.c
functional FIP code to handle FCF/fabric selection. Some questions that arise:

What triggers FLOGI/LOGO? We would now have link, enabled/disabled, selection
strategy and FCF selection to consider.

Can a new FCF be selected when one is already selected and an FLOGI has occurred?
Can a FCF be de-selected when the FLOGI has been sent?

Maybe we can only change things when disabled, that probably makes the most sense..

When does discovery happen? When the ctlr is created (no opportunity for
mode/selection strategy to be set)? When the mode is changed (same problem)?
What about when the cltr is enabled (but that's when we were going to FLOGI)?

This isn't a complete list of considerations, just what came to mind when writing
this. Anyway, the policies started to get complicated, or maybe my lack of policies
made the POC implementation more complicated. Either way it made me think about
use cases and how valuable FCF/fabric selection is.

After further consideration I think that most of the access decisions, when
connecting to a FC fabric, should be done by the fabric services. I'm not sure
the endstations should be whitelisting or blacklisting FCFs or even making
decisions about which ones to use when they're already prioritized amongst themselves
(on the same fabric). I think it might be nice for debugging, but I don't have a
need at the moment.

I think user selection may be more valuable with VN2VN, which may interest you
more anyway, as you said that you were running a VN2VN setup. Since there
aren't fabric services to rely on I can see it useful for VN_Ports to whitelist or
blacklist other VN_Ports. I think the first step to support something like this
would be to represent the fcoe_rports in fcoe_sysfs or in the FC Transport such
that they can be selected or de-slected.

I think that's a fine goal, but with this series, I think I'd like to focus on
just getting away from using module parameters for control interfaces. I think
this current series allows for selection as I've described above since it will
now start the FCoE Controller in a DISABLED state such that configurations can
now be made before the FLOGI.

@Bhanu

I implemented the changes for bnx2fc and I think it should be mostly fine. I
wasn't able to test the code, so I'd appreciate any feedback about whether
there are bugs or not.

@Bart:

Fixed the 'const char *p' and cast issue in fcoe_parse_mode().

Now checking for string length and passing correctly NULL terminated string
to fcoe_parse_mode().

Thanks, //Rob

---

v1 covermail

The following series implements a move from using module parameters
as control interfaces to /sys/bus/fcoe based interfaces. A sysfs infrastructure
was added to the kernel a few cycles ago, this series builds on that work.

It moves the create, vn2vn_create, destroy, enable and disable interfaces
from /sys/module/libfcoe/parameters/ to various places under /sys/bus/fcoe/.
These interfaces simply are not module configurations- they are control
interfaces.

A second goal of this series is to change the initialization sequence for
a FCoE device. The result of this series is that interfaces created using
libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
starting steps-

1) Create/alloc the port
   - Allocate kernel memory and create per-instance sysfs devices
   - No discovery or login

2) Configure the port
   - Change mode, set ddp_min, etc...

3) Start the port
   - Begins discovery and/or login (depending on mode)

4) Destroy the port
   - Logout and free all memory

I'm looking for feedback on using sysfs files as control interfaces that
the user (application) would write interface names to. I modeled this
series off of the bonding sysfs interface, but it was suggested to me that
it might not be a good example. I belive bonding uses two values per-file
a '+' or a '-" to add or delete and then the ifname apended. I am simply
writing the ifname to the ctlr_create or ctlr_destroy.

Series compiled and tested against v3.5. libfcoe.ko compile warning fixed
upstream after v3.5, anyone who compiles this can ignore section mismatch
warning. Also note that a modified fcoemon is needed to use the fcoe system
service against this kernel modification. I'd be happy to provide that
fcoemon code on request.

---

Robert Love (5):
      libfcoe: Save some memory and optimize name lookups
      libfcoe: Add fcoe_sysfs debug logging level
      libfcoe, fcoe, bnx2fc: Add new fcoe control interface
      fcoe: Use the fcoe_sysfs control interface
      bnx2fc: Use the fcoe_sysfs control interface


 Documentation/ABI/testing/sysfs-bus-fcoe |   42 +++++++
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c        |  169 ++++++++++++++++++++++-----
 drivers/scsi/fcoe/fcoe.c                 |  148 +++++++++++++++++++++---
 drivers/scsi/fcoe/fcoe_ctlr.c            |   17 +--
 drivers/scsi/fcoe/fcoe_sysfs.c           |  186 +++++++++++++++++++++++++-----
 drivers/scsi/fcoe/fcoe_transport.c       |  104 +++++++++++++++++
 drivers/scsi/fcoe/libfcoe.h              |   11 +-
 include/scsi/fcoe_sysfs.h                |   11 ++
 include/scsi/libfcoe.h                   |   16 ++-
 9 files changed, 608 insertions(+), 96 deletions(-)

-- 

Thanks, //Rob

[RFC PATCH v4 1/5] libfcoe: Save some memory and optimize name lookups

[Robert Love]

Instead of creating a structure with an enum and a pointer
to a string, simply allocate an array of strings and use
the enum values for the indicies.

This means that we do not need to iterate through the list
of entries when looking up a string name by its enum key.

This will also help with a latter patch that will add
more fcoe_sysfs attributes that will also use the
fcoe_enum_name_search macro. One attribute will also do
a reverse lookup which requires less code when the
enum-to-string mappings are organized as this patch makes
them to be.

Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe_sysfs.c |   44 +++++++++++++++-------------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 5e75168..5a74a47 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -210,25 +210,23 @@ static ssize_t show_fcoe_fcf_device_##field(struct device *dev,	\
 #define fcoe_enum_name_search(title, table_type, table)			\
 static const char *get_fcoe_##title##_name(enum table_type table_key)	\
 {									\
-	int i;								\
-	char *name = NULL;						\
-									\
-	for (i = 0; i < ARRAY_SIZE(table); i++) {			\
-		if (table[i].value == table_key) {			\
-			name = table[i].name;				\
-			break;						\
-		}							\
-	}								\
-	return name;							\
+	if (table_key >= ARRAY_SIZE(table))				\
+		return NULL;						\
+	return table[table_key];					\
 }
 
-static struct {
-	enum fcf_state value;
-	char           *name;
-} fcf_state_names[] = {
-	{ FCOE_FCF_STATE_UNKNOWN,      "Unknown" },
-	{ FCOE_FCF_STATE_DISCONNECTED, "Disconnected" },
-	{ FCOE_FCF_STATE_CONNECTED,    "Connected" },
+static char *fip_conn_type_names[] = {
+	[ FIP_CONN_TYPE_UNKNOWN ] = "Unknown",
+        [ FIP_CONN_TYPE_FABRIC ]  = "Fabric",
+	[ FIP_CONN_TYPE_VN2VN ]   = "VN2VN",
+};
+fcoe_enum_name_search(ctlr_mode, fip_conn_type, fip_conn_type_names)
+#define FCOE_CTLR_MODE_MAX_NAMELEN 50
+
+static char *fcf_state_names[] = {
+	[ FCOE_FCF_STATE_UNKNOWN ]      = "Unknown",
+	[ FCOE_FCF_STATE_DISCONNECTED ] = "Disconnected",
+	[ FCOE_FCF_STATE_CONNECTED ]    = "Connected",
 };
 fcoe_enum_name_search(fcf_state, fcf_state, fcf_state_names)
 #define FCOE_FCF_STATE_MAX_NAMELEN 50
@@ -246,17 +244,7 @@ static ssize_t show_fcf_state(struct device *dev,
 }
 static FCOE_DEVICE_ATTR(fcf, state, S_IRUGO, show_fcf_state, NULL);
 
-static struct {
-	enum fip_conn_type value;
-	char               *name;
-} fip_conn_type_names[] = {
-	{ FIP_CONN_TYPE_UNKNOWN, "Unknown" },
-	{ FIP_CONN_TYPE_FABRIC, "Fabric" },
-	{ FIP_CONN_TYPE_VN2VN, "VN2VN" },
-};
-fcoe_enum_name_search(ctlr_mode, fip_conn_type, fip_conn_type_names)
-#define FCOE_CTLR_MODE_MAX_NAMELEN 50
-
+#define FCOE_MAX_MODENAME_LEN 20
 static ssize_t show_ctlr_mode(struct device *dev,
 			      struct device_attribute *attr,
 			      char *buf)

[RFC PATCH v4 2/5] libfcoe: Add fcoe_sysfs debug logging level

[Robert Love]

Add a macro to print fcoe_sysfs debug statements.

Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe_sysfs.c |    7 +++++++
 drivers/scsi/fcoe/libfcoe.h    |   11 ++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 5a74a47..638e8a2 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -24,6 +24,13 @@
 
 #include <scsi/fcoe_sysfs.h>
 
+/*
+ * OK to include local libfcoe.h for debug_logging, but cannot include
+ * <scsi/libfcoe.h> otherwise non-netdev based fcoe solutions would have
+ * have to include more than fcoe_sysfs.h.
+ */
+#include "libfcoe.h"
+
 static atomic_t ctlr_num;
 static atomic_t fcf_num;
 
diff --git a/drivers/scsi/fcoe/libfcoe.h b/drivers/scsi/fcoe/libfcoe.h
index 6af5fc3..63d8fae 100644
--- a/drivers/scsi/fcoe/libfcoe.h
+++ b/drivers/scsi/fcoe/libfcoe.h
@@ -2,9 +2,10 @@
 #define _FCOE_LIBFCOE_H_
 
 extern unsigned int libfcoe_debug_logging;
-#define LIBFCOE_LOGGING	    0x01 /* General logging, not categorized */
-#define LIBFCOE_FIP_LOGGING 0x02 /* FIP logging */
-#define LIBFCOE_TRANSPORT_LOGGING	0x04 /* FCoE transport logging */
+#define LIBFCOE_LOGGING	          0x01 /* General logging, not categorized */
+#define LIBFCOE_FIP_LOGGING       0x02 /* FIP logging */
+#define LIBFCOE_TRANSPORT_LOGGING 0x04 /* FCoE transport logging */
+#define LIBFCOE_SYSFS_LOGGING     0x08 /* fcoe_sysfs logging */
 
 #define LIBFCOE_CHECK_LOGGING(LEVEL, CMD)		\
 do {							\
@@ -28,4 +29,8 @@ do {							\
 			      printk(KERN_INFO "%s: " fmt,		\
 				     __func__, ##args);)
 
+#define LIBFCOE_SYSFS_DBG(cdev, fmt, args...)				\
+	LIBFCOE_CHECK_LOGGING(LIBFCOE_SYSFS_LOGGING,			\
+			      pr_info("ctlr_%d: " fmt, cdev->id, ##args);)
+
 #endif /* _FCOE_LIBFCOE_H_ */

[RFC PATCH v4 3/5] libfcoe, fcoe, bnx2fc: Add new fcoe control interface

[Robert Love]

This patch does a few things.

1) Makes /sys/bus/fcoe/ctlr_{create,destroy} interfaces.
   These interfaces take an <ifname> and will either
   create an FCoE Controller or destroy an FCoE
   Controller depending on which file is written to.

   The new FCoE Controller will start in a DISABLED
   state and will not do discovery or login until it
   is ENABLED. This pause will allow us to configure
   the FCoE Controller before enabling it.

2) Makes the 'mode' attribute of a fcoe_ctlr_device
   writale. This allows the user to configure the mode
   in which the FCoE Controller will start in when it
   is ENABLED.

   Possible modes are 'Fabric', or 'VN2VN'.

   The default mode for a fcoe_ctlr{,_device} is 'Fabric'.
   Drivers must implement the set_fcoe_ctlr_mode routine
   to support this feature.

   libfcoe offers an exported routine to set a FCoE
   Controller's mode. The mode can only be changed
   when the FCoE Controller is DISABLED.

   This patch also removes the get_fcoe_ctlr_mode pointer
   in the fcoe_sysfs function template, the code in
   fcoe_ctlr.c to get the mode and the assignment of
   the fcoe_sysfs function pointer to the fcoe_ctlr.c
   implementation (in fcoe and bnx2fc). fcoe_sysfs can
   return that value for the mode without consulting the
   LLD.

3) Make a 'enabled' attribute of a fcoe_ctlr_device. On a
   read, fcoe_sysfs will return the attribute's value. On
   a write, fcoe_sysfs will call the LLD (if there is a
   callback) to notifiy that the enalbed state has changed.

This patch maintains the old FCoE control interfaces as
module parameters, but it adds comments pointing out that
the old interfaces are deprecated.

Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-fcoe |   42 +++++++++
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c        |    1 
 drivers/scsi/fcoe/fcoe.c                 |    1 
 drivers/scsi/fcoe/fcoe_sysfs.c           |  137 ++++++++++++++++++++++++++++--
 drivers/scsi/fcoe/fcoe_transport.c       |  104 +++++++++++++++++++++++
 include/scsi/fcoe_sysfs.h                |   11 ++
 include/scsi/libfcoe.h                   |   16 +++-
 7 files changed, 299 insertions(+), 13 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-fcoe b/Documentation/ABI/testing/sysfs-bus-fcoe
index 469d09c..a57bf37 100644
--- a/Documentation/ABI/testing/sysfs-bus-fcoe
+++ b/Documentation/ABI/testing/sysfs-bus-fcoe
@@ -1,14 +1,54 @@
+What:		/sys/bus/fcoe/
+Date:		August 2012
+KernelVersion:	TBD
+Contact:	Robert Love <robert.w.love@intel.com>, devel@open-fcoe.org
+Description:	The FCoE bus. Attributes in this directory are control interfaces.
+Attributes:
+
+	ctlr_create: 'FCoE Controller' instance creation interface. Writing an
+		     <ifname> to this file will allocate and populate sysfs with a
+		     fcoe_ctlr_device (ctlr_X). The user can then configure any
+		     per-port settings and finally write to the fcoe_ctlr_device's
+		     'start' attribute to begin the kernel's discovery and login
+		     process.
+
+	ctlr_destroy: 'FCoE Controller' instance removal interface. Writing a
+		       fcoe_ctlr_device's sysfs name to this file will log the
+		       fcoe_ctlr_device out of the fabric or otherwise connected
+		       FCoE devices. It will also free all kernel memory allocated
+		       for this fcoe_ctlr_device and any structures associated
+		       with it, this includes the scsi_host.
+
 What:		/sys/bus/fcoe/ctlr_X
 Date:		March 2012
 KernelVersion:	TBD
 Contact:	Robert Love <robert.w.love@intel.com>, devel@open-fcoe.org
-Description:	'FCoE Controller' instances on the fcoe bus
+Description:	'FCoE Controller' instances on the fcoe bus.
+
+		The FCoE Controller now has a three stage creation process.
+		1) Write interface name to ctlr_create 2) Configure the FCoE
+		Controller (ctlr_X) 3) Enable the FCoE Controller to begin
+		discovery and login. The FCoE Controller is destroyed by
+		writing it's name, i.e. ctlr_X to the ctlr_delete file.
+
 Attributes:
 
 	fcf_dev_loss_tmo: Device loss timeout peroid (see below). Changing
 			  this value will change the dev_loss_tmo for all
 			  FCFs discovered by this controller.
 
+	mode:		  Display or change the FCoE Controller's mode. Possible
+			  modes are 'Fabric' and 'VN2VN'. If a FCoE Controller
+			  is started in 'Fabric' mode then FIP FCF discovery is
+			  initiated and ultimately a fabric login is attempted.
+			  If a FCoE Controller is started in 'VN2VN' mode then
+			  FIP VN2VN discovery and login is performed. A FCoE
+			  Controller only supports one mode at a time.
+
+	enabled:	  Whether an FCoE controller is enabled or disabled.
+			  0 if disabled, 1 if enabled. Writing either 0 or 1
+			  to this file will enable or disable the FCoE controller.
+
 	lesb_link_fail:   Link Error Status Block (LESB) link failure count.
 
 	lesb_vlink_fail:  Link Error Status Block (LESB) virtual link
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index ae1cb76..97d9a58 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2555,7 +2555,6 @@ module_init(bnx2fc_mod_init);
 module_exit(bnx2fc_mod_exit);
 
 static struct fcoe_sysfs_function_template bnx2fc_fcoe_sysfs_templ = {
-	.get_fcoe_ctlr_mode = fcoe_ctlr_get_fip_mode,
 	.get_fcoe_ctlr_link_fail = bnx2fc_ctlr_get_lesb,
 	.get_fcoe_ctlr_vlink_fail = bnx2fc_ctlr_get_lesb,
 	.get_fcoe_ctlr_miss_fka = bnx2fc_ctlr_get_lesb,
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 078d262..99e9d02 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -155,7 +155,6 @@ static void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *);
 static void fcoe_fcf_get_vlan_id(struct fcoe_fcf_device *);
 
 static struct fcoe_sysfs_function_template fcoe_sysfs_templ = {
-	.get_fcoe_ctlr_mode = fcoe_ctlr_get_fip_mode,
 	.get_fcoe_ctlr_link_fail = fcoe_ctlr_get_lesb,
 	.get_fcoe_ctlr_vlink_fail = fcoe_ctlr_get_lesb,
 	.get_fcoe_ctlr_miss_fka = fcoe_ctlr_get_lesb,
diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 638e8a2..9196444 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -21,8 +21,10 @@
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/etherdevice.h>
+#include <linux/ctype.h>
 
 #include <scsi/fcoe_sysfs.h>
+#include <scsi/libfcoe.h>
 
 /*
  * OK to include local libfcoe.h for debug_logging, but cannot include
@@ -78,6 +80,8 @@ MODULE_PARM_DESC(fcf_dev_loss_tmo,
 	((x)->lesb.lesb_err_block)
 #define fcoe_ctlr_fcs_error(x)			\
 	((x)->lesb.lesb_fcs_error)
+#define fcoe_ctlr_enabled(x)			\
+	((x)->enabled)
 #define fcoe_fcf_state(x)			\
 	((x)->state)
 #define fcoe_fcf_fabric_name(x)			\
@@ -228,7 +232,18 @@ static char *fip_conn_type_names[] = {
 	[ FIP_CONN_TYPE_VN2VN ]   = "VN2VN",
 };
 fcoe_enum_name_search(ctlr_mode, fip_conn_type, fip_conn_type_names)
-#define FCOE_CTLR_MODE_MAX_NAMELEN 50
+
+static enum fip_conn_type fcoe_parse_mode(const char *buf)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(fip_conn_type_names); i++) {
+		if (strcasecmp(buf, fip_conn_type_names[i]) == 0)
+			return i;
+	}
+
+	return FIP_CONN_TYPE_UNKNOWN;
+}
 
 static char *fcf_state_names[] = {
 	[ FCOE_FCF_STATE_UNKNOWN ]      = "Unknown",
@@ -259,17 +274,116 @@ static ssize_t show_ctlr_mode(struct device *dev,
 	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
 	const char *name;
 
-	if (ctlr->f->get_fcoe_ctlr_mode)
-		ctlr->f->get_fcoe_ctlr_mode(ctlr);
-
 	name = get_fcoe_ctlr_mode_name(ctlr->mode);
 	if (!name)
 		return -EINVAL;
-	return snprintf(buf, FCOE_CTLR_MODE_MAX_NAMELEN,
+	return snprintf(buf, FCOE_MAX_MODENAME_LEN,
 			"%s\n", name);
 }
-static FCOE_DEVICE_ATTR(ctlr, mode, S_IRUGO,
-			show_ctlr_mode, NULL);
+
+static ssize_t store_ctlr_mode(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
+	char mode[FCOE_MAX_MODENAME_LEN + 1];
+
+	if (count > FCOE_MAX_MODENAME_LEN)
+		return -EINVAL;
+
+	strncpy(mode, buf, count);
+
+	if (mode[count - 1] == '\n')
+		mode[count - 1] = '\0';
+	else
+		mode[count] = '\0';
+
+	switch (ctlr->enabled) {
+	case FCOE_CTLR_ENABLED:
+		LIBFCOE_SYSFS_DBG(ctlr, "Cannot change mode when enabled.");
+		return -EBUSY;
+	case FCOE_CTLR_DISABLED:
+		if (!ctlr->f->set_fcoe_ctlr_mode) {
+			LIBFCOE_SYSFS_DBG(ctlr,
+					  "Mode change not supported by LLD.");
+			return -ENOTSUPP;
+		}
+
+		ctlr->mode = fcoe_parse_mode(mode);
+		if (ctlr->mode == FIP_CONN_TYPE_UNKNOWN) {
+			LIBFCOE_SYSFS_DBG(ctlr,
+					  "Unknown mode %s provided.", buf);
+			return -EINVAL;
+		}
+
+		ctlr->f->set_fcoe_ctlr_mode(ctlr);
+		LIBFCOE_SYSFS_DBG(ctlr, "Mode changed to %s.", buf);
+
+		return count;
+	case FCOE_CTLR_UNUSED:
+	default:
+		LIBFCOE_SYSFS_DBG(ctlr, "Mode change not supported.");
+		return -ENOTSUPP;
+	};
+}
+
+static FCOE_DEVICE_ATTR(ctlr, mode, S_IRUGO | S_IWUSR,
+			show_ctlr_mode, store_ctlr_mode);
+
+static ssize_t store_ctlr_enabled(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
+	int rc;
+
+	switch (ctlr->enabled) {
+	case FCOE_CTLR_ENABLED:
+		if (*buf == '1')
+			return count;
+		ctlr->enabled = FCOE_CTLR_DISABLED;
+		break;
+	case FCOE_CTLR_DISABLED:
+		if (*buf == '0')
+			return count;
+		ctlr->enabled = FCOE_CTLR_ENABLED;
+		break;
+	case FCOE_CTLR_UNUSED:
+		return -ENOTSUPP;
+	};
+
+	rc = ctlr->f->set_fcoe_ctlr_enabled(ctlr);
+	if (rc)
+		return rc;
+
+	return count;
+}
+
+static char *ctlr_enabled_state_names[] = {
+	[ FCOE_CTLR_ENABLED ]  = "1",
+	[ FCOE_CTLR_DISABLED ] = "0",
+};
+fcoe_enum_name_search(ctlr_enabled_state, ctlr_enabled_state,
+		      ctlr_enabled_state_names)
+#define FCOE_CTLR_ENABLED_MAX_NAMELEN 50
+
+static ssize_t show_ctlr_enabled_state(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
+	const char *name;
+
+	name = get_fcoe_ctlr_enabled_state_name(ctlr->enabled);
+	if (!name)
+		return -EINVAL;
+	return snprintf(buf, FCOE_CTLR_ENABLED_MAX_NAMELEN,
+			"%s\n", name);
+}
+
+static FCOE_DEVICE_ATTR(ctlr, enabled, S_IRUGO | S_IWUSR,
+			show_ctlr_enabled_state,
+			store_ctlr_enabled);
 
 static ssize_t
 store_private_fcoe_ctlr_fcf_dev_loss_tmo(struct device *dev,
@@ -354,6 +468,7 @@ static struct attribute_group fcoe_ctlr_lesb_attr_group = {
 
 static struct attribute *fcoe_ctlr_attrs[] = {
 	&device_attr_fcoe_ctlr_fcf_dev_loss_tmo.attr,
+	&device_attr_fcoe_ctlr_enabled.attr,
 	&device_attr_fcoe_ctlr_mode.attr,
 	NULL,
 };
@@ -438,9 +553,16 @@ struct device_type fcoe_fcf_device_type = {
 	.release = fcoe_fcf_device_release,
 };
 
+struct bus_attribute fcoe_bus_attr_group[] = {
+	__ATTR(ctlr_create, S_IWUSR, NULL, fcoe_ctlr_create_store),
+	__ATTR(ctlr_destroy, S_IWUSR, NULL, fcoe_ctlr_destroy_store),
+	__ATTR_NULL
+};
+
 struct bus_type fcoe_bus_type = {
 	.name = "fcoe",
 	.match = &fcoe_bus_match,
+	.bus_attrs = fcoe_bus_attr_group,
 };
 
 /**
@@ -561,6 +683,7 @@ struct fcoe_ctlr_device *fcoe_ctlr_device_add(struct device *parent,
 
 	ctlr->id = atomic_inc_return(&ctlr_num) - 1;
 	ctlr->f = f;
+	ctlr->mode = FIP_CONN_TYPE_FABRIC;
 	INIT_LIST_HEAD(&ctlr->fcfs);
 	mutex_init(&ctlr->lock);
 	ctlr->dev.parent = parent;
diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index ac76d8a..83e78f9 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -627,6 +627,110 @@ static int libfcoe_device_notification(struct notifier_block *notifier,
 	return NOTIFY_OK;
 }
 
+ssize_t fcoe_ctlr_create_store(struct bus_type *bus,
+			       const char *buf, size_t count)
+{
+	struct net_device *netdev = NULL;
+	struct fcoe_transport *ft = NULL;
+	struct fcoe_ctlr_device *ctlr_dev = NULL;
+	int rc = 0;
+	int err;
+
+	mutex_lock(&ft_mutex);
+
+	netdev = fcoe_if_to_netdev(buf);
+	if (!netdev) {
+		LIBFCOE_TRANSPORT_DBG("Invalid device %s.\n", buf);
+		rc = -ENODEV;
+		goto out_nodev;
+	}
+
+	ft = fcoe_netdev_map_lookup(netdev);
+	if (ft) {
+		LIBFCOE_TRANSPORT_DBG("transport %s already has existing "
+				      "FCoE instance on %s.\n",
+				      ft->name, netdev->name);
+		rc = -EEXIST;
+		goto out_putdev;
+	}
+
+	ft = fcoe_transport_lookup(netdev);
+	if (!ft) {
+		LIBFCOE_TRANSPORT_DBG("no FCoE transport found for %s.\n",
+				      netdev->name);
+		rc = -ENODEV;
+		goto out_putdev;
+	}
+
+	/* pass to transport create */
+	err = ft->alloc ? ft->alloc(netdev) : -ENODEV;
+	if (err) {
+		fcoe_del_netdev_mapping(netdev);
+		rc = -ENOMEM;
+		goto out_putdev;
+	}
+
+	err = fcoe_add_netdev_mapping(netdev, ft);
+	if (err) {
+		LIBFCOE_TRANSPORT_DBG("failed to add new netdev mapping "
+				      "for FCoE transport %s for %s.\n",
+				      ft->name, netdev->name);
+		rc = -ENODEV;
+		goto out_putdev;
+	}
+
+	LIBFCOE_TRANSPORT_DBG("transport %s %s to create fcoe on %s.\n",
+			      ft->name, (ctlr_dev) ? "succeeded" : "failed",
+			      netdev->name);
+
+out_putdev:
+	dev_put(netdev);
+out_nodev:
+	mutex_unlock(&ft_mutex);
+	if (rc)
+		return rc;
+	return count;
+}
+
+ssize_t fcoe_ctlr_destroy_store(struct bus_type *bus,
+				const char *buf, size_t count)
+{
+	int rc = -ENODEV;
+	struct net_device *netdev = NULL;
+	struct fcoe_transport *ft = NULL;
+
+	mutex_lock(&ft_mutex);
+
+	netdev = fcoe_if_to_netdev(buf);
+	if (!netdev) {
+		LIBFCOE_TRANSPORT_DBG("invalid device %s.\n", buf);
+		goto out_nodev;
+	}
+
+	ft = fcoe_netdev_map_lookup(netdev);
+	if (!ft) {
+		LIBFCOE_TRANSPORT_DBG("no FCoE transport found for %s.\n",
+				      netdev->name);
+		goto out_putdev;
+	}
+
+	/* pass to transport destroy */
+	rc = ft->destroy(netdev);
+	if (rc)
+		goto out_putdev;
+
+	fcoe_del_netdev_mapping(netdev);
+	LIBFCOE_TRANSPORT_DBG("transport %s %s to destroy fcoe on %s.\n",
+			      ft->name, (rc) ? "failed" : "succeeded",
+			      netdev->name);
+	rc = count; /* required for successful return */
+out_putdev:
+	dev_put(netdev);
+out_nodev:
+	mutex_unlock(&ft_mutex);
+	return rc;
+}
+EXPORT_SYMBOL(fcoe_ctlr_destroy_store);
 
 /**
  * fcoe_transport_create() - Create a fcoe interface
diff --git a/include/scsi/fcoe_sysfs.h b/include/scsi/fcoe_sysfs.h
index 604cb9b..7e23148 100644
--- a/include/scsi/fcoe_sysfs.h
+++ b/include/scsi/fcoe_sysfs.h
@@ -34,7 +34,8 @@ struct fcoe_sysfs_function_template {
 	void (*get_fcoe_ctlr_symb_err)(struct fcoe_ctlr_device *);
 	void (*get_fcoe_ctlr_err_block)(struct fcoe_ctlr_device *);
 	void (*get_fcoe_ctlr_fcs_error)(struct fcoe_ctlr_device *);
-	void (*get_fcoe_ctlr_mode)(struct fcoe_ctlr_device *);
+	void (*set_fcoe_ctlr_mode)(struct fcoe_ctlr_device *);
+	int  (*set_fcoe_ctlr_enabled)(struct fcoe_ctlr_device *);
 	void (*get_fcoe_fcf_selected)(struct fcoe_fcf_device *);
 	void (*get_fcoe_fcf_vlan_id)(struct fcoe_fcf_device *);
 };
@@ -48,6 +49,12 @@ enum fip_conn_type {
 	FIP_CONN_TYPE_VN2VN,
 };
 
+enum ctlr_enabled_state {
+	FCOE_CTLR_ENABLED,
+	FCOE_CTLR_DISABLED,
+	FCOE_CTLR_UNUSED,
+};
+
 struct fcoe_ctlr_device {
 	u32				id;
 
@@ -64,6 +71,8 @@ struct fcoe_ctlr_device {
 	int                             fcf_dev_loss_tmo;
 	enum fip_conn_type              mode;
 
+	enum ctlr_enabled_state         enabled;
+
 	/* expected in host order for displaying */
 	struct fcoe_fc_els_lesb         lesb;
 };
diff --git a/include/scsi/libfcoe.h b/include/scsi/libfcoe.h
index 22b07cc..02dcc05 100644
--- a/include/scsi/libfcoe.h
+++ b/include/scsi/libfcoe.h
@@ -289,8 +289,11 @@ static inline bool is_fip_mode(struct fcoe_ctlr *fip)
  * @attached:	whether this transport is already attached
  * @list:	list linkage to all attached transports
  * @match:	handler to allow the transport driver to match up a given netdev
+ * @alloc:      handler to allocate per-instance FCoE structures
+ *		(no discovery or login)
  * @create:	handler to sysfs entry of create for FCoE instances
- * @destroy:	handler to sysfs entry of destroy for FCoE instances
+ * @destroy:    handler to delete per-instance FCoE structures
+ *		(frees all memory)
  * @enable:	handler to sysfs entry of enable for FCoE instances
  * @disable:	handler to sysfs entry of disable for FCoE instances
  */
@@ -299,6 +302,7 @@ struct fcoe_transport {
 	bool attached;
 	struct list_head list;
 	bool (*match) (struct net_device *device);
+	int (*alloc) (struct net_device *device);
 	int (*create) (struct net_device *device, enum fip_state fip_mode);
 	int (*destroy) (struct net_device *device);
 	int (*enable) (struct net_device *device);
@@ -358,7 +362,7 @@ int fcoe_get_paged_crc_eof(struct sk_buff *skb, int tlen,
 
 /* FCoE Sysfs helpers */
 void fcoe_fcf_get_selected(struct fcoe_fcf_device *);
-void fcoe_ctlr_get_fip_mode(struct fcoe_ctlr_device *);
+void fcoe_ctlr_set_fip_mode(struct fcoe_ctlr_device *);
 
 /**
  * struct netdev_list
@@ -374,4 +378,12 @@ struct fcoe_netdev_mapping {
 int fcoe_transport_attach(struct fcoe_transport *ft);
 int fcoe_transport_detach(struct fcoe_transport *ft);
 
+/* sysfs store handler for ctrl_control interface */
+ssize_t fcoe_ctlr_create_store(struct bus_type *bus,
+			       const char *buf, size_t count);
+ssize_t fcoe_ctlr_destroy_store(struct bus_type *bus,
+				const char *buf, size_t count);
+
 #endif /* _LIBFCOE_H */
+
+

[RFC PATCH v4 4/5] fcoe: Use the fcoe_sysfs control interface

[Robert Love]

This patch adds support for the new fcoe_sysfs
control interface to fcoe.ko. It keeps the deprecated
interface in tact and therefore either the legacy
or the new control interfaces can be used. A mixed mode
is not supported. A user must either use the new
interfaces or the old ones, but not both.

The fcoe_ctlr's link state is now driven by both the
netdev link state as well as the fcoe_ctlr_device's
enabled attribute. The link must be up and the
fcoe_ctlr_device must be enabled before the FCoE
Controller starts discovery or login.

Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c      |  147 +++++++++++++++++++++++++++++++++++++----
 drivers/scsi/fcoe/fcoe_ctlr.c |   17 ++---
 2 files changed, 140 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 99e9d02..995ef96 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -117,6 +117,11 @@ static int fcoe_destroy(struct net_device *netdev);
 static int fcoe_enable(struct net_device *netdev);
 static int fcoe_disable(struct net_device *netdev);
 
+/* fcoe_syfs control interface handlers */
+static int fcoe_ctlr_alloc(struct net_device *netdev);
+static int fcoe_ctlr_enabled(struct fcoe_ctlr_device *cdev);
+
+
 static struct fc_seq *fcoe_elsct_send(struct fc_lport *,
 				      u32 did, struct fc_frame *,
 				      unsigned int op,
@@ -155,6 +160,8 @@ static void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *);
 static void fcoe_fcf_get_vlan_id(struct fcoe_fcf_device *);
 
 static struct fcoe_sysfs_function_template fcoe_sysfs_templ = {
+	.set_fcoe_ctlr_mode = fcoe_ctlr_set_fip_mode,
+	.set_fcoe_ctlr_enabled = fcoe_ctlr_enabled,
 	.get_fcoe_ctlr_link_fail = fcoe_ctlr_get_lesb,
 	.get_fcoe_ctlr_vlink_fail = fcoe_ctlr_get_lesb,
 	.get_fcoe_ctlr_miss_fka = fcoe_ctlr_get_lesb,
@@ -1966,6 +1973,7 @@ static int fcoe_dcb_app_notification(struct notifier_block *notifier,
 static int fcoe_device_notification(struct notifier_block *notifier,
 				    ulong event, void *ptr)
 {
+	struct fcoe_ctlr_device *cdev;
 	struct fc_lport *lport = NULL;
 	struct net_device *netdev = ptr;
 	struct fcoe_ctlr *ctlr;
@@ -2022,13 +2030,29 @@ static int fcoe_device_notification(struct notifier_block *notifier,
 
 	fcoe_link_speed_update(lport);
 
-	if (link_possible && !fcoe_link_ok(lport))
-		fcoe_ctlr_link_up(ctlr);
-	else if (fcoe_ctlr_link_down(ctlr)) {
-		stats = per_cpu_ptr(lport->stats, get_cpu());
-		stats->LinkFailureCount++;
-		put_cpu();
-		fcoe_clean_pending_queue(lport);
+	cdev = fcoe_ctlr_to_ctlr_dev(ctlr);
+
+	if (link_possible && !fcoe_link_ok(lport)) {
+		switch (cdev->enabled) {
+		case FCOE_CTLR_DISABLED:
+			pr_info("Link up while interface is disabled.\n");
+			break;
+		case FCOE_CTLR_ENABLED:
+		case FCOE_CTLR_UNUSED:
+			fcoe_ctlr_link_up(ctlr);
+		};
+	} else if (fcoe_ctlr_link_down(ctlr)) {
+		switch (cdev->enabled) {
+		case FCOE_CTLR_DISABLED:
+			pr_info("Link down while interface is disabled.\n");
+			break;
+		case FCOE_CTLR_ENABLED:
+		case FCOE_CTLR_UNUSED:
+			stats = per_cpu_ptr(lport->stats, get_cpu());
+			stats->LinkFailureCount++;
+			put_cpu();
+			fcoe_clean_pending_queue(lport);
+		};
 	}
 out:
 	return rc;
@@ -2041,6 +2065,8 @@ out:
  * Called from fcoe transport.
  *
  * Returns: 0 for success
+ *
+ * Deprecated: use fcoe_ctlr_enabled()
  */
 static int fcoe_disable(struct net_device *netdev)
 {
@@ -2100,6 +2126,33 @@ out:
 }
 
 /**
+ * fcoe_ctlr_enabled() - Enable or disable an FCoE Controller
+ * @cdev: The FCoE Controller that is being enabled or disabled
+ *
+ * fcoe_sysfs will ensure that the state of 'enabled' has
+ * changed, so no checking is necessary here. This routine simply
+ * calls fcoe_enable or fcoe_disable, both of which are deprecated.
+ * When those routines are removed the functionality can be merged
+ * here.
+ */
+static int fcoe_ctlr_enabled(struct fcoe_ctlr_device *cdev)
+{
+	struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(cdev);
+	struct fc_lport *lport = ctlr->lp;
+	struct net_device *netdev = fcoe_netdev(lport);
+
+	switch (cdev->enabled) {
+	case FCOE_CTLR_ENABLED:
+		return fcoe_enable(netdev);
+	case FCOE_CTLR_DISABLED:
+		return fcoe_disable(netdev);
+	case FCOE_CTLR_UNUSED:
+	default:
+		return -ENOTSUPP;
+	};
+}
+
+/**
  * fcoe_destroy() - Destroy a FCoE interface
  * @netdev  : The net_device object the Ethernet interface to create on
  *
@@ -2207,16 +2260,26 @@ static void fcoe_dcb_create(struct fcoe_interface *fcoe)
 #endif
 }
 
+enum fcoe_create_link_state {
+	FCOE_CREATE_LINK_DOWN,
+	FCOE_CREATE_LINK_UP,
+};
+
 /**
- * fcoe_create() - Create a fcoe interface
- * @netdev  : The net_device object the Ethernet interface to create on
- * @fip_mode: The FIP mode for this creation
+ * _fcoe_create() - (internal) Create a fcoe interface
+ * @netdev  :   The net_device object the Ethernet interface to create on
+ * @fip_mode:   The FIP mode for this creation
+ * @link_state: The ctlr link state on creation
  *
- * Called from fcoe transport
+ * Called from either the libfcoe 'create' module parameter
+ * via fcoe_create or from fcoe_syfs's ctlr_create file.
  *
- * Returns: 0 for success
+ * libfcoe's 'create' module parameter is deprecated so some
+ * consolidation of code can be done when that interface is
+ * removed.
  */
-static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode)
+static int _fcoe_create(struct net_device *netdev, enum fip_state fip_mode,
+			enum fcoe_create_link_state link_state)
 {
 	int rc = 0;
 	struct fcoe_ctlr_device *ctlr_dev;
@@ -2263,7 +2326,26 @@ static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode)
 	/* start FIP Discovery and FLOGI */
 	lport->boot_time = jiffies;
 	fc_fabric_login(lport);
-	if (!fcoe_link_ok(lport)) {
+
+	/*
+	 * If the fcoe_ctlr_device is to be set to DISABLED
+	 * it must be done after the lport is added to the
+	 * hostlist, but before the rtnl_lock is released.
+	 * This is because the rtnl_lock protects the
+	 * hostlist that fcoe_device_notification uses. If
+	 * the FCoE Controller is intended to be created
+	 * DISABLED then 'enabled' needs to be considered
+	 * handling link events. 'enabled' must be set
+	 * before the lport can be found in the hostlist
+	 * when a link up event is received.
+	 */
+	if (link_state == FCOE_CREATE_LINK_UP)
+		ctlr_dev->enabled = FCOE_CTLR_ENABLED;
+	else
+		ctlr_dev->enabled = FCOE_CTLR_DISABLED;
+
+	if (link_state == FCOE_CREATE_LINK_UP &&
+	    !fcoe_link_ok(lport)) {
 		rtnl_unlock();
 		fcoe_ctlr_link_up(ctlr);
 		mutex_unlock(&fcoe_config_mutex);
@@ -2278,6 +2360,37 @@ out_nortnl:
 }
 
 /**
+ * fcoe_create() - Create a fcoe interface
+ * @netdev  : The net_device object the Ethernet interface to create on
+ * @fip_mode: The FIP mode for this creation
+ *
+ * Called from fcoe transport
+ *
+ * Returns: 0 for success
+ */
+static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode)
+{
+	return _fcoe_create(netdev, fip_mode, FCOE_CREATE_LINK_UP);
+}
+
+/**
+ * fcoe_ctlr_alloc() - Allocate a fcoe interface from fcoe_sysfs
+ * @netdev: The net_device to be used by the allocated FCoE Controller
+ *
+ * This routine is called from fcoe_sysfs. It will start the fcoe_ctlr
+ * in a link_down state. The allows the user an opportunity to configure
+ * the FCoE Controller from sysfs before enabling the FCoE Controller.
+ *
+ * Creating in with this routine starts the FCoE Controller in Fabric
+ * mode. The user can change to VN2VN or another mode before enabling.
+ */
+static int fcoe_ctlr_alloc(struct net_device *netdev)
+{
+	return _fcoe_create(netdev, FIP_MODE_FABRIC,
+			    FCOE_CREATE_LINK_DOWN);
+}
+
+/**
  * fcoe_link_speed_update() - Update the supported and actual link speeds
  * @lport: The local port to update speeds for
  *
@@ -2378,10 +2491,13 @@ static int fcoe_reset(struct Scsi_Host *shost)
 	struct fcoe_port *port = lport_priv(lport);
 	struct fcoe_interface *fcoe = port->priv;
 	struct fcoe_ctlr *ctlr = fcoe_to_ctlr(fcoe);
+	struct fcoe_ctlr_device *cdev = fcoe_ctlr_to_ctlr_dev(ctlr);
 
 	fcoe_ctlr_link_down(ctlr);
 	fcoe_clean_pending_queue(ctlr->lp);
-	if (!fcoe_link_ok(ctlr->lp))
+
+	if (cdev->enabled != FCOE_CTLR_DISABLED &&
+	    !fcoe_link_ok(ctlr->lp))
 		fcoe_ctlr_link_up(ctlr);
 	return 0;
 }
@@ -2454,6 +2570,7 @@ static struct fcoe_transport fcoe_sw_transport = {
 	.attached = false,
 	.list = LIST_HEAD_INIT(fcoe_sw_transport.list),
 	.match = fcoe_match,
+	.alloc = fcoe_ctlr_alloc,
 	.create = fcoe_create,
 	.destroy = fcoe_destroy,
 	.enable = fcoe_enable,
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 2ebe03a..7583425 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2864,22 +2864,21 @@ void fcoe_fcf_get_selected(struct fcoe_fcf_device *fcf_dev)
 }
 EXPORT_SYMBOL(fcoe_fcf_get_selected);
 
-void fcoe_ctlr_get_fip_mode(struct fcoe_ctlr_device *ctlr_dev)
+void fcoe_ctlr_set_fip_mode(struct fcoe_ctlr_device *ctlr_dev)
 {
 	struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(ctlr_dev);
 
 	mutex_lock(&ctlr->ctlr_mutex);
-	switch (ctlr->mode) {
-	case FIP_MODE_FABRIC:
-		ctlr_dev->mode = FIP_CONN_TYPE_FABRIC;
-		break;
-	case FIP_MODE_VN2VN:
-		ctlr_dev->mode = FIP_CONN_TYPE_VN2VN;
+	switch (ctlr_dev->mode) {
+	case FIP_CONN_TYPE_VN2VN:
+		ctlr->mode = FIP_MODE_VN2VN;
 		break;
+	case FIP_CONN_TYPE_FABRIC:
 	default:
-		ctlr_dev->mode = FIP_CONN_TYPE_UNKNOWN;
+		ctlr->mode = FIP_MODE_FABRIC;
 		break;
 	}
+
 	mutex_unlock(&ctlr->ctlr_mutex);
 }
-EXPORT_SYMBOL(fcoe_ctlr_get_fip_mode);
+EXPORT_SYMBOL(fcoe_ctlr_set_fip_mode);

[RFC PATCH v4 5/5] bnx2fc: Use the fcoe_sysfs control interface

[Robert Love]

This patch adds support for the new fcoe_sysfs
control interface to bnx2fc.ko. It keeps the deprecated
interface in tact and therefore either the legacy
or the new control interfaces can be used. A mixed mode
is not supported. A user must either use the new
interfaces or the old ones, but not both.

The fcoe_ctlr's link state is now driven by both the
netdev link state as well as the fcoe_ctlr_device's
enabled attribute. The link must be up and the
fcoe_ctlr_device must be enabled before the FCoE
Controller starts discovery or login.

Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |  168 +++++++++++++++++++++++++++++++------
 1 file changed, 139 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 97d9a58..9e15998 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -62,6 +62,10 @@ static int bnx2fc_destroy(struct net_device *net_device);
 static int bnx2fc_enable(struct net_device *netdev);
 static int bnx2fc_disable(struct net_device *netdev);
 
+/* fcoe_syfs control interface handlers */
+static int bnx2fc_ctlr_alloc(struct net_device *netdev);
+static int bnx2fc_ctlr_enabled(struct fcoe_ctlr_device *cdev);
+
 static void bnx2fc_recv_frame(struct sk_buff *skb);
 
 static void bnx2fc_start_disc(struct bnx2fc_interface *interface);
@@ -864,6 +868,7 @@ static void bnx2fc_indicate_netevent(void *context, unsigned long event,
 				     u16 vlan_id)
 {
 	struct bnx2fc_hba *hba = (struct bnx2fc_hba *)context;
+	struct fcoe_ctlr_device *cdev;
 	struct fc_lport *lport;
 	struct fc_lport *vport;
 	struct bnx2fc_interface *interface, *tmp;
@@ -925,28 +930,45 @@ static void bnx2fc_indicate_netevent(void *context, unsigned long event,
 
 		bnx2fc_link_speed_update(lport);
 
+		cdev = fcoe_ctlr_to_ctlr_dev(ctlr);
+
 		if (link_possible && !bnx2fc_link_ok(lport)) {
-			/* Reset max recv frame size to default */
-			fc_set_mfs(lport, BNX2FC_MFS);
-			/*
-			 * ctlr link up will only be handled during
-			 * enable to avoid sending discovery solicitation
-			 * on a stale vlan
-			 */
-			if (interface->enabled)
-				fcoe_ctlr_link_up(ctlr);
+			switch (cdev->enabled) {
+			case FCOE_CTLR_DISABLED:
+				pr_info("Link up while interface is disabled.\n");
+				break;
+			case FCOE_CTLR_ENABLED:
+			case FCOE_CTLR_UNUSED:
+				/* Reset max recv frame size to default */
+				fc_set_mfs(lport, BNX2FC_MFS);
+				/*
+				 * ctlr link up will only be handled during
+				 * enable to avoid sending discovery
+				 * solicitation on a stale vlan
+				 */
+				if (interface->enabled)
+					fcoe_ctlr_link_up(ctlr);
+			};
 		} else if (fcoe_ctlr_link_down(ctlr)) {
-			mutex_lock(&lport->lp_mutex);
-			list_for_each_entry(vport, &lport->vports, list)
-				fc_host_port_type(vport->host) =
-							FC_PORTTYPE_UNKNOWN;
-			mutex_unlock(&lport->lp_mutex);
-			fc_host_port_type(lport->host) = FC_PORTTYPE_UNKNOWN;
-			per_cpu_ptr(lport->stats,
-				    get_cpu())->LinkFailureCount++;
-			put_cpu();
-			fcoe_clean_pending_queue(lport);
-			wait_for_upload = 1;
+			switch (cdev->enabled) {
+			case FCOE_CTLR_DISABLED:
+				pr_info("Link down while interface is disabled.\n");
+				break;
+			case FCOE_CTLR_ENABLED:
+			case FCOE_CTLR_UNUSED:
+				mutex_lock(&lport->lp_mutex);
+				list_for_each_entry(vport, &lport->vports, list)
+					fc_host_port_type(vport->host) =
+					FC_PORTTYPE_UNKNOWN;
+				mutex_unlock(&lport->lp_mutex);
+				fc_host_port_type(lport->host) =
+					FC_PORTTYPE_UNKNOWN;
+				per_cpu_ptr(lport->stats,
+					    get_cpu())->LinkFailureCount++;
+				put_cpu();
+				fcoe_clean_pending_queue(lport);
+				wait_for_upload = 1;
+			};
 		}
 	}
 	mutex_unlock(&bnx2fc_dev_lock);
@@ -1996,7 +2018,9 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
 		set_bit(BNX2FC_CNIC_REGISTERED, &hba->reg_with_cnic);
 }
 
-
+/**
+ * Deperecated: Use bnx2fc_enabled()
+ */
 static int bnx2fc_disable(struct net_device *netdev)
 {
 	struct bnx2fc_interface *interface;
@@ -2022,7 +2046,9 @@ static int bnx2fc_disable(struct net_device *netdev)
 	return rc;
 }
 
-
+/**
+ * Deprecated: Use bnx2fc_enabled()
+ */
 static int bnx2fc_enable(struct net_device *netdev)
 {
 	struct bnx2fc_interface *interface;
@@ -2048,17 +2074,57 @@ static int bnx2fc_enable(struct net_device *netdev)
 }
 
 /**
- * bnx2fc_create - Create bnx2fc FCoE interface
+ * bnx2fc_ctlr_enabled() - Enable or disable an FCoE Controller
+ * @cdev: The FCoE Controller that is being enabled or disabled
  *
- * @buffer: The name of Ethernet interface to create on
- * @kp:     The associated kernel param
+ * fcoe_sysfs will ensure that the state of 'enabled' has
+ * changed, so no checking is necessary here. This routine simply
+ * calls fcoe_enable or fcoe_disable, both of which are deprecated.
+ * When those routines are removed the functionality can be merged
+ * here.
+ */
+static int bnx2fc_ctlr_enabled(struct fcoe_ctlr_device *cdev)
+{
+	struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(cdev);
+	struct fc_lport *lport = ctlr->lp;
+	struct net_device *netdev = bnx2fc_netdev(lport);
+
+	switch (cdev->enabled) {
+	case FCOE_CTLR_ENABLED:
+		return bnx2fc_enable(netdev);
+	case FCOE_CTLR_DISABLED:
+		return bnx2fc_disable(netdev);
+	case FCOE_CTLR_UNUSED:
+	default:
+		return -ENOTSUPP;
+	};
+}
+
+enum bnx2fc_create_link_state {
+	BNX2FC_CREATE_LINK_DOWN,
+	BNX2FC_CREATE_LINK_UP,
+};
+
+/**
+ * _bnx2fc_create() - Create bnx2fc FCoE interface
+ * @netdev  :   The net_device object the Ethernet interface to create on
+ * @fip_mode:   The FIP mode for this creation
+ * @link_state: The ctlr link state on creation
  *
- * Called from sysfs.
+ * Called from either the libfcoe 'create' module parameter
+ * via fcoe_create or from fcoe_syfs's ctlr_create file.
+ *
+ * libfcoe's 'create' module parameter is deprecated so some
+ * consolidation of code can be done when that interface is
+ * removed.
  *
  * Returns: 0 for success
  */
-static int bnx2fc_create(struct net_device *netdev, enum fip_state fip_mode)
+static int _bnx2fc_create(struct net_device *netdev,
+			  enum fip_state fip_mode,
+			  enum bnx2fc_create_link_state link_state)
 {
+	struct fcoe_ctlr_device *cdev;
 	struct fcoe_ctlr *ctlr;
 	struct bnx2fc_interface *interface;
 	struct bnx2fc_hba *hba;
@@ -2153,7 +2219,15 @@ static int bnx2fc_create(struct net_device *netdev, enum fip_state fip_mode)
 	/* Make this master N_port */
 	ctlr->lp = lport;
 
-	if (!bnx2fc_link_ok(lport)) {
+	cdev = fcoe_ctlr_to_ctlr_dev(ctlr);
+
+	if (link_state == BNX2FC_CREATE_LINK_UP)
+		cdev->enabled = FCOE_CTLR_ENABLED;
+	else
+		cdev->enabled = FCOE_CTLR_DISABLED;
+
+	if (link_state == BNX2FC_CREATE_LINK_UP &&
+	    !bnx2fc_link_ok(lport)) {
 		fcoe_ctlr_link_up(ctlr);
 		fc_host_port_type(lport->host) = FC_PORTTYPE_NPORT;
 		set_bit(ADAPTER_STATE_READY, &interface->hba->adapter_state);
@@ -2161,7 +2235,10 @@ static int bnx2fc_create(struct net_device *netdev, enum fip_state fip_mode)
 
 	BNX2FC_HBA_DBG(lport, "create: START DISC\n");
 	bnx2fc_start_disc(interface);
-	interface->enabled = true;
+
+	if (link_state == BNX2FC_CREATE_LINK_UP)
+		interface->enabled = true;
+
 	/*
 	 * Release from kref_init in bnx2fc_interface_setup, on success
 	 * lport should be holding a reference taken in bnx2fc_if_create
@@ -2187,6 +2264,37 @@ mod_err:
 }
 
 /**
+ * bnx2fc_create() - Create a bnx2fc interface
+ * @netdev  : The net_device object the Ethernet interface to create on
+ * @fip_mode: The FIP mode for this creation
+ *
+ * Called from fcoe transport
+ *
+ * Returns: 0 for success
+ */
+static int bnx2fc_create(struct net_device *netdev, enum fip_state fip_mode)
+{
+	return _bnx2fc_create(netdev, fip_mode, BNX2FC_CREATE_LINK_UP);
+}
+
+/**
+ * bnx2fc_ctlr_alloc() - Allocate a bnx2fc interface from fcoe_sysfs
+ * @netdev: The net_device to be used by the allocated FCoE Controller
+ *
+ * This routine is called from fcoe_sysfs. It will start the fcoe_ctlr
+ * in a link_down state. The allows the user an opportunity to configure
+ * the FCoE Controller from sysfs before enabling the FCoE Controller.
+ *
+ * Creating in with this routine starts the FCoE Controller in Fabric
+ * mode. The user can change to VN2VN or another mode before enabling.
+ */
+static int bnx2fc_ctlr_alloc(struct net_device *netdev)
+{
+	return _bnx2fc_create(netdev, FIP_MODE_FABRIC,
+			      BNX2FC_CREATE_LINK_DOWN);
+}
+
+/**
  * bnx2fc_find_hba_for_cnic - maps cnic instance to bnx2fc hba instance
  *
  * @cnic:	Pointer to cnic device instance
@@ -2311,6 +2419,7 @@ static struct fcoe_transport bnx2fc_transport = {
 	.name = {"bnx2fc"},
 	.attached = false,
 	.list = LIST_HEAD_INIT(bnx2fc_transport.list),
+	.alloc = bnx2fc_ctlr_alloc,
 	.match = bnx2fc_match,
 	.create = bnx2fc_create,
 	.destroy = bnx2fc_destroy,
@@ -2555,6 +2664,7 @@ module_init(bnx2fc_mod_init);
 module_exit(bnx2fc_mod_exit);
 
 static struct fcoe_sysfs_function_template bnx2fc_fcoe_sysfs_templ = {
+	.set_fcoe_ctlr_enabled = bnx2fc_ctlr_enabled,
 	.get_fcoe_ctlr_link_fail = bnx2fc_ctlr_get_lesb,
 	.get_fcoe_ctlr_vlink_fail = bnx2fc_ctlr_get_lesb,
 	.get_fcoe_ctlr_miss_fka = bnx2fc_ctlr_get_lesb,

Re: [Open-FCoE] [RFC PATCH v4 0/5] Add new fcoe_sysfs based control interfaces to libfcoe, bnx2fc and fcoe

[Neil Horman]

On Tue, Oct 09, 2012 at 03:18:50PM -0700, Robert Love wrote:
> v4:
> 
> @Neil:
> 	Policy is now:
> 
> 	'mode' attribute:
> 	       input: "Fabric" "VN2VN" (case insensitive)
> 	       output: "Fabric" "VN2VN"
> 
> 	'enabled' attribute:
> 	       input: "1" "0"
> 	       output: "1" "0"
> 
Thanks, for the series
Acked-by: Neil Horman <nhorman@tuxdriver.com>

> @Mark:
> 	Initial patch now optimizes enum-to-string memory usage and
> 	string retreival
> 
> --
> 
> v3:
> 
> This series applies to the v3.6 kernel.
> 
> @Bart: Fixed bus_create_file -> bus_attrs
>        Removed sscanf with non-NULL buffer, only checking for '1' or '0' now
>        Removed unnecessary whitespace change
> 
> @Bhanu: Incorporated check in _bnx2fc_create (thanks for the code)
> 
> Additional changes: Added check for 'enabled' in reset in fcoe.c
> 	   	    Made mode strncmp case insensitive so user can
> 		    # echo "vn2vn" > /sys/.../ctlr_0/mode # or
> 		    # echo "VN2VN" > /sys/.../ctlr_0/mode # or even
> 		    # echo "FaBRiC" > /sys/.../ctlr_0/mode
> 
> --
> 
> v2:
> 
> The following series adds /sys/bus/fcoe based control interfaces to
> libfcoe. A fcoe_sysfs infrastructure was added to the kernel a few
> cycles ago, this series builds on that work. The patches deprecate
> the old create, vn2vn_create, destroy, enable and disable interfaces
> that exist in /sys/module/libfcoe/parameters/.
> 
> Another goal of this series is to change the initialization sequence for
> a FCoE device. The result of this series is that interfaces created using
> libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
> starting steps-
> 
> 1) Create/alloc the FCoE Controller
>    - Allocate kernel memory and create per-instance sysfs devices
>    - The FCoE Controller is created disabled, no discovery or login
>      until it is enabled.
> 
>    # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_create
> 
> 2) Configure the FCoE Controller
>    - Change mode, set ddp_min (future), set dcb_required (future), etc...
> 
>    # echo 2 > /sys/bus/fcoe/ctlr_0/mode #sets mode to VN2VN
> 
> 3) Enable the FCoE Controller
>    - Begins discovery and login in 'Fabric' mode. or
>    - Begins FIP FCID claim process, discovery and login in 'VN2VN' mode
> 
> 4) Destroy the FCoE Controller
>    - Logout and free all memory
> 
>    # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_destroy
> 
> This series converts both fcoe.ko and bnx2fc.ko to use the new
> fcoe_sysfs based interfaces. The legacy interfaces can remain in kernel
> for a kernel cycle or two before being removed.
> 
> I'm looking for feedback on the use of /sys/bus/fcoe/ctlr_create and
> /sys/bus/fcoe/ctlr_destroy and that those interfaces take an ifname.
> I modeled it after the bonding interface, but I'm not sure if that's
> acceptible.
> 
> Incorpoated v1 feedback:
> 
> @Chris:
> 
> I spent a lot of time looking into FCF selection.
> 
> I implemented a POC series where I made the 'enabled' attribute
> of a fcoe_fcf_device (i.e. /sys/bus/fcoe/devices/fcf_X) writable. The
> fcoe_ctlr_device (i.e. /sys/bus/fcoe/devices/ctlr_X) has a
> 'selection_strategy' attribute that would allow the user to toggle between
> AUTO (current kernel selection algorithm) and USER (user writes to FCF's
> 'selection' attribute).
> 
> I also wrote a little libudev based application that listened for fcoe_fcf_device
> sysfs add/remove events. My plan was to have fcoemon monitor the discovery
> of FCFs and then have it write to the 'selected' attribute of the FCF the user
> had chosen.
> 
> Working on this series convinced me that any FCF selection needs further
> consideration. First of all, it's fairly complicated to update the fcoe_ctlr.c
> functional FIP code to handle FCF/fabric selection. Some questions that arise:
> 
> What triggers FLOGI/LOGO? We would now have link, enabled/disabled, selection
> strategy and FCF selection to consider.
> 
> Can a new FCF be selected when one is already selected and an FLOGI has occurred?
> Can a FCF be de-selected when the FLOGI has been sent?
> 
> Maybe we can only change things when disabled, that probably makes the most sense..
> 
> When does discovery happen? When the ctlr is created (no opportunity for
> mode/selection strategy to be set)? When the mode is changed (same problem)?
> What about when the cltr is enabled (but that's when we were going to FLOGI)?
> 
> This isn't a complete list of considerations, just what came to mind when writing
> this. Anyway, the policies started to get complicated, or maybe my lack of policies
> made the POC implementation more complicated. Either way it made me think about
> use cases and how valuable FCF/fabric selection is.
> 
> After further consideration I think that most of the access decisions, when
> connecting to a FC fabric, should be done by the fabric services. I'm not sure
> the endstations should be whitelisting or blacklisting FCFs or even making
> decisions about which ones to use when they're already prioritized amongst themselves
> (on the same fabric). I think it might be nice for debugging, but I don't have a
> need at the moment.
> 
> I think user selection may be more valuable with VN2VN, which may interest you
> more anyway, as you said that you were running a VN2VN setup. Since there
> aren't fabric services to rely on I can see it useful for VN_Ports to whitelist or
> blacklist other VN_Ports. I think the first step to support something like this
> would be to represent the fcoe_rports in fcoe_sysfs or in the FC Transport such
> that they can be selected or de-slected.
> 
> I think that's a fine goal, but with this series, I think I'd like to focus on
> just getting away from using module parameters for control interfaces. I think
> this current series allows for selection as I've described above since it will
> now start the FCoE Controller in a DISABLED state such that configurations can
> now be made before the FLOGI.
> 
> @Bhanu
> 
> I implemented the changes for bnx2fc and I think it should be mostly fine. I
> wasn't able to test the code, so I'd appreciate any feedback about whether
> there are bugs or not.
> 
> @Bart:
> 
> Fixed the 'const char *p' and cast issue in fcoe_parse_mode().
> 
> Now checking for string length and passing correctly NULL terminated string
> to fcoe_parse_mode().
> 
> Thanks, //Rob
> 
> ---
> 
> v1 covermail
> 
> The following series implements a move from using module parameters
> as control interfaces to /sys/bus/fcoe based interfaces. A sysfs infrastructure
> was added to the kernel a few cycles ago, this series builds on that work.
> 
> It moves the create, vn2vn_create, destroy, enable and disable interfaces
> from /sys/module/libfcoe/parameters/ to various places under /sys/bus/fcoe/.
> These interfaces simply are not module configurations- they are control
> interfaces.
> 
> A second goal of this series is to change the initialization sequence for
> a FCoE device. The result of this series is that interfaces created using
> libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
> starting steps-
> 
> 1) Create/alloc the port
>    - Allocate kernel memory and create per-instance sysfs devices
>    - No discovery or login
> 
> 2) Configure the port
>    - Change mode, set ddp_min, etc...
> 
> 3) Start the port
>    - Begins discovery and/or login (depending on mode)
> 
> 4) Destroy the port
>    - Logout and free all memory
> 
> I'm looking for feedback on using sysfs files as control interfaces that
> the user (application) would write interface names to. I modeled this
> series off of the bonding sysfs interface, but it was suggested to me that
> it might not be a good example. I belive bonding uses two values per-file
> a '+' or a '-" to add or delete and then the ifname apended. I am simply
> writing the ifname to the ctlr_create or ctlr_destroy.
> 
> Series compiled and tested against v3.5. libfcoe.ko compile warning fixed
> upstream after v3.5, anyone who compiles this can ignore section mismatch
> warning. Also note that a modified fcoemon is needed to use the fcoe system
> service against this kernel modification. I'd be happy to provide that
> fcoemon code on request.
> 
> ---
> 
> Robert Love (5):
>       libfcoe: Save some memory and optimize name lookups
>       libfcoe: Add fcoe_sysfs debug logging level
>       libfcoe, fcoe, bnx2fc: Add new fcoe control interface
>       fcoe: Use the fcoe_sysfs control interface
>       bnx2fc: Use the fcoe_sysfs control interface
> 
> 
>  Documentation/ABI/testing/sysfs-bus-fcoe |   42 +++++++
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c        |  169 ++++++++++++++++++++++-----
>  drivers/scsi/fcoe/fcoe.c                 |  148 +++++++++++++++++++++---
>  drivers/scsi/fcoe/fcoe_ctlr.c            |   17 +--
>  drivers/scsi/fcoe/fcoe_sysfs.c           |  186 +++++++++++++++++++++++++-----
>  drivers/scsi/fcoe/fcoe_transport.c       |  104 +++++++++++++++++
>  drivers/scsi/fcoe/libfcoe.h              |   11 +-
>  include/scsi/fcoe_sysfs.h                |   11 ++
>  include/scsi/libfcoe.h                   |   16 ++-
>  9 files changed, 608 insertions(+), 96 deletions(-)
> 
> -- 
> 
> Thanks, //Rob
> _______________________________________________
> devel mailing list
> devel@open-fcoe.org
> https://lists.open-fcoe.org/mailman/listinfo/devel
> 

Re: [RFC PATCH v4 0/5] Add new fcoe_sysfs based control interfaces to libfcoe, bnx2fc and fcoe

[Bhanu Prakash Gollapudi]

On 10/09/2012 03:18 PM, Robert Love wrote:
> v4:
>
> @Neil:
> 	Policy is now:
>
> 	'mode' attribute:
> 	       input: "Fabric" "VN2VN" (case insensitive)
> 	       output: "Fabric" "VN2VN"
>
> 	'enabled' attribute:
> 	       input: "1" "0"
> 	       output: "1" "0"
>
> @Mark:
> 	Initial patch now optimizes enum-to-string memory usage and
> 	string retreival
>
> --
>
> v3:
>
> This series applies to the v3.6 kernel.
>
> @Bart: Fixed bus_create_file -> bus_attrs
>         Removed sscanf with non-NULL buffer, only checking for '1' or '0' now
>         Removed unnecessary whitespace change
>
> @Bhanu: Incorporated check in _bnx2fc_create (thanks for the code)
>
> Additional changes: Added check for 'enabled' in reset in fcoe.c
> 	   	    Made mode strncmp case insensitive so user can
> 		    # echo "vn2vn" > /sys/.../ctlr_0/mode # or
> 		    # echo "VN2VN" > /sys/.../ctlr_0/mode # or even
> 		    # echo "FaBRiC" > /sys/.../ctlr_0/mode
>
> --
>
> v2:
>
> The following series adds /sys/bus/fcoe based control interfaces to
> libfcoe. A fcoe_sysfs infrastructure was added to the kernel a few
> cycles ago, this series builds on that work. The patches deprecate
> the old create, vn2vn_create, destroy, enable and disable interfaces
> that exist in /sys/module/libfcoe/parameters/.
>
> Another goal of this series is to change the initialization sequence for
> a FCoE device. The result of this series is that interfaces created using
> libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
> starting steps-
>
> 1) Create/alloc the FCoE Controller
>     - Allocate kernel memory and create per-instance sysfs devices
>     - The FCoE Controller is created disabled, no discovery or login
>       until it is enabled.
>
>     # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_create
>
> 2) Configure the FCoE Controller
>     - Change mode, set ddp_min (future), set dcb_required (future), etc...
>
>     # echo 2 > /sys/bus/fcoe/ctlr_0/mode #sets mode to VN2VN
>
> 3) Enable the FCoE Controller
>     - Begins discovery and login in 'Fabric' mode. or
>     - Begins FIP FCID claim process, discovery and login in 'VN2VN' mode
>
> 4) Destroy the FCoE Controller
>     - Logout and free all memory
>
>     # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_destroy
>
> This series converts both fcoe.ko and bnx2fc.ko to use the new
> fcoe_sysfs based interfaces. The legacy interfaces can remain in kernel
> for a kernel cycle or two before being removed.
>
> I'm looking for feedback on the use of /sys/bus/fcoe/ctlr_create and
> /sys/bus/fcoe/ctlr_destroy and that those interfaces take an ifname.
> I modeled it after the bonding interface, but I'm not sure if that's
> acceptible.
>
> Incorpoated v1 feedback:
>
> @Chris:
>
> I spent a lot of time looking into FCF selection.
>
> I implemented a POC series where I made the 'enabled' attribute
> of a fcoe_fcf_device (i.e. /sys/bus/fcoe/devices/fcf_X) writable. The
> fcoe_ctlr_device (i.e. /sys/bus/fcoe/devices/ctlr_X) has a
> 'selection_strategy' attribute that would allow the user to toggle between
> AUTO (current kernel selection algorithm) and USER (user writes to FCF's
> 'selection' attribute).
>
> I also wrote a little libudev based application that listened for fcoe_fcf_device
> sysfs add/remove events. My plan was to have fcoemon monitor the discovery
> of FCFs and then have it write to the 'selected' attribute of the FCF the user
> had chosen.
>
> Working on this series convinced me that any FCF selection needs further
> consideration. First of all, it's fairly complicated to update the fcoe_ctlr.c
> functional FIP code to handle FCF/fabric selection. Some questions that arise:
>
> What triggers FLOGI/LOGO? We would now have link, enabled/disabled, selection
> strategy and FCF selection to consider.
>
> Can a new FCF be selected when one is already selected and an FLOGI has occurred?
> Can a FCF be de-selected when the FLOGI has been sent?
>
> Maybe we can only change things when disabled, that probably makes the most sense..
>
> When does discovery happen? When the ctlr is created (no opportunity for
> mode/selection strategy to be set)? When the mode is changed (same problem)?
> What about when the cltr is enabled (but that's when we were going to FLOGI)?
>
> This isn't a complete list of considerations, just what came to mind when writing
> this. Anyway, the policies started to get complicated, or maybe my lack of policies
> made the POC implementation more complicated. Either way it made me think about
> use cases and how valuable FCF/fabric selection is.
>
> After further consideration I think that most of the access decisions, when
> connecting to a FC fabric, should be done by the fabric services. I'm not sure
> the endstations should be whitelisting or blacklisting FCFs or even making
> decisions about which ones to use when they're already prioritized amongst themselves
> (on the same fabric). I think it might be nice for debugging, but I don't have a
> need at the moment.
>
> I think user selection may be more valuable with VN2VN, which may interest you
> more anyway, as you said that you were running a VN2VN setup. Since there
> aren't fabric services to rely on I can see it useful for VN_Ports to whitelist or
> blacklist other VN_Ports. I think the first step to support something like this
> would be to represent the fcoe_rports in fcoe_sysfs or in the FC Transport such
> that they can be selected or de-slected.
>
> I think that's a fine goal, but with this series, I think I'd like to focus on
> just getting away from using module parameters for control interfaces. I think
> this current series allows for selection as I've described above since it will
> now start the FCoE Controller in a DISABLED state such that configurations can
> now be made before the FLOGI.
>
> @Bhanu
>
> I implemented the changes for bnx2fc and I think it should be mostly fine. I
> wasn't able to test the code, so I'd appreciate any feedback about whether
> there are bugs or not.

Robert, changes look good to me, except for this minor comment:

I see the message "fcoe_ctlr_create_store: transport bnx2fc failed to 
create fcoe on p1p1.300-fcoe." when creating the interface because 
ctlr_dev is NULL. We should be checking for the return value of 
ft->alloc() I think.
>
> @Bart:
>
> Fixed the 'const char *p' and cast issue in fcoe_parse_mode().
>
> Now checking for string length and passing correctly NULL terminated string
> to fcoe_parse_mode().
>
> Thanks, //Rob
>
> ---
>
> v1 covermail
>
> The following series implements a move from using module parameters
> as control interfaces to /sys/bus/fcoe based interfaces. A sysfs infrastructure
> was added to the kernel a few cycles ago, this series builds on that work.
>
> It moves the create, vn2vn_create, destroy, enable and disable interfaces
> from /sys/module/libfcoe/parameters/ to various places under /sys/bus/fcoe/.
> These interfaces simply are not module configurations- they are control
> interfaces.
>
> A second goal of this series is to change the initialization sequence for
> a FCoE device. The result of this series is that interfaces created using
> libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following
> starting steps-
>
> 1) Create/alloc the port
>     - Allocate kernel memory and create per-instance sysfs devices
>     - No discovery or login
>
> 2) Configure the port
>     - Change mode, set ddp_min, etc...
>
> 3) Start the port
>     - Begins discovery and/or login (depending on mode)
>
> 4) Destroy the port
>     - Logout and free all memory
>
> I'm looking for feedback on using sysfs files as control interfaces that
> the user (application) would write interface names to. I modeled this
> series off of the bonding sysfs interface, but it was suggested to me that
> it might not be a good example. I belive bonding uses two values per-file
> a '+' or a '-" to add or delete and then the ifname apended. I am simply
> writing the ifname to the ctlr_create or ctlr_destroy.
>
> Series compiled and tested against v3.5. libfcoe.ko compile warning fixed
> upstream after v3.5, anyone who compiles this can ignore section mismatch
> warning. Also note that a modified fcoemon is needed to use the fcoe system
> service against this kernel modification. I'd be happy to provide that
> fcoemon code on request.
>
> ---
>
> Robert Love (5):
>        libfcoe: Save some memory and optimize name lookups
>        libfcoe: Add fcoe_sysfs debug logging level
>        libfcoe, fcoe, bnx2fc: Add new fcoe control interface
>        fcoe: Use the fcoe_sysfs control interface
>        bnx2fc: Use the fcoe_sysfs control interface
>
>
>   Documentation/ABI/testing/sysfs-bus-fcoe |   42 +++++++
>   drivers/scsi/bnx2fc/bnx2fc_fcoe.c        |  169 ++++++++++++++++++++++-----
>   drivers/scsi/fcoe/fcoe.c                 |  148 +++++++++++++++++++++---
>   drivers/scsi/fcoe/fcoe_ctlr.c            |   17 +--
>   drivers/scsi/fcoe/fcoe_sysfs.c           |  186 +++++++++++++++++++++++++-----
>   drivers/scsi/fcoe/fcoe_transport.c       |  104 +++++++++++++++++
>   drivers/scsi/fcoe/libfcoe.h              |   11 +-
>   include/scsi/fcoe_sysfs.h                |   11 ++
>   include/scsi/libfcoe.h                   |   16 ++-
>   9 files changed, 608 insertions(+), 96 deletions(-)
>