netdev - Re: [PATCH nf-next RFC 1/3] xt_statistic: taking GRO/GSO into account for nth-match

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aShi608hEPxDLvsr@strlen.de>
Date: Thu, 27 Nov 2025 15:40:43 +0100
From: Florian Westphal <fw@...len.de>
To: Jesper Dangaard Brouer <hawk@...nel.org>
Cc: netfilter-devel@...r.kernel.org,
	Pablo Neira Ayuso <pablo@...filter.org>, netdev@...r.kernel.org,
	phil@....cc, Eric Dumazet <eric.dumazet@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	kernel-team@...udflare.com, mfleming@...udflare.com,
	matt@...dmodwrite.com
Subject: Re: [PATCH nf-next RFC 1/3] xt_statistic: taking GRO/GSO into
 account for nth-match

Jesper Dangaard Brouer <hawk@...nel.org> wrote:
> The iptables statistic nth mode is documented to match one packet every nth
> packets. When packets gets GRO/GSO aggregated before traversing the statistic
> nth match, then they get accounted as a single packet.
> 
> This patch takes into account the number of packet frags a GRO/GSO packet
> contains for the xt_statistic match.

I doubt we can do this upstream.  Two reasons that come to mind, in no
particular order:

1. It introduces asymmetry.  All matches use "skb == packet" design.
   Examples that come to mind: xt_limit, xt_length.
2. This adds a compat issue with nftables:
    iptables-translate -A INPUT -m statistic --mode nth --packet 0  --every 10
    nft 'add rule ip filter INPUT numgen inc mod 10 0 counter'

'numgen' increments a counter for every skb, i.e. reg := i++;.
But, after this patch -m statistics doesn't work this way anymore
and the two rules no longer do the same thing.

But even if we'd ignore this or add a flag to control behavior, I don't
see how this could be implemented in nft.

And last but not least, I'm not sure the premise is correct.
Yes, when you think of 'packet sampling', then we don't 'match'
often enough for gro/gso case.

However, when doing '-m statistic ... -j LOG' (or whatever), then the
entire GSO superpacket is logged, i.e. several 'packets' got matched
at once.

So the existing algorithm works correctly even when considering
aggregation because on average the correct amount of segments gets
matched (logged).

With this proposed new algo, we can now match 100% of skbs / aggregated
segments, even for something like '--every 10'.  And that seems fishy to
me.

As far as I understood its only 'more correct' in your case because the
logging backend picks one individual segment out from the NFLOG'd
superpacket.

But if it would NOT do that, then you now sample (almost) all segments
seen on wire.  Did I misunderstand something here?