Hi, On Tue, Sep 11, 2018 at 02:39:29AM +0200, Pablo Neira Ayuso wrote: > On Mon, Sep 03, 2018 at 11:15:54PM +0200, Phil Sutter wrote: > > Hi Pablo, > > > > On Mon, Sep 03, 2018 at 01:57:18PM +0200, Pablo Neira Ayuso wrote: > > > On Wed, Aug 29, 2018 at 04:33:38PM +0200, Phil Sutter wrote: > > > > If an added rule's listing differs from the input (either expected or > > > > not), reinsert that output and check payload again to make sure the > > > > asymmetry doesn't lead to (internal) changes in ruleset. > > > > > > Hm, what is the goal for this one? Is this going to silence the > > > existing warnings? > > > > The goal is to assert asymmetry between input and output doesn't lead to > > changes in the rule internally. This sounds sloppy at first sight, but > > there is a rather fundamental principle behind it: Assuming that we have > > this asymmetry in certain situations and it's deliberate, we want to be > > sure it happens on input, not upon reinsertion. So take the following > > example: > > > > | ip protocol tcp tcp dport 22 > > > > The rule is in parts redundant, and will optimize to just: > > > > | tcp dport 22 > > > > If the optimization happens on input, it means that netlink output > > generated for the first version is the same as for the second one. If > > the netlink output though changes when the second version is inserted, > > we might have either missed the optimization while parsing the first > > version or there is a bug in that rule output lacks crucial details > > which get lost if you do something like: > > > > | (nft list ruleset; nft flush ruleset) | nft -f - > > > > In both cases, the code should be fixed. > > OK, so this increases test coverage by inserting the optimized rule, > so we make sure it loads fine. Not just that, we also (may) detect bugs in dependency killing after delinearizing. Imagine user does: | add rule inet t c nfproto ipv4 tcp dport 22 accept Which is loaded correctly (i.e., 'nfproto ipv4' is kept) but when listing, the code would incorrectly drop the nfproto statement. When replaying the rule (which is then just 'tcp dport 22 accept'), a difference in netlink dump can be observed. > Thanks for explaining and for your patience, I have applied this. Sure, I very much enjoy your feedback. It's worth waiting for. :) Cheers, Phil
