Subject: [nfsv4] Do the changes to LAYOUTRETURN violate the mversioning rules?

From: Tom Haynes <[email-masked]>
Date: Thu, 3 May 2012 15:18:06 -0700

Trond brought up in the review today whether what were doing violated the
minor versioning rules?

FWIW we originally added a new enum to get the error codes into the LAYOUTRETURN.
We removed that once it was pointed out that the lrf_body<> could be used to
convey the errors in the same manner as done by the Objects-Based Layouts.

In particular, it was claimed that neither the Files-Based or Blocks-Based Layouts
currently (i.e., in NFSv4.1) used that field. Furthermore, there is no standard's
based means for that to be done.

What that means to me is that all non-Objects-Based NFSv4.1 based servers have to 
ignore that field. And further, in NFSv4.2, we state that:

  4246     I.e., the Objects-Based Layout protocol can continue to utilize
  4247     lrf_body as specified in [9].  For both Files-Based Layouts, the

A minimal NFSv4.2 server which uses its NFSv4.1 LAYOUTRETURN handling code
will either

  1) Ignore lrf_body (files and blocks)
  2) Continue to use pnfs_osd_ioerr4 (objects)

Further, if we look at the XDR, we see:

   /// struct layoutreturn_file4 {
   ///         offset4         lrf_offset;
   ///         length4         lrf_length;
   ///         stateid4        lrf_stateid;
   /// %       /* layouttype4 specific data */
   ///         opaque          lrf_body<>;
   /// };

I.e., lrf_body<> is an opaque array.

Now if we look at the minorversioning rules in RFC5661, section 2.7:

   3.   Minor versions must not modify the structure of an existing
        operation's arguments or results.

        Again, the complexity of handling multiple structure definitions
        for a single operation is too burdensome.  New operations should
        be added instead of modifying existing structures for a minor
        version.

        This rule does not preclude the following adaptations in a minor
        version:

        *  adding bits to flag fields, such as new attributes to
           GETATTR's bitmap4 data type, and providing corresponding
           variants of opaque arrays, such as a notify4 used together
           with such bitmaps

        *  adding bits to existing attributes like ACLs that have flag
           words

        *  extending enumerated types (including NFS4ERR_*) with new
           values

        *  adding cases to a switched union

We are doing part of the first adaption, providing a new corresponding variant of an
opaque array. I'll further argue that the layout type is the "flag" we need to
set.

It is a stretch, but just a small one. :->

-- 
[email-masked], ex-cfb

---

From: Boaz Harrosh <[email-masked]>
Date: Fri, 4 May 2012 10:57:08 +0300

> opaque array. I'll further argue that the layout type is the "flag" we need to

I'd say the length word of the opaque is the flag. In V4.1 that length word was
forced to zero by files/blocks.

Therefor If length is none zero this means a 4.2 Server should handle it, as by
the new structures. A 4.2 enabled client connected to a 4.1 Server should force
length byte to zero so to not confuse old Servers.

And in combination with the layout-type as you said as well.

> It is a stretch, but just a small one. :->

I don't think it's a stretch, it is easily handled by any server, Only extra
code is at client to not encode lrf_body<> if 4.1 was negotiated.

Thanks
Boaz

---

From: "Haynes, Tom" <[email-masked]>
Date: Fri, 4 May 2012 16:17:52 +0000

>forced to zero by files/blocks.

Good point, thanks!

---

From: "J. Bruce Fields" <[email-masked]>
Date: Fri, 4 May 2012 13:36:18 -0400

On Fri, May 04, 2012 at 04:17:52PM +0000, Haynes, Tom wrote:
> >I'd say the length word of the opaque is the flag. In V4.1 that length

The length is not a flag field.  I can see how it's sort of like that
case, but it's not exactly the same thing.

> >word was

Could people who want to do this at least propose language updating the
above (rfc 5661 section 2.7) to cover this case?

--b.

---

From: "Haynes, Tom" <[email-masked]>
Date: Fri, 4 May 2012 18:36:08 +0000

>above (rfc 5661 section 2.7) to cover this case?

What would we put?

If an opaque field is not being used, then it is okay to start using it in
a minor release?

IMHO, there should have been a flag field to cover lrf_body in 5661. There
isn't and instead of adding an exception to the minor release rules, I'd
rather we be more diligent in the future to make sure we do add flags
where we use such opaque values.

What I think we then do is add a paragraph to Section 13.9 of the NFSv4.2
draft stating:

While this modification does not meet the listed criteria (see Section 2.7
of [2]) for inclusion in a minor version, the fact that this opaque array
is ignored by Files-Based and Blocks-Based Layout protocol servers makes
this a safe change. Further, the Objects-Based Layout   protocol is able
to continue using the mechanism established in [9].

---

From: "Myklebust, Trond" <[email-masked]>
Date: Fri, 4 May 2012 18:37:52 +0000

> >forced to zero by files/blocks.

So basically, you are saying that changing that length to non-zero will
break a lot of server expectations?

Cheers
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
[email-masked]
www.netapp.com

---

From: Boaz Harrosh <[email-masked]>
Date: Fri, 4 May 2012 22:20:04 +0300

> break a lot of server expectations?

