support for DIRECT access via ggated and ggatec

[david_crossfamilyweb.com]

Summary

Support DIRECT access via ggated and ggatec.

ggatec supports specifying -o direct in addition to the typical rw/wo/ro access specifier to request DIRECT access from ggated.

ggated supports adding ,DIRECT and ,NODIRECT on export lines. Specfying DIRECT will force any accesses to have DIRECT set, setting NODIRECT will preclude any a client from choosing DIRECT. Not specificed allows the client to request (or not)

Test Plan

ggated exports of:
host RO /path/ggated/ro
host RW /path/ggated/rw
host WO /path/ggated/wo
host RO,DIRECT /path/ggated/rod
host RW,DIRECT /path/ggated/rwd
host WO,DIRECT /path/ggated/wod
host RO,NODIRECT /path/ggated/rond
host RW,NODIRECT /path/ggated/rwnd
host WO,NODIRECT /path/ggated/wond

Client tested with:
./ggatec create -o ro server /path/ggated/ro
./ggatec create -o rw server /path/ggated/ro
./ggatec create -o wo server /path/ggated/ro

./ggatec create -o ro -o direct server /path/ggated/ro
./ggatec create -o wo -o direct server /path/ggated/ro
./ggatec create -o wo -o direct server /path/ggated/ro

./ggatec create -o ro server /path/ggated/wo
./ggatec create -o rw server /path/ggated/wo
./ggatec create -o wo server /path/ggated/wo

./ggatec create -o ro -o direct server /path/ggated/wo
./ggatec create -o rw -o direct server /path/ggated/wo
./ggatec create -o wo -o direct server /path/ggated/wo

./ggatec create -o ro server /path/ggated/rw
./ggatec create -o wo server /path/ggated/rw
./ggatec create -o rw server /path/ggated/rw

./ggatec create -o ro -o direct server /path/ggated/rw
./ggatec create -o wo -o direct server /path/ggated/rw
./ggatec create -o rw -o direct server /path/ggated/rw

./ggatec create -o ro server /path/ggated/rod
./ggatec create -o wo server /path/ggated/rod
./ggatec create -o rw server /path/ggated/rod

./ggatec create -o ro -o direct server /path/ggated/rod
./ggatec create -o wo -o direct server /path/ggated/rod
./ggatec create -o rw -o direct server /path/ggated/rod

./ggatec create -o ro server /path/ggated/wod
./ggatec create -o wo server /path/ggated/wod
./ggatec create -o rw server /path/ggated/wod

./ggatec create -o ro -o direct server /path/ggated/wod
./ggatec create -o wo -o direct server /path/ggated/wod
./ggatec create -o rw -o direct server /path/ggated/wod

./ggatec create -o ro server /path/ggated/rwd
./ggatec create -o wo server /path/ggated/rwd
./ggatec create -o rw server /path/ggated/rwd

./ggatec create -o ro -o direct server /path/ggated/rwd
./ggatec create -o wo -o direct server /path/ggated/rwd
./ggatec create -o rw -o direct server /path/ggated/rwd

./ggatec create -o ro server /path/ggated/rond
./ggatec create -o wo server /path/ggated/rond
./ggatec create -o rw server /path/ggated/rond

./ggatec create -o ro -o direct server /path/ggated/rond
./ggatec create -o wo -o direct server /path/ggated/rond
./ggatec create -o rw -o direct server /path/ggated/rond

./ggatec create -o ro server /path/ggated/wond
./ggatec create -o wo server /path/ggated/wond
./ggatec create -o rw server /path/ggated/wond

./ggatec create -o ro -o direct server /path/ggated/wond
./ggatec create -o wo -o direct server /path/ggated/wond
./ggatec create -o rw -o direct server /path/ggated/wond

./ggatec create -o ro server /path/ggated/rwnd
./ggatec create -o wo server /path/ggated/rwnd
./ggatec create -o rw server /path/ggated/rwnd

./ggatec create -o ro -o direct server /path/ggated/rwnd
./ggatec create -o wo -o direct server /path/ggated/rwnd
./ggatec create -o rw -o direct server /path/ggated/rwnd

Validated via a (removed) debugging logline that showed the open(2) flags. All flags passed correctly.

[cy]

Can you please regenerate a full patch using -U99999?

[david_crossfamilyweb.com]

Updated for -U99999 as requested.

Additionally:

• fixed manpage for ggatec, removed an old way I had for specifying direct access that I had already removed from code but missed in the documentation
• updated ggatel to also support O_DIRECT.

*WARNING* I haven't tested AT ALL (not even compiled) ggatel with these changes (but they were just imports from ggatec).. I will be doing that now, but I was hoping to kickstart the review process while I can test those

[zlei]

sbin/ggate/ggatec/ggatec.c

81

IIUC, the original options <ro|wo|rw> are mutually exclusive, but they can be combined with direct, then it seems [-o <ro|wo|rw, direct>] or [-o <ro|wo|rw]> [-o <direct>] is better than option = {ro, wo, rw, direct}.

547

This blank is intentional as per style(9) .

