On Wed, 2012-09-12 at 15:12 +0200, Michal Židek wrote:
On 09/12/2012 02:39 PM, Michal Židek wrote:
> Patch 1 - tools_util.h now provides signal_sssd function. It
> replaces send_sighup function from sss_debuglevel.c. It can
> be used to send any signal to monitor (used in patch 2).
>
> Patch 2 -
https://fedorahosted.org/sssd/ticket/1311
I have questions, nack until answered.
Comments inline:
From 17da45856fc0a723ae9d09431cb372c68a2ef07a Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzidek(a)redhat.com>
Date: Tue, 11 Sep 2012 18:44:52 +0200
Subject: [PATCH 2/2] sss_cache tool invalidates records in memory
cache.
---
src/responder/nss/nsssrv.c | 37 +++++++++++-
src/responder/nss/nsssrv_mmap_cache.c | 110
++++++++++++++++++++++++++++++++++
src/responder/nss/nsssrv_mmap_cache.h | 2 +
src/tools/sss_cache.c | 9 +++
4 files changed, 157 insertions(+), 1 deletion(-)
diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
index 64267e8..59f92c5 100644
--- a/src/responder/nss/nsssrv.c
+++ b/src/responder/nss/nsssrv.c
@@ -51,10 +51,13 @@
#define SHELL_REALLOC_INCREMENT 5
#define SHELL_REALLOC_MAX 50
+static int nss_logrotate(DBusMessage *message,
+ struct sbus_connection *conn);
+
struct sbus_method monitor_nss_methods[] = {
{ MON_CLI_METHOD_PING, monitor_common_pong },
{ MON_CLI_METHOD_RES_INIT, monitor_common_res_init },
- { MON_CLI_METHOD_ROTATE, responder_logrotate },
+ { MON_CLI_METHOD_ROTATE, nss_logrotate },
{ NULL, NULL }
};
@@ -66,6 +69,37 @@ struct sbus_interface monitor_nss_interface = {
NULL
};
+static int nss_logrotate(DBusMessage *message,
+ struct sbus_connection *conn)
+{
+ errno_t ret;
+ struct resp_ctx *rctx =
talloc_get_type(sbus_conn_get_private_data(conn),
+ struct resp_ctx);
+ struct nss_ctx *nctx = (struct nss_tx*)rctx->pvt_ctx;
+
+ ret = monitor_common_rotate_logs(rctx->cdb,
rctx->confdb_service_path);
+ if (ret != EOK) return ret;
Main questions:
the invalidation functions seem to be called unconditionally, what
prevent cache invalidation durint a normal log rotation ?
Shouldn't cache invalidation happen exclusively when sss_cache is asking
to drop caches ?
+ /* Invalidate memory caches if used */
+ if (nctx->pwd_mc_ctx != NULL) {
you do not need to check here ^^ you already check inside
sss_mmap_cache_validate if the input is null and return in that case.
+ ret = sss_mmap_cache_invalidate(nctx->pwd_mc_ctx);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("passwd mmap cache invalidation failed\n"));
+ }
+ }
+
+ if (nctx->grp_mc_ctx != NULL) {
same as above
+ ret = sss_mmap_cache_invalidate(nctx->grp_mc_ctx);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("group mmap cache invalidation failed\n"));
+ }
+ }
+
+ return monitor_common_pong(message, conn);
+}
static errno_t nss_get_etc_shells(TALLOC_CTX *mem_ctx, char
***_shells)
{
int i = 0;
@@ -266,6 +300,7 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
struct sss_cmd_table *nss_cmds;
struct be_conn *iter;
struct nss_ctx *nctx;
+ struct tevent_signal *tes;
int memcache_timeout;
int ret, max_retries;
int hret;
diff --git a/src/responder/nss/nsssrv_mmap_cache.c
b/src/responder/nss/nsssrv_mmap_cache.c
index 07498a9..0a87754 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -58,6 +58,8 @@ struct sss_mc_ctx {
uint32_t seed; /* pseudo-random seed to avoid collision
attacks */
time_t valid_time_slot; /* maximum time the entry is valid in
seconds */
+ int n_elem; /* max number of entries stored in mmap
cache */
+
void *mmap_base; /* base address of mmap */
size_t mmap_size; /* total size of mmap */
@@ -648,6 +650,9 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx,
const char *name,
/* We can use MC_ALIGN64 for this */
n_elem = MC_ALIGN64(n_elem);
+ /* store n_elem into context, so it can be used later for
invalidation */
+ mc_ctx->n_elem = n_elem;
+
/* hash table is double the size because it will store both
forward and
* reverse keys (name/uid, name/gid, ..) */
mc_ctx->ht_size = MC_HT_SIZE(n_elem * 2);
@@ -725,3 +730,108 @@ done:
return ret;
}
Nack:
I do not like the following function as it is largely a copy/paste of
sss_mmap_cache_init.
Why donp;t you simply pass a pointer to struct sss_mc_ctx *mc_ctx
release it, and then simply call sss_mmap_cache_init() from scratch ?
Also n_elem is currently hardcoded, so saving it is not really useful,
however I would also *really* not save it as on cache invalidation you
will want to go and see if the configuration also changed and get the
new number of elements.
This is a perfect time to do that because you are killing the old file
and creating a new one so changing size during this operation is
actually a good idea (if needed).
Same for memcache_timeout parameters (which are already parametrized and
can be fetched from CONFDB_NSS_CONF_ENTRY / CONFDB_MEMCACHE_TIMEOUT
+errno_t sss_mmap_cache_invalidate(struct sss_mc_ctx *mc_ctx)
+{
+ unsigned int rseed;
+ int payload;
+ errno_t ret;
+
+ if (mc_ctx == NULL) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("Memory cache not used. Nothing to do here.\n"));
+ return EOK;
+ }
+ switch (mc_ctx->type) {
+ case SSS_MC_PASSWD:
+ payload = SSS_AVG_PASSWD_PAYLOAD;
+ break;
+ case SSS_MC_GROUP:
+ payload = SSS_AVG_GROUP_PAYLOAD;
+ break;
+ default:
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Memory cache corrupted.\n"));
+ return EINVAL;
+ }
+
+ /* close the old file descriptor */
+ ret = close(mc_ctx->fd);
+ if (ret != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to close file descriptor:
%d"
+ ": %s", ret, strerror(ret)));
+ return ret;
+ }
+ mc_ctx->fd = -1;
+
+ /* hash table is double the size because it will store both
forward and
+ * reverse keys (name/uid, name/gid, ..) */
+ mc_ctx->ht_size = MC_HT_SIZE(mc_ctx->n_elem * 2);
+ mc_ctx->dt_size = MC_DT_SIZE(mc_ctx->n_elem, payload);
+ mc_ctx->ft_size = MC_FT_SIZE(mc_ctx->n_elem);
+ mc_ctx->mmap_size = MC_HEADER_SIZE +
+ MC_ALIGN64(mc_ctx->dt_size) +
+ MC_ALIGN64(mc_ctx->ft_size) +
+ MC_ALIGN64(mc_ctx->ht_size);
+
+
+ /* create a new file */
+ ret = sss_mc_create_file(mc_ctx);
+ if (ret) {
+ goto done;
+ }
+
+ ret = ftruncate(mc_ctx->fd, mc_ctx->mmap_size);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to resize file %s: %d(%
s)\n",
+ mc_ctx->file, ret,
strerror(ret)));
+ goto done;
+ }
+
+ /* map the file to memory */
+ mc_ctx->mmap_base = mmap(NULL, mc_ctx->mmap_size,
+ PROT_READ | PROT_WRITE,
+ MAP_SHARED, mc_ctx->fd, 0);
+ if (mc_ctx->mmap_base == MAP_FAILED) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to mmap file %s(%ld): %
d(%s)\n",
+ mc_ctx->file, mc_ctx->mmap_size,
+ ret, strerror(ret)));
+ goto done;
+ }
+
+ mc_ctx->data_table = MC_PTR_ADD(mc_ctx->mmap_base,
MC_HEADER_SIZE);
+ mc_ctx->free_table = MC_PTR_ADD(mc_ctx->data_table,
+ MC_ALIGN64(mc_ctx->dt_size));
+ mc_ctx->hash_table = MC_PTR_ADD(mc_ctx->free_table,
+ MC_ALIGN64(mc_ctx->ft_size));
+
+ memset(mc_ctx->data_table, 0x00, mc_ctx->dt_size);
+ memset(mc_ctx->free_table, 0x00, mc_ctx->ft_size);
+ memset(mc_ctx->hash_table, 0xff, mc_ctx->ht_size);
+
+ /* generate a pseudo-random seed.
+ * Needed to fend off dictionary based collision attacks */
+ rseed = time(NULL) * getpid();
+ mc_ctx->seed = rand_r(&rseed);
+
+ sss_mc_header_update(mc_ctx, SSS_MC_HEADER_ALIVE);
+
+ ret = EOK;
+
+done:
+ if (ret) {
+ if (mc_ctx->mmap_base && mc_ctx->mmap_base != MAP_FAILED) {
+ munmap(mc_ctx->mmap_base, mc_ctx->mmap_size);
+ }
+ if (mc_ctx->fd != -1) {
+ close(mc_ctx->fd);
+ ret = unlink(mc_ctx->file);
+ if (ret == -1) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Failed to rm mmap file %s: %d(%s)\n",
+ mc_ctx->file, ret, strerror(ret)));
+ }
+ }
+ }
+ return ret;
+}
diff --git a/src/responder/nss/nsssrv_mmap_cache.h
b/src/responder/nss/nsssrv_mmap_cache.h
index 81241b2..3a808e6 100644
--- a/src/responder/nss/nsssrv_mmap_cache.h
+++ b/src/responder/nss/nsssrv_mmap_cache.h
@@ -48,4 +48,6 @@ errno_t sss_mmap_cache_gr_store(struct sss_mc_ctx
*mcc,
gid_t gid, size_t memnum,
char *membuf, size_t memsize);
+errno_t sss_mmap_cache_invalidate(struct sss_mc_ctx*);
+
#endif /* _NSSSRV_MMAP_CACHE_H_ */
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 950ff1c..63f31ad 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -141,6 +141,15 @@ int main(int argc, const char *argv[])
ERROR("No cache object matched the specified search\n");
ret = ENOENT;
goto done;
+ } else {
+ /*Local cache changed -> signal monitor to invalidate
fastcache */
+ DEBUG(SSSDBG_TRACE_FUNC, ("Sending SIGHUP to
monitor.\n"));
+ ret = signal_sssd(SIGHUP);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Failed to send SIGHUP to monitor.\n"));
+ goto done;
+ }
}
}
--
1.7.11.2
--
Simo Sorce * Red Hat, Inc * New York