Honor the persisted GONE flag on reload of bans, and keep the

duplicate counters consistent on reloads.

Add a test case for this.

Add a section on bans and offline silos in the docs.

bin/varnishd/cache/cache_ban.c

@@ -634,7 +634,7 @@ static void
 ban_reload(const uint8_t *ban, unsigned len)
 {
 	struct ban *b, *b2;
-	int gone = 0;
+	int dup = 0;
 	double t0, t1, t2 = 9e99;
 
 	ASSERT_CLI();
@@ -651,11 +651,8 @@ ban_reload(const uint8_t *ban, unsigned len)
 			return;
 		if (t1 < t0)
 			break;
-		if (ban_equal(b->spec, ban)) {
-			gone |= BAN_F_GONE;
-			VSC_C_main->bans_dups++;
-			VSC_C_main->bans_gone++;
-		}
+		if (ban_equal(b->spec, ban))
+			dup = 1;
 	}
 
 	VSC_C_main->bans++;
@@ -666,11 +663,14 @@ ban_reload(const uint8_t *ban, unsigned len)
 	b2->spec = malloc(len);
 	AN(b2->spec);
 	memcpy(b2->spec, ban, len);
-	b2->flags |= gone;
 	if (ban[BANS_FLAGS] & BANS_FLAG_REQ) {
 		VSC_C_main->bans_req++;
 		b2->flags |= BAN_F_REQ;
 	}
+	if (dup)
+		VSC_C_main->bans_dups++;
+	if (dup || (ban[BANS_FLAGS] & BANS_FLAG_GONE))
+		ban_mark_gone(b2);
 	if (b == NULL)
 		VTAILQ_INSERT_TAIL(&ban_head, b2, list);
 	else

bin/varnishtest/tests/p00009.vtc

+varnishtest "Check that reloaded bans with gone flag are really gone on restart"
+
+shell "rm -f ${tmpdir}/_.per[12]"
+
+server s1 {
+	rxreq
+	txresp -hdr "x-foo: foo"
+
+	accept
+	rxreq
+	txresp -hdr "x-foo: bar"
+} -start
+
+varnish v1 \
+	-arg "-pfeature=+wait_silo" \
+	-arg "-pban_lurker_sleep=0" \
+	-storage "-sper1=persistent,${tmpdir}/_.per1,10m -sper2=persistent,${tmpdir}/_.per2,10m" \
+	-vcl+backend {
+	}
+varnish v1 -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.x-foo == "foo"
+} -run
+
+varnish v1 -cliok "ban req.url == /test"
+varnish v1 -cliok "ban req.url == /test"
+varnish v1 -cliok "ban.list"
+
+# Expect ban_magic plus the 2 we added
+varnish v1 -expect bans == 3
+
+# Expect 1 of the 2 added to be marked dup
+varnish v1 -expect bans_dups == 1
+
+# Expect ban_magic plus our 1 dup to be marked gone
+varnish v1 -expect bans_gone == 2
+
+# Restart 
+varnish v1 -stop
+varnish v1 -start
+varnish v1 -cliok "ban.list"
+
+# Check that our object is still there
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.x-foo == "foo"
+} -run
+
+# Expect our duplicate
+varnish v1 -expect bans_dups == 1
+# One more than before restart due to the new ban_magic
+varnish v1 -expect bans_gone == 3

doc/sphinx/include/storage_backends.rst

@@ -105,6 +105,11 @@ open at any given point in time. Full silos are *sealed*. When Varnish
 starts after a shutdown it will discard the content of any silo that
 isn't sealed.
 
+Note that taking persistent silos offline and at the same time using
+bans can cause problems. This because bans added while the silo was
+offline will not be applied to the silo when it reenters the cache,
+and can make previously banned objects reappear.
+
 Transient Storage
 -----------------