Subject: Re: [nfsv4] Review of draft-ietf-nfsv4-rfc5667bis-06

From: Chuck Lever <[email-masked]>
Date: Wed, 1 Mar 2017 08:40:35 -0800

> On Feb 28, 2017, at 12:41 PM, David Noveck <[email-masked]> wrote:
> 
> > Again, the WG still has one author of that document
> > who is available to us to confirm the intent of
> > that text.
> 
> Tom could weigh in here, but this is text that was written over ten
> years ago.  I'm sure Tom would have interesting things to say about this,
> but, if he is anything like me, he wouldn't remember his precise intent
> in writing any particular sentence.  He would look at the same evidence 
> as we have available to us, and draw conclusions about intent in the
> same way we would, albeit with more knowledge of the circumstances
> of composition.
> 
> In particular, I expect  he would look at the following facts in determining
> intent, just as you or I might:
> 	• That there a reference to a write list and that the XDR thus is specifically designed to accommodate a variable number of items, rather being an optional item (a max-length-one variable-length array)..  True, this is in RFC5666, but given the centrality of NFS to this effort, it is hard to make the case that this was intended to support protocols other than NFS.
> 	• That the fourth paragraph of Section 5 of RFC5667 specifies a matching procedure which would make no sense if the intent was other than to support multiple write chunks.
> 	• That section 5 has an example include a write list of three elements.
> Presumably those items and similar evidence led you to write section 5,4,2 in rfc5667bis.  I will listen to what Tom has to say, but it would be very hard for him to convince me that you somehow misunderstood the intent of RFCs 5666 an 5667.

What I claimed was that the text in RFC 5667 regarding the
use of multiple chunks was incomplete. Based on conversations
I've had with Tom, at least one of the authors recognized
that this text was incomplete.

Further, I claim that there is no prototype implementation
of this specification. So we don't know if it works.

We do know that no-one has seen a need to implement it.

I agree that the original intent was to support multiple
chunks. But I also see that there has been no need to do
it, so far, and that is an important data point.

> > > > However, it might be interesting to some to leave
> > > > some flexibility for future use.
> > >
> > > Do you mean future use within Version One?  Or we
> > > could limit Version One to what it is now., in the expectation
> > > that it will not exist for very long.
> 
> > Again I am at odds with your view. Version One
> > has plenty of capability to last for quite a
> > while, 
> 
> There is no point in us in arguing "very long" vs. "quite a while".  Regardless of how long one expects Version One to be useful,  the question is whether we anticipate sticking strictly with the capabilities of the existing servers  or provide some way to provide within the scope of Version One support for the features that I believe to have been intended in RFC 5666 and 5667.
> 
> I'm not sure where you stand on this issue.  On the one hand, you say, "However, it might be interesting to some to leave some flexibility for future use".  Given your preference for the per-version ULB model, I assumed that meant we would have to have some way of distinguishing servers that provided the current level of support from other servers.  I made some suggestions in that regard which you didn't like.  So the question is whether you have some other way of doing this in mind or you whether you have decided that these limitations will endure for  the entirety of Version One, however one might characterize its duration.

It's not a question of whether or not I liked a particular
proposed mechanism. My job as editor of rfc5667bis is to
keep this document on track and limit creeping scope.

I'm having trouble considering more work and more special
casing in rf5667bis to detect support for features that
do not yet have a real world application, and for which
there is little ability to prototype and test.

Given your publicly-stated desire to see this document
published soon, I'm surprised you would consider
introducing new mechanisms at this point.

Further, I do not like the implication that we should do
something only because no-one has thought of a better
approach. We always have the option of not doing
something that adds complexity with little or no actual
gain.

I'm interested right now in hearing other people's opinions
on whether it is worth completing rfc5667bis as strictly a
document of existing implementations, or whether it should
continue to include what amounts to a speculative feature.

I could make due with permitting only single chunks in
Version One, and explore support for multiple chunks
(and/or more complex COMPOUNDs) in Version Two.

