netdev - RE: [PATCH net-next v4 2/9] dpll: spec: Add Netlink spec in YAML

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM6PR11MB4657AD95547A14234941F9399B1AA@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Thu, 17 Aug 2023 18:40:00 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Vadim Fedorenko
	<vadim.fedorenko@...ux.dev>
CC: Jiri Pirko <jiri@...nulli.us>, Jonathan Lemon <jonathan.lemon@...il.com>,
	Paolo Abeni <pabeni@...hat.com>, "Olech, Milena" <milena.olech@...el.com>,
	"Michalik, Michal" <michal.michalik@...el.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, poros <poros@...hat.com>, mschmidt
	<mschmidt@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>, Bart Van Assche
	<bvanassche@....org>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, Jiri Pirko <jiri@...dia.com>
Subject: RE: [PATCH net-next v4 2/9] dpll: spec: Add Netlink spec in YAML

>From: Jakub Kicinski <kuba@...nel.org>
>Sent: Tuesday, August 15, 2023 4:44 AM
>
>On Fri, 11 Aug 2023 21:03:33 +0100 Vadim Fedorenko wrote:
>> +attribute-sets:
>> +  -
>> +    name: dpll
>> +    enum-name: dpll_a
>> +    attributes:
>> +      -
>> +        name: id
>> +        type: u32
>> +        value: 1
>
>value: 1 is the default
>
>> +      -
>> +        name: module-name
>> +        type: string
>> +      -
>> +        name: clock-id
>> +        type: u64
>
>I don't see a pad, you have a u64 without a pad?
>
>> +      -
>> +        name: mode
>> +        type: u8
>
>It's an enum, should always be u32 or bigger at protocol level.
>Netlink pads to 4B, you're not saving any space.
>
>> +        enum: mode
>> +      -
>> +        name: mode-supported
>> +        type: u8
>
>Same.
>
>> +        enum: mode
>> +        multi-attr: true
>> +      -
>> +        name: lock-status
>> +        type: u8
>
>And here.
>
>> +        enum: lock-status
>> +      -
>> +        name: temp
>> +        type: s32
>> +      -
>> +        name: type
>> +        type: u8
>
>And here, etc.
>
>Why are all attributes in a single attr space? :(
>More than half of them are prefixed with a pin- does it really
>not scream to you that they belong to a different space?
>

I agree, but there is an issue with this, currently:

name: pin-parent-device
subset-of: dpll
attributes:
  -
    name: id
    type: u32
  -
    name: pin-direction
    type: u32
  -
    name: pin-prio
    type: u32
  -
    name: pin-state
    type: u32

Where "id" is a part of device space, rest attrs would be a pin space..
Shall we have another argument for device id in a pin space?

Thank you!
Arkadiusz


>> +operations:
>> +  enum-name: dpll_cmd
>> +  list:
>> +    -
>> +      name: device-id-get
>> +      doc: |
>> +        Get id of dpll device that matches given attributes
>> +      value: 1
>
>is the default
>--
>pw-bot: cr