From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net] fib_semantics: Don't match route with mismatching tclassid Date: Wed, 14 Feb 2018 10:46:05 -0700 Message-ID: <0b984dc9-1206-552f-80e4-66bac17c3ab1@gmail.com> References: <20180214172822.25952-1-sbrivio@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Alexey Kuznetsov , Thomas Graf , Jianlin Shi , netdev@vger.kernel.org To: Stefano Brivio , "David S . Miller" Return-path: Received: from mail-pl0-f68.google.com ([209.85.160.68]:40282 "EHLO mail-pl0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161295AbeBNRqI (ORCPT ); Wed, 14 Feb 2018 12:46:08 -0500 Received: by mail-pl0-f68.google.com with SMTP id g18so8936421plo.7 for ; Wed, 14 Feb 2018 09:46:08 -0800 (PST) In-Reply-To: <20180214172822.25952-1-sbrivio@redhat.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2/14/18 10:28 AM, Stefano Brivio wrote: > In fib_nh_match(), if output interface or gateway are passed in > the FIB configuration, we don't have to check next hops of > multipath routes to conclude whether we have a match or not. > > However, we might still have routes with different realms > matching the same output interface and gateway configuration, > and this needs to cause the match to fail. Otherwise the first > route inserted in the FIB will match, regardless of the realms: > > # ip route add 1.1.1.1 dev eth0 table 1234 realms 1/2 > # ip route append 1.1.1.1 dev eth0 table 1234 realms 3/4 > # ip route list table 1234 > 1.1.1.1 dev eth0 scope link realms 1/2 > 1.1.1.1 dev eth0 scope link realms 3/4 > # ip route del 1.1.1.1 dev ens3 table 1234 realms 3/4 > # ip route list table 1234 > 1.1.1.1 dev ens3 scope link realms 3/4 > > whereas route with realms 3/4 should have been deleted instead. > > Explicitly check for fc_flow passed in the FIB configuration > (this comes from RTA_FLOW extracted by rtm_to_fib_config()) and > fail matching if it differs from nh_tclassid. > > The handling of RTA_FLOW for multipath routes later in > fib_nh_match() is still needed, as we can have multiple RTA_FLOW > attributes that need to be matched against the tclassid of each > next hop. > > Reported-by: Jianlin Shi > Signed-off-by: Stefano Brivio > --- > net/ipv4/fib_semantics.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c > index c586597da20d..2223ebac5b64 100644 > --- a/net/ipv4/fib_semantics.c > +++ b/net/ipv4/fib_semantics.c > @@ -646,6 +646,10 @@ int fib_nh_match(struct fib_config *cfg, struct fib_info *fi, > fi->fib_nh, cfg, extack)) > return 1; > } > +#ifdef CONFIG_IP_ROUTE_CLASSID > + if (cfg->fc_flow != fi->fib_nh->nh_tclassid) > + return 1; > +#endif you should verify fc_flow is set. user is allowed to delete first matching route based on data that is given. > if ((!cfg->fc_oif || cfg->fc_oif == fi->fib_nh->nh_oif) && > (!cfg->fc_gw || cfg->fc_gw == fi->fib_nh->nh_gw)) > return 0; >