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: Fri, 27 Nov 2015 22:15:42 -0800 From: roopa <roopa@...ulusnetworks.com> To: David Miller <davem@...emloft.net> CC: ebiederm@...ssion.com, rshearma@...cade.com, netdev@...r.kernel.org Subject: Re: [PATCH net-next v5] mpls: support for dead routes On 11/25/15, 8:18 AM, David Miller wrote: > From: Roopa Prabhu <roopa@...ulusnetworks.com> > Date: Tue, 24 Nov 2015 15:22:22 -0800 > >> v4 -v5 >> - if kmemdup fails, modify the original route in place. This is a >> corner case and only side effect is that in the remote case >> of kmemdup failure, the changes will not be atomically visible >> to datapath. > I really don't like this. > > Either you need to make the changes appear atomic to the data path, > and you therefore must fail the operation if kmemdup() fails, or it > doesn't matter and you should just always change the route in-place. > > As far as I can tell it can't possibly matter. The alive counter is > never modified by the data path, it is only tested to make a multipath > decision. Likewise it's rather harmless to send a frame or two via a > device currently going down. > > But if you're convinced it matters, then is matters, and you can't > fake things when kmemdup() fails. And in that case I would recommend > that you use a two pass algorithm, one pass allocates all of the > new routes, and the second fills them in, inserts them, and frees > the old ones. > > That is the only way you can unwind and fail cleanly. > > And oh yeah, that's right, you can't really fail this and make the > ifdown not proceed. > > So you're stuck, right? > > That's why this has to be designed in a way where memory allocations > are not necessary. These notifiers aren't really designed to facilitate > situations that require resource acquisitions that can fail. > Get your point. I am not convinced that the atomic update matters much for the transient case. I was trying to accommodate comments i got during the review and it seemed ok to cover both cases by optionally doing the atomic update when we can. But, I get your position on this. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists