Reorganize libfcoe control interfaces

This patch makes the 'mode' attribute of a fcoe_ctlr_device writale. This allows the user to store the mode with with the ctlr will be in.

Possible modes would be '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_ctlr's mode.

Signed-off-by Robert Love <[email protected]>

drivers/scsi/fcoe/fcoe_sysfs.c MODULE_PARM_DESC(fcf_dev_loss_tmo, · line 64

Quoted 9 lines · Robert Love

drivers/scsi/fcoe/fcoe_sysfs.c +9 -0

+static enum fip_conn_type fcoe_parse_mode(const char *buf,
+				  const struct fcoe_ctlr_mode_table *tbl)
+{
+	int modeint = -1, i, rv;
+	char *p, modestr[FCOE_MAX_MODENAME_LEN + 1] = { 0, };
+
+	for (p = (char *)buf; *p; p++)
+		if (!(isdigit(*p) || isspace(*p)))
+			break;

If you change the declaration of p from "char *p" into "const char *p" you won't need a cast in the above for loop.

[ ... ]

drivers/scsi/fcoe/fcoe_sysfs.c static ssize_t show_ctlr_mode(struct device *dev, · line 327

Quoted 13 lines

drivers/scsi/fcoe/fcoe_sysfs.c +11 -2

-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);
+
+	if (!ctlr->f->set_fcoe_ctlr_mode)
+		return -EINVAL;
+
+	ctlr->mode = fcoe_parse_mode(buf, ctlr_mode_tbl);

As far as I know sysfs doesn't terminate buf with a '\0' before calling a store method. Does that mean that you are passing a string that is not '\0'-terminated to a function that expects a '\0'-terminated string ?

Bart.

This patch is the first in a series that will remove libfcoe's create, destroy, enable and disable module parameters and replace them with interface files in the new /sys/bus/fcoe subsystem.

Old layout:

/sys/module/libfcoe/parameters/{create,destroy,enable,disable,vn2vn_create}

New layout:

/sys/bus/fcoe/ctlr_{create,destroy}
/sys/bus/fcoe/ctlr_X/{enable,disable,start}

This patch moves fcoe drivers to the following initialization sequence-

1) create/alloc
2) configure
3) start

A control sysfs interface at /sys/bus/fcoe/ctlr_create is added. Writing the interface name to this file will allocate memory and create a sysfs entry for a new fcoe_ctlr_device. The user may then tune the interface in any desired way. After configuration the user will echo any value into the /sys/bus/fcoe/devices/ctlr_X/start interface to proceed with logging in.

VN2VN logins will still use the module parameters. A follow up patch to this one will make the 'mode' attribute of the fcoe_ctlr_device writable. Which will allow a user to change the ctlr's mode to 'VN2VN'.

Signed-off-by Robert Love <[email protected]>

---
  Documentation/ABI/testing/sysfs-bus-fcoe |   43 ++++++++++++
  drivers/scsi/fcoe/fcoe.h                 |    9 +++
  drivers/scsi/fcoe/fcoe_ctlr.c            |    2 -
  drivers/scsi/fcoe/fcoe_sysfs.c           |   78 ++++++++++++++++++++++
  drivers/scsi/fcoe/fcoe_transport.c       |  105 +++++++++++++++++++++++++++++-
  include/scsi/fcoe_sysfs.h                |    4 +
  include/scsi/libfcoe.h                   |   14 ++++
  7 files changed, 250 insertions(+), 5 deletions(-)

drivers/scsi/fcoe/fcoe_ctlr.c +0 -0

  	fip->mode = mode;
  	INIT_LIST_HEAD(&fip->fcfs);
  	mutex_init(&fip->ctlr_mutex);

drivers/scsi/fcoe/fcoe_ctlr.c +45 -0

@@ -627,6 +626,108 @@ 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 = -ENODEV;
+	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) {

drivers/scsi/fcoe/fcoe_ctlr.c +47 -0

+
+out_putdev:
+	dev_put(netdev);
+out_nodev:
+	mutex_unlock(&ft_mutex);
+	return rc;
+}
+
+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

include/scsi/fcoe_sysfs.h +4 -0

index 421ae67..8c5ea70 100644
@@ -36,6 +36,9 @@ struct fcoe_sysfs_function_template {
  	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_start)(struct fcoe_ctlr_device *);
