On Wed, Jun 03, 2020 at 01:50:11AM +0200, Stefano Brivio wrote: > While checking the validity of insertion in __nft_rbtree_insert(), > we currently ignore conflicting elements and intervals only if they > are not active within the next generation. > > However, if we consider expired elements and intervals as > potentially conflicting and overlapping, we'll return error for > entries that should be added instead. This is particularly visible > with garbage collection intervals that are comparable with the > element timeout itself, as reported by Mike Dillinger. > > Other than the simple issue of denying insertion of valid entries, > this might also result in insertion of a single element (opening or > closing) out of a given interval. With single entries (that are > inserted as intervals of size 1), this leads in turn to the creation > of new intervals. For example: > > # nft add element t s { 192.0.2.1 } > # nft list ruleset > [...] > elements = { 192.0.2.1-255.255.255.255 } > > Always ignore expired elements active in the next generation, while > checking for conflicts. > > It might be more convenient to introduce a new macro that covers > both inactive and expired items, as this type of check also appears > quite frequently in other set back-ends. This is however beyond the > scope of this fix and can be deferred to a separate patch. > > Other than the overlap detection cases introduced by commit > 7c84d41416d8 ("netfilter: nft_set_rbtree: Detect partial overlaps > on insertion"), we also have to cover the original conflict check > dealing with conflicts between two intervals of size 1, which was > introduced before support for timeout was introduced. This won't > return an error to the user as -EEXIST is masked by nft if > NLM_F_EXCL is not given, but would result in a silent failure > adding the entry. Applied, thanks.