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