Re: fcoe cleanups and fcoe TODO

[Mike Christie <michaelc-hcNo3dDEHLuVc3sceRu5cw@public.gmane.org>]

Love, Robert W wrote:
>> Hey,
>>
>> Following are some cleanup patches for things I found while trying
>> to get setup. I did not get to test the patches well, because
>> the rearch code does not seem to work every well the software
>> target. I basically loaded the driver and made sure I could login
>> and find storage. Trying to do heavy READ and WRITE IO lead to
> problems.
>> The patches are made over the re-arch git tree.
>>
> Awesome, I've reviewed a few and will get them in shortly.


Oh yeah, I just rebased my tree and saw all the checkins from the 
weekend so my patches do not apply from a tree that is pulled today. Are 
you going to fix that for me, or do you want me to rediff and resend 
when you are done reviewing. I heard the iscsi maintainer is really nice 
and will rediff for you :)


>> - I am not sure if the setup functions like fc_lport_init are nice
>> in how it sets the transport functions pointers for you or if it will
> lead
>> to problems for drivers that want to partially hook in, but find
>> out that the lib has set some functions it did not want. Maybe we want
>> to set the libfc functions in the LLD like is done for the
>> scsi_host_template.
>>
> Yeah, we've debated whether we should have the lib init the pointers and
> then have the LLD override them or have the LLD set them first and have
> the lib fill in the un-initialized ones. The current design allows a LLD
> to override the default functions with its own. Is your suggestion to
> expose all of the functions that could satisfy the tt
> (fc_transport_template) pointers through the libfc API and have the LLD
> set them all? 


It was, but for now we probably do not need to change it unless someone 
cares. I think it is best to do this last since it is not a big deal and 
we still do not know what drivers are using and things are more stable.


> 
> Throughout this re-architecture the tt and libfc API have grown and
> shrunk. Lately we've bloated the tt to expose functions that were
> previously called directly from one UL system (fc_scsi, fc_lport,
> fc_rport or fc_disc) to another. The number of functions needed in the
> tt and also required in the libfc API seemed like too much and having a
> little _init() function for each UL system seemed cleaner. We're getting
> closer to having a cleaner API between each UL system so maybe we can
> move the assignment of tt pointers back to the LLD.
> 
>> I think we need to see more hardware in the end. For now since there is
>> only
>> software fcoe that is out, this could probably be ok and as we see more
>> drivers we can evolve the split for the hardware like was done with
> iscsi.
>> For iscsi we ended up with 4 different offload modules so there was no
> way
>> we could have designed for all of them and the hardware/firmware
>> guys's quirks :)
>>
> We definitely expect this library to evolve as more LLDs use it. 
> 
>> - In general without knowing how hardware is really going to hook into
>> the lib it is hard to say how the rearch is going :) If we cannot
>> move more of the fc_remote_port to the rport (or if we just do not
>> want to move it all there because other drivers like lpfc and qla2xxx
>> will never use it) then we might want to rename the fc_remote_port
>> to avoid confusion with the rport and make it clear that the libfc
> remote
>> port is only needed for libfc operations.
>>
> We'd like to reduce fc_remote_port to nothing and only use fc_rport.
> We're still not sure we can completely achieve this without either
> having some private data or adding fields to the fc_rport.



On a related note, are there going to be values that we need to export 
in sysfs that are specific to fcoe? For remote port values are we going 
to export them on the fc_rport (is the fc_rport for anything and 
everything related to FC) or some new struct?

If it is a new struct, what if a driver does not use any 
fc_transport_template callouts and does not hook into libfc?


> 
>> Should the fc_transport_template be merged with the
> fc_function_template?
>> The function callouts to do things lke abort IO or cleanup commands
> look
>> similar. But in general should the fc_function_template be common for
> all
>> fc drivers and should the fc_transport_template be just for the ones
> that
>> are
>> going to hook into the libfc? If the latter maybe we want to rename
> this to
>> make the visibility clear.
>>
> So something like libfc_transport_template?


That makes it clear, but I do not know if it is a good name. It sounds 
ugly like something I would name it (I am not good at naming as you can 
tell from some of the iscsi function names). All I know is that the 
original is not so clear :)