> > and Version Two is years away from
> > appearing in storage products in a robust form.
> 
> Probably so, but to me, the fact that something is going to take a
> while to do makes it more appropriate to push forward, rather than
> less.  And whatever you think about Version One,, it lacks:
> 	• General remote invalidation support
> 	• A default inline threshold size that is appropriate to a protocol that has neither remote invalidation support nor message continuation.
> 	• The ability to decide on and use a threshold bigger than the default. 

Right now Linux has Remote Invalidation support, a
4KB default inline threshold (when interoperating
with another Linux system), and the ability to decide
on and use a larger threshold (up to 64KB). In other
words, everything you've named here, minus "general
Remote Invalidation".

The mechanism it uses to do these things is documented
in a published personal draft:

  draft-cel-nfsv4-rpcrdma-cm-pvt-msg

Linux client happens to support responder's choice
Remote Invalidation. Proper generic support for Remote
Invalidation will be needed to include other clients,
though the only other current client implementation
would need significant internal re-architecture to use
Remote Invalidation of any kind.

Given the existence of the CM private data mechanism,
IMO we are safe focusing on adding real value to
Version Two rather than rushing it forward.

> On Tue, Feb 28, 2017 at 12:46 PM, Chuck Lever <[email-masked]> wrote:
> 
> > On Feb 27, 2017, at 6:09 PM, David Noveck <[email-masked]> wrote:
> >
> > > Earlier I had believed we had some historical reasons
> > > for matching the intent of RFC 5667.
> >
> > I think we do, where that intent is clear.
> >
> > > But if we are
> > > interested only in documenting the behavior of
> > > existing implementations,
> >
> > For some of the cases you mention, there is no possible
> > ambiguity.  The clear intent of RFC5666 and RFC5667
> > is that multiple READs and WRITEs were supposed to be
> > allowed and considerable effort was expended in explaining
> > how they would work.
> 
> If you're referring to handling multiple Read or
> Write chunks per RPC, there is no clear intent.
> That text in RFC 5667 is clearly unfinished, and
> its state is a major reason the WG embarked on
> this update.
> 
> 
> > So it appears that some the things on your liist are bugs.  We
> > may be forced, for practical reasons, to turn a blind eye to these
> > bugs for some period of time, but this is not the same sort of
> > situation as the PZRC-read-chunk issue which is a fairly minor nit
> > based on text with rfc56667 that defies clear interpretation.  Sigh!
> 
> Again, the WG still has one author of that document
> who is available to us to confirm the intent of
> that text.
> 
> The problem is existing implementations do not
> match the capabilities described in RFC 5667.
> 
> 
> > > note that:
> >
> > interesting/depressing list deleted
> 
> Not depressing. Basically this list means that the
> world has survived adequately with incomplete
> implementations of RPC-over-RDMA and NFSv4 for quite
> some time. There has been no need to support multiple
> NFSv4 READ and WRITE operations in a COMPOUND on RDMA
> or on any other transport.
> 
> The issue is whether the WG believes rectifying that
> situation is an immediate need, or something that
> would be nice to allow for the future, or something
> that is not necessary for the remaining future of
> RPC-over-RDMA Version One.
> 
> 
> > > This could be an argument to, in addition, remove a
> > > large piece of text that discusses multiple Write chunks.
> >
> > Perhaps it could.
> >
> > > However, it might be interesting to some to leave
> > > some flexibility for future use.
> >
> > Do you mean future use within Version One?  Or we
> > could limit Version One to what it is now., in the expectation
> > that it will not exist for very long.
> 
> Again I am at odds with your view. Version One
> has plenty of capability to last for quite a
> while, and Version Two is years away from
> appearing in storage products in a robust form.
> 
> 
> > Then we
> > could use base Version Two to implement what
> > Version One was supposed to be and get a small set
> > of performance goodies at the same time.
> >
> > If we on;t want to write off Version One in that way,
> > we need a reliable means of distinguishing servers
> > with full COMPOUND support  from those that you
> > describe.
> 
> > For example, when you say a server only supports a single READ
> > operation, what does it do if it gets a COMPOUND with more than
> > one?  We could have something to work with if:
> >       • All such servers did the same thing.
> >       • It didn't result in disconnection or memory corruption.
> > I thought about an extension adding a full_rdma_compound_support
> > attribute but that doesn't work for V4.0.
> >
> > BTW, do any of these old-fashioned servers with these bugs, recognize
> > and report DDP-eligibility violations?  If not this could be a fool proof way
> > to distinguish servers who may have these implememtation gaps from newer
> > ones that should have full COMPOUND support for RDMA.
> 
> Given the experience we had trying to detect
> the completeness of protocol features with
> RPC-over-RDMA, I really don't relish going
> down that path.
> 
> As far as I am aware, no client I'm aware of
> sends such COMPOUNDs. There is some support in
> the Linux NFS server to handle such COMPOUNDS,
> but as you might guess, there's been no testing
> of this facility with real clients.
> 
> 
> > On Mon, Feb 27, 2017 at 11:38 AM, David Noveck <[email-masked]> wrote:
> > One of the issues that Chuck has to deal with is the need
> > to make new implementations interoperable with existing
> > implementation.  That has been the source of some new
> > MUSTs.
> >
> > Regarding the use of MUSTs, I think these terms are overused
> > in general and that RFC2119's suggestion that these be used
> > sparingly (which ironically uses a "MUST") is too often ignored,
> > including by  the IESG.
> >
> > Regarding your suggestion that 5667bis is using "MUST" too
> > much, a comparison with RFC5667 is instructive.  Including
> > "MUST NOT"s, the RFC2119 term "MUST" Is  used:
> >       • 22 times in RFC5667, a 10-page spec.
> >       • 14 times in rf5667bis-06, an 18-page spec.
> > Part of Chuck's advantage here is that he deleted a lot of the
> > duplication in which RFC5667 ether repeated what was in
> > RFC5666 specialized to NFS (useless) or contradicted it (which
> > really had to go).
> >
> >
> > On Mon, Feb 27, 2017 at 8:16 AM, Tom Talpey <[email-masked]> wrote:
> > On 2/26/2017 3:29 PM, Chuck Lever wrote:
> > On Feb 25, 2017, at 3:54 PM, David Noveck <[email-masked]> wrote:
> >
> > RFC 5667 Section 4 says:
> >
> > Similarly, a single RDMA Read list entry MAY be posted by the client
> > to supply the opaque file data for a WRITE request or the pathname
> > for a SYMLINK request.
> >
> > Part of the problem here is that, as you discuss later, this statement is
> > ambiguous, as the meaning of "read list entry" is not clear.
> >
> > The server MUST ignore any Read list for
> > other NFS procedures,
> >
> > As I understand it, this statement cannot apply to PZRCs, and rfc5666bis
> > has already dealt with that issue.  So, if one tried to maintain this paragraph,
> > in something like the RFC5667-form, some modification would have been
> > necessary to avoid essentially preventing any use of PZRCs
> >
> > as well as additional Read list entries beyond
> > the first in the list.
> >
> > I take "Read list entry" to mean Read chunk, composed of
> > multiple list entries that share the same XDR position.
> > This comports with similar language describing Write
> > chunks where a single list entry is indeed allowed to
> > have multiple segments.
> >
> > Makes sense to me.
> >
> > However, the original intent might have been "single
> > Read segment".
> >
> > It might have been but there is no way to be sure.
> >
> > We can ask Tom Talpey. If he does not recall, then
> > we have no way to be sure.
> >
> > I agree the paragraph in question could have been more clear. I'll
> > hazard a guess that it should have been written as "Read list" instead
> > of "Read list entry", meaning, an entire scatter list is provided.
> > This woud certainly match the semantic for the result of an ordinary
> > NFS Read.
> >
> > I will also observe that the statement is a MAY. That is, it prescribes
> > no behavior, and offers a choice to the implementer. It does not rule
> > out the option of posting a list.
> >
> > I think you guys need to stop worrying about writing these "rules"
> > down so literally. The only goal of RFC5667 was to isolate the tidbits
> > of NFS behaviors separate from the core rpcrdma transport. The
> > document makes relatively few MUST requirements.
> >
> > Tom.
> >
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/nfsv4
> >
> >
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/nfsv4
> 
> --
> Chuck Lever
> 
> 
> 
> 
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Wed, 1 Mar 2017 14:57:13 -0500

