If the aic94xx chip doesn't have a SAS address in the chip's flash memory,
use the request_firmware() interface to get one from userspace. This
way, there's no debate as to who or how an address gets generated--it's
totally up to the administrator to provide it if the card doesn't have one.
Signed-off-by: Darrick J. Wong <[email protected]>
---
drivers/scsi/aic94xx/aic94xx.h | 1 -
drivers/scsi/aic94xx/aic94xx_hwi.c | 40 +++++++++++++++++++++++++----------
drivers/scsi/aic94xx/aic94xx_init.c | 2 --
3 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx.h b/drivers/scsi/aic94xx/aic94xx.h
index 32f513b..935d558 100644
--- a/drivers/scsi/aic94xx/aic94xx.h
+++ b/drivers/scsi/aic94xx/aic94xx.h
@@ -58,7 +58,6 @@
extern struct kmem_cache *asd_dma_token_cache;
extern struct kmem_cache *asd_ascb_cache;
-extern char sas_addr_str[2*SAS_ADDR_SIZE + 1];
static inline void asd_stringify_sas_addr(char *p, const u8 *sas_addr)
{
diff --git a/drivers/scsi/aic94xx/aic94xx_hwi.c b/drivers/scsi/aic94xx/aic94xx_hwi.c
index 0cd7eed..82a12cc 100644
--- a/drivers/scsi/aic94xx/aic94xx_hwi.c
+++ b/drivers/scsi/aic94xx/aic94xx_hwi.c
@@ -27,6 +27,7 @@
#include <linux/pci.h>
#include <linux/delay.h>
#include <linux/module.h>
+#include <linux/firmware.h>
#include "aic94xx.h"
#include "aic94xx_reg.h"
@@ -38,16 +39,34 @@ u32 MBAR0_SWB_SIZE;
/* ---------- Initialization ---------- */
-static void asd_get_user_sas_addr(struct asd_ha_struct *asd_ha)
+#define SAS_STRING_ADDR_SIZE 16
+static int asd_get_user_sas_addr(struct asd_ha_struct *asd_ha)
{
- extern char sas_addr_str[];
- /* If the user has specified a WWN it overrides other settings
- */
- if (sas_addr_str[0] != '\0')
- asd_destringify_sas_addr(asd_ha->hw_prof.sas_addr,
- sas_addr_str);
- else if (asd_ha->hw_prof.sas_addr[0] != 0)
- asd_stringify_sas_addr(sas_addr_str, asd_ha->hw_prof.sas_addr);
+ const struct firmware *fw;
+ int res;
+
+ /* adapter came with a sas address */
+ if (asd_ha->hw_prof.sas_addr[0])
+ return 0;
+
+ ASD_DPRINTK("No address found for %s; asking for one...\n",
+ pci_name(asd_ha->pcidev));
+
+ /* else go ask userspace */
+ res = request_firmware(&fw, "sas_addr", &asd_ha->pcidev->dev);
+ if (res)
+ return res;
+
+ if (fw->size < SAS_STRING_ADDR_SIZE) {
+ res = -ENODEV;
+ goto out;
+ }
+
+ asd_destringify_sas_addr(asd_ha->hw_prof.sas_addr, fw->data);
+
+out:
+ release_firmware(fw);
+ return res;
}
static void asd_propagate_sas_addr(struct asd_ha_struct *asd_ha)
@@ -657,8 +676,7 @@ int asd_init_hw(struct asd_ha_struct *asd_ha)
asd_init_ctxmem(asd_ha);
- asd_get_user_sas_addr(asd_ha);
- if (!asd_ha->hw_prof.sas_addr[0]) {
+ if (asd_get_user_sas_addr(asd_ha)) {
asd_printk("No SAS Address provided for %s\n",
pci_name(asd_ha->pcidev));
err = -ENODEV;
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index b70d6e7..5c99f27 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -54,8 +54,6 @@ MODULE_PARM_DESC(collector, "\n"
"\tThe aic94xx SAS LLDD supports both modes.\n"
"\tDefault: 0 (Direct Mode).\n");
-char sas_addr_str[2*SAS_ADDR_SIZE + 1] = "";
-
static struct scsi_transport_template *aic94xx_transport_template;
static int asd_scan_finished(struct Scsi_Host *, unsigned long);
static void asd_scan_start(struct Scsi_Host *);
On Mon, 08 Oct 2007, Darrick J. Wong wrote:
> If the aic94xx chip doesn't have a SAS address in the chip's flash memory,
> use the request_firmware() interface to get one from userspace. This
> way, there's no debate as to who or how an address gets generated--it's
> totally up to the administrator to provide it if the card doesn't have one.
So how about factoring that out to a transport-level interface. How
about something along the lines of the following patch, whereby the
software driver upon detecting no valid WWPN, makes an upcall to each
interface's 'request_wwn()'. The data passed in from shost_gendev
should be enough for some helper script to cull relevent device bits
and perhaps offer some level of persistence... Off base?
Darrick, forgive the FC example, I don't do SAS...
--
av
--
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 7a7cfe5..5e0d953 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -35,6 +35,7 @@
#include <linux/netlink.h>
#include <net/netlink.h>
#include <scsi/scsi_netlink_fc.h>
+#include <linux/firmware.h>
#include "scsi_priv.h"
#include "scsi_transport_fc_internal.h"
@@ -3251,6 +3252,30 @@ fc_vport_sched_delete(struct work_struct *work)
vport->channel, stat);
}
+int
+fc_request_wwn(struct Scsi_Host *shost, u64 *wwn)
+{
+ const struct firmware *fw;
+ int stat;
+
+ stat = request_firmware(&fw, "fc_addr", &shost->shost_gendev);
+ if (stat)
+ return stat;
+
+ if (fw->size < 16) {
+ stat = -EINVAL;
+ goto out;
+ }
+
+ stat = fc_parse_wwn(fw->data, wwn);
+ if (stat)
+ return stat;
+
+out:
+ release_firmware(fw);
+ return stat;
+}
+EXPORT_SYMBOL(fc_request_wwn);
/* Original Author: Martin Hicks */
MODULE_AUTHOR("James Smart");
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index e466d88..e80c36c 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -734,4 +734,6 @@ void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 event_number,
*/
int fc_vport_terminate(struct fc_vport *vport);
+int fc_request_wwn(struct Scsi_Host *, u64 *);
+
#endif /* SCSI_TRANSPORT_FC_H */
On Mon, Oct 08, 2007 at 03:48:32PM -0700, Andrew Vasquez wrote:
> So how about factoring that out to a transport-level interface. How
> about something along the lines of the following patch, whereby the
> software driver upon detecting no valid WWPN, makes an upcall to each
> interface's 'request_wwn()'. The data passed in from shost_gendev
> should be enough for some helper script to cull relevent device bits
> and perhaps offer some level of persistence... Off base?
Hrm... jejb made a remark that it might be better to pass the
scsi_host's device into request_firmware() as your example does, so I'll
pitch in a patch to do likewise with libsas--the scsi_host knows the
actual device it's coming from, and userland can sort that all out later
anyway via DEVPATH.
I suppose one could also have multiple scsi_hosts per PCI device, which
means that my first patch would stumble horribly in more than a few
cases.
> Darrick, forgive the FC example, I don't do SAS...
That's ok, I don't do FC. :) Looks mostly good to me...
--D
On Mon, 08 Oct 2007, Darrick J. Wong wrote:
> On Mon, Oct 08, 2007 at 03:48:32PM -0700, Andrew Vasquez wrote:
>
> > So how about factoring that out to a transport-level interface. How
> > about something along the lines of the following patch, whereby the
> > software driver upon detecting no valid WWPN, makes an upcall to each
> > interface's 'request_wwn()'. The data passed in from shost_gendev
> > should be enough for some helper script to cull relevent device bits
> > and perhaps offer some level of persistence... Off base?
>
> Hrm... jejb made a remark that it might be better to pass the
> scsi_host's device into request_firmware() as your example does, so I'll
> pitch in a patch to do likewise with libsas--the scsi_host knows the
> actual device it's coming from, and userland can sort that all out later
> anyway via DEVPATH.
>
> I suppose one could also have multiple scsi_hosts per PCI device, which
> means that my first patch would stumble horribly in more than a few
> cases.
This is done already in the FC case -- NPIV. Though with that
interface, the administrator is already responsible for assigning
proper WWNN/WWPN during creation.
> > Darrick, forgive the FC example, I don't do SAS...
>
> That's ok, I don't do FC. :) Looks mostly good to me...
--
av
Why do you prefer request_firmware() vs something over sysfs ?
Does environments like the kdump kernel also have access to data needed
by request_firmware() ?
-- james s
Andrew Vasquez wrote:
> On Mon, 08 Oct 2007, Darrick J. Wong wrote:
>
>> On Mon, Oct 08, 2007 at 03:48:32PM -0700, Andrew Vasquez wrote:
>>
>>> So how about factoring that out to a transport-level interface. How
>>> about something along the lines of the following patch, whereby the
>>> software driver upon detecting no valid WWPN, makes an upcall to each
>>> interface's 'request_wwn()'. The data passed in from shost_gendev
>>> should be enough for some helper script to cull relevent device bits
>>> and perhaps offer some level of persistence... Off base?
>> Hrm... jejb made a remark that it might be better to pass the
>> scsi_host's device into request_firmware() as your example does, so I'll
>> pitch in a patch to do likewise with libsas--the scsi_host knows the
>> actual device it's coming from, and userland can sort that all out later
>> anyway via DEVPATH.
>>
>> I suppose one could also have multiple scsi_hosts per PCI device, which
>> means that my first patch would stumble horribly in more than a few
>> cases.
>
> This is done already in the FC case -- NPIV. Though with that
> interface, the administrator is already responsible for assigning
> proper WWNN/WWPN during creation.
>
>>> Darrick, forgive the FC example, I don't do SAS...
>> That's ok, I don't do FC. :) Looks mostly good to me...
>
> --
> av
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Tue, 09 Oct 2007, James Smart wrote:
> Why do you prefer request_firmware() vs something over sysfs ?
>
> Does environments like the kdump kernel also have access to data needed
> by request_firmware() ?
There's already much in the way of automation and infrastructure
present in supporting the request_firwmare() interfaces (perhaps not
the best of names) which can provide for a level of flexibility beyond
a basic 'soft_port_name' interface.
Though I don't see why both can't coexist cleanly -- I take it the use
case you are considering is: software recognizes no valid WWPN
available, query via request_firmware() fails, software halts
initialization (rather than fail), and awaits the admin to poke
'0x123456.. > /sys/.../fc_host/soft_port_name', causing a ping to the
driver and continuation of initialization with requested portname?
On Tue, Oct 09, 2007 at 09:41:47AM -0700, Andrew Vasquez wrote:
> On Tue, 09 Oct 2007, James Smart wrote:
>
> > Why do you prefer request_firmware() vs something over sysfs ?
> >
> > Does environments like the kdump kernel also have access to data needed
> > by request_firmware() ?
Assuming the driver-loading parts of the kdump kernel's initrd are the
same (udev, bunch of modules, firmwares, etc) as the regular kernel's
initrd, this shouldn't be a problem.
In the specific case of aic94xx, one needs request_firmware() and
associated infrastructure to load firmware blobs into the controller in
order to issue any I/O at all.
> There's already much in the way of automation and infrastructure
> present in supporting the request_firwmare() interfaces (perhaps not
> the best of names) which can provide for a level of flexibility beyond
> a basic 'soft_port_name' interface.
>
> Though I don't see why both can't coexist cleanly -- I take it the use
> case you are considering is: software recognizes no valid WWPN
> available, query via request_firmware() fails, software halts
> initialization (rather than fail), and awaits the admin to poke
> '0x123456.. > /sys/.../fc_host/soft_port_name', causing a ping to the
> driver and continuation of initialization with requested portname?
Hmm... could we use such a sysfs attribute to reassign adapter WWNs at
arbitrary times? Is that even a good idea?
--D
Darrick J. Wong wrote:
>> Though I don't see why both can't coexist cleanly -- I take it the use
>> case you are considering is: software recognizes no valid WWPN
>> available, query via request_firmware() fails, software halts
>> initialization (rather than fail), and awaits the admin to poke
>> '0x123456.. > /sys/.../fc_host/soft_port_name', causing a ping to the
>> driver and continuation of initialization with requested portname?
>
> Hmm... could we use such a sysfs attribute to reassign adapter WWNs at
> arbitrary times? Is that even a good idea?
As the threads on this topic has shown - allowing any kind of override
or WWN generation isn't a well-liked idea. But there are plausible
scenarios where it makes sense.
Here's one pro for setting WWNs at arbitrary times...
If the box is migrating applications (say containers) that want
different SAN connectivity, where that connectivity (view) is based
on their WWN, it would be really nice to selectively set the WWN and
not touch the SAN config. Granted, in FC, NPIV can do the same thing,
but this could be done on an adapter or configuration that can't do NPIV.
So, what's the decision - are we only allowing this for physical adapters
that don't have a name ? or are we allowing it to be more dynamic ?
My thoughts on request_firmware() vs sysfs:
via request_firmware
pro: It seems to imply a stronger level of control, as it seems more
hidden from the casual admin and/or tools. Too many random things
now peruse sysfs attributes without knowing their meaning.
Also, easier (more correct) to pass multiple elements (WWNN & WWPN)
if needed.
con: ?? when does it get used - only at initial attachment and
under failure ? If you were to make it more arbitrary, doesn't
it imply some sysfs use to poke the driver/transport to get the
new value ? and if arbitrary, how to marry that with kdump so the
initrd is up to date. Also, it's yet another mgmt interface - how many
does an admin need to know about ? Hotplug scripts to identify the
adapter and specify the proper names are more complex for the admin
than sysfs.
via sysfs:
pro: Keeps all mgmt in the sysfs area, which admins are starting to
become comfortable with. Easily supports a dynamic, change at any time
model.
con: Drives to a set-after-attachment model, and doesn't support kdump
well (requires initrd tools). Multiple elements can be a bit cumbersome
if one follows the one-value-per-attribute sysfs rules.
Must protect against random rogue sysfs tools.
Overall, I guess I like the stronger controls of request_firmware(), but dislike
using yet another mgmt interface for admins.
-- james s
James Smart wrote:
> So, what's the decision - are we only allowing this for physical adapters
> that don't have a name ? or are we allowing it to be more dynamic ?
At a minimum, I think(?) we all agree that current upstream aic94xx
behavior is nice: allow the admin to override the WWN manually, even if
the adapter already has one.
As to the question of request_firmware() versus sysfs, it's IMO largely
a question of taste -- do you like the "get property" on-demand pull
model, or a push model that presumes the property must be set before it
is needed? And which solution requires the least amount of additional
userspace machinery in order to be usable?
Jeff
--- James Smart <[email protected]> wrote:
> Here's one pro for setting WWNs at arbitrary times...
> If the box is migrating applications (say containers) that want
> different SAN connectivity, where that connectivity (view) is based
> on their WWN, it would be really nice to selectively set the WWN and
> not touch the SAN config.
No. The WWN stays the same and the SAN is re-configured to be
aware of the node migration. Access patterns could be more
complex than just a single portal allowing access to/from the
SAN from/to the node.
WWN is supposed to be persistent. This is what it actually is.
It is NOT supposed to be "(auto)generated" and/or changed at a whim.
This also has security implications, and enterprise networks may
refuse to use software (read "kernel") which allows for arbitrary
changes to a node's WWN.
If access to a node is compromised, a rogue agent may change
the node's WWN in order to access SAN zones that it normally
will be denied access to.
Luben