lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 10 Apr 2019 21:54:18 -0700 From: David Ahern <dsahern@...il.com> To: Martin Lau <kafai@...com> Cc: David Ahern <dsahern@...nel.org>, "davem@...emloft.net" <davem@...emloft.net>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "idosch@...lanox.com" <idosch@...lanox.com> Subject: Re: [PATCH net-next 10/10] ipv6: Refactor __ip6_route_redirect On 4/10/19 1:45 PM, Martin Lau wrote: > On Wed, Apr 10, 2019 at 11:45:04AM -0700, David Ahern wrote: >>>> @@ -2420,7 +2449,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net, >>>> int flags) >>>> { >>>> struct ip6rd_flowi *rdfl = (struct ip6rd_flowi *)fl6; >>>> - struct rt6_info *ret = NULL, *rt_cache; >>>> + struct rt6_info *ret = NULL; >>>> struct fib6_info *rt; >>> The "rt" naming is becoming hard to review. >>> It is fortunate that the type context is kept in this diff. >>> >>> In this local case, the s/rt/f6i/ rename is quite doable? >> >> I looked into it and I feel such a mass rename just causes noise: >> $ egrep -r 'fib6_info \*rt[,;]' net | wc -l >> 50 >> >> so that's 50 functions that need to be updated and then all rt >> references within the functions. A lot of noise which is why I have not >> sent a name change patch. > Agree that it is a bit noisy (but obvious) change. > I think it was a mistake to decide to postpone this name change while > introducing fib6_info back then. It would be easier to review if > they were done together: > https://marc.info/?l=linux-netdev&m=151968617902168&w=2 > > I think a few of us are still ok-ish because we know what > have been going on. For some, it will be confusing and > it should not be suprised to see this 'struct fib6_info *rt' > being copy-and-pasted to the future patch that makes it > even harder to read/maintain. > As I mentioned in that URL: "My thought is to follow up with another patch that does the rename of rt to f6i for all fib6_info. Given how large this change is already I did not want to add extra diffs for that. If there is agreement to fold that part in now, I can do it." That was the theory. The reality is that it is a non-trivial change that can not be done cleanly: 'rt' does not allow a clean search and replace and when you consider the related changes (e.g., fib6_info *rt_last). I am 80% through net/ipv6/route.c with ip6_fib.c to go. This is the current diff stat: $ git diff --stat include/net/if_inet6.h | 2 +- include/net/ip6_fib.h | 20 +-- net/ipv6/addrconf.c | 84 ++++----- net/ipv6/route.c | 674 ++++++++++++++++++++++++++++++++++----------------------------------- 4 files changed, 390 insertions(+), 390 deletions(-) and that assumes I have done this right (ie., a clean compile). By the time I am done that is going to be on the order of 500 insert / 500 delete of manual name changes. I see that as unnecessary churn with no benefit.
Powered by blists - more mailing lists