| 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: <ZwaHgKED9kHvWeHP@nanopsycho.orion>
Date: Wed, 9 Oct 2024 15:39:12 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: netdev@...r.kernel.org, Maciek Machnikowski <maciek@...hnikowski.net>,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, donald.hunter@...il.com,
arkadiusz.kubalewski@...el.com, saeedm@...dia.com, leon@...nel.org,
tariqt@...dia.com
Subject: Re: [PATCH net-next 1/2] dpll: add clock quality level attribute and
op
Wed, Oct 09, 2024 at 03:33:13PM CEST, vadim.fedorenko@...ux.dev wrote:
>On 09/10/2024 13:25, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...dia.com>
>>
>> In order to allow driver expose quality level of the clock it is
>> running, introduce a new netlink attr with enum to carry it to the
>> userspace. Also, introduce an op the dpll netlink code calls into the
>> driver to obtain the value.
>
>The idea is good, it matches with the work Maciek is doing now in terms
>of improving POSIX clock interface. See a comment below.
>
>> Signed-off-by: Jiri Pirko <jiri@...dia.com>
>> ---
>> Documentation/netlink/specs/dpll.yaml | 28 +++++++++++++++++++++++++++
>> drivers/dpll/dpll_netlink.c | 22 +++++++++++++++++++++
>> include/linux/dpll.h | 4 ++++
>> include/uapi/linux/dpll.h | 21 ++++++++++++++++++++
>> 4 files changed, 75 insertions(+)
>>
>> diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
>> index f2894ca35de8..77a8e9ddb254 100644
>> --- a/Documentation/netlink/specs/dpll.yaml
>> +++ b/Documentation/netlink/specs/dpll.yaml
>> @@ -85,6 +85,30 @@ definitions:
>> This may happen for example if dpll device was previously
>> locked on an input pin of type PIN_TYPE_SYNCE_ETH_PORT.
>> render-max: true
>> + -
>> + type: enum
>> + name: clock-quality-level
>> + doc: |
>> + level of quality of a clock device.
>> + entries:
>> + -
>> + name: prc
>> + value: 1
>> + -
>> + name: ssu-a
>> + -
>> + name: ssu-b
>> + -
>> + name: eec1
>> + -
>> + name: prtc
>> + -
>> + name: eprtc
>> + -
>> + name: eeec
>> + -
>> + name: eprc
>> + render-max: true
>> -
>> type: const
>> name: temp-divider
>> @@ -252,6 +276,10 @@ attribute-sets:
>> name: lock-status-error
>> type: u32
>> enum: lock-status-error
>> + -
>> + name: clock-quality-level
>> + type: u32
>> + enum: clock-quality-level
>> -
>> name: pin
>> enum-name: dpll_a_pin
>> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>> index fc0280dcddd1..689a6d0ff049 100644
>> --- a/drivers/dpll/dpll_netlink.c
>> +++ b/drivers/dpll/dpll_netlink.c
>> @@ -169,6 +169,25 @@ dpll_msg_add_temp(struct sk_buff *msg, struct dpll_device *dpll,
>> return 0;
>> }
>> +static int
>> +dpll_msg_add_clock_quality_level(struct sk_buff *msg, struct dpll_device *dpll,
>> + struct netlink_ext_ack *extack)
>> +{
>> + const struct dpll_device_ops *ops = dpll_device_ops(dpll);
>> + enum dpll_clock_quality_level ql;
>> + int ret;
>> +
>> + if (!ops->clock_quality_level_get)
>> + return 0;
>> + ret = ops->clock_quality_level_get(dpll, dpll_priv(dpll), &ql, extack);
>> + if (ret)
>> + return ret;
>> + if (nla_put_u32(msg, DPLL_A_CLOCK_QUALITY_LEVEL, ql))
>> + return -EMSGSIZE;
>> +
>> + return 0;
>> +}
>> +
>> static int
>> dpll_msg_add_pin_prio(struct sk_buff *msg, struct dpll_pin *pin,
>> struct dpll_pin_ref *ref,
>> @@ -557,6 +576,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
>> if (ret)
>> return ret;
>> ret = dpll_msg_add_lock_status(msg, dpll, extack);
>> + if (ret)
>> + return ret;
>> + ret = dpll_msg_add_clock_quality_level(msg, dpll, extack);
>> if (ret)
>> return ret;
>> ret = dpll_msg_add_mode(msg, dpll, extack);
>> diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>> index 81f7b623d0ba..e99cdb8ab02c 100644
>> --- a/include/linux/dpll.h
>> +++ b/include/linux/dpll.h
>> @@ -26,6 +26,10 @@ struct dpll_device_ops {
>> struct netlink_ext_ack *extack);
>> int (*temp_get)(const struct dpll_device *dpll, void *dpll_priv,
>> s32 *temp, struct netlink_ext_ack *extack);
>> + int (*clock_quality_level_get)(const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + enum dpll_clock_quality_level *ql,
>> + struct netlink_ext_ack *extack);
>> };
>> struct dpll_pin_ops {
>> diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
>> index b0654ade7b7e..0572f9376da4 100644
>> --- a/include/uapi/linux/dpll.h
>> +++ b/include/uapi/linux/dpll.h
>> @@ -79,6 +79,26 @@ enum dpll_lock_status_error {
>> DPLL_LOCK_STATUS_ERROR_MAX = (__DPLL_LOCK_STATUS_ERROR_MAX - 1)
>> };
>> +/**
>> + * enum dpll_clock_quality_level - if previous status change was done due to a
>> + * failure, this provides information of dpll device lock status error. Valid
>> + * values for DPLL_A_LOCK_STATUS_ERROR attribute
>> + */
>> +enum dpll_clock_quality_level {
>> + DPLL_CLOCK_QUALITY_LEVEL_PRC = 1,
>> + DPLL_CLOCK_QUALITY_LEVEL_SSU_A,
>> + DPLL_CLOCK_QUALITY_LEVEL_SSU_B,
>> + DPLL_CLOCK_QUALITY_LEVEL_EEC1,
>> + DPLL_CLOCK_QUALITY_LEVEL_PRTC,
>> + DPLL_CLOCK_QUALITY_LEVEL_EPRTC,
>> + DPLL_CLOCK_QUALITY_LEVEL_EEEC,
>> + DPLL_CLOCK_QUALITY_LEVEL_EPRC,
>
>I think it would be great to provide some explanation of levels here.
>People coming from SDH area may not be familiar with some of them. Or at
>least mention ITU-T/IEEE recommendations documents to get the meanings
>of these levels.
Okay, I can put a reference to ITU document.
>
>> +
>> + /* private: */
>> + __DPLL_CLOCK_QUALITY_LEVEL_MAX,
>> + DPLL_CLOCK_QUALITY_LEVEL_MAX = (__DPLL_CLOCK_QUALITY_LEVEL_MAX - 1)
>> +};
>> +
>> #define DPLL_TEMP_DIVIDER 1000
>> /**
>> @@ -180,6 +200,7 @@ enum dpll_a {
>> DPLL_A_TEMP,
>> DPLL_A_TYPE,
>> DPLL_A_LOCK_STATUS_ERROR,
>> + DPLL_A_CLOCK_QUALITY_LEVEL,
>> __DPLL_A_MAX,
>> DPLL_A_MAX = (__DPLL_A_MAX - 1)
>
Powered by blists - more mailing lists