Applied, thanks. See comments below though.
On Thu, Jan 30, 2020 at 01:16:57AM +0100, Stefano Brivio wrote:
> After exporting field lengths via NFTNL_SET_DESC_CONCAT attributes,
> we now need to adjust parsing of user input and generation of
> netlink key data to complete support for concatenation of set
> ranges.
>
> Instead of using separate elements for start and end of a range,
> denoting the end element by the NFT_SET_ELEM_INTERVAL_END flag,
> as it's currently done for ranges without concatenation, we'll use
> the new attribute NFTNL_SET_ELEM_KEY_END as suggested by Pablo. It
> behaves in the same way as NFTNL_SET_ELEM_KEY, but it indicates
> that the included key represents the upper bound of a range.
>
> For example, "packets with an IPv4 address between 192.0.2.0 and
> 192.0.2.42, with destination port between 22 and 25", needs to be
> expressed as a single element with two keys:
>
> NFTA_SET_ELEM_KEY: 192.0.2.0 . 22
> NFTA_SET_ELEM_KEY_END: 192.0.2.42 . 25
>
> To achieve this, we need to:
>
> - adjust the lexer rules to allow multiton expressions as elements
> of a concatenation. As wildcards are not allowed (semantics would
> be ambiguous), exclude wildcards expressions from the set of
> possible multiton expressions, and allow them directly where
> needed. Concatenations now admit prefixes and ranges
>
> - generate, for each element in a range concatenation, a second key
> attribute, that includes the upper bound for the range
>
> - also expand prefixes and non-ranged values in the concatenation
> to ranges: given a set with interval and concatenation support,
> the kernel has no way to tell which elements are ranged, so they
> all need to be. For example, 192.0.2.0 . 192.0.2.9 : 1024 is
> sent as:
>
> NFTA_SET_ELEM_KEY: 192.0.2.0 . 1024
> NFTA_SET_ELEM_KEY_END: 192.0.2.9 . 1024
>
> - aggregate ranges when elements received by the kernel represent
> concatenated ranges, see concat_range_aggregate()
I think concat_range_aggregate() can be remove.
NFTA_SET_ELEM_KEY and the NFTA_SET_ELEM_KEY_END are now coming in the
same element. From the set element delinearization path this could
just build the range, correct?
[...]
> diff --git a/include/rule.h b/include/rule.h
> index a7f106f715cf..c232221e541b 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -372,6 +372,11 @@ static inline bool set_is_interval(uint32_t set_flags)
> return set_flags & NFT_SET_INTERVAL;
> }
>
> +static inline bool set_is_non_concat_range(struct set *s)
> +{
> + return (s->flags & NFT_SET_INTERVAL) && s->desc.field_count <= 1;
> +}
I might make a second pass to revisit this new helper.
Probably, we can pass struct set to all set_is_*() helpers instead,
and use set_is_interval() for the legacy interval representation
that is using the segtree infrastructure.
