Date: Mon, 2 Aug 1999 13:07:25 -0400 (EDT) From: Alfred Perlstein <bright@rush.net> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: hackers@FreeBSD.ORG Subject: Re: confusion about nfsm_srvmtofh bad behavior? Message-ID: <Pine.BSF.3.96.990802113034.20420m-100000@cygnus.rush.net> In-Reply-To: <199908021517.IAA13341@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 2 Aug 1999, Matthew Dillon wrote:
>
> ::> it then rewinds the mbuf pointers (i think) because of the
> ::> over "dissection" above.
> ::> ---
> ::>
> ::> why does it do the copy, then rewind it, it seems like it knows
> ::> it's doing something wrong and instead of fixing it, it just
> ::> compensates after the fact.
> ::
> ::yes, replying to my own message.
> ::
> ::the only thing i can think of is that the extra data is safely
> ::ignored because the routines that use these macros seem to
> ::pass the version of NFS to all the function that they call...
> ::
> ::however unless i'm wrong (which i probably am) nfsV2 stuff
> ::could be made faster if it was correctly noted and less data
> ::was copied. It would also DTRT and not access data it isn't
> ::supposed to :)
> ::
> ::it seems like all of the V3 handles are the same length so
> ::there isn't much to do there...
> ::
> ::-Alfred
> :
> : Well, I must say that it certainly looks like a bug. It is not going
> : to blow anything up since the nfsm_dissect() will break out if it runs
> : out of buffer space, but it certainly seems inefficient. I am somewhat
> : loath to fix anything in NFS that does not create a demonstrateable
> : problem for fear of creating new problems, though, it is quite possible
> : that the server code depends on the extra junk in the file handle for
> : V2 mounts - A full audit of nfs_nqlease.c and nfs_serv.c would be
> : necessary before this could be fixed.
> :
> : -Matt
> : Matthew Dillon
> : <dillon@backplane.com>
>
> Oh, p.s. But in the mean time, if you or someone would like to commit
> an XXX comment to document the potential bug / performance problem, I
> think that would be very appropriate. e.g.
>
> /*
> * Extract file handle from NFS stream. XXX note that the extraction of
> * the file handle for an NFSv2 mount appears to be rather odd. It is copying
> * NFSX_V3FH bytes instead of NFSX_V2FH and then rewinding the mbuf index.
> */
>
> #define nfsm_srvmtofh(f) \
> { int fhlen = NFSX_V3FH; \
> if (nfsd->nd_flag & ND_NFSV3) { \
> nfsm_dissect(tl, u_int32_t *, NFSX_UNSIGNED); \
> fhlen = fxdr_unsigned(int, *tl); \
> ...
The whole file needs some **** documentation. :)
I stared at that for so long wondering why the heck it was doing that.
Is there a chance you could give this some of your famous regression
testing?
http://big.endian.org/~bright/freebsd/patches/nfsm_subs.diff
This is a patch that Peter Wemm proposed however he had this:
! if (fhlen < NFSX_V3FH) { \
! bzero((caddr_t)(f) + fhlen, NFSX_V3FH - fhlen); \
! } \
right before the while(0) instead of the else clause with the
full bzero.
i'd rather get rid of the extra copying going on and since
previously it was filled with garbage from the rest of the RPC
structure i don't think it's nessesary.
-Alfred Perlstein - [bright@rush.net|bright@wintelcom.net]
systems administrator and programmer
Wintelcom - http://www.wintelcom.net/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.3.96.990802113034.20420m-100000>