Hi Pablo, On Mon, 2 Dec 2019 14:14:05 +0100 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > Hi Stefano, > > This patchset extends the netlink API to allow to express an interval > with one single element. > > This simplifies this interface since userspace does not need to send two > independent elements anymore, one of the including the > NFT_SET_ELEM_INTERVAL_END flag. > > The idea is to use the _DESC to specify that userspace speaks the kernel > that new API representation. In your case, the new description attribute > that tells that this set contains interval + concatenation implicitly > tells the kernel that userspace supports for this new API. Thanks! I just had a quick look, I think the new set implementation would indeed look more elegant this way. As to design choices, I'm afraid I'm not familiar enough with the big picture to comment on the general idea, but my uninformed opinion agrees with this approach. :) For what it's worth, I'd review this in deeper detail next. > If you're fine with this, I can scratch a bit of time to finish the > libnftnl part. The nft code will need a small update too. You will not > need to use the nft_set_pipapo object as scratchpad area anymore. On my side, I'm almost done with nft/libnftnl/kernel changes for the NFT_SET_DESC_CONCAT thing. How should we proceed? Do you want me to share those patches so that you can add this bit on top, or should this come first, or in a separate series? I could also just share the new nft/libnftnl patches (I should have them ready between today and tomorrow), and proceed adapting the kernel part according to your changes. Related question: to avoid copying data around, I'm now dynamically allocating a struct nft_data_desc in nf_tables_newset() with a reference from struct nft_set: desc->dlen, desc->klen, desc->size would all live there, together with the "subkey" stuff. Is it a bad idea? I can undo it easily, I just don't know if there's a specific reason why those fields are repeated in struct nft_set. -- Stefano
