[SSSD] [PATCHES] sss_cache tool invalidates records in memory cache
Simo Sorce
simo at redhat.com
Thu Sep 13 17:59:33 UTC 2012
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 at 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
More information about the sssd-devel
mailing list