Re: [PATCH nf] netfilter: nft_quota: make nft_overquota() really over the quota

[Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>]

Hi,

On Tue, Apr 15, 2025 at 02:03:59PM +0000, Zhongqiu Duan wrote:
[...]
> They behave differently in the case of consumed bytes equal to quota.
> 
> From nft_quota:
> 
> 	overquota = nft_overquota(priv, pkt->skb);
> 	if (overquota ^ nft_quota_invert(priv))
> 		regs->verdict.code = NFT_BREAK;
> 
> The xt_quota compares skb length with remaining quota, but the nft_quota
> compares it with consumed bytes.
> 
> The xt_quota can match consumed bytes up to quota at maximum. But the
> nft_quota break match when consumed bytes equal to quota.
> 
> i.e., nft_quota match consumed bytes in [0, quota - 1], not [0, quota].

Thanks for explaining.

> Also note that after applying this patch, nft_quota obj will report when
> consumed bytes exceed the quota, but nfacct can report when consumed
> bytes are greater than or equal to the quota.
> 
> From nft_quota:
> 
> 	if (overquota &&
> 	    !test_and_set_bit(NFT_QUOTA_DEPLETED_BIT, &priv->flags))
> 		nft_obj_notify(...);
> 
> From nfacct:
> 
> 	if (now >= *quota &&
> 	    !test_and_set_bit(NFACCT_OVERQUOTA_BIT, &nfacct->flags)) {
> 		nfnl_overquota_report(net, nfacct);
> 	}
> 
> Should we report when quota is exhausted but not exceeded?

I think it is good if nfacct and nft_quota behave in the same way.

I'd suggest you collapse this patch.

Please, route this patch v2 through the nf-next tree.

Thanks.

diff --git a/net/netfilter/nft_quota.c b/net/netfilter/nft_quota.c
index 0bb43c723061..9fd6985f54c5 100644
--- a/net/netfilter/nft_quota.c
+++ b/net/netfilter/nft_quota.c
@@ -51,13 +51,15 @@ static void nft_quota_obj_eval(struct nft_object *obj,
 			       const struct nft_pktinfo *pkt)
 {
 	struct nft_quota *priv = nft_obj_data(obj);
+	u64 consumed = atomic64_add_return(pkt->skb->len, priv->consumed);
+	u64 quota = atomic64_read(&priv->quota);
 	bool overquota;
 
-	overquota = nft_overquota(priv, pkt->skb);
+	overquota = (consumed > quota);
 	if (overquota ^ nft_quota_invert(priv))
 		regs->verdict.code = NFT_BREAK;
 
-	if (overquota &&
+	if (consumed >= quota &&
 	    !test_and_set_bit(NFT_QUOTA_DEPLETED_BIT, &priv->flags))
 		nft_obj_notify(nft_net(pkt), obj->key.table, obj, 0, 0,
 			       NFT_MSG_NEWOBJ, 0, nft_pf(pkt), 0, GFP_ATOMIC);