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

[Tom Talpey <[e-mail]>]

Dave, I don't agree that the document gets this right, but I defer
to the WG if it wishes to proceed.

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]>> 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]>> 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]>
>     https://www.ietf.org/mailman/listinfo/nfsv4
> 
> 
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>