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

[Zhongqiu Duan <dzq.aishenghu0@gmail.com>]

On Thu, Apr 17, 2025 at 01:21:58PM +0200, Pablo Neira Ayuso wrote:
> 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.
> 

Okay, I will send v2 to the nf-next tree.

Thanks for your attention.

> 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);