> keep this document on track and limit creeping scope.

Fair enough.  Instead of saying:

I made some suggestions in that regard which you didn't like

I should have said:

I made some suggestions in that regard that you felt would
lead the document off track and might result in an desirable

expansion of scope.

> there is little ability to prototype and test.

I hear you.

> introducing new mechanisms at this point.

I wanted to explore the ways this could be done.   I guess
the question regarding "at this point" is exactly where we are.
A few days ago, I thought the document was quite close to
WGLC.  Now it appears it is not and I'm not sure exactly where
this document is.

> approach.

When did I say/imply that?

> We always have the option of not doing

Right, but right now we don't have the option of doing nothing.
Either we do a fairly major surgery on an existing document,
or we do something else.  In any case, you are not comfortable
with any of these alternatives and so we might as well drop
discussion of them.

However, given how ingrained the idea of list of chunks is, I
don't think that this can be though of as a minimally invasive
featurectomy.  That's why I thought of alternatives that you
have trouble with.  If you and the rest of the working group,
think the surgery to remove support for multi-chunk operation
is the most expeditious way to proceed, I don't have a problem
with it.

> continue to include what amounts to a speculative feature.

I'm also interested in hearing other people's opinions.

I'm having trouble with the idea that, a major part of the
current rfc5667bis, which I thought was pretty close to
WGLC, has suddenly become "speculative".

