Re: how to do fc_remote_port_delete correctly

[James Smart <James.Smart-iH1Dq9VlAzfQT0dZR+AlfA@public.gmane.org>]

Joe Eykholt wrote:
> It seems like there are two slightly different sets of rules depending
> on whether dd_fcrport_size is zero or not, as specified by the LLDD.
> 
> In the first model, where dd_fcrport_size is zero, the transport
> never sets dd_data at all.  My understanding now is that its OK
> for the LLDD to set it non-NULL, but not OK to change it after that.
> I guess it would be OK but unnecessary to NULL it at dev_loss timeout
> just before freeing the attached context.  These are the usage rules
> I didn't fully understand. These rules are really established by
> how the LLDDs I/O routines use dd_data.

No. In all cases where you see dd_data, the dd_data field is a transport owned 
field.  The LLDD can change the contents of data pointed to by dd_data, but an 
LLDD  is not supposed to change the dd_data field itself.

> 
> In the second model, it doesn't seem like the LLDD has full control over
> the contents pointed to by dd_data either, since when the remote
> port is re-added the area pointed to by dd_data is cleared by the
> transport, so we always start fresh.  This is fine, but has
> implications on how the context is used during devloss.  For example,
> it shouldn't be used for list linkage unless it's unlinked
> before fc_remote_port_add.  All that's in the LLDDs control, so it's OK.

The only place the transport should be zero'ing the contents are:
a) when it's allocated
b) when it was deleted but kept around to hold the target id binding, then
    reallocated. Note: by deleted, I mean it fully transitioned through
    devloss_tmo_callbk() so for all intents and purposes, its as if it was
    freed and realloc'd.

If there's another case, it's bug and we should be fixing it.

> 
> For libfc, I'm leaning towards continuing to use a non-zero dd_fcrport_size
> and the fc_rport_libfc_priv struct.   libfc could use a separately
> allocated struct like fc_disc_rport for the discovery and
> rport (PLOGI, PRLI, etc.) state machines.

makes sense

> 
> This is all an effort to clean up some issues caused by creating "rogue"
> fc_rports in libfc so that we would always have both an fc_rport_libfc_priv
> and an fc_rport allocated together, even before fc_remote_port_add().
> It causes issues when we do remote_port_add and have to transition
> the state from the rogue to the "real" rport.
> In the meantime, the rogue could still be accessed by incoming requests,
> or new RSCNs, and those changes wouldn't get reflected to the real rport.
> It's messy, and hard to analyze all the potential problems, so I'm
> trying to fix that.
> 
> I really appreciate your help!  Thanks a bunch!
> 
> 	Joe

Ok..  Good Luck.

-- james