On Tue, Aug 13, 2024 at 08:12:18PM +0200, Phil Sutter wrote:
> Hi Pablo,
>
> On Wed, Aug 07, 2024 at 04:23:56PM +0200, Pablo Neira Ayuso wrote:
> > This patch adds a timeout marker for those elements that never expire
> > when the element are created, so timeout updates are possible.
> >
> > Note that maximum supported timeout in milliseconds which is conveyed
> > within a netlink attribute is 0x10c6f7a0b5ec which translates to
> > 0xffffffffffe85300 jiffies64, higher milliseconds values result in an
> > ERANGE error. Use U64_MAX as an internal marker to be stored in the set
> > element timeout field for permanent elements.
> >
> > If userspace provides no timeout for an element, then the default set
> > timeout applies. However, if no default set timeout is specified and
> > timeout flag is set on, then such new element gets the never expires
> > marker.
> >
> > Note that, in older kernels, it is already possible to define elements
> > that never expire by declaring a set with the set timeout flag set on
> > and no global set timeout, in this case, new element with no explicit
> > timeout never expire do not allocate the timeout extension, hence, they
> > never expire. This approach makes it complicated to accomodate element
> > timeout update, because element extensions do not support reallocations.
> > Therefore, allocate the timeout extension and use the new marker for
> > this case, but do not expose it to userspace to retain backward
> > compatibility in the set listing.
>
> I fail to miss the point why this marker is needed at all:
Long story short: I did my best to support this without this marker
but I could not find a design that works without it.
> Right now, new set elements receive EXT_TIMEOUT upon creation if either
> NFTA_SET_ELEM_TIMEOUT is present (i.e., user specified per-element
> timeout) or set->timeout is non-zero (i.e., set has a default timeout).
There is one exception though:
table inet x {
set y {
typeof ip saddr
flags timeout
}
}
in this case, there is no default set timeout. Older kernels already
allow to add elements with no EXT_TIMEOUT that never expire with this
approach, however, this is not practical for element updates, because
set element extension reallocation is not supported.
> Why not change this logic and add EXT_TIMEOUT to all elements which will
> timeout and initialize it either to the user-defined value or the set's
> default? Then, only elements which don't timeout remain without
> EXT_TIMEOUT. Which is not a problem, because their expiration value does
> not need to be reset and thus they don't need space for one.
No EXT_TIMEOUT means users cannot update the timeout policy for such
element. I assume users can update from "timeout never" to "timeout 1h"
as a valid usecase.
> The only downside is that elements which stick to the set's default
> timeout increase in size (for the copied set->timeout value), but this
> patch series merges EXT_EXPIRATION and EXT_TIMEOUT anyway so their size
> increases already. OTOH, non-expiring set elements won't need a spurious
> EXT_TIMEOUT holding never expires marker.
