scsi_transport_fc: Add sysfs entry for HBA API management library
Subsystems (4)
PATCH 1/1 scsi_transport_fc: Add sysfs entry for HBA API management library
The user space HBA API vendor libraries need to know which HBA/CNAs hosts to manage. Currently, libhbalinux is used to manage a few drivers that use libfcoe and libfc. Right now libhbalinux keys off of the string " over " in the FC Host's symbolic_name attribute to determine if it should manage a given host. fnic, bnx2fc and fcoe/netdev based hosts all use " over " in their symboic_names and that is currently what libhbalinux looks for to determine if it should manage hosts created by those drivers.
Clearly using " over " in the symbolic_name isn't descriptive and it is an awkward way to determine whether libhbalinux should manage a host. Also, drivers may wish to use more descriptive and accurate symbolic_names because these strings are displayed in fabric management applications; forcing them to use " over " in their symbolic_name is unnecessarily restrictive.
This patch adds a read-only attribute to the FC Host that will expose which management library should be used to manage it.
This attribute will not be present in sysfs for drivers that do no implement the .show_hbaapi_lib routine.
--- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 4 ++++ drivers/scsi/fcoe/fcoe.c | 4 ++++ drivers/scsi/fnic/fnic_main.c | 4 ++++ drivers/scsi/scsi_transport_fc.c | 5 ++++- include/scsi/libfc.h | 2 ++ include/scsi/scsi_transport_fc.h | 4 ++++ 6 files changed, 22 insertions(+), 1 deletion(-)
drivers/scsi/bnx2fc/bnx2fc_fcoe.c +4 -0
@@ -731,6 +731,9 @@ static int bnx2fc_shost_config(struct fc_lport *lport, struct device *dev)BNX2FC_NAME ,BNX2FC_VERSION , interface->netdev->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }
@@ -2583,6 +2586,7 @@ static struct fc_function_template bnx2fc_transport_function = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = (sizeof (struct fc_rport_libfc_priv ) + sizeof(struct bnx2fc_rport)),
drivers/scsi/fcoe/fcoe.c +4 -0
@@ -201,6 +201,7 @@ static struct fc_function_template fcoe_nport_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = sizeof(struct fc_rport_libfc_priv), .show_rport_maxframe_size = 1,
@@ -755,6 +756,9 @@ static int fcoe_shost_config(struct fc_lport *lport, struct device *dev)"%s v%s over %s" ,FCOE_NAME ,FCOE_VERSION , fcoe_netdev(lport)->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }
drivers/scsi/fnic/fnic_main.c +4 -0
@@ -138,6 +138,7 @@ static struct fc_function_template fnic_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .show_rport_maxframe_size = 1, .show_rport_supported_classes = 1, .show_host_fabric_name = 1,
@@ -704,6 +705,9 @@ static int __devinit fnic_probe(struct pci_dev *pdev,sprintf (fc_host_symbolic_name (lp->host),DRV_NAME " v" DRV_VERSION " over %s" , fnic->name); +strncpy (fc_host_hbaapi_library (lp->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +spin_lock_irqsave (&fnic_list_lock, flags);list_add_tail (&fnic->list, &fnic_list);spin_unlock_irqrestore (&fnic_list_lock, flags);
drivers/scsi/scsi_transport_fc.c +4 -1
@@ -313,7 +313,7 @@ static void fc_scsi_scan_rport(struct work_struct *work);#define FC_STARGET_NUM_ATTRS 3#define FC_RPORT_NUM_ATTRS 10#define FC_VPORT_NUM_ATTRS 9 -#define FC_HOST_NUM_ATTRS 29 +#define FC_HOST_NUM_ATTRS 30struct fc_internal {struct scsi_transport_template t;
@@ -422,6 +422,7 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev, fc_host->speed =FC_PORTSPEED_UNKNOWN ; fc_host->fabric_name =-1 ;memset (fc_host->symbolic_name,0 ,sizeof (fc_host->symbolic_name)); +memset (fc_host->hbaapi_library,0 ,sizeof (fc_host->hbaapi_library));memset (fc_host->system_hostname,0 ,sizeof (fc_host->system_hostname)); fc_host->tgtid_bind_type =FC_TGTID_BIND_BY_WWPN ;
@@ -1529,6 +1530,7 @@ fc_private_host_rd_attr(max_npiv_vports, "%u\n", 20);fc_private_host_rd_attr (serial_number,"%s\n" , (FC_SERIAL_NUMBER_SIZE +1 ));fc_private_host_rd_attr (manufacturer,"%s\n" ,FC_SERIAL_NUMBER_SIZE +1 );fc_private_host_rd_attr (model,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 ); +fc_private_host_rd_attr (hbaapi_library,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 );fc_private_host_rd_attr (model_description,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 );fc_private_host_rd_attr (hardware_version,"%s\n" ,FC_VERSION_STRING_SIZE +1 );fc_private_host_rd_attr (driver_version,"%s\n" ,FC_VERSION_STRING_SIZE +1 );
@@ -2262,6 +2264,7 @@ fc_attach_transport(struct fc_function_template *ft)SETUP_HOST_ATTRIBUTE_RD (speed);SETUP_HOST_ATTRIBUTE_RD (fabric_name);SETUP_HOST_ATTRIBUTE_RD (symbolic_name); +SETUP_HOST_ATTRIBUTE_RD (hbaapi_library);SETUP_HOST_ATTRIBUTE_RW (system_hostname);/* Transport-managed attributes */
include/scsi/libfc.h +2 -0
@@ -36,6 +36,8 @@#include <scsi/fc_frame.h> +#define FC_LIBHBALINUX_NAME "libhbalinux" +#define FC_FC4_PROV_SIZE (FC_TYPE_FCP + 1)/* size of tables */ /*
include/scsi/scsi_transport_fc.h +4 -0
@@ -514,6 +514,7 @@ struct fc_host_attrs {u32 speed;u64 fabric_name;char symbolic_name[FC_SYMBOLIC_NAME_SIZE ]; +char hbaapi_library[FC_SYMBOLIC_NAME_SIZE ];char system_hostname[FC_SYMBOLIC_NAME_SIZE ];u32 dev_loss_tmo;
@@ -588,6 +589,8 @@ struct fc_host_attrs { (((struct fc_host_attrs *)(x)->shost_data)->fabric_name)#define fc_host_symbolic_name (x) \ (((struct fc_host_attrs *)(x)->shost_data)->symbolic_name) +#define fc_host_hbaapi_library (x) \ + (((struct fc_host_attrs *)(x)->shost_data)->hbaapi_library)#define fc_host_system_hostname (x) \ (((struct fc_host_attrs *)(x)->shost_data)->system_hostname)#define fc_host_tgtid_bind_type (x) \
@@ -748,6 +751,7 @@ struct fc_function_template { unsigned long show_host_speed:1; unsigned long show_host_fabric_name:1; unsigned long show_host_symbolic_name:1; + unsigned long show_host_hbaapi_library:1; unsigned long show_host_system_hostname:1; unsigned long disable_target_scan:1;
Hannes Reinecke
maintainerQuoted 141 lines · Robert Love
The user space HBA API vendor libraries need to know which HBA/CNAs hosts to manage. Currently, libhbalinux is used to manage a few drivers that use libfcoe and libfc. Right now libhbalinux keys off of the string " over " in the FC Host's
symbolic_nameattribute to determine if it should manage a given host. fnic, bnx2fc and fcoe/netdev based hosts all use " over " in theirsymboic_namesand that is currently what libhbalinux looks for to determine if it should manage hosts created by those drivers.Clearly using " over " in the
symbolic_nameisn't descriptive and it is an awkward way to determine whether libhbalinux should manage a host. Also, drivers may wish to use more descriptive and accuratesymbolic_namesbecause these strings are displayed in fabric management applications; forcing them to use " over " in theirsymbolic_nameis unnecessarily restrictive.This patch adds a read-only attribute to the FC Host that will expose which management library should be used to manage it.
This attribute will not be present in sysfs for drivers that do no implement the .show_hbaapi_lib routine.
Signed-off-by Robert Love <[email protected]>Tested-by Ross Brattain <[email protected]>--- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 4 ++++ drivers/scsi/fcoe/fcoe.c | 4 ++++ drivers/scsi/fnic/fnic_main.c | 4 ++++ drivers/scsi/scsi_transport_fc.c | 5 ++++- include/scsi/libfc.h | 2 ++ include/scsi/scsi_transport_fc.h | 4 ++++ 6 files changed, 22 insertions(+), 1 deletion(-)drivers/scsi/bnx2fc/bnx2fc_fcoe.c +4
-0@@ -731,6 +731,9 @@ static int bnx2fc_shost_config(struct fc_lport *lport, struct device *dev)BNX2FC_NAME ,BNX2FC_VERSION , interface->netdev->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }@@ -2583,6 +2586,7 @@ static struct fc_function_template bnx2fc_transport_function = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = (sizeof (struct fc_rport_libfc_priv ) + sizeof(struct bnx2fc_rport)),drivers/scsi/fcoe/fcoe.c +4
-0@@ -201,6 +201,7 @@ static struct fc_function_template fcoe_nport_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = sizeof(struct fc_rport_libfc_priv), .show_rport_maxframe_size = 1,@@ -755,6 +756,9 @@ static int fcoe_shost_config(struct fc_lport *lport, struct device *dev)"%s v%s over %s" ,FCOE_NAME ,FCOE_VERSION , fcoe_netdev(lport)->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }drivers/scsi/fnic/fnic_main.c +4
-0@@ -138,6 +138,7 @@ static struct fc_function_template fnic_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .show_rport_maxframe_size = 1, .show_rport_supported_classes = 1, .show_host_fabric_name = 1,@@ -704,6 +705,9 @@ static int __devinit fnic_probe(struct pci_dev *pdev,sprintf (fc_host_symbolic_name (lp->host),DRV_NAME " v" DRV_VERSION " over %s" , fnic->name); +strncpy (fc_host_hbaapi_library (lp->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +spin_lock_irqsave (&fnic_list_lock, flags);list_add_tail (&fnic->list, &fnic_list);spin_unlock_irqrestore (&fnic_list_lock, flags);drivers/scsi/scsi_transport_fc.c +4
-1@@ -313,7 +313,7 @@ static void fc_scsi_scan_rport(struct work_struct *work);#define FC_STARGET_NUM_ATTRS 3#define FC_RPORT_NUM_ATTRS 10#define FC_VPORT_NUM_ATTRS 9 -#define FC_HOST_NUM_ATTRS 29 +#define FC_HOST_NUM_ATTRS 30struct fc_internal {struct scsi_transport_template t;@@ -422,6 +422,7 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev, fc_host->speed =FC_PORTSPEED_UNKNOWN ; fc_host->fabric_name =-1 ;memset (fc_host->symbolic_name,0 ,sizeof (fc_host->symbolic_name)); +memset (fc_host->hbaapi_library,0 ,sizeof (fc_host->hbaapi_library));memset (fc_host->system_hostname,0 ,sizeof (fc_host->system_hostname)); fc_host->tgtid_bind_type =FC_TGTID_BIND_BY_WWPN ;@@ -1529,6 +1530,7 @@ fc_private_host_rd_attr(max_npiv_vports, "%u\n", 20);fc_private_host_rd_attr (serial_number,"%s\n" , (FC_SERIAL_NUMBER_SIZE +1 ));fc_private_host_rd_attr (manufacturer,"%s\n" ,FC_SERIAL_NUMBER_SIZE +1 );fc_private_host_rd_attr (model,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 ); +fc_private_host_rd_attr (hbaapi_library,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 );fc_private_host_rd_attr (model_description,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 );fc_private_host_rd_attr (hardware_version,"%s\n" ,FC_VERSION_STRING_SIZE +1 );fc_private_host_rd_attr (driver_version,"%s\n" ,FC_VERSION_STRING_SIZE +1 );@@ -2262,6 +2264,7 @@ fc_attach_transport(struct fc_function_template *ft)SETUP_HOST_ATTRIBUTE_RD (speed);SETUP_HOST_ATTRIBUTE_RD (fabric_name);SETUP_HOST_ATTRIBUTE_RD (symbolic_name); +SETUP_HOST_ATTRIBUTE_RD (hbaapi_library);SETUP_HOST_ATTRIBUTE_RW (system_hostname);/* Transport-managed attributes */ include/scsi/libfc.h +0
-0
include/scsi/libfc.h line 43
Quoted 8 lines
include/scsi/libfc.h +2
-0@@ -36,6 +36,8 @@#include <scsi/fc_frame.h> +#define FC_LIBHBALINUX_NAME "libhbalinux" +#define FC_FC4_PROV_SIZE (FC_TYPE_FCP + 1)/* size of tables */ /*
Bah.
You can't be serious.
I totally agree that matching on 'over' is ridiculous.
But hard-coding 'libhbalinux' is also a bad move, simply for the fact that libhbalinux is a user-space library and no-one knows how long it'll stay around. Or if some vendor would like it to be renamed for whatever reason.
No, please. I would rather define a libfc version, and having this one displayed fc host attributes. This way userspace could identify
a) if libfc is used
b) which libfc _version_ is used
and libbhalinux could match onto those.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- 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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 174 lines · Hannes Reinecke
Quoted 170 lines · Robert Love
The user space HBA API vendor libraries need to know which HBA/CNAs hosts to manage. Currently, libhbalinux is used to manage a few drivers that use libfcoe and libfc. Right now libhbalinux keys off of the string " over " in the FC Host's
symbolic_nameattribute to determine if it should manage a given host. fnic, bnx2fc and fcoe/netdev based hosts all use " over " in theirsymboic_namesand that is currently what libhbalinux looks for to determine if it should manage hosts created by those drivers.Clearly using " over " in the
symbolic_nameisn't descriptive and it is an awkward way to determine whether libhbalinux should manage a host. Also, drivers may wish to use more descriptive and accuratesymbolic_namesbecause these strings are displayed in fabric management applications; forcing them to use " over " in theirsymbolic_nameis unnecessarily restrictive.This patch adds a read-only attribute to the FC Host that will expose which management library should be used to manage it.
This attribute will not be present in sysfs for drivers that do no implement the .show_hbaapi_lib routine.
Signed-off-by Robert Love <[email protected]>Tested-by Ross Brattain <[email protected]>--- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 4 ++++ drivers/scsi/fcoe/fcoe.c | 4 ++++ drivers/scsi/fnic/fnic_main.c | 4 ++++ drivers/scsi/scsi_transport_fc.c | 5 ++++- include/scsi/libfc.h | 2 ++ include/scsi/scsi_transport_fc.h | 4 ++++ 6 files changed, 22 insertions(+), 1 deletion(-)drivers/scsi/bnx2fc/bnx2fc_fcoe.c +4
-0@@ -731,6 +731,9 @@ static int bnx2fc_shost_config(struct fc_lport *lport, struct device *dev)BNX2FC_NAME ,BNX2FC_VERSION , interface->netdev->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }@@ -2583,6 +2586,7 @@ static struct fc_function_template bnx2fc_transport_function = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = (sizeof(struct fc_rport_libfc_priv) +
sizeof(struct bnx2fc_rport)),drivers/scsi/fcoe/fcoe.c +1
-0@@ -201,6 +201,7 @@ static struct fc_function_template fcoe_nport_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = sizeof(struct fc_rport_libfc_priv),.show_rport_maxframe_size = 1,
drivers/scsi/fcoe/fcoe.c +3
-0@@ -755,6 +756,9 @@ static int fcoe_shost_config(struct fc_lport *lport, struct device *dev)"%s v%s over %s" ,FCOE_NAME ,FCOE_VERSION , fcoe_netdev(lport)->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }drivers/scsi/fnic/fnic_main.c +1
-0@@ -138,6 +138,7 @@ static struct fc_function_template fnic_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .show_rport_maxframe_size = 1, .show_rport_supported_classes = 1,.show_host_fabric_name = 1,drivers/scsi/fnic/fnic_main.c +3
-0@@ -704,6 +705,9 @@ static int __devinit fnic_probe(struct pci_dev *pdev,sprintf (fc_host_symbolic_name (lp->host),DRV_NAME " v" DRV_VERSION " over %s" , fnic->name); +strncpy (fc_host_hbaapi_library (lp->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +spin_lock_irqsave (&fnic_list_lock, flags);list_add_tail (&fnic->list, &fnic_list);spin_unlock_irqrestore(&fnic_list_lock, flags);drivers/scsi/scsi_transport_fc.c +1
-1@@ -313,7 +313,7 @@ static void fc_scsi_scan_rport(struct work_struct *work);#define FC_STARGET_NUM_ATTRS 3#define FC_RPORT_NUM_ATTRS 10#define FC_VPORT_NUM_ATTRS 9 -#define FC_HOST_NUM_ATTRS 29 +#define FC_HOST_NUM_ATTRS 30struct fc_internal {struct scsi_transport_template t;drivers/scsi/scsi_transport_fc.c +1
-0@@ -422,6 +422,7 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev, fc_host->speed = FC_PORTSPEED_UNKNOWN; fc_host->fabric_name =-1 ;memset (fc_host->symbolic_name,0 ,sizeof (fc_host->symbolic_name)); +memset (fc_host->hbaapi_library,0 ,sizeof (fc_host->hbaapi_library)); memset(fc_host->system_hostname, 0,
sizeof(fc_host->system_hostname));fc_host->tgtid_bind_type = FC_TGTID_BIND_BY_WWPN;drivers/scsi/scsi_transport_fc.c +2
-0@@ -1529,6 +1530,7 @@ fc_private_host_rd_attr(max_npiv_vports, "%u\n", 20);fc_private_host_rd_attr (serial_number,"%s\n" , (FC_SERIAL_NUMBER_SIZE +1 ));fc_private_host_rd_attr (manufacturer,"%s\n" ,FC_SERIAL_NUMBER_SIZE + 1);fc_private_host_rd_attr (model,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 ); +fc_private_host_rd_attr(hbaapi_library, "%s\n",
FC_SYMBOLIC_NAME_SIZE+ 1);
fc_private_host_rd_attr(model_description, "%s\n",FC_SYMBOLIC_NAME_SIZE+ 1);
fc_private_host_rd_attr(hardware_version, "%s\n",FC_VERSION_STRING_SIZE+ 1);
fc_private_host_rd_attr(driver_version, "%s\n",FC_VERSION_STRING_SIZE+ 1);drivers/scsi/scsi_transport_fc.c +1
-0@@ -2262,6 +2264,7 @@ fc_attach_transport(struct fc_function_template *ft)SETUP_HOST_ATTRIBUTE_RD (speed);SETUP_HOST_ATTRIBUTE_RD (fabric_name);SETUP_HOST_ATTRIBUTE_RD (symbolic_name); +SETUP_HOST_ATTRIBUTE_RD (hbaapi_library);SETUP_HOST_ATTRIBUTE_RW (system_hostname);/* Transport-managed attributes */include/scsi/libfc.h +2
-0@@ -36,6 +36,8 @@#include <scsi/fc_frame.h> +#define FC_LIBHBALINUX_NAME "libhbalinux" +#define FC_FC4_PROV_SIZE (FC_TYPE_FCP + 1)/* size of tables */
Quoted 1 line
/*
Bah.
Quoted 7 lines
You can't be serious.
I totally agree that matching on 'over' is ridiculous.
But hard-coding 'libhbalinux' is also a bad move, simply for the fact that libhbalinux is a user-space library and no-one knows how long it'll stay around. Or if some vendor would like it to be renamed for whatever reason.
OK, point taken. I can't say I'm too surprised I got flamed for this one. :) However, as you agree, there is a real problem to solve.
Quoted 7 lines
No, please. I would rather define a libfc version, and having this one displayed fc host attributes.
This way userspace could identify
a) if libfc is used
b) which libfc _version_ is used
and libbhalinux could match onto those.
So, this is not good enough. The question I am trying to answer is, "How does a management library know which fc_host it should manage?" A libfc version would only tell me who is using libfc and which version. This could include a HBA/CNA that uses libfc (kernel), but noes not use libhbalinux (user).
I'd further contend that the kernel is already reporting this information and in string form. It's just using a string that carries no
context: " over ", which really tells you nothing useful. The current patch simply adds context to the string by moving it to be a new attribute.
What if I keep the hbaapi_library attribute on the fc_host, but I make it RW and set it to "" by default. This would allow something in user space to set the HBA API management library to be used. For example, fcoemon could start fcoe on a interface and then set the management library for that interface. Alternatively, an HBA/CNA install script/app could do the same. This would allow the driver to report which library should be used, but the decision making stays in user space.
One last thought on the RW hbaapi_library idea. There is a timing problem, which is that the current fcoe "control" interface discovery/login happens on creation, so the name of the mgmt library won't get set until sometime during the interface discovery/login phase. My new fcoe_sysfs based interfaces allow for a configuration pause before discover/login, which would solve that minor timing issue. So, in my mind this would fit in nicely with the interface change that I'm proposing in another thread.
Thoughts?
Thanks, //Rob
Quoted 222 lines · Robert W
Quoted 174 lines · Hannes Reinecke
Quoted 170 lines · Robert Love
The user space HBA API vendor libraries need to know which HBA/CNAs hosts to manage. Currently, libhbalinux is used to manage a few drivers that use libfcoe and libfc. Right now libhbalinux keys off of the string " over " in the FC Host's
symbolic_nameattribute to determine if it should manage a given host. fnic, bnx2fc and fcoe/netdev based hosts all use " over " in theirsymboic_namesand that is currently what libhbalinux looks for to determine if it should manage hosts created by those drivers.Clearly using " over " in the
symbolic_nameisn't descriptive and it is an awkward way to determine whether libhbalinux should manage a host. Also, drivers may wish to use more descriptive and accuratesymbolic_namesbecause these strings are displayed in fabric management applications; forcing them to use " over " in theirsymbolic_nameis unnecessarily restrictive.This patch adds a read-only attribute to the FC Host that will expose which management library should be used to manage it.
This attribute will not be present in sysfs for drivers that do no implement the .show_hbaapi_lib routine.
Signed-off-by Robert Love <[email protected]>Tested-by Ross Brattain <[email protected]>--- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 4 ++++ drivers/scsi/fcoe/fcoe.c | 4 ++++ drivers/scsi/fnic/fnic_main.c | 4 ++++ drivers/scsi/scsi_transport_fc.c | 5 ++++- include/scsi/libfc.h | 2 ++ include/scsi/scsi_transport_fc.h | 4 ++++ 6 files changed, 22 insertions(+), 1 deletion(-)drivers/scsi/bnx2fc/bnx2fc_fcoe.c +4
-0@@ -731,6 +731,9 @@ static int bnx2fc_shost_config(struct fc_lport *lport, struct device *dev)BNX2FC_NAME ,BNX2FC_VERSION , interface->netdev->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }@@ -2583,6 +2586,7 @@ static struct fc_function_template bnx2fc_transport_function = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = (sizeof(struct fc_rport_libfc_priv) +
sizeof(struct bnx2fc_rport)),drivers/scsi/fcoe/fcoe.c +1
-0@@ -201,6 +201,7 @@ static struct fc_function_template fcoe_nport_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .dd_fcrport_size = sizeof(struct fc_rport_libfc_priv),.show_rport_maxframe_size = 1,
drivers/scsi/fcoe/fcoe.c +3
-0@@ -755,6 +756,9 @@ static int fcoe_shost_config(struct fc_lport *lport, struct device *dev)"%s v%s over %s" ,FCOE_NAME ,FCOE_VERSION , fcoe_netdev(lport)->name); +strncpy (fc_host_hbaapi_library (lport->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +return 0 ; }drivers/scsi/fnic/fnic_main.c +1
-0@@ -138,6 +138,7 @@ static struct fc_function_template fnic_fc_functions = { .get_host_port_state = fc_get_host_port_state, .show_host_port_state = 1, .show_host_symbolic_name = 1, + .show_host_hbaapi_library = 1, .show_rport_maxframe_size = 1, .show_rport_supported_classes = 1,.show_host_fabric_name = 1,drivers/scsi/fnic/fnic_main.c +3
-0@@ -704,6 +705,9 @@ static int __devinit fnic_probe(struct pci_dev *pdev,sprintf (fc_host_symbolic_name (lp->host),DRV_NAME " v" DRV_VERSION " over %s" , fnic->name); +strncpy (fc_host_hbaapi_library (lp->host),FC_LIBHBALINUX_NAME , +FC_SYMBOLIC_NAME_SIZE ); +spin_lock_irqsave (&fnic_list_lock, flags);list_add_tail (&fnic->list, &fnic_list);spin_unlock_irqrestore(&fnic_list_lock, flags);drivers/scsi/scsi_transport_fc.c +1
-1@@ -313,7 +313,7 @@ static void fc_scsi_scan_rport(struct work_struct *work);#define FC_STARGET_NUM_ATTRS 3#define FC_RPORT_NUM_ATTRS 10#define FC_VPORT_NUM_ATTRS 9 -#define FC_HOST_NUM_ATTRS 29 +#define FC_HOST_NUM_ATTRS 30struct fc_internal {struct scsi_transport_template t;drivers/scsi/scsi_transport_fc.c +1
-0@@ -422,6 +422,7 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev, fc_host->speed = FC_PORTSPEED_UNKNOWN; fc_host->fabric_name =-1 ;memset (fc_host->symbolic_name,0 ,sizeof (fc_host->symbolic_name)); +memset (fc_host->hbaapi_library,0 ,sizeof (fc_host->hbaapi_library)); memset(fc_host->system_hostname, 0,
sizeof(fc_host->system_hostname));fc_host->tgtid_bind_type = FC_TGTID_BIND_BY_WWPN;drivers/scsi/scsi_transport_fc.c +2
-0@@ -1529,6 +1530,7 @@ fc_private_host_rd_attr(max_npiv_vports, "%u\n", 20);fc_private_host_rd_attr (serial_number,"%s\n" , (FC_SERIAL_NUMBER_SIZE +1 ));fc_private_host_rd_attr (manufacturer,"%s\n" ,FC_SERIAL_NUMBER_SIZE + 1);fc_private_host_rd_attr (model,"%s\n" ,FC_SYMBOLIC_NAME_SIZE +1 ); +fc_private_host_rd_attr(hbaapi_library, "%s\n",
FC_SYMBOLIC_NAME_SIZE+ 1);
fc_private_host_rd_attr(model_description, "%s\n",FC_SYMBOLIC_NAME_SIZE+ 1);
fc_private_host_rd_attr(hardware_version, "%s\n",FC_VERSION_STRING_SIZE+ 1);
fc_private_host_rd_attr(driver_version, "%s\n",FC_VERSION_STRING_SIZE+ 1);drivers/scsi/scsi_transport_fc.c +1
-0@@ -2262,6 +2264,7 @@ fc_attach_transport(struct fc_function_template *ft)SETUP_HOST_ATTRIBUTE_RD (speed);SETUP_HOST_ATTRIBUTE_RD (fabric_name);SETUP_HOST_ATTRIBUTE_RD (symbolic_name); +SETUP_HOST_ATTRIBUTE_RD (hbaapi_library);SETUP_HOST_ATTRIBUTE_RW (system_hostname);/* Transport-managed attributes */include/scsi/libfc.h +2
-0@@ -36,6 +36,8 @@#include <scsi/fc_frame.h> +#define FC_LIBHBALINUX_NAME "libhbalinux" +#define FC_FC4_PROV_SIZE (FC_TYPE_FCP + 1)/* size of tables */
Quoted 1 line
/*
Bah.
Quoted 7 lines
You can't be serious.
I totally agree that matching on 'over' is ridiculous.
But hard-coding 'libhbalinux' is also a bad move, simply for the fact that libhbalinux is a user-space library and no-one knows how long it'll stay around. Or if some vendor would like it to be renamed for whatever reason.OK, point taken. I can't say I'm too surprised I got flamed for this one. :) However, as you agree, there is a real problem to solve.
Quoted 7 lines
No, please. I would rather define a libfc version, and having this one displayed fc host attributes.
This way userspace could identify
a) if libfc is used
b) which libfc _version_ is used
and libbhalinux could match onto those.So, this is not good enough. The question I am trying to answer is, "How does a management library know which
fc_hostit should manage?" A libfc version would only tell me who is using libfc and which version. This could include a HBA/CNA that uses libfc (kernel), but noes not use libhbalinux (user).I'd further contend that the kernel is already reporting this information and in string form. It's just using a string that carries no
context: " over ", which really tells you nothing useful. The current patch simply adds context to the string by moving it to be a new attribute.What if I keep the
hbaapi_libraryattribute on thefc_host, but I make it RW and set it to "" by default. This would allow something in user space to set the HBA API management library to be used. For example, fcoemon could start fcoe on a interface and then set the management library for that interface. Alternatively, an HBA/CNA install script/app could do the same. This would allow the driver to report which library should be used, but the decision making stays in user space.One last thought on the RW
hbaapi_libraryidea. There is a timing problem, which is that the current fcoe "control" interface discovery/login happens on creation, so the name of the mgmt library won't get set until sometime during the interface discovery/login phase. My newfcoe_sysfsbased interfaces allow for a configuration pause before discover/login, which would solve that minor timing issue. So, in my mind this would fit in nicely with the interface change that I'm proposing in another thread.
Quoted 2 lines
Thoughts?
Rob, you and I spoke about this this afternoon, and after re-reading your patch, I'm still having trouble understanding why any user of the fc host management libraries in userspace need additional information in sysfs. Given your patch above, it seems libhbalinux (and quite likely any other hw dependent management libraries) are detecting which devices they should manage by querying /sys/class/fc_host/<host>/symbolic_name (as well as other sysfs attributes potentially), to detect what hosts they can manage. If libhbalinux needs to further identify hosts that it can manage, can you just use additional sysfs attributes already in existence? Specifically I would think of /sys/class/fc_host/<host>/device/[vendor|device|subsystem_vendor|etc]. It would seem to me those attributes can help you determine which hosts are manageable by libhbalinux. About the only problem I can see with that is if multiple user libraries identify the same host, at which point libhbaapi needs to have some mechanism for determining which library to use for a given host. If it doesn't, that strikes me as a bug in need of fixing.
Perhaps I'm still missing something here (very possible), but I'm just not seeing it at the moment.
Regards
Neil
Quoted 6 lines
Thanks, //Rob
_______________________________________________
devel mailing list
[email protected]
https://lists.open-fcoe.org/mailman/listinfo/devel