From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init Date: Mon, 17 Sep 2018 09:59:32 -0700 Message-ID: References: <20180913203814.189698-1-posk@google.com> <351e6fbc-5356-b5c1-0638-0554843ad833@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, netdev@vger.kernel.org To: Peter Oskolkov Return-path: Received: from mail-pg1-f195.google.com ([209.85.215.195]:46937 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726795AbeIQW1s (ORCPT ); Mon, 17 Sep 2018 18:27:48 -0400 Received: by mail-pg1-f195.google.com with SMTP id b129-v6so7941015pga.13 for ; Mon, 17 Sep 2018 09:59:36 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 9/17/18 9:11 AM, Peter Oskolkov wrote: > On Thu, Sep 13, 2018 at 9:11 PM David Ahern wrote: >> >> On 9/13/18 1:38 PM, Peter Oskolkov wrote: >> >>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >>> index 3eed045c65a5..a3902f805305 100644 >>> --- a/net/ipv6/route.c >>> +++ b/net/ipv6/route.c >>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort) >>> >>> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort) >>> { >>> - rt->dst.flags |= fib6_info_dst_flags(ort); >>> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT; >> >> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags >> argument is passed to ip6_dst_alloc which is always invoked before >> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst. > > ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only > one flag is copied later (DST_HOST) outside of ip6_rt_init_dst(). > If the flag assignment is completely removed from ip6_rt_init_dst(), > then DST_NOPOLICY flag will be lost. > > Which may be OK, but is more than what this patch tries to solve (do not > copy DST_NOCOUNT flag). In the 4.17 kernel (prior to the fib6_info change), ip6_rt_cache_alloc calls __ip6_dst_alloc with 0 for flags so this is correct. The mistake is ip6_rt_copy_init -> ip6_rt_init_dst -> fib6_info_dst_flags. I believe the right fix is to drop the 'rt->dst.flags |= fib6_info_dst_flags(ort);' from ip6_rt_init_dst.