Subject: [nfsv4] Revising the pNFS access permissions draft

From: <[email-masked]>
Date: Wed, 16 Jun 2010 21:20:59 -0400

I'm starting to work with Sorin on revising the pNFS access permissions draft:
https://datatracker.ietf.org/doc/draft-faibish-nfsv4-pnfs-access-permissions-check/

It's currently at -02, and there will be a significant rework to produce -03 by
the Maastricht meeting cutoff (and hopefully earlier).

I wanted to get some input and feedback on the direction to take in this revision.
My concerns are around section 3, which contain 3 possible mechanisms for clients
to deal with problems encountered in trying to access a pNFS data server.

The first two mechanisms are intended for use by a client that has discovered that
it cannot access a pNFS data server and wants to inform the pNFS server that it
can't get to that data server:
- 3.1 observes that the object layout already supports this error report in
	its layout-specific data passed in LAYOUTRETURN and proposes revising
	the block and file layouts to do likewise with their layout-specific
	data.
- 3.2 proposes to add a new layoutreturn_type4 value to the enumeration of the
	type value passed to LAYOUT RETURN, accompanied by a data structure for
	the detailed error report so that there's a common way to report this error
	across all layouts.
We should only take one of these forward.  For backwards compatibility reasons, I
prefer the 3.2 approach of adding a new enumeration value, because I don't see
how to determine whether a client passed error info for the block and file layouts
under the 3.1 approach.  The 3.2 approach also passes richer information about
what went wrong.  I could envision text that did not require use of this new
return type with the object layout.

Note that both approaches can also be used to cope with inability to reach the
data server in addition to access denial - there's an nfsstat4 passed in each
case that can convey the appropriate error code.

The third mechanism (in 3.3) is a callback to allow a pNFS server to ask clients
to perform access checks and report the results.  I've heard enough concerns about
this callback that I propose to just drop it.  The callback is not needed to deal
with the basic error reporting problem (either the 3.1 or 3.2 mechanism suffices),
and the concerns I've heard can be summarized by saying it's not clear that the
possible troubleshooting benefits to the pNFS server are worth the effort
to implement this callback, issue it and make sense of its results.

If you care about what we do here, please comment, as we'll probably be revising
this draft in the next week or so.

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953             FAX: +1 (508) 293-7786
[email-masked]        Mobile: +1 (978) 394-7754
----------------------------------------------------

---

From: <[email-masked]>
Date: Fri, 18 Jun 2010 14:43:01 -0400

Hi Benny,

Many thanks for looking at the draft - courtesy of your comments I found
a use case that was overlooked plus a serious design mistake in the
draft (!).  Here's what I propose to do about your concerns:

> The proposed additional return type seems to narrow to me as it only
allows
> to return all the layouts for the given fsid

This appears to bring up an additional use case - our initial work had
assumed that all files in an FS would be striped across all of the
storage devices (data servers) used by that FS.  In 20/20 hindsight,
that's overly restrictive, so I propose adding a new
LAYOUT4_RET_REC_FILE_NO_ACCESS return type in addition to the new
LAYOUT4_RET_REC_FSID_NO_ACCESS return type.  The FILE_NO_ACCESS type
should be useful when the inaccessible storage device only turns up in a
subset of the layouts.  We still need FSID_NO_ACCESS for situations in
which all (or almost all) layouts for the FSID are  
useless due to the inaccessible device server (to avoid having to return
them one-at-a-time).

OTOH, I don't think an ALL_NO_ACCESS case is needed.

> and then it mixes the layoutreturn_file4 fields into
layoutreturn_device4 and for
> some reason drops layoutreturn_file4 altogether.
>
> What do the different fields in layoutreturn_device4 supposed to mean?

That was a serious design mistake in the draft.  The LAYOUTRETURN4_FILE
case in the layoutreturn4 switch (including the layoutreturn_file4 data
structure) needs to be preserved, and then the two
NO_ACCESS cases need to be added to the layoutreturn4 switch:
- for the FSID_NO_ACCESS case, the data structure contains only the
deviceid and NFS status (more on not including additional error info
below).
- for the FILE_NO_ACCESS case, the data structure would need to include
the fields from layoutreturn_file4 that allow partial layout return plus
the device id and status for the error, i.e., roughly specified in the
-02 draft:

     struct layoutreturn_file_no_access4 {
           offset4         lrf_offset;
           length4         lrf_length;
           stateid4        lrf_stateid;
           deviceid4       lrf_deviceid;
           nfsstat4        lrf_status;
           /* layouttype4 specific data */
           opaque          lrf_body<>;
	}

