NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

lib/40728: expanding NIS-Netgroups for users and groups is very slow



>Number:         40728
>Category:       lib
>Synopsis:       expanding NIS-Netgroups for users and groups is very slow
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Mon Feb 23 12:50:00 +0000 2009
>Originator:     W. Stukenbrock
>Release:        NetBSD 4.0
>Organization:
Dr. Nagler & Company GmbH
        
>Environment:
        
        
System: NetBSD s012 4.0 NetBSD 4.0 (NSW-S012) #1: Thu Sep 11 12:21:03 CEST 2008 
root@s012:/usr/src/sys/arch/amd64/compile/NSW-S012 amd64
Architecture: x86_64
Machine: amd64
>Description:
        The problem has been recognized while running "ls -l" as root and as 
non-root-user.
        When running as root the statement takes significant longer (something 
around 20 seconds) as running as non-root-user on
        the same directory. The call for the non-root-user takes less than 2 
seconds.
        The system is a YP-client with no local ypserv and setup in passwd and 
group compat mode with compat-nis.
        I've analysed the problem by tracing the network and found a lot of 
repeated YP-request to check the existens of
        passwd.adjunct and passwd.master.byname.
        A look into the ls-command sources shows, that it uses the 
pwcache-module - user_from_uid().
        The pwcache-module tries to minimize the lookup-effort of the 
getpwent-routines by setting stayopen to 1.
        Dooing this should keep all possible fd's etc. active in order to avoid 
redundant requests etc.
        In the getpwent (or getgrent) routines the ns-switch is used.
        The pwcache calls now getpwuid() to solve the user_from_uid() call.
        This call will go though _compat_getpwuid() in getpwent.c, will find 
the included netgroup (e.g. "+@NU_USER_FOR_HOST") in the
        password file and starts netgroup processing (setnetgrent(), 
getnetgrent(), endnetgrent()).
        It will call the ns-switch for passwd_compat for every member in the 
netgroup with the function getpwnam_r.
        This will call _nis_getpwnam_r() - we have compat set to NIS - and is 
here the problem.
        Even if we have set stayopen - _nis_getpwnam_r() uses a local state 
information and ignores the global stayopen condition.
        So it will start over with determining the domainname and for the user 
root check if there are passwd.adjunct or passwd.master.byname
        maps on the server.
        The same happens to the group lookup stuff.
        The problem gets gets bigger and bigger with the member count of the 
included netgroup. Each lookup will take up to 3 times the required
        time for each member of the group. And if you are out of luck, the 
pwcache module is not able to cache a lot due to the simple
        hash mechanism there.
        A look the implementation of _nis_getpwnam_r() and _nis_getpwuid_r() 
shows, that they are just callint ypmatch(...) and don't touch
        any other state information than the domainname and the cached 
information for passwd.adjunct or passwd.master.byname maps..
        So it is easy to avoid the repeated lookups by useing the global state 
buffer for NIS if stayopen is set there.
        No additional synchronisation is needed for the current implementation, 
because all calls enter a module-monitor prior the first call
        to the ns-switch stuff. If this will be changed in the future, the 
current state information should be copied to the local state info
        in theese routines with an adequate mutex aquired, but this is a 
possbile todo for the future.

        With the following patch the run time of ls -l" as user root is much 
faster as before and reaches the time of a non-root-user,
        if there are no passwd.adjunct and passwd.master.byname maps.

        An analog problem may be in the HESIOD code part, but we are not useing 
HESIOD-maps here, so I cannot test anything and therefore
        I haven't changed the code there.
>How-To-Repeat:
        Setup a large netgroup and add it to /etc/passwd and/or /etc/group.
        Run agains a server without passwd.master (and passwd.adjunct) maps.
        Try "ls -l" as root and trace the network trafic. You will see two 
request for passwd.adjunct and passwd.master.byname
        for each member in the netgroup and they report all the time, that the 
maps are not present.
>Fix:
        The following patches will use the global state buffer in getpwent.c 
and getgrent.c if stayopen has been set.

--- getgrent.c  2009/02/23 10:50:32     1.1
+++ getgrent.c  2009/02/23 12:45:45
@@ -1194,9 +1194,17 @@
        _DIAGASSERT(result != NULL);
 
        *result = NULL;
