Hi Stefano, Stefano Brivio schrieb am 03.07.2020 11:28 (GMT +02:00): > On Fri, 3 Jul 2020 00:30:10 +0200 (CEST) > "Timo Sigurdsson" <public_timo.s@xxxxxxxxxxxxxx> wrote: > >> Another issue I stumbled upon was that auto-merge may actually >> generate wrong/incomplete intervals if you have multiple 'add >> element' statements within an nftables script file. I consider this a >> serious issue if you can't be sure whether the addresses or intervals >> you add to a set actually end up in the set. I reported this here >> [2]. The workaround for it is - again - to add all addresses in a >> single statement. >> ... >> [2] https://bugzilla.netfilter.org/show_bug.cgi?id=1438 > > Practically speaking I think it's a bug, but I can't find a formal, > complete definition of automerge, so one can also say it "adds items up > to and including the first conflicting one", and there you go, it's > working as intended. Actually, I think it's a bug regardless of how auto-merge is defined exactly. Simply because I don't think that this add element family table myset { A } add element family table myset { B } should give a different result compared to this add element family table myset { A, B } But that's basically what's happening in my example. > In general, when we discussed this "automerge" feature for > multi-dimensional sets in nftables (not your case, but I aimed at > consistency), I thought it was a mistake to introduce it altogether, > because it's hard to define it and whatever definition one comes up > with might not match what some users think. I understand that depending on the use case, one may have different expectations and that merging entries may cause issues. One example I read before was about adding a single IP address to a set which already contains the full /24 interval. If the addition would be simply ignored, you either wouldn't be able to delete the entry again or you'd have to break up the interval when doing so. So, I understand that it's impossible to make everybody happy. > Let's try to reorder the insertions: > > # nft add element t s '{ 10.0.0.1/16 . 192.168.0.0/16 }' > # nft add element t s '{ 10.0.0.1/24 . 172.16.0.1 }' > # nft add element t s '{ 10.0.1.1/30 . 192.168.1.1/24 }' > Error: Could not process rule: File exists > add element t s { 10.0.1.1/30 . 192.168.1.1/24 } > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > ...because that entry would never match anything: it's inserted after a > more generic one that already covers it completely, and we'd like to > tell the user that it doesn't make sense. > > Now, this is pretty much the only advantage of not allowing overlaps: > telling the user that some insertion doesn't make sense, and thus it > was probably not what the user wanted to do. Two thoughts here: 1) If there is a real conflict, say in a verdict map with diverging entries, then sure, refusing to accept the conflicting entries is certainly useful. But if there is an overlap that bears no meaning other than you'd either have an additional entry in your set that would never be matched, then I think there should be a way to just allow this or ignore the addition altogether. 2) Another problem here in practice is that nft doesn't emit a warning but fails entirely if you load a large set from a script with a duplicate entry (without auto-merge). You could avoid this by adding each element individually, but that's very slow for large sets, so not really an option either. > So... I wouldn't know how deal with your use case, even in theory, in a > consistent way. Should we rather introduce a flag that allows any type > of overlapping (default with ipset), which is a way for the user to > tell us they don't actually care about entries not having any effect? Giving advice here is difficult, since I'm not in any position to make judgements about other use cases that might be affected by this. But, generally speaking, yes I think some option that makes overlaps ignorable like `ipset -exist' would be useful as well as good documentation what is to be expected from each flag/option, so the user at least knows about the limitations. And if there is a way to determine this programmatically, there could be a distinction along the lines what I described above between conflicting statements (leading to an error) and statements that simply overlap or are superfluous (leading to a simple warning unless a flag/option is used to allow and ignore them). > And, in that case, would you expect the entry to be listed in the > resulting set, in case of full overlap (where one set is a subset, not > necessarily proper, of the other one)? I would have expected the entries to be merged, so if I add 192.168.0.0/24 and then 192.168.0.2, I'd only expect to get 192.168.0.0/24 returned when listing the set. But even if both entries were added and the second one would never match, that would be fine for my use case. The problem is, when working with multiple blacklists, I cannot keep track of which entries are already added to the set, especially if one list may contain complete networks and another list contains individual IP addresses. Thanks and regards, Timo
