On Wed, Jun 27, 2018 at 6:27 PM, Roopa Prabhu <ro...@cumulusnetworks.com> wrote: > From: Roopa Prabhu <ro...@cumulusnetworks.com> > > After commit f9d4b0c1e969 ("fib_rules: move common handling of newrule > delrule msgs into fib_nl2rule"), rule_find is strict about checking > for an existing rule. rule_find must check against all > user given attributes, else it may match against a subset > of attributes and return an existing rule. > > In the below case, without support for protocol match, rule_find > will match only against 'table main' and return an existing rule. > > $ip -4 rule add table main protocol boot > RTNETLINK answers: File exists > > This patch adds protocol support to rule_find, forcing it to > check protocol match if given by the user. > > Fixes: f9d4b0c1e969 ("fib_rules: move common handling of newrule delrule msgs > into fib_nl2rule") > Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com> > --- > I spent some time looking at all match keys today and protocol > was the only missing one (protocol is not in a released kernel yet). > The only way this could be avoided is to move back to the old loose > rule_find. I am worried about this new strict checking surprising users, > but going back to the previous loose checking does not seem right either. > If there is a reason to believe that users did rely on the previous > behaviour, I will be happy to revert. Here is another example of old and > new behaviour. > > old rule_find behaviour: > $ip -4 rule add table main protocol boot > $ip -4 rule add table main protocol boot > $ip -4 rule add table main protocol boot > $ip rule show > 0: from all lookup local > 32763: from all lookup main proto boot > 32764: from all lookup main proto boot > 32765: from all lookup main proto boot > 32766: from all lookup main > 32767: from all lookup default > > new rule_find behaviour (after this patch): > $ip -4 rule add table main protocol boot > $ip -4 rule add table main protocol boot > RTNETLINK answers: File exists >
I found the case where the new rule_find breaks for add. $ip -4 rule add table main tos 10 fwmark 1 $ip -4 rule add table main tos 10 RTNETLINK answers: File exists The key masks in the new and old rule need to be compared . And it cannot be easily compared today without an elaborate if-else block. Its best to introduce key masks for easier and accurate rule comparison. But this is best done in net-next. I will submit an incremental patch tomorrow to restore previous rule_exists for the add case and the rest should be good. The current patch in context is needed regardless. Thanks (and sorry about the iterations).