-       memset(&state, 0, sizeof(state));
-       rv = __grscan_nis(retval, grp, buffer, buflen, &state, 1, NULL, gid);
-       __grend_nis(&state);
+/* remark: we run under a global mutex inside of this module ... */
+       if (_nis_state.stayopen)
+         { /* use global state only if stayopen is set - otherwiese we would 
blow up getgrent_r() ... */
+            rv = __grscan_nis(retval, grp, buffer, buflen, &_nis_state, 1, 
NULL, gid);
+         }
+       else
+         {
+           memset(&state, 0, sizeof(state));
+           rv = __grscan_nis(retval, grp, buffer, buflen, &state, 1, NULL, 
gid);
+           __grend_nis(&state);
+         }
        if (rv == NS_SUCCESS)
                *result = grp;
        return rv;
@@ -1246,9 +1254,17 @@
        _DIAGASSERT(result != NULL);
 
        *result = NULL;
-       memset(&state, 0, sizeof(state));
-       rv = __grscan_nis(retval, grp, buffer, buflen, &state, 1, name, 0);
-       __grend_nis(&state);
+/* remark: we run under a global mutex inside of this module ... */
+       if (_nis_state.stayopen)
+         { /* use global state only if stayopen is set - otherwiese we would 
blow up getgrent_r() ... */
+            rv = __grscan_nis(retval, grp, buffer, buflen, &_nis_state, 1, 
name, 0);
+         }
+       else
+         {
+           memset(&state, 0, sizeof(state));
+           rv = __grscan_nis(retval, grp, buffer, buflen, &state, 1, name, 0);
+           __grend_nis(&state);
+         }
        if (rv == NS_SUCCESS)
                *result = grp;
        return rv;
--- getpwent.c  2009/02/23 10:50:32     1.1
+++ getpwent.c  2009/02/23 11:09:31
@@ -1529,14 +1529,21 @@
        _DIAGASSERT(result != NULL);
 
        *result = NULL;
-       memset(&state, 0, sizeof(state));
-       rv = _nis_start(&state);
-       if (rv != NS_SUCCESS)
-               return rv;
        snprintf(buffer, buflen, "%u", (unsigned int)uid);
-       rv = _nis_pwscan(retval, pw, buffer, buflen,
-           &state, PASSWD_BYUID(&state));
-       _nis_end(&state);
+/* remark: we run under a global mutex inside of this module ... */
+       if (_nis_state.stayopen)
+         { /* use global state only if stayopen is set - otherwiese we would 
blow up getpwent_r() ... */
+           rv = _nis_pwscan(retval, pw, buffer, buflen,
+               &_nis_state, PASSWD_BYUID(&_nis_state));
+         }
+       else
+         { /* keep old semantic if no stayopen set - no need to call 
_nis_start() here - _nis_pwscan() will do it for us ... */
+           /* use same way as in getgrent.c ... */
+           memset(&state, 0, sizeof(state));
+           rv = _nis_pwscan(retval, pw, buffer, buflen,
+               &state, PASSWD_BYUID(&state));
+           _nis_end(&state);
+         }
        if (rv != NS_SUCCESS)
                return rv;
        if (uid == pw->pw_uid) {
@@ -1593,13 +1600,20 @@
 
        *result = NULL;
        snprintf(buffer, buflen, "%s", name);
-       memset(&state, 0, sizeof(state));
-       rv = _nis_start(&state);
-       if (rv != NS_SUCCESS)
-               return rv;
-       rv = _nis_pwscan(retval, pw, buffer, buflen,
-           &state, PASSWD_BYNAME(&state));
-       _nis_end(&state);
+/* remark: we run under a global mutex inside of this module ... */
+       if (_nis_state.stayopen)
+         { /* use global state only if stayopen is set - otherwiese we would 
blow up getpwent_r() ... */
+           rv = _nis_pwscan(retval, pw, buffer, buflen,
+               &_nis_state, PASSWD_BYNAME(&_nis_state));
+         }
+       else
+         { /* keep old semantic if no stayopen set - no need to call 
_nis_start() here - _nis_pwscan() will do it for us ... */
+           /* use same way as in getgrent.c ... */
+           memset(&state, 0, sizeof(state));
+           rv = _nis_pwscan(retval, pw, buffer, buflen,
+               &state, PASSWD_BYNAME(&state));
+           _nis_end(&state);
+         }
        if (rv != NS_SUCCESS)
                return rv;
        if (strcmp(name, pw->pw_name) == 0) {

>Unformatted:
        
        


Home | Main Index | Thread Index | Old Index