Skip to content

Commit

Permalink
NFS bug fix and improvement (#412)
Browse files Browse the repository at this point in the history
* fs/nfs: Remove all nfs_checkmount call.  The check just waste cpu cycle since nobody will set nm_mounted to false, and remove the unused fields(nm_mounted and n_flags) and related flags too
* fs/nfs: Fix the definition not confirm to RFC 1813 and other minor issue(unused, typo, duplication, alignment...)
* fs/nfs: Always represent error with negative number and remove the unused arguments from function
* fs/nfs: Set socket receive timeout with nfs_args->timeo and fix warning:

nfs/nfs.h:59:28: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 #define NFS_TIMEO          (1 * NFS_HZ)   /* Default timeout = 1 second */
                            ^
nfs/nfs_vfsops.c:1857:23: note: in expansion of macro 'NFS_TIMEO'
   nprmt.timeo       = NFS_TIMEO;
                            ^
                       ^~~~~~~~~

* fs/nfs: Update the file attribute correctly in nfs_filetruncate and simplify the attrbitue conversion between NFSv3 and NuttX
* fs/nfs: Remove the unfinished and buggy EXCLUSIVE creation mode
* fs/nfs: Call nfs_fsinfo in nfs_bind instead of nfs_statfs since we should update the buffer size before transfer happen, and handle the attribute existence variance correctly.
* fs/nfs: Shouldn't insert node into list again in nfs_dup and fix other typo issue
* fs/nfs: Should skip . and .. in nfs_readdir
* fs/nfs: Remove the unnecessary dynamic allocation and the duplicated root handle storage
  • Loading branch information
xiaoxiang781216 authored and gregory-nutt committed Mar 1, 2020
1 parent 350adb2 commit 915f094
Show file tree
Hide file tree
Showing 9 changed files with 523 additions and 905 deletions.
19 changes: 4 additions & 15 deletions fs/nfs/nfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,14 @@
* Pre-processor Definitions
****************************************************************************/

#define NFS_TICKS 1 /* Number of system ticks */
#define NFS_HZ CLOCKS_PER_SEC /* Ticks/sec */
#define NFS_TIMEO (1 * NFS_HZ) /* Default timeout = 1 second */
#define NFS_MINTIMEO (1 * NFS_HZ) /* Min timeout to use */
#define NFS_MAXTIMEO (60 * NFS_HZ) /* Max timeout to backoff to */
#define NFS_TIMEOUTMUL 2 /* Timeout/Delay multiplier */
#define NFS_TIMEO 10 /* Default timeout = 1 second */
#define NFS_MINTIMEO 10 /* Min timeout to use */
#define NFS_MAXTIMEO 255 /* Max timeout to backoff to */
#define NFS_MAXREXMIT 100 /* Stop counting after this many */
#define NFS_RETRANS 10 /* Num of retrans for soft mounts */
#define NFS_WSIZE 8192 /* Def. write data size <= 8192 */
#define NFS_RSIZE 8192 /* Def. read data size <= 8192 */
#define NFS_READDIRSIZE 8192 /* Def. readdir size */
#define NFS_NPROCS 23

/* Ideally, NFS_DIRBLKSIZ should be bigger, but I've seen servers with
* broken NFS/ethernet drivers that won't work with anything bigger (Linux..)
Expand All @@ -85,9 +81,6 @@
* Public Data
****************************************************************************/

extern uint32_t nfs_true;
extern uint32_t nfs_false;
extern uint32_t nfs_xdrneg1;
#ifdef CONFIG_NFS_STATISTICS
extern struct nfsstats nfsstats;
#endif
Expand Down Expand Up @@ -115,11 +108,7 @@ extern "C" {
#define EXTERN extern
#endif

EXTERN void nfs_semtake(FAR struct nfsmount *nmp);
EXTERN void nfs_semgive(FAR struct nfsmount *nmp);
EXTERN int nfs_checkmount(FAR struct nfsmount *nmp);
EXTERN int nfs_fsinfo(FAR struct nfsmount *nmp);
EXTERN int nfs_request(struct nfsmount *nmp, int procnum,
EXTERN int nfs_request(FAR struct nfsmount *nmp, int procnum,
FAR void *request, size_t reqlen,
FAR void *response, size_t resplen);
EXTERN int nfs_lookup(FAR struct nfsmount *nmp, FAR const char *filename,
Expand Down
35 changes: 15 additions & 20 deletions fs/nfs/nfs_mount.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,18 @@

struct nfsmount
{
struct nfsnode *nm_head; /* A list of all files opened on this mountpoint */
sem_t nm_sem; /* Used to assure thread-safe access */
nfsfh_t nm_fh; /* File handle of root dir */
char nm_path[90]; /* server's path of the directory being mounted */
struct nfs_fattr nm_fattr; /* nfs file attribute cache */
struct rpcclnt *nm_rpcclnt; /* RPC state */
struct socket *nm_so; /* RPC socket */
struct sockaddr nm_nam; /* Addr of server */
bool nm_mounted; /* true: The file system is ready */
uint8_t nm_fhsize; /* Size of root file handle (host order) */
uint8_t nm_sotype; /* Type of socket */
uint8_t nm_retry; /* Max retries */
uint16_t nm_timeo; /* Timeout value (in system clock ticks) */
uint16_t nm_rsize; /* Max size of read RPC */
uint16_t nm_wsize; /* Max size of write RPC */
uint16_t nm_readdirsize; /* Size of a readdir RPC */
uint16_t nm_buflen; /* Size of I/O buffer */
FAR struct nfsnode *nm_head; /* A list of all files opened on this mountpoint */
sem_t nm_sem; /* Used to assure thread-safe access */
nfsfh_t *nm_fh; /* File handle of root dir */
char nm_path[90]; /* server's path of the directory being mounted */
struct nfs_fattr nm_fattr; /* nfs file attribute cache */
FAR struct rpcclnt *nm_rpcclnt; /* RPC state */
struct sockaddr nm_nam; /* Addr of server */
uint8_t nm_fhsize; /* Size of root file handle (host order) */
uint16_t nm_rsize; /* Max size of read RPC */
uint16_t nm_wsize; /* Max size of write RPC */
uint16_t nm_readdirsize; /* Size of a readdir RPC */
uint16_t nm_buflen; /* Size of I/O buffer */

/* Set aside memory on the stack to hold the largest call message. NOTE
* that for the case of the write call message, it is the reply message that
Expand All @@ -104,7 +99,7 @@ struct nfsmount
struct rpc_call_readdir readdir;
struct rpc_call_fs fsstat;
struct rpc_call_setattr setattr;
struct rpc_call_fs fs;
struct rpc_call_fs fsinfo;
struct rpc_reply_write write;
} nm_msgbuffer;

Expand All @@ -116,10 +111,10 @@ struct nfsmount
* possible WRITE call message or READ response message.
*/

uint32_t nm_iobuffer[1]; /* Actual size is given by nm_buflen */
uint32_t nm_iobuffer[1]; /* Actual size is given by nm_buflen */
};

/* The size of the nfsmount structure will debug on the size of the allocated I/O
/* The size of the nfsmount structure will depend on the size of the allocated I/O
* buffer.
*/

Expand Down
29 changes: 10 additions & 19 deletions fs/nfs/nfs_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,6 @@

#include "nfs_proto.h"

/****************************************************************************
* Pre-processor Definitions
****************************************************************************/

/* Flags for struct nfsnode n_flag */

#define NFSNODE_OPEN (1 << 0) /* File is still open */
#define NFSNODE_MODIFIED (1 << 1) /* Might have a modified buffer */

/****************************************************************************
* Public Types
****************************************************************************/
Expand All @@ -69,16 +60,16 @@

struct nfsnode
{
struct nfsnode *n_next; /* Retained in a singly linked list. */
uint8_t n_crefs; /* Reference count (for nfs_dup) */
uint8_t n_type; /* File type */
uint8_t n_fhsize; /* Size in bytes of the file handle */
uint8_t n_flags; /* Node flags */
uint16_t n_mode; /* File mode for fstat() */
time_t n_mtime; /* File modification time */
time_t n_ctime; /* File creation time */
nfsfh_t n_fhandle; /* NFS File Handle */
uint64_t n_size; /* Current size of file */
FAR struct nfsnode *n_next; /* Retained in a singly linked list. */
uint8_t n_crefs; /* Reference count (for nfs_dup) */
uint8_t n_type; /* File type */
uint8_t n_fhsize; /* Size in bytes of the file handle */
uint16_t n_mode; /* File mode for fstat() */
time_t n_atime; /* File access time */
time_t n_mtime; /* File modification time */
time_t n_ctime; /* File creation time */
nfsfh_t n_fhandle; /* NFS File Handle */
uint64_t n_size; /* Current size of file */
};

#endif /* __FS_NFS_NFS_NODE_H */
55 changes: 20 additions & 35 deletions fs/nfs/nfs_proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@

/* Specific to NFS Version 3 */

#define NFSX_V3FH (sizeof (fhandle_t)) /* size this server uses */
#define NFSX_V3FH (sizeof(struct fhandle)) /* size this server uses */
#define NFSX_V3FHMAX 64 /* max. allowed by protocol */
#define NFSX_V3FATTR 84
#define NFSX_V3SATTR 60 /* max. all fields filled in */
#define NFSX_V3SRVSATTR (sizeof (struct nfsv3_sattr))
#define NFSX_V3POSTOPATTR (NFSX_V3FATTR + NFSX_UNSIGNED)
#define NFSX_V3WCCDATA (NFSX_V3POSTOPATTR + 8 * NFSX_UNSIGNED)
#define NFSX_V3WCCDATA (NFSX_V3POSTOPATTR + 7 * NFSX_UNSIGNED)
#define NFSX_V3COOKIEVERF 8
#define NFSX_V3WRITEVERF 8
#define NFSX_V3CREATEVERF 8
Expand Down Expand Up @@ -185,13 +185,6 @@
#define NFSV3FSINFO_HOMOGENEOUS 0x08
#define NFSV3FSINFO_CANSETTIME 0x10

/* Conversion macros */

#define vtonfsv3_mode(m) txdr_unsigned((m) & 07777)
#define nfstov_mode(a) (fxdr_unsigned(u_int16_t, (a))&07777)
#define vtonfsv3_type(a) txdr_unsigned(nfsv3_type[((int32_t)(a))])
#define nfsv3tov_type(a) nv3tov_type[fxdr_unsigned(uint32_t,(a))&0x7]

/* Mode bit values */

#define NFSMODE_IXOTH (1 << 0) /* Execute permission for others on a file */
Expand Down Expand Up @@ -230,7 +223,7 @@ typedef enum
} nfstype;

/* File Handle variable is up to 64 bytes for version 3. This structures a
* ariable sized and are provided only for setting aside maximum memory
* variable sized and are provided only for setting aside maximum memory
* allocations for a file handle.
*/

Expand Down Expand Up @@ -305,7 +298,7 @@ struct nfsv3_sattr
uint32_t sa_gidfollows; /* TRUE: Mode value follows */
uint32_t sa_gid; /* Mode value */
uint32_t sa_sizefollows; /* TRUE: Size value follows */
uint32_t sa_size; /* Size value */
nfsuint64 sa_size; /* Size value */
uint32_t sa_atimetype; /* Don't change, use server timer, or use client time */
nfstime3 sa_atime; /* Client time */
uint32_t sa_mtimetype; /* Don't change, use server timer, or use client time */
Expand All @@ -314,6 +307,7 @@ struct nfsv3_sattr

struct nfs_statfs
{
uint32_t obj_attributes_follow;
struct nfs_fattr obj_attributes;
nfsuint64 sf_tbytes;
nfsuint64 sf_fbytes;
Expand All @@ -324,16 +318,10 @@ struct nfs_statfs
uint32_t sf_invarsec;
};

struct post_attr
{
uint32_t obj_attributesfalse;
struct nfs_fattr attributes;
};

struct nfsv3_fsinfo
{
//struct post_attr obj_attributes;
uint32_t obj_attributesfalse;
uint32_t obj_attributes_follow;
struct nfs_fattr obj_attributes;
uint32_t fs_rtmax;
uint32_t fs_rtpref;
uint32_t fs_rtmult;
Expand Down Expand Up @@ -393,6 +381,18 @@ struct CREATE3resok
struct wcc_data dir_wcc;
};

struct SETATTR3args
{
struct file_handle fhandle; /* Variable length */
struct nfsv3_sattr new_attributes; /* Variable length */
uint32_t guard; /* Guard value */
};

struct SETATTR3resok
{
struct wcc_data wcc_data;
};

/* The actual size of the lookup argument is variable. These structures are, therefore,
* only useful in setting aside maximum memory usage for the LOOKUP arguments.
*/
Expand All @@ -409,18 +409,6 @@ struct LOOKUP3args
struct LOOKUP3filename name; /* Variable length */
};

struct SETATTR3args
{
struct file_handle fhandle; /* Variable length */
struct nfsv3_sattr new_attributes; /* Variable length */
uint32_t guard; /* Guard value */
};

struct SETATTR3resok
{
struct wcc_data wcc_data;
};

/* Actual size of LOOKUP3args */

#define SIZEOF_LOOKUP3filename(b) (sizeof(uint32_t) + (((b)+3) & ~3))
Expand Down Expand Up @@ -464,6 +452,7 @@ struct nfs_wrhdr_s
uint64_t offset;
uint32_t count;
uint32_t stable;
uint32_t length;
};

struct WRITE3args
Expand Down Expand Up @@ -528,10 +517,6 @@ struct RMDIR3resok
struct wcc_data dir_wcc;
};

/* The actual size of the lookup argument is variable. This structures is, therefore,
* only useful in setting aside maximum memory usage for the LOOKUP arguments.
*/

struct READDIR3args
{
struct file_handle dir; /* Variable length */
Expand Down
Loading

0 comments on commit 915f094

Please sign in to comment.