lrf_body<> would be identical to its use in layoutreturn_file4.

> Also, the client should be able to report an array of errors in case
it encountered more
> than one error.

I don't like an array of "errors" for a single device because that's
what syslog and the like are for.  The report is telling the NFSv4
server that it needs to rearrange pNFS because the client has
encountered fatal data server access problems.  The admin who cares
about why needs to go look @ the syslog info.

OTOH, are you suggesting that an array of inaccessible devices be
returned (e.g., because a single underlying event took out access to all
of them)?  That could make sense, although it's optimizing an error case
- perhaps use an array of bad deviceids plus a single NFS status that
applies to all devices in the array?

> I wonder if a new operation would work better.
> First, it will allow the client to report transient errors
> without necessarily returning the layout.

I don't think so - IMHO, syslog should be used for transient errors that
don't require the NFSv4 server to do anything.

> Second, the client could return single layouts rather than the whole
fsid if appropriate.

I agree - see above proposal for a second new return type.  I don't
think a new operation is needed.

> If it's a nfs DS.  How about a block device?
> Is nfsstat4 good for reporting SCSI errors?
> If so, I believe that a mapping should be formally spec'ed.

I disagree - I/O stacks already know how to map SCSI errors into
filesystem-level errors, and I'd prefer not to tinker with that, as it
ain't broken.  FWIW, most SCSI errors will be reported as NFS4ERR_IO.
I'd be willing to add a sentence to say that most SCSI errors are
appropriate to report as NFS4ERR_IO if that would be helpful.

> Or maybe passing back a SCSI sense might be better for blocks?

Maybe, very maybe - it would probably be specific to the block layout.
My default would be to not do this, because for SCSI, a 5-tuple is
required (SCSI transport Service Response, SCSI status, sense key, ASC
and ASCQ - FWIW, only the last 3 of these are sense data), but that'll
have to be optional.  With my "block layout RFC co-author" hat on, I can
see some "nice to have" value to this (e.g., if the SCSI status is
RESERVATION CONFLICT, displaying that on the NFS server console may
rapidly focus attention on the problem), but I don't see this as
essential.

Something to keep in mind are that reporting the SCSI error often isn't
enough - the admin generally has to look at the data server/storage
device logs to figure out what actually happened (that activity can be
assisted by the client reporting all of its SCSI errors via syslog).
Further, there may not be a useful SCSI error to report - two examples
are (1) loss of access to a storage device due to a "backhoe-like"
network event in the data center results in a SCSI transport Service
Response of "SERVICE DELIVERY OR TARGET FAILURE" with no additional SCSI
info, and (2) when SAS is being used to access a SATA drive, the SATA
error is what's really wanted, not the SCSI error.  

> The callback could be nice but may be an  overkill..

I'm glad you agree - that callback's headed for the bit-bucket in the
-03 version ...

Thanks,
--David

> -----Original Message-----
> From: Benny Halevy [mailto:[email-masked]]
> Sent: Thursday, June 17, 2010 5:55 PM
> To: Black, David
> Cc: [email-masked]
> Subject: Re: [nfsv4] Revising the pNFS access permissions draft
> 
> On Jun. 16, 2010, 21:20 -0400, <[email-masked]> wrote:
https://datatracker.ietf.org/doc/draft-faibish-nfsv4-pnfs-access-permiss
ions-check/
> The proposed additional return type seems to narrow to me as it only
allows
> to return all the layouts for the given fsid and then it mixes the
> layoutreturn_file4 fields into layoutreturn_device4 and for some
reason
> drops layoutreturn_file4 altogether.
> 
> What do the different fields in layoutreturn_device4 supposed to mean?
> Also, the client should be able to report an array of errors in case
> it encountered more than one error.
> 
> I wonder if a new operation would work better.
> First, it will allow the client to report transient errors
> without necessarily returning the layout.
> Second, the client could return single layouts rather
> than the whole fsid if appropriate.
> 
> >
> If it's a nfs DS.  How about a block device?
> Is nfsstat4 good for reporting SCSI errors?
> If so, I believe that a mapping should be formally spec'ed.
> Or maybe passing back a SCSI sense might be better for blocks?
> 
> >
> The callback could be nice but may be an  overkill..
> 
> Benny
> 
> >
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/nfsv4

