Re: [nfsv4] Review of draft-ietf-nfsv4-rfc5667bis-09

[karen deitke <[e-mail]>]

On 4/24/2017 5:56 AM, Tom Talpey wrote:
> On 4/21/2017 10:43 AM, Chuck Lever wrote:
>> I agree that SHOULD/MAY makes things cloudier, and does not
>> seem to align with well-defined RFC2119 usage.
>>
>> Another way we've dealt with similar disagreements between
>> specification and implementation is to decide that one of
>> the implementations is incorrect.
>>
>> Can we agree that:
>>
>> - GARBAGE_ARGS is a bit of a layering violation, though it's
>> understandable why it might be returned
>>
>> - RPC clients are already prepared for GARBAGE_ARGS
>
> Are you certain of this? And out of curiosity, what is returned
> to the consumer for GARBAGE_ARGS versus ERR_CHUNK?
Consumer will see EIO for both.
>
>>
>> - In RPC-over-RDMA Version One, we are not trying to recover
>> (in the sense of resending a simpler COMPOUND) but are rather
>> trying to ensure the offending RPC is properly terminated on
>> the client, and does not further block other RPCs or deadlock
>> the transport
>>
>> Thus I claim it is harmless if a server returns GARBAGE_ARGS
>> instead of ERR_CHUNK.
>
> "Harmless" is a bit relative. The operation fails, through no fault
> of the consumer. And, frankly, in a very mysterious way.
>
> Again, I think there is more to say here. It's a limitation of the
> protocol whose implications should be made clear (contraining the
> complexity of COMPOUNDs, limiting scatter/gather lengths, etc).
>
> Tom.
>
>
>>
>> As a result, I can change the Read list text in S5.4.1 to be
>> the same as the Write list text, removing the mention of
>> GARBAGE_ARGS.
>>
>> Would that sit comfortably with everyone?
>>
>>
>>> On Apr 20, 2017, at 7:21 PM, David Noveck <[e-mail]> wrote:
>>>
>>>> The "or" is a similar situation, it prescribes a choice, which
>>>> does not define a protocol.
>>>
>>> Fair enough, but the point that needs to be made is that, with
>>> regard to Version One, Chuck and  the working group is not
>>> free to define a protocol.  As a result we have the kind of
>>> ugliness you object to, but it is inherent in the choice to try to
>>> revive Version One as-is.
>>>
>>>>   If an NFS version 4 client sends an RPC Call with a Read list that
>>>>   contains more chunks than an NFS version 4 server is prepared to
>>>>   process, the server SHOULD reject the request by responding with an
>>>>   RDMA_ERROR message with the rdma_err value set to ERR_CHUNK. The
>>>>   server MAY reject the RPC with an RDMA_MSG message containing an RPC
>>>>   Reply with an accept status of GARBAGE_ARGS.
>>>
>>> I think I know what you intend here and I've seen stuff like this in 
>>> RFCs but I don't
>>> wthink e can do this because this is not in line with the 
>>> definitions of "SHOULD"
>>> and "MAY" that appear in RFC2119.
>>>
>>> "SHOULD" means that you are supposed to do something but can avoid 
>>> it if
>>> you have a good reason and are aware of the consequences of not 
>>> doing it.
>>> In this case the "good" reason is that someone coded the implementation
>>> to do something else, which is not all that good a reason. The 
>>> consequences of
>>> returning the GARBAGEARGS are exactly zero, since the client has to 
>>> be prepared
>>> for either it or ERR_CHUNK.
>>>
>>> "MAY" means the implementation can choose to do the action or not, 
>>> which is line
>>> with the reality here but essentially contradicts the SHOULD.
>>>
>>>> This at least makes it clear which response is "preferred".
>>>
>>> But it is isn't really the job of the RFC2119 terms to say which is 
>>> "preferred" or
>>> "'preferred'".  These terms are supposed to describe 
>>> interoperability and the
>>> interoperability situation is that the server MUST return ERR_CHUNK or
>>>  GARBAGEARGS and the client needs to be prepared for either. That is 
>>> the
>>> unpleasant reality.  If you want to indicate a preference, you can 
>>> say something
>>> like:
>>>     • Returning ERR_CHUNK is preferrable.
>>>     • Returinng ERR_CHUNK is more in line with the appropriate 
>>> protocol layering since this issue relates to a limitation of the 
>>> transport implementation.
>>>     • Use of GARBAGEARGS is an unfortunate artifact of 
>>> inappropriately layered implementations and is only allowed for 
>>> reasons of compatibility with existing implementations. It is 
>>> desirable to avoid it.
>>>> And one would hope a future draft would decide.
>>>
>>> Not sure what draft you are thinking of.  I don't see us doing an 
>>> rfc5667bisbis (rfc5667tris).
>>>
>>> By the time we did that, the implementations with these restrictions 
>>> will probably be gone.
>>>
>>>> I have a second question though. How does the client determine what is
>>>> the actual error? As in, how many chunks were allowed?
>>>
>>> This is not fixable in Version One.  It would be in Version Two, but 
>>> by then
>>> the need will probably be gone.
>>>
>>>> Does the upper
>>>> layer have to recover, and if so what are the implications?
>>>
>>> I think something could be put in to indicate that clients should 
>>> break up COMPOUNDS
>>> so the only have a single chunk each.
>>>
>>>> Yes, I know 5667 did not explore this very well.
>>>
>>> It didn't explore it at all.  And 5666's error reporting facilities 
>>> were extremely limited.
>>>
>>>> Mea culpa.
>>>
>>> I don'tt think you have anything to apologize for.
>>>
>>>
>>>
>>>
>>> On Thu, Apr 20, 2017 at 5:28 PM, Tom Talpey <[e-mail]> wrote:
>>>
>>>
>>> On 4/19/2017 11:14 AM, Chuck Lever wrote:
>>> Hi Tom-
>>>
>>> On Apr 18, 2017, at 11:08 PM, Tom Talpey <[e-mail]> wrote:
>>>
>>> I noticed the same thing, and I'll add that the MUST reject condition
>>> is very confusing because it allows an "or". In my opinion a MUST is
>>> always a single requirement, never ambiguous.
>>>
>>> I agree this kind of thing is tricky. I wrote it as "the server MUST
>>> reject the RPC". That's the single requirement. The choice is how the
>>> rejection is conveyed to the client.
>>>
>>> The statement "MUST reject" is not testable. So, while it may be
>>> understood what is intended, there is nothing implementable in the
>>> MUST. The "or" is a similar situation, it prescribes a choice, which
>>> does not define a protocol.
>>>
>>> Is there some reason you want to allow such a choice? I think you'll
>>> find that, worded properly, it becomes actually much less implementable
>>> and interoperable than you may think.
>>>
>>> The Solaris server can return an RPC-level error in cases like this.
>>>
>>> Well, this is happening because the Solaris server is (probably) just
>>> handing the chunk list up to the RPC layer, and it's the RPC (XDR)
>>> processing that detects any problem.
>>>
>>> On the other hand, an implementation could do the opposite, it could
>>> process the chunks at the lower layer, before ever invoking RPC
>>> processing. This would naturally lead to a non-RPC error.
>>>
>>> The challenge in defining the protocol is to hide these possibilities.
>>>
>>> I think there are similar choices allowed in rfc5666bis. Let's say
>>> that in a perfect world, I would go with only ERR_CHUNK, but I'm
>>> documenting existing implementation behavior here.
>>>
>>> I'm not sure it matters to the client: both errors are permanent and
>>> the RPC is terminated on the client.
>>>
>>> I'm open to alternatives.
>>>
>>> The icky way to do this is to split into two weak requirements.
>>>
>>>    If an NFS version 4 client sends an RPC Call with a Read list that
>>>    contains more chunks than an NFS version 4 server is prepared to
>>>    process, the server SHOULD reject the request by responding with an
>>>    RDMA_ERROR message with the rdma_err value set to ERR_CHUNK. The
>>>    server MAY reject the RPC with an RDMA_MSG message containing an RPC
>>>    Reply with an accept status of GARBAGE_ARGS.
>>>
>>> This at least makes it clear which response is "preferred". And one
>>> would hope a future draft would decide.
>>>
>>> I have a second question though. How does the client determine what is
>>> the actual error? As in, how many chunks were allowed? Does the upper
>>> layer have to recover, and if so what are the implications?
>>>
>>> Yes, I know 5667 did not explore this very well. Mea culpa.
>>>
>>>
>>>
>>> Tom.
>>>
>>>
>>>
>>> On 4/18/2017 6:32 PM, karen deitke wrote:
>>> Hi Chuck, its unclear what you mean by "is prepared to process" in 
>>> the text below.
>>> Other than that, looks good.
>>>
>>> Karen
>>>
>>> 5.4.1
>>>   If an NFS version 4 client sends an RPC Call with a Write list that
>>>   contains more chunks than an NFS version 4 server is prepared to
>>>   process, the server MUST reject the RPC by responding with an
>>>   RDMA_ERROR message with the rdma_err value set to ERR_CHUNK.
>>>
>>>
>>>   If an NFS version 4 client sends an RPC Call with a Read list that
>>>   contains more chunks than an NFS version 4 server is prepared to
>>>   process, the server MUST reject the RPC by responding with an
>>>   RDMA_MSG message containing an RPC Reply with an accept status of
>>>   GARBAGE_ARGS, or with an RDMA_ERROR message with the rdma_err value
>>>   set to ERR_CHUNK.
>>>
>>>
>>> On 4/18/2017 1:21 PM, David Noveck wrote:
>>> *Overall Evaluation*
>>> *
>>> *
>>> Major improvement over RFC5667.  Almost ready to ship.  No technical
>>> issues.
>>>
>>> A lot of my comments are basically editorial and are offered on a
>>> take-it-or-lease-it basis.
>>>
>>> I think some clarification in Section 5.4.1 is needed although not
>>> necessarily in the ways suggested below,
>>>
>>> *Comments by Section*
>>> *5.4.1. Multiple DDP-eligible Data Items*
>>> Giventhat READ_PLUS no longer has any DDP-eligible data items, the
>>> situation described in the fifth bullet can no longer arise. I suggest
>>> deleting the bullet.
>>> The penultimate paragraph can be read as applying to some situations
>>> in which it shouldn't and where the extra chunks would very naturally
>>> ignored. For example, if you had on write chunk together with a READ
>>> operation which failed, the server would have more chunks (i.e. one)
>>> than the number it is prepared to process (i.e. zero). Suggest, as a
>>> possible replacement:
>>>
>>>    Normally, when an NFS version 4 client sends an RPC Call with a
>>>    Write list that contains multiple chunks. each such, when matched
>>>    with a DDP-eligible data item in the response, directs the
>>>    placement of the data item as specified by
>>>    [I.D.-nfsv4-rfc5666bis]. When there are DDP-eligible data items
>>>    matched to write chunks that an NFS version 4 server is not
>>>    prepared to process, the server MUST reject the RPC by responding
>>>    with an RDMA_ERROR message with the rdma_err value set to ERR_CHUNK.
>>>
>>> With regard to the last paragraph, I am curious that this paragraph,
>>> unlike the previous one, allows GARBGEARGS. Is this so because that
>>> would be allowed if the chunks in question had offsets other than
>>> those that correspond to DDP-eligible data items? If so, please
>>> consider the following possible replacement.
>>>
>>>    Normally, when an NFS version 4 client sends an RPC Call with a
>>>    Read list that contains multiple chunks, each such, when properly
>>>    matched with a DDP-eliigible data item in the request, directs the
>>>    fetching of the the data item as specified by
>>>    [I.D.-nfsv4-rfc5666bis]. When there are DDP-eligible data items
>>>    matched to read chunks that an NFS version 4 server is not
>>>    prepared to process, the server MUST reject the RPC by responding
>>>    with an RDMA_ERROR message with the rdma_err value set to ERR_CHUNK.
>>>
>>> *5.6. Session-Related Considerations*
>>> In the third sentence of the second paragraph, suggest replacing "no
>>> different" by "not different".
>>> In the last sentence of the last paragraph, suggest replacing "is not"
>>> by "were not"
>>>
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>>
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>> -- 
>>> Chuck Lever
>>>
>>>
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>>
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>>
>>> _______________________________________________
>>> nfsv4 mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/nfsv4
>>
>> -- 
>> Chuck Lever
>>
>>
>>
>> _______________________________________________
>> nfsv4 mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/nfsv4
>>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4