That does not mean that I think we need to keep things
as they are, but I think it has to be understood that to
remove this, we would have to do some pretty substantial
surgery on the current document.

> (and/or more complex COMPOUNDs) in Version Two.

If that;s what you want to do, and the rest of the working group is
OK with it, I don't have a problem.

But note that the following documents are written to support multiple
chunks in a request:

   - RFC5666
   - RFC5667
   - rfc5666bis
   - rfc56667 (at least up until -06)
   - draft-cel-rpcrdma-version-two

so the exploration involved is going to require prototype implementation,
if anyone is interested in doing that.

>>       • A default inline threshold size that is appropriate to a
protocol that has neither remote invalidation support nor message
continuation.

> words, everything you've named here, minus "general

I hadn't known that.

>   draft-cel-nfsv4-rpcrdma-cm-pvt-msg

If, as I believe you are, suggesting that this is an alternative to
accelerating work on Version Two, then I would be OK with that,
provided that we push forward on this work instead.  I will be looking
at this document with a view to seeing what barriers exist to making
it a working group document.  This shouldn't be too hard, given that
prototypes exist and that it provides some of the performance
help we need.

> Remote Invalidation of any kind.

OK.  If expanding the client set is blocked, perhaps it would
be best if this document were made a working group item with a
view toward encouraging other servers to support this mechanism
interoperably.

> Version Two rather than rushing it forward.

I don't think that's what I was proposing, but there isn't
much point in arguing about it.  I'm OK with putting
additional focus on the CM private data mechanism
instead. I don't think I'm proposing "rushing" that forward
either, but you will have a chance to object to any particular
steps you feel are imprudent.

On Wed, Mar 1, 2017 at 11:40 AM, Chuck Lever <[email-masked]> wrote:

