FC transport: Calling fc_remote_port_add for online port

FC transport: Calling fc_remote_port_add for online port

[Christof Schmitt]

When a LLD calls fc_remote_port_add for a port that is already in the
state FC_PORTSTATE_ONLINE, the FC transport class will create a new
fc_rport struct and sysfs will show two entries for the same port.

How should this be handled? Does the LLD have to track the state and
only call fc_remote_port add for new ports and ports in the state
FC_PORTSTATE_BLOCKED or FC_PORTTYPE_NOTPRESENT?

Or should the FC transport class allow that a LLD can call
fc_remote_port_add at any time? The patch does this change in the FC
transport class.

Thoughts, comments?

--
Christof Schmitt

---
 drivers/scsi/scsi_transport_fc.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--- a/drivers/scsi/scsi_transport_fc.c	2009-06-23 14:41:20.000000000 +0200
+++ b/drivers/scsi/scsi_transport_fc.c	2009-06-23 15:22:21.000000000 +0200
@@ -2580,9 +2580,7 @@ fc_remote_port_add(struct Scsi_Host *sho
 	spin_lock_irqsave(shost->host_lock, flags);
 
 	list_for_each_entry(rport, &fc_host->rports, peers) {
-
-		if ((rport->port_state == FC_PORTSTATE_BLOCKED) &&
-			(rport->channel == channel)) {
+		if (rport->channel == channel) {
 
 			switch (fc_host->tgtid_bind_type) {
 			case FC_TGTID_BIND_BY_WWPN:

Re: FC transport: Calling fc_remote_port_add for online port

[James Smart]

The LLD should never be calling fc_remote_port_add() twice without an 
intervening fc_remote_port_delete(). Which is another way of saying - 
yes, it's what you say relative to the rport state values.

The idea is - the LLD is tracking an external port structure, using 
<portid, <wwnn,wwpn>> to track identity, and is keeping a "present" and 
"not-present" case. If a new port is deemed "present", it calls 
fc_remote_port_add() and when connectivity  is lost (aka goes to "not 
present") to that port, it calls fc_remote_port_delete(). It should be 
very straight forward.
 
The rport structure itself, to aid the midlayer,  to hide temporary 
connectivity losses due to link bounces, controller resets, etc - may 
stay around after the delete call, and midlayer calls may enter the 
driver for it, but the transport via helper functions, should pick them 
off and reject them. Eventually, when the rport exceeds its max 
connectivity loss hide value (devloss_tmo), the devloss_tmo_callbk() is 
made to tell the driver the rport is truly gone (if it cares), and all 
the midlayer structures and scsi objects below the rport are 
terminated.   After this point, for target id bindings, we keep the 
rport structure around - but only as a generic container to hold the 
binding values. For all intents and purposes, there's no relationship to 
the old rport or the old rport pointer value any more.  And if a port 
eventually comes back that matches the bindings, we flip the binding 
container back into a real rport structure as if we had just allocated it.

-- james s


Christof Schmitt wrote:
> When a LLD calls fc_remote_port_add for a port that is already in the
> state FC_PORTSTATE_ONLINE, the FC transport class will create a new
> fc_rport struct and sysfs will show two entries for the same port.
>
> How should this be handled? Does the LLD have to track the state and
> only call fc_remote_port add for new ports and ports in the state
> FC_PORTSTATE_BLOCKED or FC_PORTTYPE_NOTPRESENT?
>
> Or should the FC transport class allow that a LLD can call
> fc_remote_port_add at any time? The patch does this change in the FC
> transport class.
>
> Thoughts, comments?
>
> --
> Christof Schmitt
>
> ---
>  drivers/scsi/scsi_transport_fc.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> --- a/drivers/scsi/scsi_transport_fc.c	2009-06-23 14:41:20.000000000 +0200
> +++ b/drivers/scsi/scsi_transport_fc.c	2009-06-23 15:22:21.000000000 +0200
> @@ -2580,9 +2580,7 @@ fc_remote_port_add(struct Scsi_Host *sho
>  	spin_lock_irqsave(shost->host_lock, flags);
>  
>  	list_for_each_entry(rport, &fc_host->rports, peers) {
> -
> -		if ((rport->port_state == FC_PORTSTATE_BLOCKED) &&
> -			(rport->channel == channel)) {
> +		if (rport->channel == channel) {
>  
>  			switch (fc_host->tgtid_bind_type) {
>  			case FC_TGTID_BIND_BY_WWPN:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: FC transport: Calling fc_remote_port_add for online port

[Christof Schmitt]

On Fri, Jun 26, 2009 at 12:41:00PM -0400, James Smart wrote:
> The LLD should never be calling fc_remote_port_add() twice without an  
> intervening fc_remote_port_delete(). Which is another way of saying -  
> yes, it's what you say relative to the rport state values.
>
> The idea is - the LLD is tracking an external port structure, using  
> <portid, <wwnn,wwpn>> to track identity, and is keeping a "present" and  
> "not-present" case. If a new port is deemed "present", it calls  
> fc_remote_port_add() and when connectivity  is lost (aka goes to "not  
> present") to that port, it calls fc_remote_port_delete(). It should be  
> very straight forward.
>
> The rport structure itself, to aid the midlayer,  to hide temporary  
> connectivity losses due to link bounces, controller resets, etc - may  
> stay around after the delete call, and midlayer calls may enter the  
> driver for it, but the transport via helper functions, should pick them  
> off and reject them. Eventually, when the rport exceeds its max  
> connectivity loss hide value (devloss_tmo), the devloss_tmo_callbk() is  
> made to tell the driver the rport is truly gone (if it cares), and all  
> the midlayer structures and scsi objects below the rport are terminated.  
>  After this point, for target id bindings, we keep the rport structure 
> around - but only as a generic container to hold the binding values. For 
> all intents and purposes, there's no relationship to the old rport or the 
> old rport pointer value any more.  And if a port eventually comes back 
> that matches the bindings, we flip the binding container back into a real 
> rport structure as if we had just allocated it.
>
> -- james s

Thanks for clarifying this. So a LLD has to always follow the
add-delete sequence.

BTW: I think it could be useful to have this information about the
interface between LLD and FC transport in
Documentation/scsi/scsi_fc_transport.txt

--
Christof Schmitt