FC transport: Calling fc_remote_port_add for online port

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(-)

drivers/scsi/scsi_transport_fc.c +1 -4

@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

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