Hello all, We have an OpenLDAP instance proxying an active directory with back_meta and mr_passthru. We also have pcache on top, and as it do not support LDAP_MATCHING_RULE_IN_CHAIN, I looked about implementing it.
I found that we could retrieve cached results with pcacheQueryID, but I can't get why this is not used in the current code? I dit it this way (querying pcacheQueryID for previsouly cached results) and tests are successful I can now declare templates like this : pcacheAttrset 0 * pcacheTemplate (&(objectClass=)(member:1.2.840.113556.1.4.1941:=)) 0 900 Plesae see attached patch Did I miss something about the use of pcacheQueryID ? or anything else ? Do you think this could be added upstream? P.S.: Is there a reason mr_passthru is not included to OpenLDAP ? not even in contrib ? Thanks for reading
signature.asc
Description: This is a digitally signed message part.
--- servers/slapd/overlays/pcache.c.ori 2023-02-04 10:56:07.910235675 +0100 +++ servers/slapd/overlays/pcache.c 2023-02-04 10:57:14.319386931 +0100 @@ -53,6 +53,16 @@ */ #define PCACHE_MONITOR +// Not cacheable reasons +#define PCACHEMAXQUERIES_REACHED 0x1 +#define PCACHEATTRSONLY 0x2 +#define PCACHENOTEMPLATE 0x4 +#define PCACHENOATTRSET 0x8 + +// https://ldapwiki.com/wiki/LDAP_MATCHING_RULE_IN_CHAIN +#define LDAP_MATCHING_RULE_IN_CHAIN "1.2.840.113556.1.4.1941" + + /* query cache structs */ /* query */ @@ -512,7 +522,7 @@ int attr_cnt; int t_cnt = 0; struct berval bv; - char *p1, *p2; + char *p1, *p2, *p3; AttributeDescription *ad; AttributeName *attrs; @@ -532,9 +542,18 @@ p2 = strchr( p1, '=' ); if ( !p2 ) break; - if ( p2[-1] == '<' || p2[-1] == '>' ) p2--; - bv.bv_val = p1; - bv.bv_len = p2 - p1; + // Incase of extended operation + p3 = strchr( p1, ':' ); + if ( p3 && p3 < p2 ) { + // FIXME: Is this valid syntax : "member:1.2.840.113556.1.4.1941:>=3" ? + if ( p3[-1] == '<' || p3[-1] == '>' ) p3--; + bv.bv_val = p1; + bv.bv_len = p3 - p1; + } else { + if ( p2[-1] == '<' || p2[-1] == '>' ) p2--; + bv.bv_val = p1; + bv.bv_len = p2 - p1; + } ad = NULL; i = slap_bv2ad( &bv, &ad, text ); if ( i ) { @@ -1136,7 +1155,6 @@ return ret; } - static struct berval* merge_init_final(Operation *op, struct berval* init, struct berval* any, struct berval* final) @@ -1342,8 +1360,13 @@ case LDAP_FILTER_LE: mrule = fs->f_ava->aa_desc->ad_type->sat_ordering; break; + case LDAP_FILTER_EXT: + if (0 == strncmp(fi->f_mr_rule_text.bv_val, LDAP_MATCHING_RULE_IN_CHAIN, fi->f_mr_rule_text.bv_len)) { + mrule = fs->f_ava->aa_desc->ad_type->sat_equality; + } + break; default: - mrule = NULL; + mrule = NULL; } if (mrule) { const char *text; @@ -1394,6 +1417,7 @@ fi=fi->f_next; break; case LDAP_FILTER_EQUALITY: + case LDAP_FILTER_EXT: if (ret == 0) res = 1; fs=fs->f_next; @@ -1941,6 +1965,19 @@ fstr->bv_len += len; break; + case LDAP_FILTER_EXT: + // Concatenate attribute name + if ( f->f_mr_desc ) { + ad = f->f_mr_desc; + len = STRLENOF( "(::=)" ) + ad->ad_cname.bv_len + f->f_mr_rule_text.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s:%s:=)", ad->ad_cname.bv_val, f->f_mr_rule_text.bv_val ); + assert( ret == len ); + fstr->bv_len += len; + } else { + return -1; + } + break; + case LDAP_FILTER_AND: case LDAP_FILTER_OR: case LDAP_FILTER_NOT: { @@ -2955,17 +2992,22 @@ { slap_overinst *on = (slap_overinst *)op->o_bd->bd_info; cache_manager *cm = on->on_bi.bi_private; - query_manager* qm = cm->qm; + query_manager* qm = cm->qm; int i = -1; Query query; QueryTemplate *qtemp = NULL; - bindinfo *pbi = NULL; + bindinfo *pbi = NULL; - int attr_set = -1; - CachedQuery *answerable = NULL; - int cacheable = 0; + int attr_set = -1; + CachedQuery *answerable = NULL; + int cacheable = 0; + + char *fstr; + Filter *f; + + int ncachereason = 0; struct berval tempstr; @@ -3051,6 +3093,10 @@ if (answerable) break; } + if (cacheable == 0) + ncachereason |= PCACHENOTEMPLATE; + } else { + ncachereason |= PCACHENOATTRSET; } op->o_tmpfree( tempstr.bv_val, op->o_tmpmemctx ); } @@ -3098,7 +3144,16 @@ } } } + + /* Replace original query with query to cache ID */ + fstr = op->o_tmpalloc( sizeof("(pcacheQueryID=12345678-abcd-1234-abcd-123456789abc)")+1, op->o_tmpmemctx ); + sprintf(fstr, "(pcacheQueryID=%s)", answerable->q_uuid.bv_val); + Debug( pcache_debug, "Replace search filter with %s\n", fstr, 0, 0 ); + + f = str2filter(fstr); + op->oq_search.rs_filter = f; i = cm->db.bd_info->bi_op_search( op, rs ); + op->o_tmpfree( fstr, op->o_tmpmemctx ); } ldap_pvt_thread_rdwr_wunlock(&answerable->rwlock); /* locked by qtemp->qcfunc (query_containment) */ @@ -3112,11 +3167,14 @@ ldap_pvt_thread_mutex_lock(&cm->cache_mutex); if (cm->num_cached_queries >= cm->max_queries) { cacheable = 0; + ncachereason |= PCACHEMAXQUERIES_REACHED; } ldap_pvt_thread_mutex_unlock(&cm->cache_mutex); - if (op->ors_attrsonly) + if (op->ors_attrsonly) { cacheable = 0; + ncachereason |= PCACHEATTRSONLY; + } if (cacheable) { slap_callback *cb; @@ -3170,7 +3228,27 @@ } } else { - Debug( pcache_debug, "QUERY NOT CACHEABLE\n" ); + switch (ncachereason) { + case PCACHEMAXQUERIES_REACHED: + Debug( pcache_debug, "QUERY NOT CACHEABLE (max_entries reached)\n", + 0, 0, 0); + break; + case PCACHEATTRSONLY: + Debug( pcache_debug, "QUERY NOT CACHEABLE (attrs only)\n", + 0, 0, 0); + break; + case PCACHENOTEMPLATE: + Debug( pcache_debug, "QUERY NOT CACHEABLE (No template)\n", + 0, 0, 0); + break; + case PCACHENOATTRSET: + Debug( pcache_debug, "QUERY NOT CACHEABLE (No Attrset)\n", + 0, 0, 0); + break; + default: + Debug( pcache_debug, "QUERY NOT CACHEABLE (unkown reason)\n", + 0, 0, 0); + } } return SLAP_CB_CONTINUE;
