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

I'm Ok with this


On Wed, Dec 18, 2019, 3:14 PM Chuck Lever <[email protected]> wrote:

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

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