[v2,03/23] nscd: Enhance tst-nscd-basic with query counters

Message ID 10c9ef7c1c250ca75ce3193cd4de9cf7eb997d82.1774037705.git.fweimer@redhat.com (mailing list archive)
State Failed CI
Headers
Series NSS, nscd updates (for group merging and more) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer March 20, 2026, 8:41 p.m. UTC
  This helps to test that the cache is actually used.
---
 nscd/tst-nscd-basic.c  | 58 ++++++++++++++++++++++++++++++++++++++++++
 support/nscd_test.h    |  4 +++
 support/support_nscd.c | 33 ++++++++++++++++++++++++
 3 files changed, 95 insertions(+)
  

Comments

Carlos O'Donell March 23, 2026, 3:47 p.m. UTC | #1
On 3/20/26 4:41 PM, Florian Weimer wrote:
> This helps to test that the cache is actually used.

LGTM.

This also looks reasonable and I like the cross-checking we're
doing here. The only wrinkle is that I think the test are failing
at the end of the series. Conceptually though I can't fault the
work you've done here. I don't see anything wrong, so I'm approving.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>   nscd/tst-nscd-basic.c  | 58 ++++++++++++++++++++++++++++++++++++++++++
>   support/nscd_test.h    |  4 +++
>   support/support_nscd.c | 33 ++++++++++++++++++++++++
>   3 files changed, 95 insertions(+)
> 
> diff --git a/nscd/tst-nscd-basic.c b/nscd/tst-nscd-basic.c
> index 2ce1fece60..3dfde92c36 100644
> --- a/nscd/tst-nscd-basic.c
> +++ b/nscd/tst-nscd-basic.c
> @@ -31,6 +31,8 @@
>   #include <support/xmemstream.h>
>   #include <support/xstdio.h>
>   
> +#include <nscd/nscd-statdata.h>
> +
>   /* Large GECOS string for the large-gecos user.  */
>   static char *large_gecos;
>   
> @@ -452,17 +454,73 @@ do_test (void)
>     /* First run the tests without nscd running, to check that the test
>        expectations are correct.  Use a separate process to avoid
>        caching nscd availability.  */
> +  puts ("info: testing without nscd");
>     support_isolate_in_subprocess (all_tests, NULL);
>   
>     support_nscd_copy_configuration ();
>   
>     support_nscd_start ();
>   
> +  /* Initially the query counters are all zero.  */
> +  {
> +    struct statdata stat;
> +    support_nscd_get_statistics (&stat);
> +    for (int i = 0; i < lastdb; ++i)
> +      {
> +        TEST_COMPARE (stat.dbs[i].nentries, 0);
> +        TEST_COMPARE (stat.dbs[i].poshit, 0);
> +        TEST_COMPARE (stat.dbs[i].neghit, 0);
> +        TEST_COMPARE (stat.dbs[i].posmiss, 0);
> +        TEST_COMPARE (stat.dbs[i].negmiss, 0);
> +      }
> +  }
> +
> +  puts ("info: first pass with nscd");
>     all_tests (NULL);
>   
> +  /* After running the tests, we have cache entries in all databases.
> +   The counters have been empirically determined.  They mostly check
> +   that nscd is actually used. */
> +  struct statdata stat_first_pass;
> +  support_nscd_get_statistics (&stat_first_pass);
> +  TEST_COMPARE (stat_first_pass.dbs[pwddb].nentries, 10);
> +  TEST_COMPARE (stat_first_pass.dbs[pwddb].posmiss, 4);
> +  TEST_COMPARE (stat_first_pass.dbs[pwddb].negmiss, 2);
> +  TEST_COMPARE (stat_first_pass.dbs[grpdb].nentries, 10);
> +  TEST_COMPARE (stat_first_pass.dbs[grpdb].posmiss, 5);
> +  TEST_COMPARE (stat_first_pass.dbs[grpdb].negmiss, 2);
> +  TEST_COMPARE (stat_first_pass.dbs[hstdb].nentries, 5);
> +  TEST_COMPARE (stat_first_pass.dbs[hstdb].posmiss, 5);
> +  TEST_COMPARE (stat_first_pass.dbs[hstdb].negmiss, 0);
> +  TEST_COMPARE (stat_first_pass.dbs[netgrdb].nentries, 25);
> +  TEST_COMPARE (stat_first_pass.dbs[netgrdb].posmiss, 25);
> +  TEST_COMPARE (stat_first_pass.dbs[netgrdb].negmiss, 0);
> +  TEST_COMPARE (stat_first_pass.dbs[servdb].nentries, 5);
> +  TEST_COMPARE (stat_first_pass.dbs[servdb].posmiss, 3);
> +  TEST_COMPARE (stat_first_pass.dbs[servdb].negmiss, 2);
> +
>     /* Retry from cache.  */
> +  puts ("info: second pass with nscd, with caching");
>     all_tests (NULL);
>   
> +  /* Retries should have used the cache and not increment the query
> +     counters.  This means that the shared mapping is used, not the
> +     socket query interface or the in-process nss_files service
> +     module.  */
> +  {
> +    struct statdata stat;
> +    support_nscd_get_statistics (&stat);
> +    for (int i = 0; i < lastdb; ++i)
> +      {
> +        TEST_COMPARE (stat.dbs[i].nentries, stat_first_pass.dbs[i].nentries);
> +        TEST_COMPARE (stat.dbs[i].poshit, stat_first_pass.dbs[i].poshit);
> +        TEST_COMPARE (stat.dbs[i].neghit, stat_first_pass.dbs[i].neghit);
> +        TEST_COMPARE (stat.dbs[i].posmiss, stat_first_pass.dbs[i].posmiss);
> +        TEST_COMPARE (stat.dbs[i].negmiss, stat_first_pass.dbs[i].negmiss);
> +      }
> +  }
> +
> +  puts ("info: pass with interleaved invalidation");
>     invalidate = support_nscd_invalidate;
>     all_tests (NULL);
>   
> diff --git a/support/nscd_test.h b/support/nscd_test.h
> index 60208098f4..95d9c1cd38 100644
> --- a/support/nscd_test.h
> +++ b/support/nscd_test.h
> @@ -36,4 +36,8 @@ void support_nscd_stop (void);
>      services).  */
>   void support_nscd_invalidate (const char *database);
>   
> +/* Fill in *STATS with data from the running nscd daemon.  */
> +struct statdata;
> +void support_nscd_get_statistics (struct statdata *stat);
> +
>   #endif /* SUPPORT_NSCD_TEST_H */
> diff --git a/support/support_nscd.c b/support/support_nscd.c
> index 2d7d503ad6..96d1e6b874 100644
> --- a/support/support_nscd.c
> +++ b/support/support_nscd.c
> @@ -18,20 +18,25 @@
>   
>   #include <support/nscd_test.h>
>   
> +#include <errno.h>
>   #include <libgen.h>
>   #include <signal.h>
>   #include <spawn.h>
>   #include <stdbool.h>
> +#include <stdio.h>
>   #include <stdlib.h>
>   #include <support/check.h>
>   #include <support/support.h>
>   #include <support/xspawn.h>
>   #include <support/xunistd.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
>   #include <sys/wait.h>
>   #include <unistd.h>
>   
>   #include <nscd/nscd.h>
>   #include <nscd/nscd-client.h>
> +#include <nscd/nscd-statdata.h>
>   
>   static pid_t nscd_pid;
>   
> @@ -113,3 +118,31 @@ support_nscd_invalidate (const char *database)
>     TEST_COMPARE (system (cmd), 0);
>     free (cmd);
>   }
> +
> +void
> +support_nscd_get_statistics (struct statdata *stat)
> +{
> +  struct sockaddr_un sun;
> +  sun.sun_family = AF_UNIX;
> +  strcpy (sun.sun_path, _PATH_NSCDSOCKET);
> +
> +  int sock = socket (PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
> +  if (sock < 0 || connect (sock, (struct sockaddr *) &sun, sizeof (sun)) < 0)
> +    FAIL_EXIT1 ("failed to open nscd socket: %m");
> +
> +  request_header req;
> +  req.version = NSCD_VERSION;
> +  req.type = GETSTAT;
> +  req.key_len = 0;
> +
> +  if (TEMP_FAILURE_RETRY (send (sock, &req, sizeof (req), MSG_NOSIGNAL))
> +   != sizeof (req))
> +    FAIL_EXIT1 ("failed to send request to nscd: %m");
> +
> +  ssize_t ret = TEMP_FAILURE_RETRY (recv (sock, stat, sizeof (*stat), 0));
> +
> +  errno = EMSGSIZE;
> +  if (ret != sizeof (*stat) || stat->version != STATDATA_VERSION_FULL)
> +    FAIL_EXIT1 ("failed to receive statistics from nscd: %m");
> +  close (sock);
> +}
  

Patch

diff --git a/nscd/tst-nscd-basic.c b/nscd/tst-nscd-basic.c
index 2ce1fece60..3dfde92c36 100644
--- a/nscd/tst-nscd-basic.c
+++ b/nscd/tst-nscd-basic.c
@@ -31,6 +31,8 @@ 
 #include <support/xmemstream.h>
 #include <support/xstdio.h>
 
+#include <nscd/nscd-statdata.h>
+
 /* Large GECOS string for the large-gecos user.  */
 static char *large_gecos;
 
@@ -452,17 +454,73 @@  do_test (void)
   /* First run the tests without nscd running, to check that the test
      expectations are correct.  Use a separate process to avoid
      caching nscd availability.  */
+  puts ("info: testing without nscd");
   support_isolate_in_subprocess (all_tests, NULL);
 
   support_nscd_copy_configuration ();
 
   support_nscd_start ();
 
+  /* Initially the query counters are all zero.  */
+  {
+    struct statdata stat;
+    support_nscd_get_statistics (&stat);
+    for (int i = 0; i < lastdb; ++i)
+      {
+        TEST_COMPARE (stat.dbs[i].nentries, 0);
+        TEST_COMPARE (stat.dbs[i].poshit, 0);
+        TEST_COMPARE (stat.dbs[i].neghit, 0);
+        TEST_COMPARE (stat.dbs[i].posmiss, 0);
+        TEST_COMPARE (stat.dbs[i].negmiss, 0);
+      }
+  }
+
+  puts ("info: first pass with nscd");
   all_tests (NULL);
 
+  /* After running the tests, we have cache entries in all databases.
+   The counters have been empirically determined.  They mostly check
+   that nscd is actually used. */
+  struct statdata stat_first_pass;
+  support_nscd_get_statistics (&stat_first_pass);
+  TEST_COMPARE (stat_first_pass.dbs[pwddb].nentries, 10);
+  TEST_COMPARE (stat_first_pass.dbs[pwddb].posmiss, 4);
+  TEST_COMPARE (stat_first_pass.dbs[pwddb].negmiss, 2);
+  TEST_COMPARE (stat_first_pass.dbs[grpdb].nentries, 10);
+  TEST_COMPARE (stat_first_pass.dbs[grpdb].posmiss, 5);
+  TEST_COMPARE (stat_first_pass.dbs[grpdb].negmiss, 2);
+  TEST_COMPARE (stat_first_pass.dbs[hstdb].nentries, 5);
+  TEST_COMPARE (stat_first_pass.dbs[hstdb].posmiss, 5);
+  TEST_COMPARE (stat_first_pass.dbs[hstdb].negmiss, 0);
+  TEST_COMPARE (stat_first_pass.dbs[netgrdb].nentries, 25);
+  TEST_COMPARE (stat_first_pass.dbs[netgrdb].posmiss, 25);
+  TEST_COMPARE (stat_first_pass.dbs[netgrdb].negmiss, 0);
+  TEST_COMPARE (stat_first_pass.dbs[servdb].nentries, 5);
+  TEST_COMPARE (stat_first_pass.dbs[servdb].posmiss, 3);
+  TEST_COMPARE (stat_first_pass.dbs[servdb].negmiss, 2);
+
   /* Retry from cache.  */
+  puts ("info: second pass with nscd, with caching");
   all_tests (NULL);
 
+  /* Retries should have used the cache and not increment the query
+     counters.  This means that the shared mapping is used, not the
+     socket query interface or the in-process nss_files service
+     module.  */
+  {
+    struct statdata stat;
+    support_nscd_get_statistics (&stat);
+    for (int i = 0; i < lastdb; ++i)
+      {
+        TEST_COMPARE (stat.dbs[i].nentries, stat_first_pass.dbs[i].nentries);
+        TEST_COMPARE (stat.dbs[i].poshit, stat_first_pass.dbs[i].poshit);
+        TEST_COMPARE (stat.dbs[i].neghit, stat_first_pass.dbs[i].neghit);
+        TEST_COMPARE (stat.dbs[i].posmiss, stat_first_pass.dbs[i].posmiss);
+        TEST_COMPARE (stat.dbs[i].negmiss, stat_first_pass.dbs[i].negmiss);
+      }
+  }
+
+  puts ("info: pass with interleaved invalidation");
   invalidate = support_nscd_invalidate;
   all_tests (NULL);
 
diff --git a/support/nscd_test.h b/support/nscd_test.h
index 60208098f4..95d9c1cd38 100644
--- a/support/nscd_test.h
+++ b/support/nscd_test.h
@@ -36,4 +36,8 @@  void support_nscd_stop (void);
    services).  */
 void support_nscd_invalidate (const char *database);
 