> >       • That there a reference to a write list and that the XDR thus is
> specifically designed to accommodate a variable number of items, rather
> being an optional item (a max-length-one variable-length array)..  True,
> this is in RFC5666, but given the centrality of NFS to this effort, it is
> hard to make the case that this was intended to support protocols other
> than NFS.
> >       • That the fourth paragraph of Section 5 of RFC5667 specifies a
> matching procedure which would make no sense if the intent was other than
> to support multiple write chunks.
> > Presumably those items and similar evidence led you to write section
> 5,4,2 in rfc5667bis.  I will listen to what Tom has to say, but it would be
> very hard for him to convince me that you somehow misunderstood the intent
> of RFCs 5666 an 5667.
>
> > There is no point in us in arguing "very long" vs. "quite a while".
> Regardless of how long one expects Version One to be useful,  the question
> is whether we anticipate sticking strictly with the capabilities of the
> existing servers  or provide some way to provide within the scope of
> Version One support for the features that I believe to have been intended
> in RFC 5666 and 5667.
> >
> > I'm not sure where you stand on this issue.  On the one hand, you say,
> "However, it might be interesting to some to leave some flexibility for
> future use".  Given your preference for the per-version ULB model, I
> assumed that meant we would have to have some way of distinguishing servers
> that provided the current level of support from other servers.  I made some
> suggestions in that regard which you didn't like.  So the question is
> whether you have some other way of doing this in mind or you whether you
> have decided that these limitations will endure for  the entirety of
> Version One, however one might characterize its duration.
>
> >       • A default inline threshold size that is appropriate to a
> protocol that has neither remote invalidation support nor message
> continuation.

---

From: Chuck Lever <[email-masked]>
Date: Thu, 2 Mar 2017 10:59:14 -0800

> On Mar 1, 2017, at 11:57 AM, David Noveck <[email-masked]> wrote:
> 
> 	• RFC5666
> 	• RFC5667
> 	• rfc5666bis
> 	• rfc56667 (at least up until -06)
> 	• draft-cel-rpcrdma-version-two

For the moment, let's try to address the Long message
issue from RFC 5667. Basically, the original NFS ULB
specification does not match implementations when it
comes to supporting Long messages with reduced Payload
streams.

Tom has suggested to me privately that the RFC 5667
language was not intended to exclude the "reduced
Payload stream in a Long message" form.

So yesterday at Connectathon, we confirmed that:

- The Solaris client definitely sends Write + Reply
with NFSv4.0 and krb5. Therefore the Solaris server
must support it too. This matches the intent of RFC
5667.

- The Solaris client almost certainly sends PZRC +
normal Read in similar cases (for example, with a
writev(2) on NFSv4 while using krb5). Code exists,
but we haven't seen an actual request on the wire.
The Solaris server must also therefore support
this. This matches the intent of RFC 5667.

- The Linux server does not support Write + Reply,
but a confirmed fix is available for v4.12. I regard
this is as an implementation bug: the Linux server
hasn't officially supported NFS/RDMA with Kerberos
until a few months ago, so it never needed to
support this type of reply.

- The Linux server does not support PZRC + normal
Read. Likewise, this is an implementation bug for
the same reason.

- The Linux client does not send Write + Reply;
it uses Reply only. This is less efficient than it
could be, but should not be an interop problem.

- The Linux client does not send PZRC + normal
Read. Instead it will send a multi-segment PZRC.
The Solaris server does not support multi-segment
PZRCs, even though the spec allows it. We regard
this as a server implementation bug, and it is
being addressed.

It's possible that a writev(2) done on NFSv3 with
krb5 could build a substantial Read segment list,
and thus could be sent via a PZRC + normal Read
chunk.

I recognize that implementations are free to use
Long messages to send reduced Payload streams any
time they want. However, the main user of this
form seems to be NFS/RDMA on krb5. (krb5i and
krb5p always use Long messages without payload
reduction).

Therefore I propose the following:

All of the extra language added to -06 to deal
with reduced Payload streams in Long messages is
unnecessary and will be removed from -07. This
should permit the use of Long messages to convey
reduced Payload streams with all versions of NFS.

Now back to your previously scheduled thread topic.

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Sun, 5 Mar 2017 09:51:47 -0800

> with any of these alternatives and so we might as well drop

To be clear, it's a general discomfort, not something
specific to any of the particular alternatives. I don't
have an appetite for a lot of work on something that is
not immediately useful here, especially because our
mission is writing down how this stuff works right now.

> featurectomy.

I'd like to understand this more. What makes this harder
than just setting a limit?

> That's why I thought of alternatives that you

IMO, a ULB is allowed to set some limits on the use of
the facilities in the underlying transport. The approach
I'm thinking of is defining those limits in rfc5667bis,
possibly replacing the description of how to use multiple
chunks.

