Selfserv uses NSS_Init(). With the latest session bucketing implementation, this means selfserv is acting like a client. Servers should use NSS_Initialize(). Selfserv initializes thusly: rv = NSS_Init(dir); The session bucketing code introduced an optimizeSpace flag, which defaults to PR_TRUE. As a result, selfserv is a token object and session hash sizes of 32 rather than 1024. NSS_Init() is used by clients (setting optimizeSpace). NSS_Initialize() is used by servers. It contains: ((flags & NSS_INIT_OPTIMIZESPACE) == NSS_INIT_OPTIMIZESPACE)); selfserv should be calling NSS_Initialize() directly to avoid setting the NSS_INIT_OPTIMIZESPACE flag, or supplying PR_FALSE for the optimizeSpace argument to NSS_Init().
P2, targeting 3.9.
Priority: -- → P2
Summary: selfserv should OptimizeSpeed (use NSS_Initialize) → selfserv should not OptimizeSpace
Target Milestone: --- → 3.9
Created attachment 124128 [details] [diff] [review] Proposed patch for the TIP Correction - NSS_Init() is hardcoded to set PR_TRUE. Selfserv must call NSS_Initialize().
Created attachment 124129 [details] selfserv stress with and without the proposed patch 3.2% gain on fullhandshakes. 12.5% gain on restarts.
Should strsclnt be allocating this way too?
The name "secmod.db" is not correct for all platforms. Whenever the NSS libraries use it internally, they define the symbol SECMOD_DB as follows (in nss.c and in pkcs11i.h): #ifdef macintosh #define SECMOD_DB "Security Modules" #else #define SECMOD_DB "secmod.db" #endif I think we should move the above code to nss.h, a public header file, so that all callers of the public function NSS_Initialize will have access to it. Then the patch to selfserv becomes the following one-line change: - rv = NSS_Init(dir); + rv = NSS_Initialize(dir, "", "", SECMOD_DB, NSS_INIT_READONLY); And, yes, I think strsclnt should do this, too.
Nelson's suggestion is good. I agree that strsclnt should call do this, too. Although strsclnt is a client program, we use it for performance benchmarking so it should optimize speed.
Created attachment 124343 [details] [diff] [review] Proposed patch for the TIP This patch fixes both selfserv and strsclnt to OptimizeSpeed. Note, removing the SECMOD_DB defines from pkcs11i.h made compilation of pk11db.c fail, so this proposal leaves the defines in pkcs11i.h.
Attachment #124128 - Attachment is obsolete: true
Comment on attachment 124343 [details] [diff] [review] Proposed patch for the TIP Kirk, could you fix the indentation of the NSS_Initialize line in selfserv.c? I think you just need to replace the tab at the beginning of the line by four spaces. The pkcs11i.h problem can be solved by having pk11db.c include nss.h to get the definition of SECMOD_DB. Bob, is it okay for a softoken source file to include nss.h?
Attachment #124343 - Flags: review-
Re: Comment 8 nss.h does NOT presently define SECMOD_DB. It should. That should be fixed. THEN the one-line patch I suggested above will work.
Created attachment 124417 [details] [diff] [review] Proposed patch for the TIP Corrected indentation at selfserv.c:1641. Made nss/lib/softoken/pk11db.c include nss.h. Removed SECMOD_DB defines from nss/lib/softoken/pkcs11i.h.
Attachment #124343 - Attachment is obsolete: true
Comment on attachment 124417 [details] [diff] [review] Proposed patch for the TIP r=wtc. Question for Bob and Nelson: would secmod.h or secmodt.h be a better place than nss.h to define the SECMOD_DB macro?
Attachment #124417 - Flags: review+
I'm not sure where isthe best place for this #define symbol, but I feel strongly that softoken should not be dependent on nss.h. I'd give review- to the two patches of 2003-05-28 for that reason.
Comment on attachment 124417 [details] [diff] [review] Proposed patch for the TIP Kirk, I'm sorry that I need to ask you to revise this patch again. Bob and Nelson explained to me that the softoken should not depend on anything in NSS above the PKCS#11 interface, so softoken should not include nss.h. Could you remove the changes to softoken/pk11db.c and softoken/pkcs11i.h from this patch? Thanks.
Attachment #124417 - Flags: review+ → review-
Created attachment 124559 [details] [diff] [review] Proposed patch for the TIP Backed out of changes to pk11db.c and pkcs11i.h.
Attachment #124417 - Attachment is obsolete: true
Comment on attachment 124559 [details] [diff] [review] Proposed patch for the TIP r=wtc. Thanks, Kirk.
Attachment #124559 - Flags: review+
Created attachment 124563 [details] selfserv stress with and without the proposed patch Benchmarked selfserv of this mornings TIP (full, restart), and after applying this patch (full-speed, restart-speed). 13.5% gain on restarts Also ran a strsclnt locally against selfserv on dsame2 for awhile to verify strsclnt works.
Attachment #124129 - Attachment is obsolete: true
Checked into the TIP: Resolves bug 206971 - selfserv should OptimizeSpeed (use NSS_Initialize). Both selfserv and strsclnt no longer OptimizeSpace. Moved SECMOD_DB defines from nssinit.c to nss.h, make it availble for public use with NSS_Initialize(). Checking in mozilla/security/nss/cmd/selfserv/selfserv.c; /cvsroot/mozilla/security/nss/cmd/selfserv/selfserv.c,v <-- selfserv.c new revision: 1.52; previous revision: 1.51 done Checking in mozilla/security/nss/cmd/strsclnt/strsclnt.c; /cvsroot/mozilla/security/nss/cmd/strsclnt/strsclnt.c,v <-- strsclnt.c new revision: 1.33; previous revision: 1.32 done Checking in mozilla/security/nss/lib/nss/nss.h; /cvsroot/mozilla/security/nss/lib/nss/nss.h,v <-- nss.h new revision: 1.28; previous revision: 1.27 done Checking in mozilla/security/nss/lib/nss/nssinit.c; /cvsroot/mozilla/security/nss/lib/nss/nssinit.c,v <-- nssinit.c new revision: 1.63; previous revision: 1.62 done
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.