+/* Fill in *STATS with data from the running nscd daemon.  */
+struct statdata;
+void support_nscd_get_statistics (struct statdata *stat);
+
 #endif /* SUPPORT_NSCD_TEST_H */
diff --git a/support/support_nscd.c b/support/support_nscd.c
index 2d7d503ad6..96d1e6b874 100644
--- a/support/support_nscd.c
+++ b/support/support_nscd.c
@@ -18,20 +18,25 @@ 
 
 #include <support/nscd_test.h>
 
+#include <errno.h>
 #include <libgen.h>
 #include <signal.h>
 #include <spawn.h>
 #include <stdbool.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <support/check.h>
 #include <support/support.h>
 #include <support/xspawn.h>
 #include <support/xunistd.h>
+#include <sys/socket.h>
+#include <sys/un.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
 #include <nscd/nscd.h>
 #include <nscd/nscd-client.h>
+#include <nscd/nscd-statdata.h>
 
 static pid_t nscd_pid;
 
@@ -113,3 +118,31 @@  support_nscd_invalidate (const char *database)
   TEST_COMPARE (system (cmd), 0);
   free (cmd);
 }
+
+void
+support_nscd_get_statistics (struct statdata *stat)
+{
+  struct sockaddr_un sun;
+  sun.sun_family = AF_UNIX;
+  strcpy (sun.sun_path, _PATH_NSCDSOCKET);
+
+  int sock = socket (PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+  if (sock < 0 || connect (sock, (struct sockaddr *) &sun, sizeof (sun)) < 0)
+    FAIL_EXIT1 ("failed to open nscd socket: %m");
+
+  request_header req;
+  req.version = NSCD_VERSION;
+  req.type = GETSTAT;
+  req.key_len = 0;
+
+  if (TEMP_FAILURE_RETRY (send (sock, &req, sizeof (req), MSG_NOSIGNAL))
+   != sizeof (req))
+    FAIL_EXIT1 ("failed to send request to nscd: %m");
+
+  ssize_t ret = TEMP_FAILURE_RETRY (recv (sock, stat, sizeof (*stat), 0));
+
+  errno = EMSGSIZE;
+  if (ret != sizeof (*stat) || stat->version != STATDATA_VERSION_FULL)
+    FAIL_EXIT1 ("failed to receive statistics from nscd: %m");
+  close (sock);
+}