+	int  (*set_fcoe_ctlr_enable)(struct fcoe_ctlr_device *);
+	int  (*set_fcoe_ctlr_disable)(struct fcoe_ctlr_device *);
  	void (*get_fcoe_fcf_selected)(struct fcoe_fcf_device *);
  	void (*get_fcoe_fcf_vlan_id)(struct fcoe_fcf_device *);
  };

@@ -64,6 +67,7 @@ struct fcoe_ctlr_device {
 
  	int                             fcf_dev_loss_tmo;
  	enum fip_conn_type              mode;
+	u8                              started:1;
 
  	/* expected in host order for displaying */
  	struct fcoe_fc_els_lesb         lesb;

include/scsi/libfcoe.h +13 -1

index 20533cc..b19a489 100644
@@ -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);

@@ -375,4 +379,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 */
+
+

Convert bnx2fc to use the new fcoe_sysfs create, delete,
enable, disable, start and mode.

bnx2fc doesn't support VN2VN. bnx2fc will not initialize the set_fcoe_ctlr_mode routine and therefore its instances will always be in FABRIC mode. There was previously an explicit check for the ctlr's mode, but this is no longer needed because not implementing set_fcoe_ctlr_mode implies that the ctlr cannot change from the FABRIC mode.

Signed-off-by Robert Love <[email protected]>

---
  drivers/scsi/bnx2fc/bnx2fc_fcoe.c |   98 +++++++++++++++++++++++--------------
  1 file changed, 60 insertions(+), 38 deletions(-)

drivers/scsi/bnx2fc/bnx2fc_fcoe.c +0 -0

index f52f668f..560c8c8 100644

/**
+ * bnx2fc_alloc - Alocate a bnx2fc FCoE interface
+ *
+ * @cdev: The FCoE Controller Device to start
+ *
+ * Called from sysfs.
+ *
+ * Returns: 0 for success
+ */

drivers/scsi/bnx2fc/bnx2fc_fcoe.c +1 -0

+static int bnx2fc_start(struct fcoe_ctlr_device *cdev)

drivers/scsi/bnx2fc/bnx2fc_fcoe.c +10 -3

