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: <ZN8ccoE8X5J6yysk@nanopsycho>
Date: Fri, 18 Aug 2023 09:23:30 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
	Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	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

Fri, Aug 18, 2023 at 01:36:40AM CEST, kuba@...nel.org wrote:
>On Thu, 17 Aug 2023 18:40:00 +0000 Kubalewski, Arkadiusz wrote:
>> >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?
>
>Why would pin and device not have separate spaces?
>
>When referring to a pin from a "device mostly" command you can
>usually wrap the pin attributes in a nest, and vice versa.
>But it may not be needed at all here? Let's look at the commands:
>
>+    -
>+      name: device-id-get
>+        request:
>+          attributes:
>+            - module-name
>+            - clock-id
>+            - type
>+        reply:
>+          attributes:
>+            - id
>
>All attributes are in "device" space, no mixing.
>
>+      name: device-get
>+        request:
>+          attributes:
>+            - id
>+        reply: &dev-attrs
>+          attributes:
>+            - id
>+            - module-name
>+            - mode
>+            - mode-supported
>+            - lock-status
>+            - temp
>+            - clock-id
>+            - type
>
>Again, no pin attributes, so pin can be separate?
>
>+    -
>+      name: device-set
>+        request:
>+          attributes:
>+            - id
>
>Herm, this one looks like it's missing attrs :S
>
>+    -
>+      name: pin-id-get
>+        request:
>+          attributes:
>+            - module-name
>+            - clock-id
>+            - pin-board-label
>+            - pin-panel-label
>+            - pin-package-label
>+            - pin-type
>+        reply:
>+          attributes:
>+            - pin-id
>
>Mostly pin stuff. I guess the module-name and clock-id attrs can be
>copy/pasted between device and pin, or put them in a separate set
>and add that set as an attr here. Copy paste is likely much simpler.

Agreed for the copy.

Honestly, I wound thing that shared ATTR space is fine for DPLL,
the split is an overkill here. But up to you Jakub :)


>
>+    -
>+      name: pin-get
>+        request:
>+          attributes:
>+            - pin-id
>+        reply: &pin-attrs
>+          attributes:
>+            - pin-id
>+            - pin-board-label
>+            - pin-panel-label
>+            - pin-package-label
>+            - pin-type
>+            - pin-frequency
>+            - pin-frequency-supported
>+            - pin-dpll-caps
>+            - pin-parent-device

The ID of device is inside this nest.


>+            - pin-parent-pin
>
>All pin.
>
>+    -
>+      name: pin-set
>+        request:
>+          attributes:
>+            - pin-id
>+            - pin-frequency
>+            - pin-direction
>+            - pin-prio
>+            - pin-state
>+            - pin-parent-device

Same here.


>+            - pin-parent-pin
>
>And all pin.