dev_loss_tmo behavior question

dev_loss_tmo behavior question

[Mike Christie]

Hi FC driver developers,

I am trying to figure out what is the correct behavior when setting 
dev_loss_tmo.

With lpfc, qla2xx, and ibmfc if I set dev_loss_tmo using 
/sys/class/fc_remote_port/rport-xx/dev_loss_tmo, and then we add devices 
the slave_configure functions for these drivers reset the dev_loss_tmo 
to a driver value.

The addition of devs could be from something like a user rescan, or from 
a scan started by a remote port addition.

With fcoe, fnic, mptfc, bfa and zfcp the dev_loss_tmo value set from 
sysfs will not be reset by a driver value on rescans.

Which drivers should be changed?

Re: dev_loss_tmo behavior question

[Christof Schmitt]

On Tue, Jul 27, 2010 at 04:11:48PM -0500, Mike Christie wrote:
> Hi FC driver developers,
> 
> I am trying to figure out what is the correct behavior when setting
> dev_loss_tmo.
> 
> With lpfc, qla2xx, and ibmfc if I set dev_loss_tmo using
> /sys/class/fc_remote_port/rport-xx/dev_loss_tmo, and then we add
> devices the slave_configure functions for these drivers reset the
> dev_loss_tmo to a driver value.
> 
> The addition of devs could be from something like a user rescan, or
> from a scan started by a remote port addition.
> 
> With fcoe, fnic, mptfc, bfa and zfcp the dev_loss_tmo value set from
> sysfs will not be reset by a driver value on rescans.
> 
> Which drivers should be changed?

At least for zfcp, i don't see a reason for the driver to change the
value for dev_loss_tmo. A user could delete the SCSI device at any
time from userspace (echo 1 > /sys/.../delete), so the driver has to
be able to handle the device removal at any time. And then it should
not matter if the removal is triggered from userspace or from the FC
transport's dev_loss_tmo.

Christof

Re: dev_loss_tmo behavior question

[James Smart]

Mike Christie wrote:
> Hi FC driver developers,
> 
> I am trying to figure out what is the correct behavior when setting 
> dev_loss_tmo.
> 
> With lpfc, qla2xx, and ibmfc if I set dev_loss_tmo using 
> /sys/class/fc_remote_port/rport-xx/dev_loss_tmo, and then we add devices 
> the slave_configure functions for these drivers reset the dev_loss_tmo 
> to a driver value.
> 
> The addition of devs could be from something like a user rescan, or from 
> a scan started by a remote port addition.

Looking at lpfc - this is a totally insane thing to do, and definitely a bug. 
Addition or removal of sdevs should not change a rport setting.

> 
> With fcoe, fnic, mptfc, bfa and zfcp the dev_loss_tmo value set from 
> sysfs will not be reset by a driver value on rescans.
> 
> Which drivers should be changed?

lpfc, qla2xx, and ibmfc.

-- james s

Re: dev_loss_tmo behavior question

[Andrew Vasquez]

On Wed, 28 Jul 2010, James Smart wrote:

> Mike Christie wrote:
> > Hi FC driver developers,
> > 
> > I am trying to figure out what is the correct behavior when setting 
> > dev_loss_tmo.
> > 
> > With lpfc, qla2xx, and ibmfc if I set dev_loss_tmo using 
> > /sys/class/fc_remote_port/rport-xx/dev_loss_tmo, and then we add devices 
> > the slave_configure functions for these drivers reset the dev_loss_tmo 
> > to a driver value.
> > 
> > The addition of devs could be from something like a user rescan, or from 
> > a scan started by a remote port addition.
> 
> Looking at lpfc - this is a totally insane thing to do, and definitely a bug. 
> Addition or removal of sdevs should not change a rport setting.

Completely agree...

> > With fcoe, fnic, mptfc, bfa and zfcp the dev_loss_tmo value set from 
> > sysfs will not be reset by a driver value on rescans.
> > 
> > Which drivers should be changed?
> 
> lpfc, qla2xx, and ibmfc.