+	interface->enabled = true;
+
+	return 0;
+}
+
+/**
   * bnx2fc_find_hba_for_cnic - maps cnic instance to bnx2fc hba instance
   *
   * @cnic:	Pointer to cnic device instance

@@ -2271,10 +2292,8 @@ static struct fcoe_transport bnx2fc_transport = {
  	.attached = false,
  	.list = LIST_HEAD_INIT(bnx2fc_transport.list),
  	.match = bnx2fc_match,
-	.create = bnx2fc_create,
+	.alloc = bnx2fc_alloc,
  	.destroy = bnx2fc_destroy,
-	.enable = bnx2fc_enable,
-	.disable = bnx2fc_disable,
  };
 
  /**

@@ -2514,6 +2533,9 @@ module_init(bnx2fc_mod_init);
  module_exit(bnx2fc_mod_exit);
 
  static struct fcoe_sysfs_function_template bnx2fc_fcoe_sysfs_templ = {
+	.set_fcoe_ctlr_start = bnx2fc_start,
+	.set_fcoe_ctlr_enable = bnx2fc_enable,
+	.set_fcoe_ctlr_disable = bnx2fc_disable,
  	.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,

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, fcoe: Allow user to set a ctlr's mode
libfcoe: Create new libfcoe control interfaces
fcoe: Use new fcoe_sysfs control interface
bnx2fc: Use new fcoe_sysfs control interface libfcoe, fcoe: Remove libfcoe module parameters

  Documentation/ABI/testing/sysfs-bus-fcoe |   51 +++++++
  drivers/scsi/bnx2fc/bnx2fc_fcoe.c        |   98 ++++++++-----
  drivers/scsi/fcoe/fcoe.c                 |  229 +++++++++++++++---------------
  drivers/scsi/fcoe/fcoe.h                 |    9 +
  drivers/scsi/fcoe/fcoe_ctlr.c            |   24 +++
  drivers/scsi/fcoe/fcoe_sysfs.c           |  139 ++++++++++++++++++
  drivers/scsi/fcoe/fcoe_transport.c       |  174 ++++-------------------
  include/scsi/fcoe_sysfs.h                |    5 +
  include/scsi/libfcoe.h                   |   20 ++-
  9 files changed, 445 insertions(+), 304 deletions(-)

Quoted 31 lines · Robert Love

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)

Quoted 2 lines

4) Destroy the port
- Logout and free all memory

Robert, Can you please let me now what is the motivation for this change and what problem are we solving with this approach? Is this primarily to allow user to set the mode?

I'm concerned that we will be breaking user space compatibility with this change, as there should be a corresponding fcoemon/fipvlan change along with this, and existing utilities will not work. Also the way we start fcoe will be completely different and the user may need to do the scripting changes, if any.

Thanks,
Bhanu

Quoted 18 lines

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.

Quoted 32 lines · Bhanu Prakash Gollapudi

Quoted 31 lines · Robert Love

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)

Quoted 7 lines

Quoted 2 lines

4) Destroy the port
- Logout and free all memory

Robert, Can you please let me now what is the motivation for this change and what problem are we solving with this approach? Is this primarily to allow user to set the mode?

The main problem is that our control interfaces shouldn't be module parameters. I think of module parameters as things that globally alter the module.

I also think that moving to a create/configure/start model gives us more flexibility going forward. We don't have too many FC/FCoE knobs to tune right now, but if we wanted to add more we don't have a good way to do it without starting the whole discovery/login process and then making changes during the discovery/login.

I think the module parameter problem is the justification, but I'm trying to be comprehensive in coming up with a flexible interface that will allow us to evolve as well.

Quoted 5 lines

I'm concerned that we will be breaking user space compatibility with this change, as there should be a corresponding fcoemon/fipvlan change along with this, and existing utilities will not work. Also the way we start fcoe will be completely different and the user may need to do the scripting changes, if any.

See the last statement from my initial posting (it's below). I have patches to modify fcoemon to use these new interfaces. I'd be happy to share them, I just didn't want to spam this broad of a audience.

I'm concerned that we will be breaking user space compatibility with this change, as there should be a corresponding fcoemon/fipvlan change along with this, and existing utilities will not work. Also the way we start fcoe will be completely different and the user may need to do the scripting changes, if any.

Quoted 7 lines · Bhanu Prakash Gollapudi

I'm concerned that we will be breaking user space compatibility with this change, as there should be a corresponding fcoemon/fipvlan change along with this, and existing utilities will not work. Also the way we start fcoe will be completely different and the user may need to do the scripting changes, if any.

I agree with Bhanu on these concerns, even though I hope everyone's using fcoeadm/fcoemon/fipvlan. I think there needs to be more of a transition plan than requiring everyone moving to a new kernel to update the user-space tools at the same time. Removing the module parameters with the last patch might be rushed, they should probably remain for a few kernel cycles with a warning. When the tools are updated for the new interface, they should probably maintain fallback support for the module parameters for a while as well.

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.

On Mon, Sep 10, 2012 at 3:59 PM, Robert Love <[email protected]> wrote:

Quoted 4 lines

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

Quoted 3 lines

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

Quoted 3 lines

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

Quoted 2 lines

4) Destroy the port
- Logout and free all memory

Quoted 7 lines

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.

Can you give an example session that goes through the 4 steps above and what the sysfs hierarchy looks like at each step? I mostly get it from the patch descriptions, but I think it would help discussion of your proposed interfaces to see an example of them in use.

This feels a little awkward with all the special control files. Have you thought about something designed for creating kernel objects, like configfs? Similarly the separate start, enable, disable files vs.

having some sort of status attribute that can take different values. I feel like these need to be rethought as attributes instead of triggers. Is there a big difference between start and enable? Can you achieve the split between create and start by having it come up in a disabled state by default?

That being said, I'm glad this is being reworked. Do you have any other functionality in mind that this is laying the groundwork for?

Quoted 1 line · Chris Leech

On Mon, Sep 10, 2012 at 3:59 PM, Robert Love <[email protected]> wrote:

Quoted 3 lines

That being said, I'm glad this is being reworked. Do you have any other functionality in mind that this is laying the groundwork for?

I have one feature and a few ideas. I currently have a patch that adds a fabric selection feature. I add another RW attribute to the ctlr_X device. If the user writes fabric name to the file libfcoe uses it in it's FCF selection algorithm. Here's my commit message from that patch. I can share the patch if people would like to see it too. The current implementation also allows the user to force the login through a specific FCF.

Quoted 1 line · Chris Leech

On Mon, Sep 10, 2012 at 3:59 PM, Robert Love <[email protected]> wrote:

<snip>

Quoted 4 lines

Quoted 4 lines

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

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

results in:

/sys/bus/fcoe/devices/ctlr_0/

Quoted 3 lines

Quoted 3 lines

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

# echo "Fabric" > /sys/bus/fcoe/devices/ctlr_0/mode

no visible change

Quoted 3 lines

Quoted 3 lines

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

# echo 1 > /sys/bus/fcoe/devices/ctlr_0/start

Begins discovery and login. Assuming there are FCFs then results in:

/sys/bus/fcoe/devices/fcf_0

Quoted 3 lines

Quoted 3 lines

4) Destroy the port
- Logout and free all memory

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

/sys/bus/fcoe/devices/ctlr_0 is removed.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 5 lines

Can you give an example session that goes through the 4 steps above and what the sysfs hierarchy looks like at each step? I mostly get it from the patch descriptions, but I think it would help discussion of your proposed interfaces to see an example of them in use.

See above. bash-style.

Quoted 3 lines

This feels a little awkward with all the special control files. Have you thought about something designed for creating kernel objects, like configfs? Similarly the separate start, enable, disable files vs.

Let me do some more reading about configfs. I may not have given it enough thought.

Quoted 6 lines

having some sort of status attribute that can take different values. I feel like these need to be rethought as attributes instead of triggers. Is there a big difference between start and enable? Can you achieve the split between create and start by having it come up in a disabled state by default?

It's a good idea. I'll look into it.

Quoted 3 lines

That being said, I'm glad this is being reworked. Do you have any other functionality in mind that this is laying the groundwork for?

I have one feature and a few ideas. I currently have a patch that adds a fabric selection feature. I add another RW attribute to the ctlr_X device. If the user writes fabric name to the file libfcoe uses it in it's FCF selection algorithm. Here's my commit message from that patch. I can share the patch if people would like to see it too. The current implementation also allows the user to force the login through a specific FCF.

libfcoe, bnx2fc, fcoe: Add 'selection' attribute

This patch adds a 'selection' attribute to the fcoe_ctlr_device. The user can write either a '0x' prefixed fabric name or a ':' separated MAC address to this file. If a fabric name is provided the fcoe ctlr will only consider FCFs with the fabric name when choosing a FCF to login to. If a MAC address is provided the initiator will only login to a FCF with the given Ethernet address. Only one selection is valid at a time.

There are corresponding changes to fcoe-utils to take advantage of this kernel feature and to make it more accessible for the user.

To accompany this feature I created a new fipfcf application based on fipvlan that sends out a discovery solicitation and displays advertising FCFs.

I've also been talking with Mark Rustad about doing an 'auto' mode where Fabric discovery is attempted first and if it fails then it tries VN2VN discovery, but so for we've only had hallway conversations about it and nothing has been flushed out.

Thanks, //Rob�{.n�+�������+%��lzwm��b�맲��r��zX���(����ܨ}���Ơz�&j:+v�������zZ+��+zf���h���~����i���z��w���?����&�)ߢf

Quoted 11 lines · Robert W

Quoted 2 lines · Chris Leech

On Mon, Sep 10, 2012 at 3:59 PM, Robert Love
<[email protected]> wrote:

<snip>

Quoted 4 lines

Quoted 4 lines

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

Quoted 1 line

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

I'm assuming the existing functionality of automatically creating the vlan interface by fcoemon (using the cfg-ethX) continues to exist and the above is not a replacement for fcoeadm -c.

Quoted 21 lines

results in:

/sys/bus/fcoe/devices/ctlr_0/

Quoted 3 lines

Quoted 3 lines

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

# echo "Fabric" > /sys/bus/fcoe/devices/ctlr_0/mode

no visible change

Quoted 3 lines

Quoted 3 lines

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

# echo 1 > /sys/bus/fcoe/devices/ctlr_0/start

Begins discovery and login. Assuming there are FCFs then results in:

Quoted 1 line

/sys/bus/fcoe/devices/fcf_0

I'm also assuming that the above three steps can be clubbed by fcoeutils, perhaps by adding 'mode' parameter into the cfg-ethX file. That way 'service fcoe start' will be no different with the proposed model, except that there will be multiple entry points into the driver (alloc, config, start) instead of just one (create).

Quoted 1 line · Chris Leech

On Mon, Sep 10, 2012 at 3:59 PM, Robert Love <[email protected]> wrote:

This feels a little awkward with all the special control files. Have you thought about something designed for creating kernel objects, like configfs? Similarly the separate start, enable, disable files vs. Let me do some more reading about configfs. I may not have given it enough thought.