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: Mon, 8 Jul 2019 21:28:37 +0200 From: Jiri Pirko <jiri@...nulli.us> To: Michal Kubecek <mkubecek@...e.cz> Cc: netdev@...r.kernel.org, David Miller <davem@...emloft.net>, Jakub Kicinski <jakub.kicinski@...ronome.com>, Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, John Linville <linville@...driver.com>, Stephen Hemminger <stephen@...workplumber.org>, Johannes Berg <johannes@...solutions.net>, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface Mon, Jul 08, 2019 at 09:26:29PM CEST, jiri@...nulli.us wrote: >Mon, Jul 08, 2019 at 07:27:29PM CEST, mkubecek@...e.cz wrote: >>On Wed, Jul 03, 2019 at 10:41:51AM +0200, Jiri Pirko wrote: >>> Tue, Jul 02, 2019 at 04:52:41PM CEST, mkubecek@...e.cz wrote: >>> >On Tue, Jul 02, 2019 at 02:25:21PM +0200, Jiri Pirko wrote: >>> >> Tue, Jul 02, 2019 at 01:49:59PM CEST, mkubecek@...e.cz wrote: >>> >> >+ >>> >> >+ ETHTOOL_A_HEADER_DEV_INDEX (u32) device ifindex >>> >> >+ ETHTOOL_A_HEADER_DEV_NAME (string) device name >>> >> >+ ETHTOOL_A_HEADER_INFOMASK (u32) info mask >>> >> >+ ETHTOOL_A_HEADER_GFLAGS (u32) flags common for all requests >>> >> >+ ETHTOOL_A_HEADER_RFLAGS (u32) request specific flags >>> >> >+ >>> >> >+ETHTOOL_A_HEADER_DEV_INDEX and ETHTOOL_A_HEADER_DEV_NAME identify the device >>> >> >+message relates to. One of them is sufficient in requests, if both are used, >>> >> >+they must identify the same device. Some requests, e.g. global string sets, do >>> >> >+not require device identification. Most GET requests also allow dump requests >>> >> >+without device identification to query the same information for all devices >>> >> >+providing it (each device in a separate message). >>> >> >+ >>> >> >+Optional info mask allows to ask only for a part of data provided by GET >>> >> >>> >> How this "infomask" works? What are the bits related to? Is that request >>> >> specific? >>> > >>> >The interpretation is request specific, the information returned for >>> >a GET request is divided into multiple parts and client can choose to >>> >request one of them (usually one). In the code so far, infomask bits >>> >correspond to top level (nest) attributes but I would rather not make it >>> >a strict rule. >>> >>> Wait, so it is a matter of verbosity? If you have multiple parts and the >>> user is able to chose one of them, why don't you rather have multiple >>> get commands, one per bit. This infomask construct seems redundant to me. >> >>I thought it was a matter of verbosity because it is a very basic >>element of the design, it was even advertised in the cover letter among >>the basic ideas, it has been there since the very beginning and in five >>previous versions through year and a half, noone did question it. That's >>why I thought you objected against unclear description, not against the >>concept as such. >> >>There are two reasons for this design. First is to reduce the number of >>requests needed to get the information. This is not so much a problem of >>ethtool itself; the only existing commands that would result in multiple >>request messages would be "ethtool <dev>" and "ethtool -s <dev>". Maybe >>also "ethtool -x/-X <dev>" but even if the indirection table and hash >>key have different bits assigned now, they don't have to be split even >>if we split other commands. It may be bigger problem for daemons wanting >>to keep track of system configuration which would have to issue many >>requests whenever a new device appears. >> >>Second reason is that with 8-bit genetlink command/message id, the space >>is not as infinite as it might seem. I counted quickly, right now the >>full series uses 14 ids for kernel messages, with split you propose it >>would most likely grow to 44. For full implementation of all ethtool >>functionality, we could get to ~60 ids. It's still only 1/4 of the >>available space but it's not clear what the future development will look >>like. We would certainly need to be careful not to start allocating new >>commands for single parameters and try to be foreseeing about what can >>be grouped together. But we will need to do that in any case. >> >>On kernel side, splitting existing messages would make some things a bit >>easier. It would also reduce the number of scenarios where only part of >>requested information is available or only part of a SET request fails. > >Okay, I got your point. So why don't we look at if from the other angle. >Why don't we have only single get/set command that would be in general >used to get/set ALL info from/to the kernel. Where we can have these >bits (perhaps rather varlen bitfield) to for user to indicate which data >is he interested in? This scales. The other commands would be >just for action. > >Something like RTM_GETLINK/RTM_SETLINK. Makes sense? + I think this might safe a lot of complexicity aroung your proposed inner ops. > > >> >>Michal
Powered by blists - more mailing lists