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: <dbbfd954-efef-67e6-b291-539c0b0b5ac4@linux.dev>
Date: Fri, 21 Jul 2023 15:42:46 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Jakub Kicinski <kuba@...nel.org>,
 Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
 Jonathan Lemon <jonathan.lemon@...il.com>, Paolo Abeni <pabeni@...hat.com>,
 Milena Olech <milena.olech@...el.com>,
 Michal Michalik <michal.michalik@...el.com>,
 linux-arm-kernel@...ts.infradead.org, poros@...hat.com, mschmidt@...hat.com,
 netdev@...r.kernel.org, linux-clk@...r.kernel.org,
 Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH net-next 00/11] Create common DPLL configuration API

On 21.07.2023 12:14, Jiri Pirko wrote:
> There are couple of issues that came up during our internal ci run:
> 
> 10:16:04  error: drivers/dpll/dpll_netlink.c:452:5: error: no previous prototype for '__dpll_device_change_ntf' [-Werror=missing-prototypes]
> 10:16:04  error: drivers/dpll/dpll_netlink.c:1283:13: error: no previous prototype for 'dpll_netlink_fini' [-Werror=missing-prototypes]
> 10:16:04  error: drivers/dpll/dpll_core.c:221:1: error: no previous prototype for 'dpll_xa_ref_dpll_find' [-Werror=missing-prototypes]
> 
> 10:27:31  error: drivers/dpll/dpll_core.c:220:21: warning: symbol 'dpll_xa_ref_dpll_find' was not declared. Should it be static?
> 10:27:31  error: drivers/dpll/dpll_netlink.c:452:5: warning: symbol '__dpll_device_change_ntf' was not declared. Should it be static?
> 10:27:31  error: drivers/dpll/dpll_netlink.c:1283:13: warning: symbol 'dpll_netlink_fini' was not declared. Should it be static?
> 10:27:41  error: drivers/net/ethernet/intel/ice/ice_dpll.c:461:3: error: a label can only be part of a statement and a declaration is not a statement
> 
> I believe that you didn't run make with C=2, otherwise you would hit
> these.

Yeah, I'll re-run the set patch-by-patch with C=2 next time.

> 
> Checkpatch issue:
> 10:29:30  CHECK: struct mutex definition without comment
> 10:29:30  #6581: FILE: drivers/net/ethernet/intel/ice/ice_dpll.h:85:
> 10:29:30  +	struct mutex lock;

Arkadiusz will take care of "ice" part.


> Spelling errors:
> 10:45:08  error: Documentation/netlink/specs/dpll.yaml:165: prority ==> priority
> 10:45:08  error: include/uapi/linux/dpll.h:128: prority ==> priority
> 10:45:08  error: drivers/net/ethernet/intel/ice/ice_dpll.c:2008: userpsace ==> userspace
> 10:45:08  error: drivers/net/ethernet/intel/ice/ice_dpll.h:20: properities ==> properties
> 

Will fix it.

> 
> Thu, Jul 20, 2023 at 11:18:52AM CEST, vadim.fedorenko@...ux.dev wrote:
>> Implement common API for clock/DPLL configuration and status reporting.
>> The API utilises netlink interface as transport for commands and event
>> notifications. This API aim to extend current pin configuration and
> 
> s/API aim/API aims/
> 
> 
>> make it flexible and easy to cover special configurations.
> 
> I don't follow. How this could "aim to extend current pin configuration" ?
> This is a new thing. Could you re-phrase?

Not really new. PTP devices have already simple pin configurations, mlx5 is 
using it for some cards with external pins. The problem is that PTP subsystem
covers only simple configuration of the pin and doesn't cover DPLL part at all.

> 
> What's "special configuration"? Sounds odd.
> 

Yeah, "complex configurations" sounds better, will change it.

> 
>>
>> Netlink interface is based on ynl spec, it allows use of in-kernel
>> tools/net/ynl/cli.py application to control the interface with properly
>> formated command and json attribute strings. Here are few command
>> examples of how it works with `ice` driver on supported NIC:
>>
>> - dump dpll devices
>> $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
> 
> "$#" looks a bit odd. Just "$" with "sudo" when you want to emphasize
> root is needed to perform the command.
> 
> 
>> --dump device-get
>> [{'clock-id': 282574471561216,
>>   'id': 0,
>>   'lock-status': 'unlocked',
>>   'mode': 'automatic',
>>   'module-name': 'ice',
>>   'type': 'eec'},
>> {'clock-id': 282574471561216,
>>   'id': 1,
>>   'lock-status': 'unlocked',
>>   'mode': 'automatic',
>>   'module-name': 'ice',
>>   'type': 'pps'}]
>>
>> - get single pin info:
>> $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>> --do pin-get --json '{"pin-id":2}'
>> {'clock-id': 282574471561216,
>> 'module-name': 'ice',
>> 'pin-board-label': 'C827_0-RCLKA',
>> 'pin-dpll-caps': 6,
>> 'pin-frequency': 1953125,
>> 'pin-id': 2,
>> 'pin-parenti-device': [{'id': 0,
> 
> This looks like manual edit went wrong :)
> s/parenti/parent/
> 

Ahhh... yeah :)

> 
>>                          'pin-direction': 'input',
>>                          'pin-prio': 11,
>>                          'pin-state': 'selectable'},
>>                         {'id': 1,
>>                          'pin-direction': 'input',
>>                          'pin-prio': 9,
>>                          'pin-state': 'selectable'}],
>> 'pin-type': 'mux'}
>>
>> - set pin's state on dpll:
>> $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>> --do pin-set --json '{"pin-id":2, "pin-parent-device":{"id":1, "pin-state":2}}'
>>
>> - set pin's prio on dpll:
>> $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>> --do pin-set --json '{"pin-id":2, "pin-parent-device":{"id":1, "pin-prio":4}}'
>>
>> - set pin's state on parent pin:
>> $# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>> --do pin-set --json '{"pin-id":13, \
>>                       "pin-parent-pin":{"pin-id":2, "pin-state":1}}'
>>
> 
> [...]


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