---

From: <[email-masked]>
Date: Thu, 24 Jun 2010 10:12:51 -0400

Benny,

I intend to revise the Internet-Draft to correct and complete the
details of the LAYOUTRETURN design approach, including what an array of
device errors would look like, so that we have a concrete baseline to
discuss whether a separate operation would be a better design.  The
second word in "Internet-Draft" is still "draft", so the revised draft
would not be a final decision on this, but the current draft clearly
needs work just to explain the LAYOUTRETURN approach.  I'll need at
least a week to get the revised draft done, and would hope to discuss it
in Maastricht in addition to on the list.

Thanks,
--David

> Sent: Thursday, June 24, 2010 9:02 AM
> To: Black, David
> Cc: [email-masked]; Brent Welch
> On Jun. 18, 2010, 21:43 +0300, <[email-masked]> wrote:
> > Hi Benny,
> >
> Right.  That's why I think a new op makes more sense as a building
block
> in the protocol - so the client can send it as part of a compound
> preceding a layoutreturn.
> 
> >
> And what about automated handling using basic statistical analysis of
> errors in the cluster to determine when to try to, e.g., reset a
network
> switch - or linecard? Dealing with 1000's of hosts in the cluster
makes
> looking at the syslog quite painful from our experience.
> 
> > of them)?
> Yes.
> > That could make sense, although it's optimizing an error case
> For objects, since we do RAID-5 or RAID-6, it's important for us
> to know the type of error per device (like we have today).
> It's possible to hit a media failure on one device combined with
> a network error on another and the handling / recovery procedures
> are different for each, so squashing the errors into one type doesn't
> seem a good idea to me.
> 
> We can provide for a general array of ~<deviceid, byte range, error
info>
> and maybe restrict its usage by saying that a deviceid SHOULD (or MAY)
NOT
> be repeated in the same array.  I'm still not sure if restring the
client
> not to report multiple errors for the same device is the right thing
to do.
> 
> For example, the objects layout driver tries to send all the errors it
> encountered, whether its for a single device or multiple devices.
> Only if that doesn't fit the maximum size (or if the client ran
> out of memory) we squash the errors and send a "compressed" report.
> 
> In my experience frequent transient error may indicate faulty
components
> in the I/O path and its important to track them.  Just writing them to
> the syslog usually means they'll be buried until someone notices a
degradation
> in performance or something else, bad enough, happens.
> 
> I'm OK with combining it with LAYOUTRETURN as we actually do today for
> the objects layout, I just think it's not the right thing to do from
> the protocol design and extensibility perspective.
> 
> I'd rather define this as a new op and mandate that a LAYOUTRETURN
> MUST follow it than to add a new layoutreturn type, a legacy one with
> no errors and a new one with errors.
> 
> OK.
> OK. let's forget about this then. Note the objects layout type does
> no report wire-level OSD errors as well, but rather it defines a
> pre-defined set of error categories and it's the clients
responsibility
> to map whatever errors it encountered into this set (see
> http://tools.ietf.org/html/rfc5664#section-8.1 pnfs_osd_errno4).
> 
> Benny
> 
> >>> I wanted to get some input and feedback on the direction to take
in
> > this revision.
> >>> prefer the 3.2 approach of adding a new enumeration value, because
I
> > don't see
> >>> what went wrong.  I could envision text that did not require use
of
> > this new
> >>> Note that both approaches can also be used to cope with inability
to
> > reach the
> >>> The third mechanism (in 3.3) is a callback to allow a pNFS server
to
> > ask clients
> >>> to perform access checks and report the results.  I've heard
enough
> > concerns about
> >>> If you care about what we do here, please comment, as we'll
probably
> > be revising