Discussion:
Review D38047 ... and then there was one....
(too old to reply)
David Cross
2024-10-06 20:04:01 UTC
Permalink
Here’s the thing. The current implementation of nscd DOESN’T WORK at all. There is a symbol that nscd exports that libc is supposed to use as a flag to bypass lookups for nscd itself. But that symbol isn’t exported right.

You will need to recompile libc and nscd. (I just do a buildworld to make sure i get everything as there are makefile changes related to the aforementioned symbol changes.

And then after that make sure to check getgroupentries too
Please, love to get some eyes on this. As it stands nscd is completely useless for LDAP for getgroupmembership (and really ANY implementation that defines a specific implementation of getgroupmembership, since it will then bypass the non-existent NSCD version). Additionally it fixes bugs with negative caching as well as increases thread safety.
[host] ~# /usr/bin/time getent passwd > /dev/null
0.62 real 0.06 user 0.15 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.47 real 0.07 user 0.12 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.46 real 0.04 user 0.15 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.15 real 0.03 user 0.06 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.16 real 0.01 user 0.08 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.65 real 0.03 user 0.19 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.48 real 0.02 user 0.22 sys
[host] ~# /usr/bin/time getent passwd > /dev/null
0.43 real 0.06 user 0.12 sys
The test were run on most recent stable/14 with net/nss-pam-ldapd as a Name Service Switch module for LDAP lookup.
--
Marek Zarychta
--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
David Cross
2024-10-06 20:17:01 UTC
Permalink
Mark Millard
2024-10-06 22:36:28 UTC
Permalink
Marek Zarychta <zarychtam_at_plan-b.pwste.edu.pl> wrote on
Here’s the thing. The current implementation of nscd DOESN’T WORK at all. There is a symbol that nscd exports that libc is supposed to use as a flag to bypass lookups for nscd itself. But that symbol isn’t exported right.
You will need to recompile libc and nscd. (I just do a buildworld to make sure i get everything as there are makefile changes related to the aforementioned symbol changes.
[host] /usr/src# service nscd start
Starting nscd.
limits: setrlimit pipebuf: Invalid argument
/etc/rc.d/nscd: WARNING: failed to start nscd
. . .

This note is only about the "limits: setrlimit pipebuf:
Invalid argument" notice.

The main [so: 15] pipebuf related commits were done during
2024-Sep-20 UTC. If one has a kernel that predates those but
a world for which limits now tries to use the new pipebuf
material, the result is messages like that:

limits: setrlimit pipebuf: Invalid argument

(or related such messages).


For reference for main [so: 15]:

Fri, 20 Sep 2024
. . .
• git: 3458bbd39778 - main - kernel: add RLIMIT_PIPEBUF Konstantin Belousov
• git: 54a8d1fbbf65 - main - getrlimit(2): document RLIMIT_PIPEBUF Konstantin Belousov
• git: a4c04958f526 - main - libutil: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 5d92f20c7d31 - main - bin/sh: support RLIMIT_PIPEBUF Konstantin Belousov
• git: f54f41403d14 - main - usr.bin/limits: support RLIMIT_PIPEBUF Konstantin Belousov
• git: b029e29e0d8b - main - login.conf: add a placeholder for the pipebuf limit Konstantin Belousov
• git: 80133d678ecb - main - procstat: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 8ae779832c6f - main - privs: add PRIV_PIPEBUF Konstantin Belousov
• git: 7672cbef2c1e - main - pipes: reserve configured percentage of buffers zone to superuser Konstantin Belousov
. . .
• git: d6074f73af5c - main - pipe: use pipe subsystem KVA counter instead of pipe_map size Konstantin Belousov
• git: 40769168a5ee - main - pipespace_new(): decrease uidinfo pipebuf usage if reservation check failed Konstantin Belousov
. . .
• git: a52b30ff98cd - main - sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf Konstantin Belousov
• git: af96ccc6a508 - main - uifree(9): report non-zero values for all shared resources Konstantin Belousov
• git: 2c1963d46335 - main - procfs rlimit: handle pipebuf Konstantin Belousov
• git: c84d8db0ab3d - main - procfs: ensure that RLIMIT_IDENT is properly updated when a limit is added Konstantin Belousov

The combination of an older kernel and a newer world will not be
nicely behaved when any non-kernel code from the above ends up
involved.


stable/14 has now also had the commits:

Sat, 05 Oct 2024
• git: 1508dce2502d - stable/14 - procfs: ensure that RLIMIT_IDENT is properly updated when a limit is added Konstantin Belousov
. . .
• git: b7eecc86c3bd - stable/14 - kernel: add RLIMIT_PIPEBUF Konstantin Belousov
• git: d20f0dae2f97 - stable/14 - getrlimit(2): document RLIMIT_PIPEBUF Konstantin Belousov
• git: a03f7c040ce7 - stable/14 - libutil: support RLIMIT_PIPEBUF Konstantin Belousov
• git: d5ed8778bf3b - stable/14 - bin/sh: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 25902860b270 - stable/14 - usr.bin/limits: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 524b9810de6a - stable/14 - login.conf: add a placeholder for the pipebuf limit Konstantin Belousov
• git: 6600090e678e - stable/14 - procstat: support RLIMIT_PIPEBUF Konstantin Belousov
• git: fd9babb1b85f - stable/14 - privs: add PRIV_PIPEBUF Konstantin Belousov
• git: d532d9926ee7 - stable/14 - pipes: reserve configured percentage of buffers zone to superuser Konstantin Belousov
• git: 6536b979b856 - stable/14 - pipe: use pipe subsystem KVA counter instead of pipe_map size Konstantin Belousov
• git: a8c663bb4261 - stable/14 - pipespace_new(): decrease uidinfo pipebuf usage if reservation check failed Konstantin Belousov
• git: c15b2e046e8c - stable/14 - sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf Konstantin Belousov
. . .
• git: fc9070bf1d16 - stable/14 - procfs rlimit: handle pipebuf Konstantin Belousov
. . .

Again, the combination of an older kernel and a newer world will not be
nicely behaved.

===
Mark Millard
marklmi at yahoo.com



--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
David Cross
2024-10-07 05:05:19 UTC
Permalink
How many entries are in your ldap structure? I can attempt a replication here
Post by Mark Millard
Marek Zarychta <zarychtam_at_plan-b.pwste.edu.pl> wrote on
Here’s the thing. The current implementation of nscd DOESN’T WORK at all. There is a symbol that nscd exports that libc is supposed to use as a flag to bypass lookups for nscd itself. But that symbol isn’t exported right.
You will need to recompile libc and nscd. (I just do a buildworld to make sure i get everything as there are makefile changes related to the aforementioned symbol changes.
[host] /usr/src# service nscd start
Starting nscd.
limits: setrlimit pipebuf: Invalid argument
/etc/rc.d/nscd: WARNING: failed to start nscd
. . .
Invalid argument" notice.
The main [so: 15] pipebuf related commits were done during
2024-Sep-20 UTC. If one has a kernel that predates those but
a world for which limits now tries to use the new pipebuf
limits: setrlimit pipebuf: Invalid argument
(or related such messages).
Fri, 20 Sep 2024
. . .
• git: 3458bbd39778 - main - kernel: add RLIMIT_PIPEBUF Konstantin Belousov
• git: 54a8d1fbbf65 - main - getrlimit(2): document RLIMIT_PIPEBUF Konstantin Belousov
• git: a4c04958f526 - main - libutil: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 5d92f20c7d31 - main - bin/sh: support RLIMIT_PIPEBUF Konstantin Belousov
• git: f54f41403d14 - main - usr.bin/limits: support RLIMIT_PIPEBUF Konstantin Belousov
• git: b029e29e0d8b - main - login.conf: add a placeholder for the pipebuf limit Konstantin Belousov
• git: 80133d678ecb - main - procstat: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 8ae779832c6f - main - privs: add PRIV_PIPEBUF Konstantin Belousov
• git: 7672cbef2c1e - main - pipes: reserve configured percentage of buffers zone to superuser Konstantin Belousov
. . .
• git: d6074f73af5c - main - pipe: use pipe subsystem KVA counter instead of pipe_map size Konstantin Belousov
• git: 40769168a5ee - main - pipespace_new(): decrease uidinfo pipebuf usage if reservation check failed Konstantin Belousov
. . .
• git: a52b30ff98cd - main - sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf Konstantin Belousov
• git: af96ccc6a508 - main - uifree(9): report non-zero values for all shared resources Konstantin Belousov
• git: 2c1963d46335 - main - procfs rlimit: handle pipebuf Konstantin Belousov
• git: c84d8db0ab3d - main - procfs: ensure that RLIMIT_IDENT is properly updated when a limit is added Konstantin Belousov
The combination of an older kernel and a newer world will not be
nicely behaved when any non-kernel code from the above ends up
involved.
Sat, 05 Oct 2024
• git: 1508dce2502d - stable/14 - procfs: ensure that RLIMIT_IDENT is properly updated when a limit is added Konstantin Belousov
. . .
• git: b7eecc86c3bd - stable/14 - kernel: add RLIMIT_PIPEBUF Konstantin Belousov
• git: d20f0dae2f97 - stable/14 - getrlimit(2): document RLIMIT_PIPEBUF Konstantin Belousov
• git: a03f7c040ce7 - stable/14 - libutil: support RLIMIT_PIPEBUF Konstantin Belousov
• git: d5ed8778bf3b - stable/14 - bin/sh: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 25902860b270 - stable/14 - usr.bin/limits: support RLIMIT_PIPEBUF Konstantin Belousov
• git: 524b9810de6a - stable/14 - login.conf: add a placeholder for the pipebuf limit Konstantin Belousov
• git: 6600090e678e - stable/14 - procstat: support RLIMIT_PIPEBUF Konstantin Belousov
• git: fd9babb1b85f - stable/14 - privs: add PRIV_PIPEBUF Konstantin Belousov
• git: d532d9926ee7 - stable/14 - pipes: reserve configured percentage of buffers zone to superuser Konstantin Belousov
• git: 6536b979b856 - stable/14 - pipe: use pipe subsystem KVA counter instead of pipe_map size Konstantin Belousov
• git: a8c663bb4261 - stable/14 - pipespace_new(): decrease uidinfo pipebuf usage if reservation check failed Konstantin Belousov
• git: c15b2e046e8c - stable/14 - sys_pipe: consistently use cr_ruidinfo for accounting of pipebuf Konstantin Belousov
. . .
• git: fc9070bf1d16 - stable/14 - procfs rlimit: handle pipebuf Konstantin Belousov
. . .
Again, the combination of an older kernel and a newer world will not be
nicely behaved.
===
Mark Millard
marklmi at yahoo.com
Thank you for this deep investigation. Anyway, after the kernel and world update, I could test the patch, but there was no significant progress, at least in lookup timing (as I pointed out before).
--
Marek Zarychta
--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Enji Cooper
2024-10-11 15:25:02 UTC
Permalink
Yes. I was about to suggest this. Plus, any proposed commit log message
must answer the questions why, what and how. With special attention to why.
I have the same feelings as Cy.

FWIW, part of the reason why large/complex changes like this languish in my review queues is in part due to reasons like this.

Unless I am a SME in the area who is driven to understand what the change aims to achieve, I will not take the time to review large/complex chances. I have a lot of other things in my life which take priority over large code reviews.

Please break the large change down into a smaller set of changes/reviews to make it easier to review the overall change effectively.

Cheers,
-Enji

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Cy Schubert
2024-10-11 15:35:26 UTC
Permalink
In message <AE37187C-79D0-4B5E-87F0-***@gmail.com>, Enji Cooper
writes
Post by Enji Cooper
Yes. I was about to suggest this. Plus, any proposed commit log message
must answer the questions why, what and how. With special attention to why=
.
I have the same feelings as Cy.
FWIW, part of the reason why large/complex changes like this languish in my r
=
eview queues is in part due to reasons like this.
Totally!
Post by Enji Cooper
Unless I am a SME in the area who is driven to understand what the change ai=
ms to achieve, I will not take the time to review large/complex chances. I h=
ave a lot of other things in my life which take priority over large code rev=
iews.
Agreed. In the case of nscd, I applied one fix to it years ago. Like you, I
do not feel qualified to review a large and complex jumbo patch (group of
patches).
Post by Enji Cooper
Please break the large change down into a smaller set of changes/reviews to m
=
ake it easier to review the overall change effectively.
Yes. I have the same issue at $JOB. We use github enterprise. I refuse to
review such patches and yet others, less experienced, summarily approve
such patches without even looking at them, breaking our infrastructure.

The choice is clear.
Post by Enji Cooper
Cheers,
-Enji=
--
Cheers,
Cy Schubert <***@cschubert.com>
FreeBSD UNIX: <***@FreeBSD.org> Web: https://FreeBSD.org
NTP: <***@nwtime.org> Web: https://nwtime.org

e^(i*pi)+1=0




--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...