From 7592fd7d851fefba0758fec6bfcf2f363f56d4da Mon Sep 17 00:00:00 2001 From: George Hicken Date: Wed, 4 Oct 2017 16:39:57 -0700 Subject: [PATCH] Support discriminated unions correctly (#2) This makes use of the discriminated union support in the xdr2 package, and uses that to correct several protocol errors in the previous code. Additionally this adds support for chunked readdir responses and chunked file writing. This client is still woefully incomplete and likely does not handle various corner cases correctly. Basic testing performed against Linux kernel NFS server and NetApp simulator. --- nfs/example/test/main.go | 24 +++-- nfs/file.go | 81 ++++++++++------- nfs/nfs.go | 91 ++++++++++++++----- nfs/rpc/client.go | 26 +++++- nfs/target.go | 187 +++++++++++++++++++++------------------ 5 files changed, 257 insertions(+), 152 deletions(-) diff --git a/nfs/example/test/main.go b/nfs/example/test/main.go index f937103..bf2caac 100644 --- a/nfs/example/test/main.go +++ b/nfs/example/test/main.go @@ -47,6 +47,13 @@ func main() { } defer v.Close() + // discover any system files such as lost+found or .snapshot + dirs, err := ls(v, ".") + if err != nil { + log.Fatalf("ls: %s", err.Error()) + } + baseDirCount := len(dirs) + if _, err = v.Mkdir(dir, 0775); err != nil { log.Fatalf("mkdir error: %v", err) } @@ -65,14 +72,14 @@ func main() { log.Fatalf("mkdir error: %v", err) } - dirs, err := ls(v, ".") + dirs, err = ls(v, ".") if err != nil { log.Fatalf("ls: %s", err.Error()) } - // check the length. There should only be 1 entry in the target (aside from . and ..) - if len(dirs) != 3 { - log.Fatalf("expected 3 dirs, got %d", len(dirs)) + // check the length. There should only be 1 entry in the target (aside from . and .., et al) + if len(dirs) != 1+baseDirCount { + log.Fatalf("expected %d dirs, got %d", 1+baseDirCount, len(dirs)) } // 10 MB file @@ -131,8 +138,8 @@ func main() { log.Fatalf("ls: %s", err.Error()) } - if len(outDirs) != 2 { - log.Fatalf("directory shoudl be empty!") + if len(outDirs) != baseDirCount { + log.Fatalf("directory shoudl be empty of our created files!") } if err = mount.Unmount(); err != nil { @@ -140,6 +147,7 @@ func main() { } mount.Close() + util.Infof("Completed tests") } func testFileRW(v *nfs.Target, name string, filesize uint64) error { @@ -162,9 +170,9 @@ func testFileRW(v *nfs.Target, name string, filesize uint64) error { t := io.TeeReader(f, h) // Copy filesize - _, err = io.CopyN(wr, t, int64(filesize)) + n, err := io.CopyN(wr, t, int64(filesize)) if err != nil { - util.Errorf("error copying: %s", err.Error()) + util.Errorf("error copying: n=%d, %s", n, err.Error()) return err } expectedSum := h.Sum(nil) diff --git a/nfs/file.go b/nfs/file.go index 2a987b9..6f47d10 100644 --- a/nfs/file.go +++ b/nfs/file.go @@ -34,10 +34,7 @@ func (f *File) Read(p []byte) (int, error) { } type ReadRes struct { - Follows uint32 - Attrs struct { - Attrs Fattr - } + Attr PostOpAttr Count uint32 EOF uint32 Data struct { @@ -45,7 +42,7 @@ func (f *File) Read(p []byte) (int, error) { } } - readSize := uint32(min(uint64(f.fsinfo.RTPref), uint64(len(p)))) + readSize := min(f.fsinfo.RTPref, uint32(len(p))) util.Debugf("read(%x) len=%d offset=%d", f.fh, readSize, f.curr) r, err := f.call(&ReadArgs{ @@ -97,36 +94,58 @@ func (f *File) Write(p []byte) (int, error) { Contents []byte } - totalToWrite := len(p) - writeSize := uint64(min(uint64(f.fsinfo.WTPref), uint64(totalToWrite))) + type WriteRes struct { + Wcc WccData + Count uint32 + How uint32 + WriteVerf uint64 + } - _, err := f.call(&WriteArgs{ - Header: rpc.Header{ - Rpcvers: 2, - Prog: Nfs3Prog, - Vers: Nfs3Vers, - Proc: NFSProc3Write, - Cred: f.auth, - Verf: rpc.AuthNull, - }, - FH: f.fh, - Offset: f.curr, - Count: uint32(writeSize), - How: 2, - Contents: p[:writeSize], - }) + totalToWrite := uint32(len(p)) + written := uint32(0) + + for written = 0; written < totalToWrite; { + writeSize := min(f.fsinfo.WTPref, totalToWrite-written) + + res, err := f.call(&WriteArgs{ + Header: rpc.Header{ + Rpcvers: 2, + Prog: Nfs3Prog, + Vers: Nfs3Vers, + Proc: NFSProc3Write, + Cred: f.auth, + Verf: rpc.AuthNull, + }, + FH: f.fh, + Offset: f.curr, + Count: writeSize, + How: 2, + Contents: p[written : written+writeSize], + }) + + if err != nil { + util.Errorf("write(%x): %s", f.fh, err.Error()) + return int(written), err + } - if err != nil { - util.Debugf("write(%x): %s", f.fh, err.Error()) - return int(writeSize), err - } + writeres := &WriteRes{} + if err = xdr.Read(res, writeres); err != nil { + util.Errorf("write(%x) failed to parse result: %s", f.fh, err.Error()) + util.Debugf("write(%x) partial result: %+v", f.fh, writeres) + return int(written), err + } + + if writeres.Count != writeSize { + util.Debugf("write(%x) did not write full data payload: sent: %d, written: %d", writeSize, writeres.Count) + } - util.Debugf("write(%x) len=%d offset=%d written=%d total=%d", - f.fh, writeSize, f.curr, writeSize, totalToWrite) + f.curr += uint64(writeres.Count) + written += writeres.Count - f.curr = f.curr + writeSize + util.Debugf("write(%x) len=%d new_offset=%d written=%d total=%d", f.fh, totalToWrite, f.curr, writeres.Count, written) + } - return int(writeSize), nil + return int(written), nil } // Close commits the file @@ -197,7 +216,7 @@ func (v *Target) Open(path string) (io.ReadCloser, error) { return f, nil } -func min(x, y uint64) uint64 { +func min(x, y uint32) uint32 { if x > y { return y } diff --git a/nfs/nfs.go b/nfs/nfs.go index c083b36..fb7ea1f 100644 --- a/nfs/nfs.go +++ b/nfs/nfs.go @@ -32,6 +32,10 @@ const ( NFSProc3FSInfo = 19 NFSProc3Commit = 21 + // The size in bytes of the opaque cookie verifier passed by + // READDIR and READDIRPLUS. + NFS3_COOKIEVERFSIZE = 8 + // file types NF3Reg = 1 NF3Dir = 2 @@ -51,19 +55,37 @@ type Sattr3 struct { Mode SetMode UID SetUID GID SetUID - Size uint64 - Atime NFS3Time - Mtime NFS3Time + Size SetSize + Atime SetTime + Mtime SetTime } type SetMode struct { - Set uint32 - Mode uint32 + SetIt bool `xdr:"union"` + Mode uint32 `xdr:"unioncase=1"` } type SetUID struct { - Set uint32 - UID uint32 + SetIt bool `xdr:"union"` + UID uint32 `xdr:"unioncase=1"` +} + +type SetSize struct { + SetIt bool `xdr:"union"` + Size uint64 `xdr:"unioncase=1"` +} + +type TimeHow int + +const ( + DontChange TimeHow = iota + SetToServerTime + SetToClientTime +) + +type SetTime struct { + SetIt TimeHow `xdr:"union"` + Time NFS3Time `xdr:"unioncase=2"` //SetToClientTime } type NFS3Time struct { @@ -109,17 +131,23 @@ func (f *Fattr) Sys() interface{} { return nil } +type PostOpFH3 struct { + IsSet bool `xdr:"union"` + FH []byte `xdr:"unioncase=1"` +} + +type PostOpAttr struct { + IsSet bool `xdr:"union"` + Attr Fattr `xdr:"unioncase=1"` +} + type EntryPlus struct { FileId uint64 FileName string Cookie uint64 - Attr struct { - Follows uint32 - Attr Fattr - } - FHSet uint32 - FH []byte - ValueFollows uint32 + Attr PostOpAttr + Handle PostOpFH3 + // NextEntry *EntryPlus } func (e *EntryPlus) Name() string { @@ -127,7 +155,7 @@ func (e *EntryPlus) Name() string { } func (e *EntryPlus) Size() int64 { - if e.Attr.Follows == 0 { + if !e.Attr.IsSet { return 0 } @@ -135,7 +163,7 @@ func (e *EntryPlus) Size() int64 { } func (e *EntryPlus) Mode() os.FileMode { - if e.Attr.Follows == 0 { + if !e.Attr.IsSet { return 0 } @@ -143,7 +171,7 @@ func (e *EntryPlus) Mode() os.FileMode { } func (e *EntryPlus) ModTime() time.Time { - if e.Attr.Follows == 0 { + if !e.Attr.IsSet { return time.Time{} } @@ -151,7 +179,7 @@ func (e *EntryPlus) ModTime() time.Time { } func (e *EntryPlus) IsDir() bool { - if e.Attr.Follows == 0 { + if !e.Attr.IsSet { return false } @@ -159,15 +187,25 @@ func (e *EntryPlus) IsDir() bool { } func (e *EntryPlus) Sys() interface{} { - if e.Attr.Follows == 0 { + if !e.Attr.IsSet { return 0 } return e.FileId } +type WccData struct { + Before struct { + IsSet bool `xdr:"union"` + Size uint64 `xdr:"unioncase=1"` + MTime NFS3Time `xdr:"unioncase=1"` + CTime NFS3Time `xdr:"unioncase=1"` + } + After PostOpAttr +} + type FSInfo struct { - Follows uint32 + Attr PostOpAttr RTMax uint32 RTPref uint32 RTMult uint32 @@ -184,6 +222,7 @@ type FSInfo struct { func DialService(addr string, prog rpc.Mapping) (*rpc.Client, error) { pm, err := rpc.DialPortmapper("tcp", addr) if err != nil { + util.Errorf("Failed to connect to portmapper: %s", err) return nil, err } defer pm.Close() @@ -194,6 +233,9 @@ func DialService(addr string, prog rpc.Mapping) (*rpc.Client, error) { } client, err := dialService(addr, port) + if err != nil { + return nil, err + } return client, nil } @@ -222,7 +264,10 @@ func dialService(addr string, port int) (*rpc.Client, error) { Port: p, } - client, err = rpc.DialTCP("tcp", ldr, fmt.Sprintf("%s:%d", addr, port)) + raddr := fmt.Sprintf("%s:%d", addr, port) + util.Debugf("Connecting to %s", raddr) + + client, err = rpc.DialTCP("tcp", ldr, raddr) if err == nil { break } @@ -236,8 +281,10 @@ func dialService(addr string, port int) (*rpc.Client, error) { util.Debugf("using random port %d -> %d", p, port) } else { + raddr := fmt.Sprintf("%s:%d", addr, port) + util.Debugf("Connecting to %s from unprivileged port", raddr) - client, err = rpc.DialTCP("tcp", ldr, fmt.Sprintf("%s:%d", addr, port)) + client, err = rpc.DialTCP("tcp", ldr, raddr) if err != nil { return nil, err } diff --git a/nfs/rpc/client.go b/nfs/rpc/client.go index 1eca6cb..f06c385 100644 --- a/nfs/rpc/client.go +++ b/nfs/rpc/client.go @@ -13,6 +13,7 @@ import ( "sync/atomic" "time" + "github.com/vmware/go-nfs-client/nfs/util" "github.com/vmware/go-nfs-client/nfs/xdr" ) @@ -25,6 +26,9 @@ const ( Success = iota ProgUnavail ProgMismatch + ProcUnavail + GarbageArgs + SystemErr ) const ( @@ -68,11 +72,14 @@ type message struct { } func (c *Client) Call(call interface{}) (io.ReadSeeker, error) { + retries := 1 + msg := &message{ Xid: atomic.AddUint32(&xid, 1), Body: call, } +retry: w := new(bytes.Buffer) if err := xdr.Write(w, msg); err != nil { return nil, err @@ -135,11 +142,24 @@ func (c *Client) Call(call interface{}) (io.ReadSeeker, error) { case Success: return res, nil case ProgUnavail: - return nil, fmt.Errorf("PROG_UNAVAIL") + return nil, fmt.Errorf("rpc: PROG_UNAVAIL - server does not recognize the program number") case ProgMismatch: - return nil, fmt.Errorf("rpc: PROG_MISMATCH") + return nil, fmt.Errorf("rpc: PROG_MISMATCH - program version does not exist on the") + case ProcUnavail: + return nil, fmt.Errorf("rpc: PROC_UNAVAIL - unrecognized procedure number") + case GarbageArgs: + // emulate Linux behaviour for GARBAGE_ARGS + if retries > 0 { + util.Debugf("Retrying on GARBAGE_ARGS per linux semantics") + retries-- + goto retry + } + + return nil, fmt.Errorf("rpc: GARBAGE_ARGS - rpc arguments cannot be XDR decoded") + case SystemErr: + return nil, fmt.Errorf("rpc: SYSTEM_ERR - unknown error on server") default: - return nil, fmt.Errorf("rpc: unknown status %d", acceptStatus) + return nil, fmt.Errorf("rpc: unknown accepted status error: %d", acceptStatus) } case MsgDenied: diff --git a/nfs/target.go b/nfs/target.go index b5f246f..3a734d1 100644 --- a/nfs/target.go +++ b/nfs/target.go @@ -127,7 +127,7 @@ func (v *Target) Lookup(p string) (os.FileInfo, []byte, error) { return nil, nil, err } - // util.Debugf("%s -> 0x%x", dirent, fh) + //util.Debugf("%s -> 0x%x", dirent, fh) } return fattr, fh, nil @@ -140,6 +140,12 @@ func (v *Target) lookup(fh []byte, name string) (*Fattr, []byte, error) { What Diropargs3 } + type LookupOk struct { + FH []byte + Attr PostOpAttr + DirAttr PostOpAttr + } + res, err := v.call(&Lookup3Args{ Header: rpc.Header{ Rpcvers: 2, @@ -160,27 +166,15 @@ func (v *Target) lookup(fh []byte, name string) (*Fattr, []byte, error) { return nil, nil, err } - fh, err = xdr.ReadOpaque(res) - if err != nil { - return nil, nil, err - } - - util.Debugf("lookup(%s): FH 0x%x", name, fh) - - var fattrs *Fattr - attrFollows, err := xdr.ReadUint32(res) - if err != nil { + lookupres := new(LookupOk) + if err := xdr.Read(res, lookupres); err != nil { + util.Errorf("lookup(%s) failed to parse return: %s", name, err) + util.Debugf("lookup partial decode: %+v", *lookupres) return nil, nil, err } - if attrFollows != 0 { - fattrs = new(Fattr) - if err = xdr.Read(res, fattrs); err != nil { - return nil, nil, err - } - } - - return fattrs, fh, nil + util.Debugf("lookup(%s): FH 0x%x, attr: %+v", name, lookupres.FH, lookupres.Attr.Attr) + return &lookupres.Attr.Attr, lookupres.FH, nil } func (v *Target) ReadDirPlus(dir string) ([]*EntryPlus, error) { @@ -193,6 +187,10 @@ func (v *Target) ReadDirPlus(dir string) ([]*EntryPlus, error) { } func (v *Target) readDirPlus(fh []byte) ([]*EntryPlus, error) { + cookie := uint64(0) + cookieVerf := uint64(0) + eof := false + type ReadDirPlus3Args struct { rpc.Header FH []byte @@ -202,67 +200,74 @@ func (v *Target) readDirPlus(fh []byte) ([]*EntryPlus, error) { MaxCount uint32 } + type DirListPlus3 struct { + IsSet bool `xdr:"union"` + Entry EntryPlus `xdr:"unioncase=1"` + } + type DirListOK struct { - DirAttrs struct { - Follows uint32 - DirAttrs Fattr - } + DirAttrs PostOpAttr CookieVerf uint64 - Follows uint32 } - res, err := v.call(&ReadDirPlus3Args{ - Header: rpc.Header{ - Rpcvers: 2, - Prog: Nfs3Prog, - Vers: Nfs3Vers, - Proc: NFSProc3ReadDirPlus, - Cred: v.auth, - Verf: rpc.AuthNull, - }, - FH: fh, - DirCount: 512, - MaxCount: 4096, - }) + var entries []*EntryPlus + for !eof { + res, err := v.call(&ReadDirPlus3Args{ + Header: rpc.Header{ + Rpcvers: 2, + Prog: Nfs3Prog, + Vers: Nfs3Vers, + Proc: NFSProc3ReadDirPlus, + Cred: v.auth, + Verf: rpc.AuthNull, + }, + FH: fh, + Cookie: cookie, + CookieVerf: cookieVerf, + DirCount: 512, + MaxCount: 4096, + }) - if err != nil { - util.Debugf("readdir(%x): %s", fh, err.Error()) - return nil, err - } + if err != nil { + util.Debugf("readdir(%x): %s", fh, err.Error()) + return nil, err + } - // The dir list entries are so-called "optional-data". We need to check - // the Follows fields before continuing down the array. Effectively, it's - // an encoding used to flatten a linked list into an array where the - // Follows field is set when the next idx has data. See - // https://tools.ietf.org/html/rfc4506.html#section-4.19 for details. - dirlistOK := new(DirListOK) - if err = xdr.Read(res, dirlistOK); err != nil { - return nil, err - } + // The dir list entries are so-called "optional-data". We need to check + // the Follows fields before continuing down the array. Effectively, it's + // an encoding used to flatten a linked list into an array where the + // Follows field is set when the next idx has data. See + // https://tools.ietf.org/html/rfc4506.html#section-4.19 for details. + dirlistOK := new(DirListOK) + if err = xdr.Read(res, dirlistOK); err != nil { + util.Errorf("readdir failed to parse result (%x): %s", fh, err.Error()) + util.Debugf("partial dirlist: %+v", dirlistOK) + return nil, err + } - if dirlistOK.Follows == 0 { - return nil, nil - } + for { + var item DirListPlus3 + if err = xdr.Read(res, &item); err != nil { + util.Errorf("readdir failed to parse directory entry, aborting") + util.Debugf("partial dirent: %+v", item) + return nil, err + } - var entries []*EntryPlus - for { - var entry EntryPlus + if !item.IsSet { + break + } - if err = xdr.Read(res, &entry); err != nil { - return nil, err + cookie = item.Entry.Cookie + entries = append(entries, &item.Entry) } - entries = append(entries, &entry) - - if entry.ValueFollows == 0 { - break + if err = xdr.Read(res, &eof); err != nil { + util.Errorf("readdir failed to determine presence of more data to read, aborting") + return nil, err } - } - // The last byte is EOF - var eof uint32 - if err = xdr.Read(res, &eof); err != nil { - util.Debugf("ReadDirPlus(0x%x): expected EOF", fh) + util.Debugf("No EOF for dirents so calling back for more") + cookieVerf = dirlistOK.CookieVerf } return entries, nil @@ -282,7 +287,13 @@ func (v *Target) Mkdir(path string, perm os.FileMode) ([]byte, error) { Attrs Sattr3 } - res, err := v.call(&MkdirArgs{ + type MkdirOk struct { + FH PostOpFH3 + Attr PostOpAttr + DirWcc WccData + } + + args := &MkdirArgs{ Header: rpc.Header{ Rpcvers: 2, Prog: Nfs3Prog, @@ -297,31 +308,28 @@ func (v *Target) Mkdir(path string, perm os.FileMode) ([]byte, error) { }, Attrs: Sattr3{ Mode: SetMode{ - Set: uint32(1), - Mode: uint32(perm.Perm()), + SetIt: true, + Mode: uint32(perm.Perm()), }, }, - }) + } + res, err := v.call(args) if err != nil { util.Debugf("mkdir(%s): %s", path, err.Error()) + util.Debugf("mkdir args (%+v)", args) return nil, err } - follows, err := xdr.ReadUint32(res) - if err != nil { + mkdirres := new(MkdirOk) + if err := xdr.Read(res, mkdirres); err != nil { + util.Errorf("mkdir(%s) failed to parse return: %s", path, err) + util.Debugf("mkdir(%s) partial response: %+v", mkdirres) return nil, err } - if follows != 0 { - fh, err = xdr.ReadOpaque(res) - if err != nil { - return nil, err - } - } - util.Debugf("mkdir(%s): created successfully (0x%x)", path, fh) - return fh, nil + return mkdirres.FH.FH, nil } // Create a file with name the given mode @@ -346,8 +354,9 @@ func (v *Target) Create(path string, perm os.FileMode) ([]byte, error) { } type Create3Res struct { - Follows uint32 - FH []byte + FH PostOpFH3 + Attr PostOpAttr + DirWcc WccData } res, err := v.call(&Create3Args{ @@ -366,8 +375,8 @@ func (v *Target) Create(path string, perm os.FileMode) ([]byte, error) { HW: How{ Attr: Sattr3{ Mode: SetMode{ - Set: uint32(1), - Mode: uint32(perm.Perm()), + SetIt: true, + Mode: uint32(perm.Perm()), }, }, }, @@ -384,7 +393,7 @@ func (v *Target) Create(path string, perm os.FileMode) ([]byte, error) { } util.Debugf("create(%s): created successfully", path) - return status.FH, nil + return status.FH.FH, nil } // Remove a file @@ -527,8 +536,10 @@ func (v *Target) removeAll(deleteDirfh []byte) error { // If directory, recurse, then nuke it. It should be empty when we get // back. if entry.Attr.Attr.Type == NF3Dir { - if err = v.removeAll(entry.FH); err != nil { - return err + if entry.Handle.IsSet { + if err = v.removeAll(entry.Handle.FH); err != nil { + return err + } } err = v.rmDir(deleteDirfh, entry.FileName)