sbin/ggate/ggated/ggated.8

92

Shall client have the option [-o nodirect] ?

[david_crossfamilyweb.com]

sbin/ggate/ggated/ggated.8

92

The intent here is no, DIRECT has implications on the server. In my case I want to avoid these images thrashing the cache on my server from accesses, but after I did that I noticed that it had a profound impact (predictably) on the rest of the IO on that device and I realized that the operator may want to be able to have say over how this impacts their server.

If the server operator does not care, they can allow the client to pick DIRECT or not (default is not)

If the server operator DOES care they can choose to enforce DIRECT or NODIRECT at their discretion,

If they've chosen to pick DIRECT then the client should be forced to DIRECT, if they pick NODIRECT then they should be forced off.

If the client doesn't want direct, it should just not specify; but what the server specifies rules.

I chose NOT to outright reject the request since from the client's perspective it doesn't change the contract of the data operations any, it is just cache optimization on the server.

[david_crossfamilyweb.com]

Updated with PR comments

[david_crossfamilyweb.com]

Previous version did have one compile error (missing ;) in the file I warned I had not tested; also updated usage to match man pages

[markj]

This broadly looks good to me. I verified that the existing ggate regression tests pass with this change.

I left some comments, mostly regarding code style. If you address them and send me a git patch (i.e., generated by git format-patch), I will commit this.

sbin/ggate/ggated/ggated.8

111

I think the man page could use more explanation on how this option interacts with clients. For example:

• What happens if a client specifies -o direct and NODIRECT is specified in the export? (I believe the client's flag is ignored.)
• What happens if multiple clients access an export, and some pass -o direct but others do not? Does each client connection have its own fd?

Also some brief explanation of why DIRECT or NODIRECT might be useful.

sbin/ggate/ggated/ggated.c

157

Since you are making a copy of flagsstr, it can be declared const char *flagsstr.

160

162

There should be an extra newline after variable declarations.

169

171

This brace should appear at the end of the preceding line.

185

Indentation here should be by four spaces.

191

Indentation here should be by four spaces.

193

sbin/ggate/ggatel/ggatel.c

86

[david_crossfamilyweb.com]

Updated with last PR fixes

[david_crossfamilyweb.com]

How should I submit this to you for the path; I have never used git format-patch before; I tried doing git format-patch main and it generated 21 files (since I have 21 commits). Manpage suggests I email this to you? But it seems rude to just blast you with 21 patches; and I don't know your email.

[imp]

In D45056#1058221, @david_crossfamilyweb.com wrote:

How should I submit this to you for the path; I have never used git format-patch before; I tried doing git format-patch main and it generated 21 files (since I have 21 commits). Manpage suggests I email this to you? But it seems rude to just blast you with 21 patches; and I don't know your email.

This likely should just be one (maybe two, though I'm having trouble looking at it now suggesting a split) commit, so use git rebase -i to collapse them into one commit with a good commit message. git format-patch --stdout main will generate the patch on standard out if you don't like the default of 00xx-long-name.diff files that are generated otherwise. Make sure the first line of the commit is less than 50 or so characters (eg "ggate: Add support for O_DIRECT access") and has a blank line after it. Then format the rest (maybe the description of this request) with 72 columns max. Make sure that the author name is correct (Author: David E. Cross <[e-mail]> is what the other commit in the src repo with your name on it looks like, though it's been a while so a new address is fine). Once you have that, then markj can just apply that patch to his repo, do the final sanity checks and push it in.

[markj]

In D45056#1058221, @david_crossfamilyweb.com wrote:

How should I submit this to you for the path; I have never used git format-patch before; I tried doing git format-patch main and it generated 21 files (since I have 21 commits). Manpage suggests I email this to you? But it seems rude to just blast you with 21 patches; and I don't know your email.

git format-patch main would generate a patch file for each commit between main and the currently checked out branch. If you want to just create a patch file for the most recent commit, git format-patch HEAD~..HEAD should generate a file named 0001-<commit title>.patch. Please mail that to [e-mail].

[allanjude]

In D45056#1058221, @david_crossfamilyweb.com wrote:

How should I submit this to you for the path; I have never used git format-patch before; I tried doing git format-patch main and it generated 21 files (since I have 21 commits). Manpage suggests I email this to you? But it seems rude to just blast you with 21 patches; and I don't know your email.

David, did you still need more help here?

[david_crossfamilyweb.com]

In D45056#1061365, @allanjude wrote:

In D45056#1058221, @david_crossfamilyweb.com wrote:

How should I submit this to you for the path; I have never used git format-patch before; I tried doing git format-patch main and it generated 21 files (since I have 21 commits). Manpage suggests I email this to you? But it seems rude to just blast you with 21 patches; and I don't know your email.

David, did you still need more help here?

Thank you for following up; I have had an insane few weeks at work; I will be finishing this up this evening and sending it out.

[david_crossfamilyweb.com]

With many apologies for the delay, this is now sent.

I am glad my old address is remembered; my hope and desire is to get my commit access back; which is in no small part why I am working on pushing a number of improvements I have made locally upstream.