Re: [PATCH v2 7/8] scsi_transport_fc: Added a new sysfs attribute port_state

From: Benjamin Block <bblock@linux.ibm.com>
To: Hannes Reinecke <hare@suse.de>
Cc: Muneendra <muneendra.kumar@broadcom.com>,
	linux-scsi@vger.kernel.org, jsmart2021@gmail.com,
	emilne@redhat.com, mkumar@redhat.com
Subject: Re: [PATCH v2 7/8] scsi_transport_fc: Added a new sysfs attribute port_state
Date: Mon, 5 Oct 2020 10:41:53 +0200	[thread overview]
Message-ID: <20201005084153.GB8365@t480-pf1aa2c2> (raw)
In-Reply-To: <ca995d96-608b-39b9-8ded-4a6dd7598660@suse.de>

On Mon, Oct 05, 2020 at 08:49:27AM +0200, Hannes Reinecke wrote:
> On 10/2/20 6:26 PM, Benjamin Block wrote:
> > On Mon, Sep 28, 2020 at 10:20:56AM +0530, Muneendra wrote:
> > > Added a new sysfs attribute port_state under fc_transport/target*/
> > > 
> > > With this new interface the user can move the port_state from
> > > Marginal -> Online and Online->Marginal.
> > > 
> > > On Marginal :This interface will set SCMD_NORETRIES_ABORT bit in
> > > scmd->state for all the pending io's on the scsi device associated
> > > with target port.
> > > 
> > > On Online :This interface will clear SCMD_NORETRIES_ABORT bit in
> > > scmd->state for all the pending io's on the scsi device associated
> > > with target port.
> > > 
> > > Below is the interface provided to set the port state to Marginal
> > > and Online.
> > > 
> > > echo "Marginal" >> /sys/class/fc_transport/targetX\:Y\:Z/port_state
> > > echo "Online" >> /sys/class/fc_transport/targetX\:Y\:Z/port_state
> > > 
> > > Also made changes in fc_remote_port_delete,fc_user_scan_tgt,
> > > fc_timeout_deleted_rport functions  to handle the new rport state
> > > FC_PORTSTATE_MARGINAL.
> > 
> > Hey Muneendra, out of curiosity, what drives these changes to the
> > port_state in userspace, and based on what?
> > 
> > I imagine something uses the other bunch of sysfs attributes that were
> > introduced recently that get feed by FPIN notifications about congestion
> > and such, or?
> > 
> > If so, is there any plans to integrated something along the lines in
> > multipathd/multipath-tools? Or maybe I missed that.
> > 
>
> My idea here was that that 'marginal' port state is initiated by FPIN
> notifications; ideally set from the driver itself, but initially most likely
> from userspace (eg multipathd).
> 
> Problem here is that the FPIN comes in various flavours (eg congestion or
> link degradation), each of which will result in either one or several FPIN
> notifications.
> EG for link congestion it is expected to get messages at a certain frequency
> for as long as the situation occurs.
> Meaning we would have to have some sort of mechanism for checking that
> frequency, and reset the state if the condition persists.

Yeah, that makes sense.

> 
> How exactly we're going to do this, whether by external daemon or integrated
> into multipathd, is currently subject for debate and testing.
> I would prefer to have it integrated into multipathd, but it might well
> become too complex such that an external daemon might be the better option.
> 

Less "moving parts" would be great. A proper FC setup with multipath is
already a burden on many users, yet an other daemon to juggle with and
coordinate adds more to that. But I'm not doing the work rn so that's
not my choice :)

Thanks for the explanation Hannes.

-- 
Best Regards, Benjamin Block  / Linux on IBM Z Kernel Development / IBM Systems
IBM Deutschland Research & Development GmbH    /    https://www.ibm.com/privacy
Vorsitz. AufsR.: Gregor Pillen         /        Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294