Quotas: carry what was used when a rule counts differently

model.QuotaCarry says what becomes of a quota's use when its rule
changes how it counts; the firewall's apply follows it, comparing with
the rules in force: per address to a total starts at the addresses' sum,
wider groups add up their addresses, and what can't be carried (a total
to per address, per source to per destination, finer groups) starts
again, with the engine warning quota_restarts on that change. Before,
per source to per destination kept the sources' use as destinations',
and regrouping left old entries behind.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -87,6 +87,12 @@ All notable changes to Housecarl are documented here. The format follows
 
 ### Fixed
 
+- Changing how a quota counts lost or muddled what it used: per address
+  to a total started at nothing, per source to per destination kept the
+  sources' use as if it were the destinations', and other groups left
+  the old entries behind. Now a total starts at the addresses' sum,
+  wider groups add up their addresses, and what can't be carried starts
+  again with a warning (`quota_restarts`) on the change.
 - `hc stats top devices` listed each device twice (once for what it
   received, once for what it sent); it is one row with both again.
 - `hc rule add --before/--after/--first` made two changes (create at the

docs/rules.md

@@ -215,6 +215,20 @@ A per-address quota counts an address: a device that gets a new one
 starts again. With Steward's reservations IPv4 addresses stay; IPv6 is
 grouped by /64 (below).
 
+Changing the amount, `over` or the resets keeps what was used. Changing
+how the quota counts carries what can be carried:
+
+| Change | What was used |
+|---|---|
+| per address to one total | the addresses' sum |
+| to wider groups (IPv6 /64 to /56) | added up per new group |
+| a total to per address | starts again: a total can't be split |
+| per source to per destination (or back) | starts again |
+| to finer groups (/64 to /128) | starts again: a group can't be split |
+
+A change that starts a quota again says so (warning `quota_restarts`),
+when it is staged for review and in `hc` (also with `-n`).
+
 **Rules with a quota or a byte rate count every packet.** The built-in
 that accepts established connections comes before your rules, so a rule
 there only sees each connection's first packet. Rules that count traffic

docs/validation.md

@@ -97,6 +97,7 @@ These apply to rules, source NAT and redirects.
 | `rule_group` | error | a grouping prefix outside 1-32 (IPv4) or 1-128 (IPv6) |
 | `rule_traffic_accept` | error | a rule with a quota or a byte rate that accepts: it is checked before established connections are accepted, so it can only drop, reject or log |
 | `rule_every_packet_accept` | error | a rule checked against every packet (`every_packet`) that accepts: it comes before established connections are accepted, so it can only drop, reject or log |
+| `quota_restarts` | warning | a change makes a rule count its quota so that what it used can't carry over (a total to per address, per source to per destination, finer groups): it starts again. Only on the change that does it |
 | `flowtable_traffic` | warning | a forward rule checked against every packet (a quota, a byte rate, `every_packet`) while the flowtable is on: it only sees each connection's first packets |
 | `reject_with` | error | `tcp-reset` on a rule that matches more than TCP, or with an action other than reject |
 | `log_prefix` | error | a log prefix over 100 characters |

internal/engine/engine.go

@@ -191,6 +191,7 @@ func (e *Engine) commit(ctx context.Context, p *auth.Principal, description stri
 	// Only report warnings the change brings; old ones are on the status
 	// page.
 	res.Warnings = newProblems(validate.Check(e.state, e.env()), problems)
+	res.Warnings = append(res.Warnings, quotaRestarts(e.state, next)...)
 	if e.Firewall != nil {
 		if err := e.Firewall.Check(ctx, next); err != nil {
 			return res, &Error{Kind: KindFirewall, Message: "nftables refused the ruleset: " + err.Error()}
@@ -230,6 +231,23 @@ func (e *Engine) commit(ctx context.Context, p *auth.Principal, description stri
 	return res, nil
 }
 
+// quotaRestarts warns of the rules whose quota starts again because they
+// now count it in a way what they used can't be carried to.
+func quotaRestarts(before, after *model.State) []validate.Problem {
+	was := map[int64]*model.Rule{}
+	for _, r := range before.Rules {
+		was[r.ID] = r
+	}
+	var out []validate.Problem
+	for _, r := range after.Rules {
+		if carry, why := model.QuotaCarry(was[r.ID], r); carry == model.QuotaRestarts {
+			out = append(out, validate.Problem{Severity: validate.Warning, Code: "quota_restarts", Object: r.Label(),
+				Message: "its quota starts again: " + why})
+		}
+	}
+	return out
+}
+
 func newProblems(before, after []validate.Problem) []validate.Problem {
 	seen := map[string]bool{}
 	for _, p := range before {

internal/engine/engine_test.go

@@ -173,3 +173,49 @@ func TestPositionsStayStableUntilCrowded(t *testing.T) {
 		t.Errorf("order %v", order)
 	}
 }
+
+// TestQuotaRestartsWarns: a change that counts a quota so what it used
+// can't carry over warns; one that can (to a total, wider groups) doesn't.
+func TestQuotaRestartsWarns(t *testing.T) {
+	e := newEngine(t)
+	ctx := context.Background()
+	var id int64
+	if _, err := e.Apply(ctx, admin, "", Options{}, func(st *model.State, newID func() int64) error {
+		id = newID()
+		st.Rules = append(st.Rules, &model.Rule{ID: id, Chain: "forward", Action: "drop", Quota: &model.Quota{Bytes: 1 << 30, Over: true, PerSource: true}})
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+	change := func(f func(r *model.Rule)) []string {
+		res, err := e.Apply(ctx, admin, "", Options{DryRun: true}, func(st *model.State, _ func() int64) error {
+			f(st.ByID(model.KindRule, id).(*model.Rule))
+			return nil
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		var codes []string
+		for _, w := range res.Warnings {
+			if w.Code == "quota_restarts" {
+				codes = append(codes, w.Message)
+			}
+		}
+		return codes
+	}
+	if w := change(func(r *model.Rule) { r.Quota.PerSource, r.Quota.PerDestination = false, true }); len(w) != 1 || !strings.Contains(w[0], "per source can't be counted per destination") {
+		t.Errorf("per source to per destination: %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.Quota.PerSource = false }); len(w) != 0 {
+		t.Errorf("to a total (summed): %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.GroupV6 = 56 }); len(w) != 0 {
+		t.Errorf("wider groups: %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.GroupV6 = 128 }); len(w) != 1 {
+		t.Errorf("finer groups: %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.Quota.Bytes = 2 << 30 }); len(w) != 0 {
+		t.Errorf("another amount: %v", w)
+	}
+}

internal/model/objects.go

@@ -274,6 +274,45 @@ func (r *Rule) Groups() (v4, v6 int) {
 	return v4, v6
 }
 
+// What becomes of what a rule's quota used when the rule changes
+// (QuotaCarry).
+const (
+	QuotaKept      = ""          // counted as before: carried as it is
+	QuotaSummed    = "summed"    // per address to a total: the addresses' sum
+	QuotaRegrouped = "regrouped" // per address in wider groups: added up per group
+	QuotaRestarts  = "restarts"  // can't be carried: starts again
+)
+
+// QuotaCarry says what becomes of what old's quota used when the rule
+// becomes new (both the same rule, with quotas), and for QuotaRestarts
+// why. The amount, over and the resets don't matter: only how it counts.
+func QuotaCarry(old, new *Rule) (carry, why string) {
+	if old == nil || new == nil || old.Quota == nil || new.Quota == nil {
+		return QuotaKept, ""
+	}
+	was, is := old.Quota.PerAddress(), new.Quota.PerAddress()
+	words := map[string]string{"saddr": "per source", "daddr": "per destination", "": "in total"}
+	switch {
+	case was == "" && is == "":
+		return QuotaKept, ""
+	case is == "":
+		return QuotaSummed, ""
+	case was == "":
+		return QuotaRestarts, "a total can't be split by address, so each address starts at nothing"
+	case was != is:
+		return QuotaRestarts, "what was counted " + words[was] + " can't be counted " + words[is]
+	}
+	o4, o6 := old.Groups()
+	n4, n6 := new.Groups()
+	switch {
+	case o4 == n4 && o6 == n6:
+		return QuotaKept, ""
+	case n4 > o4 || n6 > o6:
+		return QuotaRestarts, "addresses are grouped more finely now, and a group's use can't be split"
+	}
+	return QuotaRegrouped, ""
+}
+
 // PortForward publishes a service on an inside host: incoming connections
 // to a public address and port are sent on (DNAT) to To[:ToPort], let
 // through the forward chain, and, with Hairpin, also work from inside.

internal/model/quota_test.go

@@ -0,0 +1,26 @@
+package model
+
+import "testing"
+
+func TestQuotaCarry(t *testing.T) {
+	rule := func(per string, v4, v6 int) *Rule {
+		return &Rule{GroupV4: v4, GroupV6: v6, Quota: &Quota{Bytes: 1, PerSource: per == "src", PerDestination: per == "dst"}}
+	}
+	for _, c := range []struct {
+		old, new *Rule
+		want     string
+	}{
+		{rule("", 0, 0), rule("", 0, 0), QuotaKept},
+		{rule("src", 0, 0), rule("src", 32, 64), QuotaKept}, // the defaults, written out
+		{rule("src", 0, 0), rule("", 0, 0), QuotaSummed},
+		{rule("", 0, 0), rule("src", 0, 0), QuotaRestarts},
+		{rule("src", 0, 0), rule("dst", 0, 0), QuotaRestarts},
+		{rule("src", 0, 0), rule("src", 24, 56), QuotaRegrouped},
+		{rule("src", 0, 0), rule("src", 24, 128), QuotaRestarts},
+		{nil, rule("src", 0, 0), QuotaKept},
+	} {
+		if got, _ := QuotaCarry(c.old, c.new); got != c.want {
+			t.Errorf("%+v -> %+v: %q, want %q", c.old, c.new, got, c.want)
+		}
+	}
+}

internal/nft/nft.go

@@ -15,6 +15,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"net/netip"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -424,6 +425,9 @@ type Firewall struct {
 	restore *QuotaUsage
 	// adjust are quota changes the next apply makes (AdjustQuota).
 	adjust []QuotaAdjust
+	// quotaRules are the rules with a quota in the ruleset in force, by
+	// ID: what their usage was counted by (nil before the first apply).
+	quotaRules map[int64]*model.Rule
 	// flowDevs are the flowtable's devices in the ruleset in force, by
 	// name, with their interface index then (0: left out, missing).
 	flowDevs map[string]int
@@ -510,6 +514,73 @@ func (u *QuotaUsage) apply(adj []QuotaAdjust) {
 	}
 }
 
+// carry keeps what quotas used when their rules now count differently
+// (model.QuotaCarry): per address to a total, the sum; to wider groups,
+// the groups' sums; what can't be carried starts again. was are the
+// rules in force before (nil: nothing to compare with).
+func (u *QuotaUsage) carry(was map[int64]*model.Rule, st *model.State) {
+	for _, r := range st.Rules {
+		old := was[r.ID]
+		carry, _ := model.QuotaCarry(old, r)
+		if carry == model.QuotaKept {
+			continue
+		}
+		name := render.QuotaName(r.ID)
+		sets := []string{name + "_v4", name + "_v6"}
+		switch carry {
+		case model.QuotaSummed:
+			var sum int64
+			for _, set := range sets {
+				for _, e := range u.QuotaSets[set] {
+					sum += e.Used
+				}
+			}
+			if u.Quotas == nil {
+				u.Quotas = map[string]int64{}
+			}
+			u.Quotas[name] = sum
+		case model.QuotaRegrouped:
+			v4, v6 := r.Groups()
+			for _, set := range sets {
+				byKey := map[string]int64{}
+				var order []string
+				for _, e := range u.QuotaSets[set] {
+					a, err := netip.ParseAddr(e.Addr)
+					if err != nil {
+						continue
+					}
+					bits := v4
+					if a.Is6() {
+						bits = v6
+					}
+					p, _ := a.Prefix(bits)
+					k := p.Addr().String()
+					if _, ok := byKey[k]; !ok {
+						order = append(order, k)
+					}
+					byKey[k] += e.Used
+				}
+				if len(order) == 0 {
+					continue
+				}
+				var es []render.Element
+				for _, k := range order {
+					es = append(es, render.Element{Addr: k, Used: byKey[k]})
+				}
+				u.QuotaSets[set] = es
+			}
+			continue
+		}
+		// What was counted the other way goes: a total, or the entries.
+		if carry != model.QuotaSummed {
+			delete(u.Quotas, name)
+		}
+		for _, set := range sets {
+			delete(u.QuotaSets, set)
+		}
+	}
+}
+
 // ResetQuota starts a rule's quota again: the named quota, or the
 // per-address quota sets (both families).
 func (f *Firewall) ResetQuota(ctx context.Context, ruleID int64, perAddress bool) error {
@@ -579,6 +650,7 @@ func (f *Firewall) Apply(ctx context.Context, st *model.State) error {
 		usage = old.QuotaUsage
 	}
 	usage.merge(f.restore)
+	usage.carry(f.quotaRules, st)
 	usage.apply(f.adjust)
 	f.adjust = nil
 	opt.Quotas, opt.QuotaSets = usage.Quotas, usage.QuotaSets
@@ -589,6 +661,14 @@ func (f *Firewall) Apply(ctx context.Context, st *model.State) error {
 		return err
 	}
 	f.restore = nil
+	f.quotaRules = map[int64]*model.Rule{}
+	for _, r := range st.Rules {
+		if r.Quota != nil {
+			c, q := *r, *r.Quota // only the quota and the grouping are read
+			c.Quota = &q
+			f.quotaRules[r.ID] = &c
+		}
+	}
 	f.flowDevs = map[string]int{}
 	for _, d := range st.Settings.FlowtableDevices {
 		if opt.Interfaces == nil || opt.Interfaces[d] {

internal/nft/nft_test.go

@@ -197,6 +197,21 @@ func TestQuotasInContainer(t *testing.T) {
 	if t2, d2 := used(); t2 > total-1000 || t2 < total-2000 || d2 != 0 {
 		t.Errorf("after giving back: total %d (was %d, gave 1000), per destination %d (was %d, reset)", t2, total, d2, dev)
 	}
+	// Counted in total now: what the addresses used carries over.
+	ping("127.0.0.3")
+	_, dev = used()
+	st.Rules[1].Quota.PerDestination = false
+	if err := fw.Apply(ctx, st); err != nil {
+		t.Fatal(err)
+	}
+	tb, _ := fw.Table(ctx)
+	if tb.Quotas["quota_r2"] < dev || dev == 0 {
+		t.Errorf("per destination to a total: %d, was %d", tb.Quotas["quota_r2"], dev)
+	}
+	st.Rules[1].Quota.PerDestination = true
+	if err := fw.Apply(ctx, st); err != nil {
+		t.Fatal(err)
+	}
 	// Reset by hand.
 	if err := fw.ResetQuota(ctx, 1, false); err != nil {
 		t.Fatal(err)
@@ -211,7 +226,7 @@ func TestQuotasInContainer(t *testing.T) {
 	// saved usage comes back.
 	ping("127.0.0.2")
 	ping("127.0.0.3")
-	tb, _ := fw.Table(ctx)
+	tb, _ = fw.Table(ctx)
 	saved := tb.QuotaUsage
 	exec.Command("docker", "exec", name, "nft", "delete", "table", "inet", "housecarl").Run()
 	fw.RestoreQuotas(&saved)
@@ -240,6 +255,45 @@ func TestQuotaAdjust(t *testing.T) {
 	}
 }
 
+// TestQuotaCarry: usage follows a rule counting its quota differently.
+func TestQuotaCarry(t *testing.T) {
+	perSource := &model.Rule{ID: 2, Quota: &model.Quota{Bytes: 1 << 30, PerSource: true}}
+	usage := func() QuotaUsage {
+		return QuotaUsage{Quotas: map[string]int64{}, QuotaSets: map[string][]render.Element{
+			"quota_r2_v4": {{Addr: "10.0.0.1", Used: 100}, {Addr: "10.0.0.2", Used: 50}},
+			"quota_r2_v6": {{Addr: "2001:db8:0:1::", Used: 7}, {Addr: "2001:db8:0:2::", Used: 3}}}}
+	}
+	was := map[int64]*model.Rule{2: perSource}
+	with := func(f func(r *model.Rule)) *model.State {
+		r := *perSource
+		q := *perSource.Quota
+		r.Quota = &q
+		f(&r)
+		return &model.State{Rules: []*model.Rule{&r}}
+	}
+	u := usage()
+	u.carry(was, with(func(r *model.Rule) { r.Quota.PerSource = false }))
+	if u.Quotas["quota_r2"] != 160 || len(u.QuotaSets["quota_r2_v4"]) != 0 {
+		t.Errorf("to a total: %+v", u)
+	}
+	u = usage()
+	u.carry(was, with(func(r *model.Rule) { r.GroupV4, r.GroupV6 = 24, 48 }))
+	want := map[string][]render.Element{"quota_r2_v4": {{Addr: "10.0.0.0", Used: 150}}, "quota_r2_v6": {{Addr: "2001:db8::", Used: 10}}}
+	if !reflect.DeepEqual(u.QuotaSets, want) {
+		t.Errorf("wider groups: %+v", u.QuotaSets)
+	}
+	u = usage()
+	u.carry(was, with(func(r *model.Rule) { r.Quota.PerSource, r.Quota.PerDestination = false, true }))
+	if len(u.QuotaSets) != 0 {
+		t.Errorf("per destination now: %+v", u.QuotaSets)
+	}
+	u = usage()
+	u.carry(nil, with(func(r *model.Rule) { r.Quota.PerSource = false }))
+	if len(u.QuotaSets["quota_r2_v4"]) != 2 || u.Quotas["quota_r2"] != 0 {
+		t.Errorf("nothing to compare with (the first apply): %+v", u)
+	}
+}
+
 // TestDeviceCountsInContainer: the per-device counters are read back and
 // survive a reload.
 func TestDeviceCountsInContainer(t *testing.T) {

scripts/e2e.sh

@@ -342,6 +342,15 @@ run "the device is back" 0 send 1000
 run "the reset is audited" 0 H audit --action quota.
 contains "quota.reset"
 contains "quota.give"
+# Counted in total now: what the device used carries over; per
+# destination can't take it, and says so.
+run "within the quota" 0 send 300000
+run "to one total" 0 H rule update "$QR" --quota 1mbytes,over
+not_contains "quota_restarts"
+run "the total carried over" 0 H quotas
+contains "e2e quota)  *[0-9.]* KB"
+run "per destination starts again" 0 H rule update "$QR" --quota 1mbytes,over,per-destination
+contains "quota_restarts"
 run "the quota rule goes" 0 H rule delete -y "$QR"
 
 # ---- the zone matrix, of the imported ruleset