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

From: Chuck Lever <[email-masked]>
Date: Sun, 15 Dec 2019 15:48:35 -0500

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

   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-
   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,
   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).

   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
   recommended that RPC-over-RDMA version 1 implementations perform
   additional checks on the content of received CM private data before
   making use of it.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 16 Dec 2019 05:24:45 -0500

Ship it.

On Sun, Dec 15, 2019, 3:48 PM Chuck Lever <[email-masked]> wrote:

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

---

From: Tom Talpey <[email-masked]>
Date: Mon, 16 Dec 2019 09:36:17 -0500

> 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?

>     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.

>     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.

>     behaving as if no RPC-over-RDMA version 1 Private Data has been

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.

>     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.

>     additional checks on the content of received CM private data before

"Additional checks" is pretty vague. Are there any specific requirements?

Tom.

---

From: Chuck Lever <[email-masked]>
Date: Mon, 16 Dec 2019 09:52:59 -0500

> On Dec 16, 2019, at 9:36 AM, Tom Talpey <[email-masked]> wrote:
> 

I will review the titles of the sub-sections here.

> upper layers. And these payloads may choose to append, or prepend

OK. Are you requesting a change to this paragraph?

> and the payload associated with it, is present somewhere in the

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.

No other scanning is necessary, it's a fail-safe design since the
CM private data contains only hints.

> "Additional checks" is pretty vague. Are there any specific requirements?

Would "perform sanity checks on the content" be preferable?

--
Chuck Lever

---

From: Tom Talpey <[email-masked]>
Date: Mon, 16 Dec 2019 15:39:11 -0500

> 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".

> 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, 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, 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.

> 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, but what's on the wire will look quite different.
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.

> 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.

---

From: David Noveck <[email-masked]>
Date: Mon, 16 Dec 2019 23:36:58 -0500

On Mon, Dec 16, 2019, 3:39 PM Tom Talpey <[email-masked]> wrote:

> 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
> 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
> 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.

> 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.

---

From: Tom Talpey <[email-masked]>
Date: Tue, 17 Dec 2019 09:12:42 -0500

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 <[email-masked] 
> <mailto:[email-masked]>> wrote:
> 
>      >> On Dec 16, 2019, at 9:36 AM, Tom Talpey <[email-masked]
>     <mailto:[email-masked]>> wrote:
>      >>
> job of the transport protocol to pass along the cm data without 
> modifying it.
> 
>     however, it
>     merely states:
> 
> 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.
> 
> What's on the wire might be encrypted and look/really/ different. 
> However, we don't and shouldn't see any of that.
> 
>     [email-masked] <mailto:[email-masked]>

---

From: Chuck Lever <[email-masked]>
Date: Tue, 17 Dec 2019 11:35:56 -0500

> On Dec 17, 2019, at 9:12 AM, Tom Talpey <[email-masked]> wrote:
> 

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.

>> On Mon, Dec 16, 2019, 3:39 PM Tom Talpey <[email-masked] <mailto:[email-masked]>> wrote:
>> 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 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.
>> If it states that it should be changed, but I read it as describing what the rpc-over-rdma protocol would see.
>>    private data chunks, If it doesn't, it is seriously broken.
>> What's on the wire might be encrypted and look/really/ different. However, we don't and shouldn't see any of that.

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Wed, 18 Dec 2019 15:14:26 -0500

> On Dec 17, 2019, at 11:35 AM, Chuck Lever <[email-masked]> wrote:
> 
> 
> 

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.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Wed, 18 Dec 2019 15:56:51 -0500

I'm Ok with this

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

> >>> 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:
> that.  It does see what it received from the transport implementation via,
> for example, the ofed interface. For that data the transport is the
> >>> If it states that it should be changed, but I read it as describing
> what the rpc-over-rdma protocol would see.

---

From: Tom Talpey <[email-masked]>
Date: Thu, 19 Dec 2019 21:28:06 -0500

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 <[email-masked] 
> <mailto:[email-masked]>> wrote:
> 
> 
> 
>      > On Dec 17, 2019, at 11:35 AM, Chuck Lever <[email-masked]
>     <mailto:[email-masked]>> wrote:
>      >
>      >
>      >
>      >> On Dec 17, 2019, at 9:12 AM, Tom Talpey <[email-masked]
>     <mailto:[email-masked]> <mailto:[email-masked]
>     <mailto:[email-masked]>>> wrote:
>     <mailto:[email-masked]>
>      >>>   <mailto:[email-masked] <mailto:[email-masked]>>> wrote:
>      >>>>>
>      >>> 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 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.
>      >>> If it states that it should be changed, but I read it as
>     describing what the rpc-over-rdma protocol would see.
>      >>> What's on the wire might be encrypted and look/really/
>     different. However, we don't and shouldn't see any of that.
>      >>> [email-masked] <mailto:[email-masked]> <mailto:[email-masked]
>     <mailto:[email-masked]>>

---

From: Chuck Lever <[email-masked]>
Date: Fri, 20 Dec 2019 10:34:31 -0500

> On Dec 19, 2019, at 9:28 PM, Tom Talpey <[email-masked]> wrote:
> 

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

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

--
Chuck Lever