libfcoe: Make fcoe_sysfs optional / fix fnic NULL exception
fnic doesn't use any of the create/destroy/enable/disable interfaces
either from the (legacy) module paramaters or the (new) fcoe_sysfs
interfaces. When fcoe_sysfs was introduced fnic wasn't changed since
it wasn't using the interfaces. libfcoe incorrectly assumed that that
all of its users were using fcoe_sysfs and when adding and deleting
FCFs would assume the existance of a fcoe_ctlr_device. fnic was not
allocating this structure because it doesn't care about the standard
user interfaces (fnic starts on link only). If/When libfcoe tried to use
the fcoe_ctlr_device's lock for the first time a NULL pointer exception
would be triggered.
Since fnic doesn't care about sysfs or user interfaces, the solution
is to drop libfcoe's assumption that all drivers are using fcoe_sysfs.
This patch accomplishes this by changing some of the structure
relationships.
We need a way to determine when a LLD is using fcoe_sysfs or not and
we can do that by checking for the existance of the fcoe_ctlr_device.
Prior to this patch, it was assumed that the fcoe_ctlr structure was
allocated with the fcoe_ctlr_device and immediately followed it in
memory. To reach the fcoe_ctlr_device we would simply go back in memory
from the fcoe_ctlr to get the fcoe_ctlr_device.
Since fnic doesn't allocate the fcoe_ctlr_device, we cannot keep that
assumption. This patch adds a pointer from the fcoe_ctlr to the
fcoe_ctlr_device. For bnx2fc and fcoe we will continue to allocate the
two structures together, but then we'll set the ctlr->cdev pointer
to point at the fcoe_ctlr_device. fnic will not change and will continue
to allocate the fcoe_ctlr itself, and ctlr->cdev will remain NULL.
When libfcoe adds fcoe_fcf's to the fcoe_ctlr it will check if ctlr->cdev
is set and only if so will it continue to interact with fcoe_sysfs.
Signed-off-by: Robert Love <robert.w.love@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Tested-by: Hiral Patel <hiralpat@cisco.com>
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 69ac554..27f2cc4 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1381,6 +1381,7 @@
return NULL;
}
ctlr = fcoe_ctlr_device_priv(ctlr_dev);
+ ctlr->cdev = ctlr_dev;
interface = fcoe_ctlr_priv(ctlr);
dev_hold(netdev);
kref_init(&interface->kref);
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index dff40b2..8626988 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -408,6 +408,7 @@
}
ctlr = fcoe_ctlr_device_priv(ctlr_dev);
+ ctlr->cdev = ctlr_dev;
fcoe = fcoe_ctlr_priv(ctlr);
dev_hold(netdev);
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 692c653..75efdbc 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -160,10 +160,16 @@
}
EXPORT_SYMBOL(fcoe_ctlr_init);
+/**
+ * fcoe_sysfs_fcf_add() - Add a fcoe_fcf{,_device} to a fcoe_ctlr{,_device}
+ * @new: The newly discovered FCF
+ *
+ * Called with fip->ctlr_mutex held
+ */
static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
{
struct fcoe_ctlr *fip = new->fip;
- struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
+ struct fcoe_ctlr_device *ctlr_dev;
struct fcoe_fcf_device *temp, *fcf_dev;
int rc = -ENOMEM;
@@ -174,8 +180,6 @@
if (!temp)
goto out;
- mutex_lock(&ctlr_dev->lock);
-
temp->fabric_name = new->fabric_name;
temp->switch_name = new->switch_name;
temp->fc_map = new->fc_map;
@@ -185,55 +189,83 @@
temp->fka_period = new->fka_period;
temp->selected = 0; /* default to unselected */
- fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp);
- if (unlikely(!fcf_dev))
- goto unlock;
-
/*
- * The fcoe_sysfs layer can return a CONNECTED fcf that
- * has a priv (fcf was never deleted) or a CONNECTED fcf
- * that doesn't have a priv (fcf was deleted). However,
- * libfcoe will always delete FCFs before trying to add
- * them. This is ensured because both recv_adv and
- * age_fcfs are protected by the the fcoe_ctlr's mutex.
- * This means that we should never get a FCF with a
- * non-NULL priv pointer.
+ * If ctlr_dev doesn't exist then it means we're a libfcoe user
+ * who doesn't use fcoe_syfs and didn't allocate a fcoe_ctlr_device.
+ * fnic would be an example of a driver with this behavior. In this
+ * case we want to add the fcoe_fcf to the fcoe_ctlr list, but we
+ * don't want to make sysfs changes.
*/
- BUG_ON(fcf_dev->priv);
- fcf_dev->priv = new;
- new->fcf_dev = fcf_dev;
+ ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
+ if (ctlr_dev) {
+ mutex_lock(&ctlr_dev->lock);
+ fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp);
+ if (unlikely(!fcf_dev)) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ /*
+ * The fcoe_sysfs layer can return a CONNECTED fcf that
+ * has a priv (fcf was never deleted) or a CONNECTED fcf
+ * that doesn't have a priv (fcf was deleted). However,
+ * libfcoe will always delete FCFs before trying to add
+ * them. This is ensured because both recv_adv and
+ * age_fcfs are protected by the the fcoe_ctlr's mutex.
+ * This means that we should never get a FCF with a
+ * non-NULL priv pointer.
+ */
+ BUG_ON(fcf_dev->priv);
+
+ fcf_dev->priv = new;
+ new->fcf_dev = fcf_dev;
+ mutex_unlock(&ctlr_dev->lock);
+ }
list_add(&new->list, &fip->fcfs);
fip->fcf_count++;
rc = 0;
-unlock:
- mutex_unlock(&ctlr_dev->lock);
-
out:
kfree(temp);
return rc;
}
+/**
+ * fcoe_sysfs_fcf_del() - Remove a fcoe_fcf{,_device} to a fcoe_ctlr{,_device}
+ * @new: The FCF to be removed
+ *
+ * Called with fip->ctlr_mutex held
+ */
static void fcoe_sysfs_fcf_del(struct fcoe_fcf *new)
{
struct fcoe_ctlr *fip = new->fip;
- struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
+ struct fcoe_ctlr_device *cdev;
struct fcoe_fcf_device *fcf_dev;
list_del(&new->list);
fip->fcf_count--;
- mutex_lock(&ctlr_dev->lock);
-
- fcf_dev = fcoe_fcf_to_fcf_dev(new);
- WARN_ON(!fcf_dev);
- new->fcf_dev = NULL;
- fcoe_fcf_device_delete(fcf_dev);
- kfree(new);
-
- mutex_unlock(&ctlr_dev->lock);
+ /*
+ * If ctlr_dev doesn't exist then it means we're a libfcoe user
+ * who doesn't use fcoe_syfs and didn't allocate a fcoe_ctlr_device
+ * or a fcoe_fcf_device.
+ *
+ * fnic would be an example of a driver with this behavior. In this
+ * case we want to remove the fcoe_fcf from the fcoe_ctlr list (above),
+ * but we don't want to make sysfs changes.
+ */
+ cdev = fcoe_ctlr_to_ctlr_dev(fip);
+ if (cdev) {
+ mutex_lock(&cdev->lock);
+ fcf_dev = fcoe_fcf_to_fcf_dev(new);
+ WARN_ON(!fcf_dev);
+ new->fcf_dev = NULL;
+ fcoe_fcf_device_delete(fcf_dev);
+ kfree(new);
+ mutex_unlock(&cdev->lock);
+ }
}
/**