On 4/28/19 12:21 AM, Hangbin Liu wrote:
> Hi David, Mateusz,
>
> Kernel commit 153380ec4b9b ("fib_rules: Added NLM_F_EXCL support to
> fib_nl_newrule") added a check and return -EEXIST if the rule is already
> exist. With it the ip rule works as expected now.
>
> But without NLM_F_EXCL people still could add duplicate rules. the result
> looks like:
>
> # ip rule
> 0: from all lookup local
> 32766: from all lookup main
> 32767: from all lookup default
> 100000: from 192.168.7.5 lookup 5
> 100000: from 192.168.7.5 lookup 5
>
> The two same rules looks unreasonable. Do you know if there is a use case
> that need this?
It does not make sense to me to allow multiple entries with the same
config; it only adds to the overhead of fib lookups.
>
> So how about just return directly if user add a exactally same rule, as if
> we did an update, like:
>
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index ffbb827723a2..c49b752ea7eb 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -756,9 +756,9 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr
> *nlh,
> if (err)
> goto errout;
>
> - if ((nlh->nlmsg_flags & NLM_F_EXCL) &&
> - rule_exists(ops, frh, tb, rule)) {
> - err = -EEXIST;
> + if (rule_exists(ops, frh, tb, rule)) {
> + if (nlh->nlmsg_flags & NLM_F_EXCL)
> + err = -EEXIST;
> goto errout_free;
> }
>
>
> What do you think?
I'm not so sure about the failure and more from a consistency with other
RTM_NEW commands which cover updates to an existing entry. In the case
of rules if there is nothing to update and the rule already exists then
- for consistency - 0 is the right return code.
