| 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: <DM6PR11MB46571C9CAA9CBE56D6A5FCFD9B7F2@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Wed, 9 Oct 2024 13:38:38 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>, "donald.hunter@...il.com"
<donald.hunter@...il.com>, "vadim.fedorenko@...ux.dev"
<vadim.fedorenko@...ux.dev>, "saeedm@...dia.com" <saeedm@...dia.com>,
"leon@...nel.org" <leon@...nel.org>, "tariqt@...dia.com" <tariqt@...dia.com>
Subject: RE: [PATCH net-next 1/2] dpll: add clock quality level attribute and
op
>From: Jiri Pirko <jiri@...nulli.us>
>Sent: Wednesday, October 9, 2024 2:26 PM
>
>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.
>
>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.
Hi Jiri,
Thanks for your work on this!
I do like the idea, but this part is a bit tricky.
I assume it is all about clock/quality levels as mentioned in
ITU-T spec "Table 11-7" of REC-G.8264?
Then what about table 11-8?
And in general about option 2(3?) networks?
AFAIR there are 3 (I don't think 3rd is relevant? But still defined
In REC-G.781, also REC-G.781 doesn't provide clock types at all,
just Quality Levels).
Assuming 2(3?) network options shall be available, either user can
select the one which is shown, or driver just provides all (if can,
one/none otherwise)?
If we don't want to give the user control and just let the
driver to either provide this or not, my suggestion would be to name
the attribute appropriately: "clock-quality-level-o1" to make clear
provided attribute belongs to option 1 network.
Then, if there would be need for different network options, just new
attribute and defines could be introduced without hassle for backward
compatibility.
Does it make sense?
Thank you!
Arkadiusz
>+ 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,
>+
>+ /* 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)
>--
>2.46.1
Powered by blists - more mailing lists