From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net] ipv6: use rt6_info members when dst is set in rt6_fill_node Date: Mon, 10 Sep 2018 13:07:11 -0600 Message-ID: References: <840f0c017c670ae9cc8fd8330ca24fcca1207918.1536398641.git.lucien.xin@gmail.com> <4abf5ec6-862b-f88e-b0ce-ddcd4308fd59@cumulusnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: network dev , davem , Roopa Prabhu To: Xin Long Return-path: Received: from mail-pg1-f196.google.com ([209.85.215.196]:45539 "EHLO mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726603AbeIKACo (ORCPT ); Mon, 10 Sep 2018 20:02:44 -0400 Received: by mail-pg1-f196.google.com with SMTP id x26-v6so10929892pge.12 for ; Mon, 10 Sep 2018 12:07:13 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 9/10/18 11:55 AM, Xin Long wrote: > On Tue, Sep 11, 2018 at 12:13 AM David Ahern wrote: >> >> On 9/9/18 12:29 AM, Xin Long wrote: >>>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >>>>> index 18e00ce..e554922 100644 >>>>> --- a/net/ipv6/route.c >>>>> +++ b/net/ipv6/route.c >>>>> @@ -4670,20 +4670,33 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb, >>>>> int iif, int type, u32 portid, u32 seq, >>>>> unsigned int flags) >>>>> { >>>>> - struct rtmsg *rtm; >>>>> + struct rt6key *fib6_prefsrc, *fib6_dst, *fib6_src; >>>>> + struct rt6_info *rt6 = (struct rt6_info *)dst; >>>>> + u32 *pmetrics, table, fib6_flags; >>>>> struct nlmsghdr *nlh; >>>>> + struct rtmsg *rtm; >>>>> long expires = 0; >>>>> - u32 *pmetrics; >>>>> - u32 table; >>>>> >>>>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*rtm), flags); >>>>> if (!nlh) >>>>> return -EMSGSIZE; >>>>> >>>>> + if (rt6) { >>>>> + fib6_dst = &rt6->rt6i_dst; >>>>> + fib6_src = &rt6->rt6i_src; >>>>> + fib6_flags = rt6->rt6i_flags; >>>>> + fib6_prefsrc = &rt6->rt6i_prefsrc; >>>>> + } else { >>>>> + fib6_dst = &rt->fib6_dst; >>>>> + fib6_src = &rt->fib6_src; >>>>> + fib6_flags = rt->fib6_flags; >>>>> + fib6_prefsrc = &rt->fib6_prefsrc; >>>>> + } >>>> >>>> Unless I am missing something at the moment, an rt6_info can only have >>>> the same dst, src and prefsrc as the fib6_info on which it is based. >>>> Thus, only the flags is needed above. That simplifies this patch a lot. >>> If dst, src and prefsrc in rt6_info are always the same as these in fib6_info, >>> why do we need them in rt6_info? we could just get it by 'from'. >>> >> >> I just sent a patch removing rt6i_prefsrc. It is set with only 1 reader >> that can be converted. >> >> rt6i_src is checked against the fib6_info to invalidate a dst if the src >> has changed, so a valid rt will always have the same rt6i_src as the >> rt->from. >> >> rt6i_dst is set to the dest address / 128 in cases, so it should be used >> for rt6_info cases above. > So that means, I will use rt6i_dst and rt6i_flags when dst is set? > how about I use rt6i_src there as well? just to make it look clear. > and plus the gw/nh dump fix in rt6_fill_node(): > - if (rt->fib6_nsiblings) { > + if (rt6) { > + if (fib6_flags & RTF_GATEWAY) > + if (nla_put_in6_addr(skb, RTA_GATEWAY, > + &rt6->rt6i_gateway) < 0) > + goto nla_put_failure; > + > + if (dst->dev && nla_put_u32(skb, RTA_OIF, dst->dev->ifindex)) > + goto nla_put_failure; > + } else if (rt->fib6_nsiblings) { > struct fib6_info *sibling, *next_sibling; > struct nlattr *mp; > > looks good to you? > sure