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

[Tom Talpey <[e-mail]>]

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?

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 <[e-mail] 
> <mailto:[e-mail]>> wrote:
> 
> 
> 
>      > On Dec 17, 2019, at 11:35 AM, Chuck Lever <[e-mail]
>     <mailto:[e-mail]>> wrote:
>      >
>      >
>      >
>      >> On Dec 17, 2019, at 9:12 AM, Tom Talpey <[e-mail]
>     <mailto:[e-mail]>> 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 <[e-mail]
>     <mailto:[e-mail]> <mailto:[e-mail]
>     <mailto:[e-mail]>>> wrote:
>      >>>   On 12/16/2019 9:52 AM, Chuck Lever wrote:
>      >>>>
>      >>>>
>      >>>>> On Dec 16, 2019, at 9:36 AM, Tom Talpey <[e-mail]
>     <mailto:[e-mail]>
>      >>>   <mailto:[e-mail] <mailto:[e-mail]>>> 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
>      >>> [e-mail] <mailto:[e-mail]> <mailto:[e-mail]
>     <mailto:[e-mail]>>
>      >>> https://www.ietf.org/mailman/listinfo/nfsv4
>      >>> _______________________________________________
>      >>> nfsv4 mailing list
>      >>> [e-mail] <mailto:[e-mail]>
>      >>> https://www.ietf.org/mailman/listinfo/nfsv4
>      >>
>      >> _______________________________________________
>      >> nfsv4 mailing list
>      >> [e-mail] <mailto:[e-mail]>
>      >> https://www.ietf.org/mailman/listinfo/nfsv4
>      >
>      > --
>      > Chuck Lever
>      >
>      >
>      >
>      > _______________________________________________
>      > nfsv4 mailing list
>      > [e-mail] <mailto:[e-mail]>
>      > https://www.ietf.org/mailman/listinfo/nfsv4
> 
>     --
>     Chuck Lever
> 
> 
> 
>     _______________________________________________
>     nfsv4 mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/nfsv4
> 
> 
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>