Re: [nfsv4] Remaining errata reports for rfc5661bis

[David Noveck <[e-mail]>]

On Fri, Sep 30, 2022 at 1:39 PM Chuck Lever III <[e-mail]>
wrote:

>
>
> On Sep 30, 2022, at 12:14 PM, David Noveck <[e-mail]> wrote:
>
>
>
> On Fri, Sep 30, 2022 at 11:00 AM Chuck Lever III <[e-mail]>
> wrote:
>
>>
>>
>> On Sep 30, 2022, at 9:27 AM, David Noveck <[e-mail]> wrote:
>>
>> The following are not, for various reasons, listed as addressed in
>> draft-dnoveck-nfsv4-rfc5661bis-05.   I'd like to start wg discussion of
>> these so that we can address as many as possible in
>> draft-ietf-nfsv4-rfc5661bis-00.
>>
>>    - *2722*
>>
>> This was actually addressed in rfc8881 😊.  See Err930 (attached) for an
>> updated list
>>
>>
>>    - *2751*
>>
>> I'm kind of uncertain about the proposed new section 12.5.4.1.   Given
>> its size, it needs some working group discussion before being put in an RFC.
>>
>>
>> I agree, further WG discussion is warranted.
>>
>
I think we will haVe to have that discussion after -00 is submitted, unless
somebody is in a big hurry.  This one will prpbably remain TBD for  a
while.

>
>>
>>    - *3067*
>>
>> I'm OK with the substance of this but:
>>
>>
>>    - I'm unclear what "deprecated" means in a Proposed Standard.
>>
>> I propose simply removing the sentence containing "deprecated".
>>
>
> I'd prefer simply saying these fields are no longer used.   I intend to
> also indicate this via comments in rfc5662bis.
>
>
>>
>>    - Regarding use of "*SHOULD*", unclear what might be valid rasons to
>>       ignore the recommendation.  Also, am dubious about the capacity of non-zero
>>       values to cause harm given thathe server is ignoring them.
>>
>> I propose removing that sentence as well.
>>
>
> OK.
>
>
>>
>>
>>    - Worried about the use of lower-case "should" in he last two
>>       sentences.
>>
>> I propose replacing "should" with "MUST" in both sentences.
>>
>
> Although I'd prefer "is to",   I can live with "MUST".
>
> The big problem with these two sentences is that the condition in the
> first is a subset of the condition an you can wind up with situations in
> which it would say you "MUST" return two different errors, which is self
> contradictory 😞
>
> Propose the following but am open to replacing "is to" by "MUST":
>
> For the case where the client does not hold any previously granted
> layout,
>
> the server is to return the error NFS4ERR_BAD_LAYOUT. Otherwise, where no
> previously
> granted layout has an iomode of LAYOUTIOMODE4_RW, the server is to return
> the error NFS4ERR_BAD_IOMODE.
>
>
> That seems to eliminate the contradiction.
>
> Re: "is to" v. "MUST": Using the phrase "is to" means clients cannot rely
> on servers to recognize and return these errors. I've always preferred the
> use of MUST in these cases -- I've known too many implementers who look at
> the current text and decide the guidance is inconvenient and therefore does
> not apply to them, and that makes the text pretty meaningless.
>

As previously indicated, I can live with "MUST" so I think we can close on
this one in -00.

>
>
>
>>    - *4118*
>>
>> I'm OK with the substance of this but:
>>
>>
>>    - Think the "*SHOULD*" in the third sentence is not appropriate.
>>       Think "is to be ignored" is better.
>>
>> I propose replacing "SHOULD" with "MUST".
>>
>
> I think it is better to say this field has no function.   As far as use of
> "MUST", how could the server depend on this being ignored.
>
>
> We are making a protocol compliance statement.
>

I don't think we should be making a protocol compliance statement in the
bis.   In rfc5661, these fields had a function which was described there,
whether the RFC2119 keywords were used or not.   Now, with the five related
errata reports which are intended to "deprecate" those fields, they are no
longer used.   there is no point in describing how unused fields are to be
used, whether RFC2119.   There is no way to use them nd the spec needs to
clearly state that.


> MUST is a legitimate and conventional way of identifying an unused
> protocol element.
>

I don't believe so.


