Robert Love wrote: > This series is the beginning of a FC sysfs reorganization. The new layout > as suggested in this post, http://marc.info/?l=linux-scsi&m=124404278711785&w=2, > creates more sysfs objects to expose FC session information and > decouples the FC sysfs layout from SCSI until FC attaches to the SCSI > mid-layer. > > The first four patches in this series are just preparation patches > that have not been merged. They can mostly be ignored. The fifth patch > introduces FC sysfs and the last patch modifies libfc, libfcoe and fcoe > to use the new FC sysfs interface. > > These patches move code out of scsi_transport_fc.[ch] and into individual > files for each of the new sysfs objects. Compilation creates a new > fc_sysfs.ko module. I'm not sure if this is a good overall approach, but > I wanted to make sure that I didn't overlook any relationships as I made > changes. This results in a patch that doesn't show a good progression of > changes, rather a big change combined with code moving from file(s) to > other file(s). This can be addressed when the patch series is further along > in development. > > To review, it might be easier to apply the patches and look at > drivers/scsi/fc/ and include/scsi/fc.h. > > Functionally both real and NPIV port creation and deletion is working under > libfc/fcoe, with most if not all attributes showing correct values. I did > not spend much time on error handling or cleanliness, these are things > that I'd prefer to address once I'm further along. > > These patches create the following sysfs layout- > > Device tree: > /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_0/host6/fcpinit/host6/ > > LDM tree: > /sys/class/fcport/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_0/host6/fcpinit/host6/ > > Each object has redistributed attributes: > #ls /sys/devices/virtual/net/eth3.101/fcport1/ > active_fc4s device fcfport_2001000deca31e81 maxframe_size power serial_number speed subsystem supported_classes supported_fc4s supported_speeds uevent > > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/ > fcfabric_0 power uevent > Please don't. Do _not_ use physical names in the path names; rather the logical names should be used. So something like /sys/devices/virtual/net/eth3.101/fcport1/fcfport-1:0/ would be preferred. > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/ > fabric_name fcvport_0 fcvport_1 max_npiv_vports npiv_vports_inuse power uevent vport_create vport_delete > Consistent numbering. Either use '_' as a separator always or for none. This mix-up doesn't make sense. And, why doesn't the fcfport doesn't show up under the fabric? One would assume that the fcfport is part of the fabric, too. And what about the remote ports? Do they not participate in the fabric? If the 'fcfabric' element is _not_ the actual fabric I would get rid of it altogether as it'll only serve to confuse the users. Like just has been happened with me :-) > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_0/ > host6 node_name port_id port_name port_type power roles symbolic_name uevent vport_last_state vport_state vport_type > > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_0/host6/ > bsg fcpinit power rport-6:0-0 rport-6:0-2 scsi_host subsystem uevent > See above. Why are the remote ports grouped under the 'host', and not under the fabric? > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_0/host6/fcpinit/ > host6 > fcpinit? What's this for? > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_0/host6/fcpinit/host6/ > device issue_lip port_state power statistics subsystem tgtid_bind_type uevent > > VN_Ports ports show up just like N_Ports: (notice fcvport_1, not fcvport_0) > # ls /sys/devices/virtual/net/eth3.101/fcport1/fcfport_2001000deca31e81/fcfabric_0/fcvport_1/ > host7 node_name port_id port_name port_type power roles symbolic_name uevent vport_last_state vport_state vport_type > > > My open questions are: > > 1) How is the FC4 choice made? > > If FC sysfs allows an FC HBA to support other FC4 types, then how and > when is the selection made? It seems that this decision would either > need to be hard-coded or provided by the user. fcoe.ko requires user > action to start a connection. It would be easy for us to also require > a FC4 selection (maybe it defaults to SCSI-FCP (init)). HW adapters > generally FLOGI on link up and do not require user interaction. How > would this decision be made? > I would consider FCP only currently. Other types would be handled by different subsystems anyway (ie net), which again have a totally different sysfs layout. > 2) How do I reorder fc_host and scsi_host? > > Previously the fc_host is attached as a child of the scsi_host. The > new layout has the scsi_host as a child of fcpinit. These patches > have the fcpinit object created by the scsi transport and attribute > container code. I'm not sure if/how to re-order these objects, it > might require changes to the scsi transport and AC code. > Easiest would be to use the same counter, so that a fc_host preallocates a scsi_host id, too. Not sure if that's possible, though. Alternatively you could create a scsi_host, but _not_ register it with sysfs. This way you would have a number preallocated, too. And when restricting this layout to FCP we will allocate a scsi_host anyway... > 3) What do I do about remote_ports? > > Currently the remote_ports are created by the scsi transport and AC > code which places them as children of the fc_host. I believe this > problem is essentially the same as #2, but with remote ports instead > of the fc_host. > We should try to avoid separate directories for objects which ultimatively map onto the same device. IE it should be possible to gather _all_ information from walking up the devpath only, without having to recurse on other side directories. This way we can match on the various attributes from udev rules; with separate directories this is impossible without a helper script. I'm happy to mail my 'path_id' script around, just to get you an idea where you end up with the separate directory approach :-) > 4) What should the FC sysfs objects be? > > Should they be class objects? I just made them of type 'struct device' > to keep them as flexible as possible. I made the fcport a class object > so that it could be anchored at /sys/class/. > > (last minute note: I just saw in the link referenced above, that the > objects were referred to as classes. I missed that initiatlly, is > there no concern about adding too many classes to /sys?) > The 'class' and 'device' distinction is basically gone with the newer sysfs code. The elements in 'class' are just pointers into the 'device' structure. > 5) How should the FC sysfs objects be named? > > Currently I have two styles. The fcport, fcfabric and fcvport objects > are all enumerated. The fcfport is given a name from its WWPN/WWNN. > It's pretty ugly as is, and the naming needs more consideration. > As mentioned above, yes, it is. Please use logical names. > I don't really like N_Ports being referred to as vports in sysfs either. > > 6) What about complicated relationships between objects? > > Right now this layout cannot elegantly support multiple physical > ports connected to the same F_Port. Each port will have it's own > sysfs directory tree and the two trees will not converge even if > they share the same F_Port/fabric/etc... vports under a physical > port will show up under the physical port's sysfs directory tree. > > 7) Should vport_create/destroy be under fcfabric or fcport? > > I've currently put the vport_create/destroy attributes under the > fcfabric. I think this is fine for now, since there are no > complicated object relationships being presented. There's a one to > one relationship between a fcport and a fcfabric so it's fairly > easy to go back to the fcport, which is needed to be passed to the > LLD so it can create a vport. If there were more than one fcport > per fcfabric it would complicate this. Should the > vport_create/destroy be moved to be attributes of the fcport instead > to avoid a potential lookup if things get more complicated in the > future? > Once the fcfabric is consolidated with fcfport they'll end up under fcport anyway :-) > 8) Should the fcfport and fcfabric be consolidated? > > The fcfport doesn't show much other than its name. The fcfabric > doesn't have many attributes either. Most of its attributes are > vport related. I could see adding some FIP information to the > fcfport, but I'm not sure we need two objects here, especially if > the vport attributes need to be moved to the fcport. > See above. Yes. > 9) Is this change worth all of the changes that will need to happen? > > This is probably the most difficult question. I view this change as > positive overall, but it's very heavy lifting. Not only does the > FC transport need an overhaul, but it looks like the scsi transport > and AC code might need some additions. Every HBA will need to be > modified and every HBA's HBAAPI vendor library will need to change > too. (I hope this would be a catalyst to move to an OS library that > everyone shares) > > I'd like to hear other opinions about this because this really is > going to be something that all HBAs will need to be involved in. > I think it's quite a valid goal. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