I'm curious though, each driver would still need to seed the rport's
dev_loss_tmo value (in the case of qla2xxx,
ha->port_down_retry_count), but, by doing so after rport-addition
(fc_remote_port_add()), the driver could still overwrite a previous
sysfs setting.  Internally, upon rport creation, the dev_loss_tmo
value is seeding with fc_dev_loss_tmo (a module parameter -- 60
seconds).  Should we extend the transport so the the 'default seeding
value' can be specified once at fc_host creation-time?

BTW: fnic looks to be doing the same 'bad' behaviour in their
slave_alloc() call too...

-- av

Re: dev_loss_tmo behavior question

[Mike Christie]

On 07/28/2010 11:57 AM, Andrew Vasquez wrote:
> I'm curious though, each driver would still need to seed the rport's
> dev_loss_tmo value (in the case of qla2xxx,
> ha->port_down_retry_count), but, by doing so after rport-addition
> (fc_remote_port_add()), the driver could still overwrite a previous
> sysfs setting.  Internally, upon rport creation, the dev_loss_tmo
> value is seeding with fc_dev_loss_tmo (a module parameter -- 60
> seconds).  Should we extend the transport so the the 'default seeding
> value' can be specified once at fc_host creation-time?
>

I was going to add a fc_transport callout that gets called when the 
rport is allocated so drivers can do other rport initialization if they 
wanted. It is only called the first time when it is actually allocated 
not every time fc_remote_port_add is called. Would that be more useful?

Re: dev_loss_tmo behavior question

[James Smart]

Mike Christie wrote:
> On 07/28/2010 11:57 AM, Andrew Vasquez wrote:
>> I'm curious though, each driver would still need to seed the rport's
>> dev_loss_tmo value (in the case of qla2xxx,
>> ha->port_down_retry_count), but, by doing so after rport-addition
>> (fc_remote_port_add()), the driver could still overwrite a previous
>> sysfs setting.  Internally, upon rport creation, the dev_loss_tmo
>> value is seeding with fc_dev_loss_tmo (a module parameter -- 60
>> seconds).  Should we extend the transport so the the 'default seeding
>> value' can be specified once at fc_host creation-time?
>>
> 
> I was going to add a fc_transport callout that gets called when the 
> rport is allocated so drivers can do other rport initialization if they 
> wanted. It is only called the first time when it is actually allocated 
> not every time fc_remote_port_add is called. Would that be more useful?

I would assume this is a fc_host attribute, used in fc_remote_port_add.

Per your last sentence, I assume :
if the rport comes back within the devloss window it definitely is kept to the 
last value set in the rport

if devloss expires, only if you have target id bindings (where you keep the 
rport struct as a container for the binding, thus it is part of the saved 
binding) would you keep the last value set, and it only applies if the binding 
matches.

if no target id bindings, then it would be set again to the fc_host value when 
fc_remote_port_add() is called.


-- james s

Re: dev_loss_tmo behavior question

[Andrew Vasquez]

On Fri, 30 Jul 2010, Mike Christie wrote:

> On 07/28/2010 11:57 AM, Andrew Vasquez wrote:
> > I'm curious though, each driver would still need to seed the rport's
> > dev_loss_tmo value (in the case of qla2xxx,
> > ha->port_down_retry_count), but, by doing so after rport-addition
> > (fc_remote_port_add()), the driver could still overwrite a previous
> > sysfs setting.  Internally, upon rport creation, the dev_loss_tmo
> > value is seeding with fc_dev_loss_tmo (a module parameter -- 60
> > seconds).  Should we extend the transport so the the 'default seeding
> > value' can be specified once at fc_host creation-time?
> >
> 
> I was going to add a fc_transport callout that gets called when the 
> rport is allocated so drivers can do other rport initialization if they 
> wanted. It is only called the first time when it is actually allocated 
> not every time fc_remote_port_add is called. Would that be more useful?

I'm fine with that too...

-- av