How did you deduce that? I'm saying the (very small) burden is on the
client, to not encode the opaque if negotiated version is 4.1.

The server need not do anything special. If opaque is empty as it will
usually be, do nothing, if present decode it.

So 4.2 Server need not do anything special, even with 4.1 clients.

And 4.1 Server will not break because 4.2 client will not encode
it.

What broken scenario do you envision. Please forgive my slowness.

Thanks
Boaz

---

From: "Haynes, Tom" <[email-masked]>
Date: Fri, 4 May 2012 12:29:41 -0700

> client, to not encode the opaque if negotiated version is 4.1.

The client can encode it regardless of the version.

A NFSv4.1 server will ignore the lrf_body.

A partial NFSv4.2 server (i.e., minimal implementation and using most of NSv4.1)
can ignore the lrf_body as if it were a NFSv4.1 server.

A full NFSv4.2 server can decide for itself whether it will ignore it or not.
I.e., a fully functional NFSv4.2 client will have a hard time telling
whether a fully functional NFSv4.2 server is using the error information.

---

From: "Myklebust, Trond" <[email-masked]>
Date: Fri, 4 May 2012 19:38:27 +0000

> whether a fully functional NFSv4.2 server is using the error information.

Umm... No... A NFSv4.1 server may quite legitimately return
NFS4ERR_BAD_XDR in reply to a non-empty lrf_body.

I'd argue that so can the NFSv4.2 server, since this is a feature that
cannot be made mandatory to implement until 4.3...

NetApp

---

From: Boaz Harrosh <[email-masked]>
Date: Fri, 4 May 2012 22:45:03 +0300

> whether a fully functional NFSv4.2 server is using the error information.

I agree. At-least the two implementations I work with Linux-knfsd and Ganesha
will behave like that. If the client started to encode them out of the blue
with no warning or negotiated 4.2 version, *nothing will happen* they are just
ignored, if not objects.

But you don't know what bugs might exist in other implementations.

BTW: In objects-layout (>= V4.1) the error reporting is mandatory in the face of
errors. The Server relays on that, for smooth operations. Otherwise the what we
call "file recovery" is only detected on close/open. Where raid5 is fixed up and
file_length deduced from objects-lengths. If error is reported info is recovered
sooner and depending on the error, recovery might be much simpler to none.
In anyway it is assumed that the clients does it on any IO error.

I guess for 4.2 blocks/files it is always optional? And Server needs to detect
those in back channel protocol. (With block it cannot actually)

Thanks
Boaz

---

From: Boaz Harrosh <[email-masked]>
Date: Fri, 4 May 2012 22:56:13 +0300

> cannot be made mandatory to implement until 4.3...

I'd suggest text is put forward in NFSv4.2 that minimal NFSv4.2 Server
should not return NFS4ERR_BAD_XDR even if this info is ignored, so to avoid
that problem. So implementation is optional until 4.3, but NFS4_OK return
is mandatory, no?

And I suggest text is put forward in NFSv4.2 that client should never
encode lrf_body if server is V4.1 as I suggested before. So to avoid
any possible NFS4ERR_BAD_XDR.

just my $0.017
Thanks
Boaz

---

From: "Haynes, Tom" <[email-masked]>
Date: Fri, 4 May 2012 14:29:11 -0700

> any possible NFS4ERR_BAD_XDR.

A simple approach:

1) NFSv4.1 servers can either ignore a non-NULL lrf_body or report NFS4ERR_BADXDR.

2) Minimal NFSv4.2 servers (i.e., a NFSv4.1 server with an extension to return
not supported for all new features) SHOULD react like the base NFSv4.1 implementation.

3) NFSv4.2 servers either ignore a non-NULL lrf_body or handle it as per the
draft.

But I think a better way to handle this is to use an errata to 5661 to state that File
and Block based layout protocols have to ignore lrf_body.

---

From: Boaz Harrosh <[email-masked]>
Date: Sun, 6 May 2012 12:27:28 +0300

> not supported for all new features) SHOULD react like the base NFSv4.1 implementation.

Why not combine 2 & 3:

2) Minimal NFSv4.2 servers (i.e., a NFSv4.1 server with an extension to return
   "not-supported" for all new features) SHOULD: ignore a non-NULL lrf_body

You already fired up the editor and compiled in new stuff, whats the fuss?
Where would that not work in any possible world?

> and Block based layout protocols have to ignore lrf_body.

Yes I like that errata a lot. It just specifies what everyone is already doing today.
Which makes it a very good errata.

Thanks, sounds like a plan
Boaz

---

From: <[email-masked]>
Date: Sun, 6 May 2012 09:57:03 -0400

> and Block based layout protocols have to ignore lrf_body. 

I think it would be cleaner if the stuff about block-based layouts were in an errata to
5663.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Haynes, Tom
Sent: Friday, May 04, 2012 5:29 PM
To: Boaz Harrosh
Cc: Haynes, Tom; Myklebust, Trond; [email-masked]
Subject: Re: [nfsv4] Do the changes to LAYOUTRETURN violate the mversioning rules?

A simple approach:

-- 
[email-masked], ex-cfb
_______________________________________________
nfsv4 mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/nfsv4