Re: Proposed updates to draft-ietf-nfsv4-rpcrdma-cm-pvt-data

Chuck Lever <[email protected]> Fri, 20 Dec 2019 10:34:31 -0500
Newsgroups gmane.ietf.nfsv4
Message-ID <[email protected]>

> On Dec 19, 2019, at 9:28 PM, Tom Talpey <[email protected]> wrote:
> 
> This text looks good, and of course yields the same result regardless
> of how shared private data may be presented.
> 
> Is it worth mentioning that the identifier could be seen at any offset,
> regardless of alignment, and if offset+8 would exceed the buffer, it
> must similarly be ignored?

That's worth mentioning. I'll add it.


> Tom.
> 
> On 12/18/2019 3:56 PM, David Noveck wrote:
>> I'm Ok with this
>> On Wed, Dec 18, 2019, 3:14 PM Chuck Lever <[email protected] <mailto:[email protected]>> wrote:
>>     > On Dec 17, 2019, at 11:35 AM, Chuck Lever <[email protected]
>>    <mailto:[email protected]>> wrote:
>>     >
>>     >
>>     >
>>     >> On Dec 17, 2019, at 9:12 AM, Tom Talpey <[email protected]
>>    <mailto:[email protected]>> wrote:
>>     >>
>>     >> Dave, I don't agree that the document gets this right, but I defer
>>     >> to the WG if it wishes to proceed.
>>     >
>>     > I agree with Dave's desire to keep the text simple and take the
>>     > point of view of the RPC/RDMA consumer. However, the document
>>     > author would like Tom to be happy with the final text.
>>     >
>>     > Tom, you suggested that you might have some alternate text for
>>     > this section. I'd like to see that before the document proceeds.
>>    Based on feedback from Tom, here is take 2:
>>    4.1.2.  Interoperability Amongst RDMA Transports
>>        The Format Identifier field defined in Section 4 is provided to
>>        enable implementations to distinguish RPC-over-RDMA version 1
>>    Private
>>        Data from private data inserted at other layers, such as the private
>>        data inserted by the iWARP MPAv2 enhancement described in [RFC6581].
>>        As part of connection establishment, the received private data
>>    buffer
>>        is searched for the Format Identifier word.  If the RPC-over-RDMA
>>        version 1 CM Private Data Format Identifier is not present, an RPC-
>>        over-RDMA version 1 receiver MUST behave as if no RPC-over-RDMA
>>        version 1 CM Private Data has been provided.
>>        Once the RPC-over-RDMA version 1 CM Private Data Format
>>    Identifier is
>>        found, the receiver parses the subsequent octets as RPC-over-RDMA
>>        version 1 CM Private Data.  As additional assurance that the private
>>        data content is valid RPC-over-RDMA version 1 CM Private Data, the
>>        receiver should check that the format version number field
>>    contains a
>>        valid and recognized version number and all reserved flag bits are
>>        zero.
>>     >> Tom.
>>     >>
>>     >> On 12/16/2019 11:36 PM, David Noveck wrote:
>>     >>> On Mon, Dec 16, 2019, 3:39 PM Tom Talpey <[email protected]
>>    <mailto:[email protected]> <mailto:[email protected]
>>    <mailto:[email protected]>>> wrote:
>>     >>>   On 12/16/2019 9:52 AM, Chuck Lever wrote:
>>     >>>>
>>     >>>>
>>     >>>>> On Dec 16, 2019, at 9:36 AM, Tom Talpey <[email protected]
>>    <mailto:[email protected]>
>>     >>>   <mailto:[email protected] <mailto:[email protected]>>> wrote:
>>     >>>>>
>>     >>>>> On 12/15/2019 3:48 PM, Chuck Lever wrote:
>>     >>>>>> As a result of expert review, changes are needed to Section
>>    4.1.2
>>     >>>>>> to clarify the purpose and implementation guidance of the Format
>>     >>>>>> Identifier field.
>>     >>>>>> I propose that the new Section read (in its entirety):
>>     >>>>>> 4.1.2.  Amongst Implementations of Other Upper-Layer Protocols
>>     >>>>>
>>     >>>>> That first word is a very odd one in a section title. Would
>>     >>>>> "Interoperability With" be more meaningful?
>>     >>>>
>>     >>>> I will review the titles of the sub-sections here.
>>     >>>>
>>     >>>>
>>     >>>>>>    The Format Identifier field in the message format defined
>>     >>>   in this
>>     >>>>>>    document is provided to enable implementations to
>>     >>>   distinguish RPC-
>>     >>>>>>    over-RDMA version 1 Private Data from private data inserted
>>     >>>   at layers
>>     >>>>>>    below RPC-over-RDMA version 1.  An example of a layer below
>>     >>>   RPC-over-
>>     >>>>>
>>     >>>>> "Below" is problematic here. The RFC6581 MPA enhanced connection
>>     >>>>> processing can insert private data at the start of the field, and
>>     >>>>> it is "below" RPC in the stack, but the peer's RFC6581 processing
>>     >>>>> strips it off. Therefore, of RPC is the "lowest upper layer" in
>>     >>>>> such a stack, there is no issue.
>>     >>>>>
>>     >>>>> However, there might well be other lower layers, with different
>>     >>>>> behaviors, injecting their own private data payloads. Or indeed,
>>     >>>>> upper layers. And these payloads may choose to append, or prepend
>>     >>>>> to the buffer.
>>     >>>>
>>     >>>> OK. Are you requesting a change to this paragraph?
>>     >>>   I think it would be better to avoid introducing a formal
>>    layering, so
>>     >>>   yes. Instead of "layers below", it may be best simply say "other
>>     >>>   layers".
>>     >>>   The next issue makes this clearer:
>>     >>>>>>    RDMA version 1 that makes use of CM Private Data is iWARP,
>>     >>>   via the
>>     >>>>>>    MPA enhancement described in [RFC6581].
>>     >>>>>>    During connection establishment, an implementation of the
>>     >>>   extension
>>     >>>>>>    described in this document checks the Format Identifier
>>     >>>   field before
>>     >>>>>>    decoding subsequent fields.  If the RPC-over-RDMA version
>>    1 CM
>>     >>>>>>    Private Data Format Identifier is not present as the first
>>     >>>   4 octets,
>>     >>>>>
>>     >>>>> So, just to be clear - this introduces a new requirement on other
>>     >>>>> layers over the same connection. They MUST NOT inject private
>>    data
>>     >>>>> at the beginning of the buffer, or if they do, they MUST strip it
>>     >>>>> off.
>>     >>>>>
>>     >>>>>>    an RPC-over-RDMA version 1 receiver MUST ignore the CM
>>     >>>   Private Data,
>>     >>>>>>    behaving as if no RPC-over-RDMA version 1 Private Data
>>    has been
>>     >>>>>>    provided (see above).
>>     >>>>>
>>     >>>>> And, if the prior requirement is not made, then the RPC layer
>>    needs
>>     >>>>> to scan the prvate data rather carefully to see if the
>>    identifier,
>>     >>>>> and the payload associated with it, is present somewhere in the
>>     >>>>> private data.
>>     >>>>
>>     >>>> You might have misread this paragraph. I don't think there's a
>>    need
>>     >>>> for any new requirements: the paragraph states if the first
>>    word in
>>     >>>> the buffer is not the RPC-over-RDMA Format Identifier, then the
>>     >>>> receiver ignores the CM private data.
>>     >>>   Ok, I didn't misread the paragraph, but I drew a different
>>    conclusion.
>>     >>>   If the protocol requires the RPC sender to place its private data
>>     >>>   payload "first", then it is logical to require the receiver
>>    to look
>>     >>>   at it only there. Lower layers may do the same (e.g. the
>>    MPAv2 ird/ord
>>     >>>   exchange), but of course those layers must strip it off
>>    before passing
>>     >>>   the remainder. So at the RPC layer, it is no longer present.
>>     >>>   The draft currently doesn't discuss this type of sharing,
>>     >>> For it to do so is complicated and unnecessary.  The point is
>>    that it is job of the transport protocol to pass along the cm data
>>    without modifying it.
>>     >>>   however, it
>>     >>>   merely states:
>>     >>>        The first 8 octets of the CM Private Data field is to be
>>     >>>   formatted as
>>     >>>        follows:
>>     >>>   In reality, the private data field is a property of the
>>    underlying
>>     >>>   transport,
>>     >>> The private data field on the wire is but rpc-over-rdma never
>>    sees that.  It does see what it received from the transport
>>    implementation via, for example, the ofed interface. For that data
>>    the transport is the custodian rather than the owner.
>>     >>>   and in the case of iWARP/MPAv2 the first bytes are already
>>     >>>   "taken". So, my concern was that the text was somehow stating
>>    that
>>     >>>   the entire private data paylod was to be inspected, when in
>>    reality
>>     >>>   it's just the part which was *not* otherwise taken by other
>>    layers.
>>     >>> If it states that it should be changed, but I read it as
>>    describing what the rpc-over-rdma protocol would see.
>>     >>>> No other scanning is necessary, it's a fail-safe design since the
>>     >>>> CM private data contains only hints.
>>     >>>   Yes, that's understood. Again, it's just a matter of
>>    describing how
>>     >>>   the field is to be found. The CM API may strip off other
>>    non-shared
>>     >>>   private data chunks, If it doesn't, it is seriously broken.
>>     >>>   but what's on the wire will look quite different.
>>     >>> What's on the wire might be encrypted and look/really/
>>    different. However, we don't and shouldn't see any of that.
>>     >>>   So the word "first" is, I find, problematic.
>>     >>>   I'm short on time right now to craft suggested text for this,
>>    but I'll
>>     >>>   take a shot at it hopefully tomorrow.
>>     >>>>>>    Because the Format Identifier field is newer than some other
>>     >>>>>>    potential users of private data (such as iWARP), there is a
>>     >>>   risk that
>>     >>>>>>    a lower layer might inject its own private data with a
>>    payload
>>     >>>>>>    somehow containing the identifier of RPC-over-RDMA version
>>     >>>   1.  It is
>>     >>>>>
>>     >>>>> Well, *and* not strip it off. This could happen if the peer
>>     >>>   implemented
>>     >>>>> a lower layer that wasn't recognized by the receiver, and the
>>    data
>>     >>>>> simply passed up. See previous paragraph.
>>     >>>>>
>>     >>>>>>    recommended that RPC-over-RDMA version 1 implementations
>>     >>>   perform
>>     >>>>>>    additional checks on the content of received CM private
>>     >>>   data before
>>     >>>>>>    making use of it.
>>     >>>>>
>>     >>>>> "Additional checks" is pretty vague. Are there any specific
>>     >>>   requirements?
>>     >>>>
>>     >>>> Would "perform sanity checks on the content" be preferable?
>>     >>>   I was asking what the sanity checks might actually be. Is there a
>>     >>>   specific
>>     >>>   requirement? If not, I don't think the sentence really says
>>    anything,
>>     >>>   and should be dropped.
>>     >>>   What I think is being implied is that after matching the
>>    identifier,
>>     >>>   somehow the range of values in the following structure needs
>>    to be
>>     >>>   checked, and if any of the values fall outside, that the
>>    entire payload
>>     >>>   be ignored. Leaving me to ask, what are these limits?
>>     >>>   Tom.
>>     >>>   Tom.
>>     >>>   _______________________________________________
>>     >>>   nfsv4 mailing list
>>     >>> [email protected] <mailto:[email protected]> <mailto:[email protected]
>>    <mailto:[email protected]>>
>>     >>> https://www.ietf.org/mailman/listinfo/nfsv4
>>     >>> _______________________________________________
>>     >>> nfsv4 mailing list
>>     >>> [email protected] <mailto:[email protected]>
>>     >>> https://www.ietf.org/mailman/listinfo/nfsv4
>>     >>
>>     >> _______________________________________________
>>     >> nfsv4 mailing list
>>     >> [email protected] <mailto:[email protected]>
>>     >> https://www.ietf.org/mailman/listinfo/nfsv4
>>     >
>>     > --
>>     > Chuck Lever
>>     >
>>     >
>>     >
>>     > _______________________________________________
>>     > nfsv4 mailing list
>>     > [email protected] <mailto:[email protected]>
>>     > https://www.ietf.org/mailman/listinfo/nfsv4
>>    --
>>    Chuck Lever
>>    _______________________________________________
>>    nfsv4 mailing list
>>    [email protected] <mailto:[email protected]>
>>    https://www.ietf.org/mailman/listinfo/nfsv4
>> _______________________________________________
>> nfsv4 mailing list
>> [email protected]
>> https://www.ietf.org/mailman/listinfo/nfsv4
> 
> _______________________________________________
> nfsv4 mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/nfsv4

--
Chuck Lever



_______________________________________________
nfsv4 mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/nfsv4