> As an example, here is language from RFC 8797 about unused bit fields:
>

Yes but this is very different case in which the bits are physically
present and essenially kept zero to reserve them for future use.

>
>       Reserved:  This 7-bit field is unused.  Senders MUST set these bits
>       to zero, and receivers MUST ignore their value.
>
>
> That document's IESG reviewers requested this formulation specifically.
>

I would agree with that given the need to make these available for
future use, but  this case is different.   In this case the fields existed
in rfc5661 but we decided that was a mistake.   The best thing to do is to
clearly explain that these fields no longer have a use, although they
continue to take up space in the XDR messages..

>
>
>>    - For the fourth sentence suggest "The client can validly finish any
>>       outstanding I/Os that reference the previously   provided
>>       device-ID-to-device-address mapping and would have to use GETDEVICEINFO to
>>       obtain the updated mapping for the previously provided
>>       device-ID-to-device-address mapping before requesting new layouts" as a
>>       replacement.
>>
>> I prefer the fourth sentence proposed in the errata. Can you say what
>> your proposed replacement fixes?
>>
>> First of all, it avoids use of lower-case "may" which is always confusing.
>
>
> But it adds "would have to use" which is awkward. A subjunctive is not
> appropriate here, and makes this text worse than it is with "may". And
> "validly finish" is also not terribly meaningful. Instead, how about:
>
> The client is permitted to finish outstanding I/O that references the
> previously provided device-ID-to-device-address mapping. Before requesting
> new layouts, the client needs to replace the previously provided
> device-ID-to-device-address mapping using a GETDEVINFO operation.
>


I'm OK with this and will include it in -00

>
>
> Is the phrase "before requesting new layouts" a protocol requirement,
>>
>
> Not really.   The client is not obligated to request new layouts even
> though most clients will.
>
>
>> and thus should it be stated using a compliance keyword?
>>
>
> I don't think so.   RFC2119 says these terms should be used "sparingly"
> which would be hard if every protocol requirement needed such a keyword.
> I  think we can specify how the protocol works without sounding like
> control freaks.
>
>
> My question simply is: is the current language specifying a point of
> interoperation? "before requesting new layouts" implies there might be an
> ordering requirement here that the implementation on one or the other side
> might choose to depend on -- or that could result in data corruption if it
> is not heeded.
>
> This text still seems somehow weak to me -- it doesn't seem to describe
> the hazards well enough (to me) but maybe I'm missing some important
> context.
>
>
>>    - *5982*
>>
>> This is tentatively addressed in -04 in a eevised description of CLOSE
>> but want to have working group dicussion of the new text before we consider
>> this done.  In any case, what is in rfc8881 us self-contradictory and needs
>> to be changed.
>>
>>
> I got my errata confused here.   There may be no errata report associated
> with this change.   Nevertheless, I urge people to look at the changes in
> REMOVE in -04/-05 to address issues with preserve-unlinked.
>
>
> Do you have a method to send us diffs?
>

I've tried to send the output rfcdiff but have always run into problems.

You can rfcdiff draft-dnoveck-nfsv4-rfc5661bis-{03,04} youself.   Except
for a few typo fixes elsewhere, the only changes are those to REMOVE, to
straighten out preserve-unlinked.

>
>
> Because the replier "is permitted (but not always obligated) to return
>> NFS4ERR_FALSE_RETRY", I read the suggestions in the original text as
>> implementation guidance, and not as Mandatory-To-Implement.
>>
>
> I agree but to me, that does not dispose of the matter.   The real issue
> is when false retries can happen.   If v4.1 truly had exactly-once
> semantics, which other parts of the spec suggest it does, the false retries
> would result only from a badly implemented peer.    As of now, we have a
> "MUST" which clients ignore due to the control-C problem.   We need to
> address that issue for the bis.
>
>
> That wasn't clear from the errata, but OK: maybe you indicated the wrong
> errata number.
>
>
> Thus I am not troubled by the original text -- implementers are free to
>> choose not to use checksumming or other techniques when direct data
>> placement is employed.
>>
>
> Yes, but if not doing so can result in data corruption, implementations
> might need to do so.   If it can't implementation guidance would have to
> make this clear.  This almost certainly will not be addressed in -00.
>
>
> --
> Chuck Lever
>
>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>