> > words, everything you've named here, minus "general

You might be remembering that I was leery of a CM-private-
data-based approach, originally.

Christoph and others in the Linux community convinced me
this mechanism would be an appropriate platform for
experimenting with the features you listed above.

> it a working group document.

Please remember this was conceived of as an enabler for
experimentation. It is a naive design. However, as simple
as the CM-private-data approach is, it does have a limited
ability to be extended. It would not be difficult to add
fields that report how many of each chunk type an
implementation supports per RPC, for example.

> This shouldn't be too hard, given that

> view toward encouraging other servers to support this mechanism

I have heard a rumor that the Solaris team is interested
in this idea, at least to allow somewhat larger inline
thresholds, both on the Solaris client and server.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 6 Mar 2017 06:22:35 -0500

> than just setting a limit?

Perhaps I'm carrying the surgical analogy too far, but I want
to make it clear that I'm not saying this would be anything like
Ben Carson separating conjoined twins.

What I'm basically saying this is a lot more than wart removal and
is probably harder than you think.

> The problem with "just setting a limit" is that it results in a
> document that doesn't make sense.

> the facilities in the underlying transport.

I agree.

> The approach I'm thinking of is defining those limits in rfc5667bis,

You've just described multiple approaches and need to
decide on one before you scrub up.

If you just define those limits, you will wind up with a
document that doesn't make sense.  rfc5666bis defines
sensible rules for matching write chunks with data items
and rfc5667bis-06 sensibly modifies those to account for
the peculiarities of READ_PLUS.

As a result, simply adding those limits will result in a
confusing document.

If you remove those matching rule modifications, you will
have to deal with the restrictions on what READ_PLUS
can return.  Those are still valid, but would be stated
differently in an environment in which there are no requests
with multiple chunks.

I had an easier task with nfsulb.  In that case, the existing text
makes sense as the generic NFS ULB description, and adding
a version-one-specific restriction is not incongruous.

For rfc5667bis, this is surgery and the patent is likely to survive.
Unfortunately, there is no way to properly compensate the
surgeon :-(

> data-based approach, originally.

I didn't remember that.

> Christoph and others in the Linux community convinced me

Sounds like one or more acknowledgements is in order.

> experimenting with the features you listed above.

It appears that the experiment has been successful :-)

> experimentation.

I'm not sure exactly what that means.  Any proposal is an
experiment in that it needs to be implemented.  If it can't,
the experiment has failed.  In this case, the experiment has
succeeded.

There was certainly nothing slapdash about this document.

> It is a naive design.

I would not use the word "naive" regarding the design.
It is simple and accomplishes what needs to be done,
which is enough for me and I hope it will be enough for
the working group.

There is one small element that does seem naive to me
and I cover it in my review, which should be out soon.

> However, as simple
> ability to be extended.

I noticed that but I wasn't particularly concerned about
extensibility.

> It would not be difficult to add

I thought about adding a flag indicating that the server
could support with more than one read chunk or write
chunk, but I didn't think you wanted to make rfc5667bis
dependent on a cm private data RFC.

> thresholds, both on the Solaris client and server.

Now the whole working group has heard that rumour.

If we go forward with this, as I expect we will, then it
would be good if we could hear from the Solaris team directly.

---

From: karen deitke <[email-masked]>
Date: Mon, 6 Mar 2017 08:37:25 -0700

On 3/6/2017 4:22 AM, David Noveck wrote:
> On Sun, Mar 5, 2017 at 12:51 PM, Chuck Lever <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>
>     > On Mar 1, 2017, at 11:57 AM, David Noveck <[email-masked]
>     <mailto:[email-masked]>> wrote:
>     >
>     > >>       • A default inline threshold size that is appropriate
>     to a protocol that has neither remote invalidation support nor
>     message continuation.
Yes, the solaris team is definitely interesting in this implementation 
for larger inline thresholds.

Karen
>     > On Wed, Mar 1, 2017 at 11:40 AM, Chuck Lever
>     <[email-masked] <mailto:[email-masked]>> wrote:
>
>     --
>     Chuck Lever
>
>
>
>
>
>