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
| ||
|
Message-Id: <20170526.105443.1489276661727770629.davem@davemloft.net> Date: Fri, 26 May 2017 10:54:43 -0400 (EDT) From: David Miller <davem@...emloft.net> To: jiri@...nulli.us Cc: xiyou.wangcong@...il.com, netdev@...r.kernel.org, jhs@...atatu.com, jiri@...lanox.com Subject: Re: [Patch net-next] net_sched: only create filter chains for new filters/actions From: Jiri Pirko <jiri@...nulli.us> Date: Fri, 26 May 2017 07:53:52 +0200 > Thu, May 25, 2017 at 06:14:56PM CEST, davem@...emloft.net wrote: >>From: Cong Wang <xiyou.wangcong@...il.com> >>Date: Tue, 23 May 2017 09:42:37 -0700 >> >>> tcf_chain_get() always creates a new filter chain if not found >>> in existing ones. This is totally unnecessary when we get or >>> delete filters, new chain should be only created for new filters >>> (or new actions). >>> >>> Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters") >>> Cc: Jamal Hadi Salim <jhs@...atatu.com> >>> Cc: Jiri Pirko <jiri@...lanox.com> >>> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com> >> >>Indeed, get and delete requests should not create new objects, ever. >> >>I have pretty much no idea why Jiri is making such a big fuss about >>this change, to be quite honest. :-) > > Because it makes already hard to read code even worse, for *no* benefit. > That's why. Jiri, if you say the same thing 100 times, it doesn't help anyone understand your arguments any better. Creating new objects when a GET or a DEL is requested is flat out wrong. And Cong is fixing that. And I also didn't find the boolean logic hard to understand at all. It is in fact a very common pattern to pass a "create" boolean into lookup functions, to tell them whether to create a new object on lookup failure or not. And then also to control that boolean via what kind of netlink request we are processing. So you tell me what's so bad about his change given the above? Give me details and real facts, like I just did, rather than vague statements about "benefit" and "hard to read". Thank you.
Powered by blists - more mailing lists