On Fri, 3 Apr 2020 15:33:31 +0200 Stefano Brivio <sbrivio@xxxxxxxxxx> wrote: > On Fri, 3 Apr 2020 14:50:29 +0200 > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > On Fri, Apr 03, 2020 at 02:27:05PM +0200, Stefano Brivio wrote: > > > On Fri, 3 Apr 2020 14:03:51 +0200 > > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > > > > On Thu, Apr 02, 2020 at 11:49:41PM +0200, Pablo Neira Ayuso wrote: > > > > > This patch adds a lazy check to validate that the first element is not a > > > > > concatenation. The segtree code does not support for concatenations, > > > > > bail out with EOPNOTSUPP. > > > > > > > > > > # nft add element x y { 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535 } > > > > > Error: Could not process rule: Operation not supported > > > > > add element x y { 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535 } > > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > > > > > > Otherwise, the segtree code barfs with: > > > > > > > > > > BUG: invalid range expression type concat > > > > > > > > Hm. > > > > > > > > I'm afraid this patch is not enough, the following ruleset crashes > > > > in old kernels with recent nft: > > > > > > > > flush ruleset > > > > > > > > table inet filter { > > > > set test { > > > > type ipv4_addr . ipv4_addr . inet_service > > > > flags interval,timeout > > > > elements = { 1.1.1.1 . 2.2.2.2 . 30 , > > > > 2.2.2.2 . 3.3.3.3 . 40 , > > > > 3.3.3.3 . 4.4.4.4 . 50 } > > > > } > > > > > > > > chain output { > > > > type filter hook output priority 0; policy accept; > > > > ip saddr . ip daddr . tcp dport @test counter > > > > } > > > > } > > > > > > First off, sorry, it didn't occur to me to run new tests on older > > > kernels. :/ > > > > > > I can't quickly run that on some older kernel right now. For my > > > understanding, where is it crashing? > > > > When listing via > > > > nft list ruleset > > > > The segtree does not know how to handle this concatenation. > > Ouch, I see. > > > The only way I found to prevent this error is to bail out when adding > > the set, ie. old kernel checks that NFT_SET_CONCAT is not supported, > > hence it bails out. > > Right, it took me a while to realise you're referring to the flag that > was present until v2 and I dropped in v3 (gold medal goes to truly > yours -- I actually proposed that :/). > > > I'm going to prepare patches to submit this to nf.git. > > I'm still wondering if we can come up with a userspace-only fix by > understanding kernel support level somehow (not via flags) and refuse > to add that type of set. I'm still checking. Nope, sorry, I can't think of anything better than what you already proposed. -- Stefano