| 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
| ||
|
Date: Tue, 11 Oct 2022 09:28:50 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Vadim Fedorenko <vfedorenko@...ek.ru>
Cc: Jakub Kicinski <kuba@...nel.org>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-clk@...r.kernel.org, Vadim Fedorenko <vadfed@...com>
Subject: Re: [RFC PATCH v3 4/6] dpll: get source/output name
Mon, Oct 10, 2022 at 09:55:57PM CEST, vfedorenko@...ek.ru wrote:
>On 10.10.2022 10:45, Jiri Pirko wrote:
>> Mon, Oct 10, 2022 at 03:18:02AM CEST, vfedorenko@...ek.ru wrote:
>> > From: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>> >
>> > Dump names of sources and outputs in response to DPLL_CMD_DEVICE_GET dump
>> > request.
>> >
>> > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>> > ---
>> > drivers/dpll/dpll_netlink.c | 24 ++++++++++++++++++++++++
>> > include/linux/dpll.h | 2 ++
>> > include/uapi/linux/dpll.h | 2 ++
>> > 3 files changed, 28 insertions(+)
>> >
>> > diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>> > index a5779871537a..e3604c10b59e 100644
>> > --- a/drivers/dpll/dpll_netlink.c
>> > +++ b/drivers/dpll/dpll_netlink.c
>> > @@ -31,12 +31,16 @@ static const struct nla_policy dpll_genl_set_source_policy[] = {
>> > [DPLLA_DEVICE_ID] = { .type = NLA_U32 },
>> > [DPLLA_SOURCE_ID] = { .type = NLA_U32 },
>> > [DPLLA_SOURCE_TYPE] = { .type = NLA_U32 },
>> > + [DPLLA_SOURCE_NAME] = { .type = NLA_STRING,
>> > + .len = DPLL_NAME_LENGTH },
>> > };
>> >
>> > static const struct nla_policy dpll_genl_set_output_policy[] = {
>> > [DPLLA_DEVICE_ID] = { .type = NLA_U32 },
>> > [DPLLA_OUTPUT_ID] = { .type = NLA_U32 },
>> > [DPLLA_OUTPUT_TYPE] = { .type = NLA_U32 },
>> > + [DPLLA_OUTPUT_NAME] = { .type = NLA_STRING,
>> > + .len = DPLL_NAME_LENGTH },
>> > };
>> >
>> > static const struct nla_policy dpll_genl_set_src_select_mode_policy[] = {
>> > @@ -100,6 +104,7 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll,
>> > {
>> > int i, ret = 0, type, prio;
>> > struct nlattr *src_attr;
>> > + const char *name;
>> >
>> > for (i = 0; i < dpll->sources_count; i++) {
>> > src_attr = nla_nest_start(msg, DPLLA_SOURCE);
>> > @@ -132,6 +137,15 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll,
>> > break;
>> > }
>> > }
>> > + if (dpll->ops->get_source_name) {
>> > + name = dpll->ops->get_source_name(dpll, i);
>> > + if (name && nla_put_string(msg, DPLLA_SOURCE_NAME,
>> > + name)) {
>> > + nla_nest_cancel(msg, src_attr);
>> > + ret = -EMSGSIZE;
>> > + break;
>> > + }
>> > + }
>> > nla_nest_end(msg, src_attr);
>> > }
>> >
>> > @@ -143,6 +157,7 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll,
>> > {
>> > struct nlattr *out_attr;
>> > int i, ret = 0, type;
>> > + const char *name;
>> >
>> > for (i = 0; i < dpll->outputs_count; i++) {
>> > out_attr = nla_nest_start(msg, DPLLA_OUTPUT);
>> > @@ -167,6 +182,15 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll,
>> > }
>> > ret = 0;
>> > }
>> > + if (dpll->ops->get_output_name) {
>> > + name = dpll->ops->get_output_name(dpll, i);
>> > + if (name && nla_put_string(msg, DPLLA_OUTPUT_NAME,
>> > + name)) {
>> > + nla_nest_cancel(msg, out_attr);
>> > + ret = -EMSGSIZE;
>> > + break;
>> > + }
>> > + }
>> > nla_nest_end(msg, out_attr);
>> > }
>> >
>> > diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>> > index 3fe957a06b90..2f4964dc28f0 100644
>> > --- a/include/linux/dpll.h
>> > +++ b/include/linux/dpll.h
>> > @@ -23,6 +23,8 @@ struct dpll_device_ops {
>> > int (*set_output_type)(struct dpll_device *dpll, int id, int val);
>> > int (*set_source_select_mode)(struct dpll_device *dpll, int mode);
>> > int (*set_source_prio)(struct dpll_device *dpll, int id, int prio);
>> > + const char *(*get_source_name)(struct dpll_device *dpll, int id);
>> > + const char *(*get_output_name)(struct dpll_device *dpll, int id);
>>
>> Hmm, why you exactly need the name for?
>>
>As with device name, user-space app can use source/output name to easily
>select one using configuration value, for example.
Can you give me some examples? I can't imagine how it can be named
differently than containing the "type string". My point is to avoid
drivers to come up with original names that basically have the same
meaning creating a mess.
Why index (which is predictable and could be used in scripts) isn't
enough?
Or, alternatively, we can leave the "resolve" up to the userspace app.
Example:
index 0
type 1PPS
index 1
type 10MHZ
index 2
type SyncE
ifindex 20
index 3
type SyncE
ifindex 30
Now when user does:
1) dpll X source set index 0
could be equivalent to
dpll X source set 1pps
dpll app would do dump of all sources and if there is only one of
type 1pps, it will resolve it to index 0 which it is going to send
via ATTR_INDEX to kernel.
2) dpll X source set index 2
could be equivalent to
dpll X source set eth1
dpll app would use rtnetlink to get ifindex 20 for eth1. Then it
will do dump of all sources and see if there is SyncE source with
ifindex 30. This will resolve to index 2 which it is going to send
via ATTR_INDEX to kernel.
>>
>> > };
>> >
>> > struct dpll_device *dpll_device_alloc(struct dpll_device_ops *ops, const char *name,
>> > diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
>> > index f6b674e5cf01..8782d3425aae 100644
>> > --- a/include/uapi/linux/dpll.h
>> > +++ b/include/uapi/linux/dpll.h
>> > @@ -26,11 +26,13 @@ enum dpll_genl_attr {
>> > DPLLA_SOURCE,
>> > DPLLA_SOURCE_ID,
>> > DPLLA_SOURCE_TYPE,
>> > + DPLLA_SOURCE_NAME,
>> > DPLLA_SOURCE_SUPPORTED,
>> > DPLLA_SOURCE_PRIO,
>> > DPLLA_OUTPUT,
>> > DPLLA_OUTPUT_ID,
>> > DPLLA_OUTPUT_TYPE,
>> > + DPLLA_OUTPUT_NAME,
>> > DPLLA_OUTPUT_SUPPORTED,
>> > DPLLA_STATUS,
>> > DPLLA_TEMP,
>> > --
>> > 2.27.0
>> >
>
Powered by blists - more mailing lists