Subject: Re: [nfsv4] New Version Notification for draft-talpey-rdma-commit-01.txt

From: Chuck Lever <[email-masked]>
Date: Sat, 11 Apr 2020 14:33:23 -0400

> On Mar 12, 2020, at 4:26 PM, Tom Talpey <ttalpey=[email-masked]> wrote:
> 
> As mentioned on nfsv4 earlier, the "RDMA Push Mode" extensions draft has
> been updated and was published on Monday. There is a ten-minute session
> in the upcoming agenda where its possible adoption as an NFSv4 Work Item
> will be discussed. While we wait for those logistics, please take a look, if
> interested in the area.

In preparation for our interim meeting on April 29, where this
document is to be discussed, I've prepared the following review
of rdma-commit-01.

Thanks, Tom, for considering the nfsv4 WG as a home for this work.

General comments:

Having not been involved in the crafting of RDDP (5040), I can't claim
expertise in the protocol mechanics described in Section 3 and later
parts of the document. My domain of expertise is as a developer and
protocol architect that makes use of RDMA transports.

I have been following the development of these ideas for several years.
The work described here is thorough and detailed, and I'm aware of at
least one partial prototype implementation. And, well-noted that
Sections 3.2 and several later sections appear to be incomplete. The
document remains a work-in-progress.

RPC/RDMA itself is not likely to employ the extensions described in this
document. "Push mode" however can be utilized in extensions to NFS that
would directly submit RDMA Read, Write, Flush, Verify, and Commit
operations in place of NFS READ, WRITE, and COMMIT procedures. Such a
construction has been proposed by:

https://tools.ietf.org/html/draft-hellwig-nfsv4-rdma-layout-00

though it predates the definition of RDMA Flush, Verify, and Commit.

The document does not indicate any parallel development of similar
operations in the InfiniBand community. It would be valuable to
understand how InfiniBand operations differ (semantically and in
their design approach) from the extensions described in this document.

In summary, this document definitely falls within the intersection of
networking and storage. Therefore, IMO that makes it appropriate for
adoption by the nfsv4 WG.

Document organization:

The information in Section 2 is usually relegated to a separate
Informative requirements document. That kind of separation might not
be practical for an individual contribution, but could be considered
for a scope of work that has a home in a particular Working Group.

An Implementation Status section would be a welcome addition to this
document.

1.1. Glossary

   Invalidate:  The removal of data from volatile intermediate locations.

Does this perfunctory definition of "invalidate" align with the use of
Invalidate in "Send With Invalidate" and other common utilizations?

Suggest a definition of "queued" and/or "posted" operations appear in the
Glossary.

Suggest a definition of "ULP" and/or "upper-layer protocol" appear in the
Glossary.

2. Problem Statement

Minor comment:

"requires that multiple copies be committed in multiple locations across
the fabric,"

Suggest instead "across the data center" or even "across cloud instances".
Note that the term "fabric" is not defined in this document, and might be
unfamiliar to the average IETF reader.

Spelling: "a persistent and/or globally visibile state,"

The problem statement introduces

traditional storage hardware, where data resides in a memory cache and is
flushed to durable storage via a host I/O operation;

and

persistent memory, where data resides in that memory and no such I/O is
necessary to make the data durable.

The document then introduces

   a new "flush to
   persistence" RDMA operation.  This operation, when invoked by a
   connected remote RDMA peer, can be used to request that previously-
   written data be moved into the persistent storage domain.

It is not entirely clear that this operation is not intended to be used
with traditional storage devices that require host I/O for data to reach
durability.

The purpose of these extensions is to leverage hardware architectures
where that I/O step is now gone. Rather than framing these as introducing
a "push mode" this document might be more accurately described as
introducing support for RDMA to persistent memory -- an off-label use
of these new operations would be pushing data to volatile memory for
further processing.

2.1. Requirements for RDMA Flush

Spelling: "checking and corection (ECC) memory"

2.2. Requirements for Atomic Write

"the resulting string (log) of transactions"

"string" is an overloaded term and perhaps not precise in this context.
Suggest instead "the resulting sequence of transactions".

2.3.  Requirements for RDMA Verify

   Typically, storage stacks such as filesystems and
   media approches such as SCSI T10 DIF or filesystem integrity checks
   such as ZFS provide for block- oir file-level protection of data at
   rest on storage devices.

Perhaps T10-DIF and ZFS might need references. Note the spelling
error "oir".

   The integrity
   check is initiated by the upper layer or application, which
   optionally computes the expected hash of a given segment of arbitrary
   size, sending the hash via an RDMA Verify operation targeting the
   RDMA segment on the responder, and the responder calculating and
   optionally verifying the hash on the indicated data, bypassing any
   volatile copies remaining in caches.

If the remote RNIC is responsible for offloading hash verification,
why wouldn't the hash computation be offloaded on the requester side as
well?

--
Chuck Lever