- Fix #1032: The size of subnet_msg_cache calculation mistake cause

memory usage increased beyond expectations.

doc/Changelog

@@ -1,5 +1,7 @@
 27 March 2024: Wouter
 	- Fix name of unit test for subnet cache response.
+	- Fix #1032: The size of subnet_msg_cache calculation mistake cause
+	  memory usage increased beyond expectations.
 
 25 March 2024: Yorgos
 	- Merge #831 from Pierre4012: Improve Windows NSIS installer

edns-subnet/subnetmod.c

@@ -310,9 +310,18 @@ delfunc(void *envptr, void *elemptr) {
 static size_t
 sizefunc(void *elemptr) {
 	struct reply_info *elem  = (struct reply_info *)elemptr;
-	return sizeof (struct reply_info) - sizeof (struct rrset_ref)
+	size_t s = sizeof (struct reply_info) - sizeof (struct rrset_ref)
 		+ elem->rrset_count * sizeof (struct rrset_ref)
 		+ elem->rrset_count * sizeof (struct ub_packed_rrset_key *);
+	size_t i;
+	for (i = 0; i < elem->rrset_count; i++) {
+		struct ub_packed_rrset_key *key = elem->rrsets[i];
+		struct packed_rrset_data *data = key->entry.data;
+		s += ub_rrset_sizefunc(key, data);
+	}
+	if(elem->reason_bogus_str)
+		s += strlen(elem->reason_bogus_str)+1;
+	return s;
 }
 
 /**
@@ -352,7 +361,7 @@ update_cache(struct module_qstate *qstate, int id)
 	struct slabhash *subnet_msg_cache = sne->subnet_msg_cache;
 	struct ecs_data *edns = &sq->ecs_client_in;
 	size_t i;
-	int only_match_scope_zero;
+	int only_match_scope_zero, diff_size;
 
 	/* We already calculated hash upon lookup (lookup_and_reply) if we were
 	 * allowed to look in the ECS cache */
@@ -417,14 +426,19 @@ update_cache(struct module_qstate *qstate, int id)
 	if(edns->subnet_source_mask == 0 && edns->subnet_scope_mask == 0)
 		only_match_scope_zero = 1;
 	else only_match_scope_zero = 0;
+	diff_size = (int)tree->size_bytes;
 	addrtree_insert(tree, (addrkey_t*)edns->subnet_addr, 
 		edns->subnet_source_mask, sq->max_scope, rep,
 		rep->ttl, *qstate->env->now, only_match_scope_zero);
+	diff_size = (int)tree->size_bytes - diff_size;
 
 	lock_rw_unlock(&lru_entry->lock);
 	if (need_to_insert) {
 		slabhash_insert(subnet_msg_cache, h, lru_entry, lru_entry->data,
 			NULL);
+	} else {
+		slabhash_update_space_used(subnet_msg_cache, h, NULL,
+			diff_size);
 	}
 }
 

util/storage/lruhash.c

@@ -528,6 +528,38 @@ lruhash_setmarkdel(struct lruhash* table, lruhash_markdelfunc_type md)
 	lock_quick_unlock(&table->lock);
 }
 
+void
+lruhash_update_space_used(struct lruhash* table, void* cb_arg, int diff_size)
+{
+	struct lruhash_entry *reclaimlist = NULL;
+
+	fptr_ok(fptr_whitelist_hash_sizefunc(table->sizefunc));
+	fptr_ok(fptr_whitelist_hash_delkeyfunc(table->delkeyfunc));
+	fptr_ok(fptr_whitelist_hash_deldatafunc(table->deldatafunc));
+	fptr_ok(fptr_whitelist_hash_markdelfunc(table->markdelfunc));
+
+	if(cb_arg == NULL) cb_arg = table->cb_arg;
+
+	/* find bin */
+	lock_quick_lock(&table->lock);
+
+	table->space_used = (size_t)((int)table->space_used + diff_size);
+
+	if(table->space_used > table->space_max)
+		reclaim_space(table, &reclaimlist);
+
+	lock_quick_unlock(&table->lock);
+
+	/* finish reclaim if any (outside of critical region) */
+	while(reclaimlist) {
+		struct lruhash_entry* n = reclaimlist->overflow_next;
+		void* d = reclaimlist->data;
+		(*table->delkeyfunc)(reclaimlist->key, cb_arg);
+		(*table->deldatafunc)(d, cb_arg);
+		reclaimlist = n;
+	}
+}
+
 void 
 lruhash_traverse(struct lruhash* h, int wr, 
 	void (*func)(struct lruhash_entry*, void*), void* arg)

util/storage/lruhash.h

@@ -303,6 +303,17 @@ void lru_touch(struct lruhash* table, struct lruhash_entry* entry);
  */
 void lruhash_setmarkdel(struct lruhash* table, lruhash_markdelfunc_type md);
 
+/**
+ * Update the size of an element in the hashtable.
+ *
+ * @param table: hash table.
+ * @param cb_override: if not NULL overrides the cb_arg for deletefunc.
+ * @param diff_size: difference in size to the hash table storage.
+ * 	This is newsize - oldsize, a positive number uses more space.
+ */
+void lruhash_update_space_used(struct lruhash* table, void* cb_override,
+	int diff_size);
+
 /************************* getdns functions ************************/
 /*** these are used by getdns only and not by unbound. ***/
 

util/storage/slabhash.c

@@ -166,6 +166,13 @@ int slabhash_is_size(struct slabhash* sl, size_t size, size_t slabs)
 	return 0;
 }
 
+void slabhash_update_space_used(struct slabhash* sl, hashvalue_type hash,
+	void* cb_arg, int diff_size)
+{
+	lruhash_update_space_used(sl->array[slab_idx(sl, hash)], cb_arg,
+		diff_size);
+}
+
 size_t slabhash_get_mem(struct slabhash* sl)
 {	
 	size_t i, total = sizeof(*sl);

util/storage/slabhash.h

@@ -161,6 +161,18 @@ size_t slabhash_get_size(struct slabhash* table);
  */
 int slabhash_is_size(struct slabhash* table, size_t size, size_t slabs);
 
+/**
+ * Update the size of an element in the hashtable, uses
+ * lruhash_update_space_used.
+ *
+ * @param table: hash table.
+ * @param hash: hash value. User calculates the hash.
+ * @param cb_override: if not NULL overrides the cb_arg for deletefunc.
+ * @param diff_size: difference in size to the hash table storage.
+ */
+void slabhash_update_space_used(struct slabhash* table, hashvalue_type hash,
+	void* cb_override, int diff_size);
+
 /**
  * Retrieve slab hash current memory use.
  * @param table: hash table.