| 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: <DM6PR11MB4657FADDDA75A5F35CF8FE3F9B9D9@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Tue, 2 Aug 2022 14:02:31 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Vadim Fedorenko <vfedorenko@...ek.ru>,
Jakub Kicinski <kuba@...nel.org>,
Jonathan Lemon <jonathan.lemon@...il.com>
CC: Vadim Fedorenko <vadfed@...com>, Aya Levin <ayal@...dia.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
Subject: RE: [RFC PATCH v2 2/3] dpll: add netlink events
>-----Original Message-----
>From: Vadim Fedorenko <vfedorenko@...ek.ru>
>Sent: Friday, July 15, 2022 1:29 AM
>>
>>On 11.07.2022 10:02, Kubalewski, Arkadiusz wrote:
>>> -----Original Message-----
>>> From: Vadim Fedorenko <vfedorenko@...ek.ru>
>>> Sent: Sunday, June 26, 2022 9:25 PM
>>>>
>>>> From: Vadim Fedorenko <vadfed@...com>
>>>>
>>>> Add netlink interface to enable notification of users about
>>>> events in DPLL framework. Part of this interface should be
>>>> used by drivers directly, i.e. lock status changes.
>>>>
>>>> Signed-off-by: Vadim Fedorenko <vadfed@...com>
>>>> ---
>>>> drivers/dpll/dpll_core.c | 2 +
>>>> drivers/dpll/dpll_netlink.c | 141 ++++++++++++++++++++++++++++++++++++
>>>> drivers/dpll/dpll_netlink.h | 7 ++
>>>> 3 files changed, 150 insertions(+)
>>>>
>>>> diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>>>> index dc0330e3687d..387644aa910e 100644
>>>> --- a/drivers/dpll/dpll_core.c
>>>> +++ b/drivers/dpll/dpll_core.c
>>>> @@ -97,6 +97,8 @@ struct dpll_device *dpll_device_alloc(struct dpll_device_ops *ops, int sources_c
>>>> mutex_unlock(&dpll_device_xa_lock);
>>>> dpll->priv = priv;
>>>>
>>>> + dpll_notify_device_create(dpll->id, dev_name(&dpll->dev));
>>>> +
>>>> return dpll;
>>>>
>>>> error:
>>>> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>>>> index e15106f30377..4b1684fcf41e 100644
>>>> --- a/drivers/dpll/dpll_netlink.c
>>>> +++ b/drivers/dpll/dpll_netlink.c
>>>> @@ -48,6 +48,8 @@ struct param {
>>>> int dpll_source_type;
>>>> int dpll_output_id;
>>>> int dpll_output_type;
>>>> + int dpll_status;
>>>> + const char *dpll_name;
>>>> };
>>>>
>>>> struct dpll_dump_ctx {
>>>> @@ -239,6 +241,8 @@ static int dpll_genl_cmd_set_source(struct param *p)
>>>> ret = dpll->ops->set_source_type(dpll, src_id, type);
>>>> mutex_unlock(&dpll->lock);
>>>>
>>>> + dpll_notify_source_change(dpll->id, src_id, type);
>>>> +
>>>> return ret;
>>>> }
>>>>
>>>> @@ -262,6 +266,8 @@ static int dpll_genl_cmd_set_output(struct param *p)
>>>> ret = dpll->ops->set_source_type(dpll, out_id, type);
>>>> mutex_unlock(&dpll->lock);
>>>>
>>>> + dpll_notify_source_change(dpll->id, out_id, type);
>>>> +
>>>> return ret;
>>>> }
>>>>
>>>> @@ -438,6 +444,141 @@ static struct genl_family dpll_gnl_family __ro_after_init = {
>>>> .pre_doit = dpll_pre_doit,
>>>> };
>>>>
>>>> +static int dpll_event_device_create(struct param *p)
>>>> +{
>>>> + if (nla_put_u32(p->msg, DPLLA_DEVICE_ID, p->dpll_id) ||
>>>> + nla_put_string(p->msg, DPLLA_DEVICE_NAME, p->dpll_name))
>>>> + return -EMSGSIZE;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int dpll_event_device_delete(struct param *p)
>>>> +{
>>>> + if (nla_put_u32(p->msg, DPLLA_DEVICE_ID, p->dpll_id))
>>>> + return -EMSGSIZE;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int dpll_event_status(struct param *p)
>>>> +{
>>>> + if (nla_put_u32(p->msg, DPLLA_DEVICE_ID, p->dpll_id) ||
>>>> + nla_put_u32(p->msg, DPLLA_LOCK_STATUS, p->dpll_status))
>>>> + return -EMSGSIZE;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int dpll_event_source_change(struct param *p)
>>>> +{
>>>> + if (nla_put_u32(p->msg, DPLLA_DEVICE_ID, p->dpll_id) ||
>>>> + nla_put_u32(p->msg, DPLLA_SOURCE_ID, p->dpll_source_id) ||
>>>> + nla_put_u32(p->msg, DPLLA_SOURCE_TYPE, p->dpll_source_type))
>>>> + return -EMSGSIZE;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int dpll_event_output_change(struct param *p)
>>>> +{
>>>> + if (nla_put_u32(p->msg, DPLLA_DEVICE_ID, p->dpll_id) ||
>>>> + nla_put_u32(p->msg, DPLLA_OUTPUT_ID, p->dpll_output_id) ||
>>>> + nla_put_u32(p->msg, DPLLA_OUTPUT_TYPE, p->dpll_output_type))
>>>> + return -EMSGSIZE;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static cb_t event_cb[] = {
>>>> + [DPLL_EVENT_DEVICE_CREATE] = dpll_event_device_create,
>>>> + [DPLL_EVENT_DEVICE_DELETE] = dpll_event_device_delete,
>>>> + [DPLL_EVENT_STATUS_LOCKED] = dpll_event_status,
>>>> + [DPLL_EVENT_STATUS_UNLOCKED] = dpll_event_status,
>>>> + [DPLL_EVENT_SOURCE_CHANGE] = dpll_event_source_change,
>>>> + [DPLL_EVENT_OUTPUT_CHANGE] = dpll_event_output_change,
>>>> +};
>>>> +/*
>>>> + * Generic netlink DPLL event encoding
>>>> + */
>>>> +static int dpll_send_event(enum dpll_genl_event event,
>>>> + struct param *p)
>>>> +{
>>>> + struct sk_buff *msg;
>>>> + int ret = -EMSGSIZE;
>>>> + void *hdr;
>>>> +
>>>> + msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
>>>> + if (!msg)
>>>> + return -ENOMEM;
>>>> + p->msg = msg;
>>>> +
>>>> + hdr = genlmsg_put(msg, 0, 0, &dpll_gnl_family, 0, event);
>>>> + if (!hdr)
>>>> + goto out_free_msg;
>>>> +
>>>> + ret = event_cb[event](p);
>>>> + if (ret)
>>>> + goto out_cancel_msg;
>>>> +
>>>> + genlmsg_end(msg, hdr);
>>>> +
>>>> + genlmsg_multicast(&dpll_gnl_family, msg, 0, 1, GFP_KERNEL);
>>>
>>> All multicasts are send only for group "1" (DPLL_CONFIG_SOURCE_GROUP_NAME),
>>> but 4 groups were defined.
>>>
>>
>>Yes, you are right! Will update it in the next round.
>>
>>>> +
>>>> + return 0;
>>>> +
>>>> +out_cancel_msg:
>>>> + genlmsg_cancel(msg, hdr);
>>>> +out_free_msg:
>>>> + nlmsg_free(msg);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +int dpll_notify_device_create(int dpll_id, const char *name)
>>>> +{
>>>> + struct param p = { .dpll_id = dpll_id, .dpll_name = name };
>>>> +
>>>> + return dpll_send_event(DPLL_EVENT_DEVICE_CREATE, &p);
>>>> +}
>>>> +
>>>> +int dpll_notify_device_delete(int dpll_id)
>>>> +{
>>>> + struct param p = { .dpll_id = dpll_id };
>>>> +
>>>> + return dpll_send_event(DPLL_EVENT_DEVICE_DELETE, &p);
>>>> +}
>>>> +
>>>> +int dpll_notify_status_locked(int dpll_id)
>>>> +{
>>>> + struct param p = { .dpll_id = dpll_id, .dpll_status = 1 };
>>>> +
>>>> + return dpll_send_event(DPLL_EVENT_STATUS_LOCKED, &p);
>>>> +}
>>>> +
>>>> +int dpll_notify_status_unlocked(int dpll_id)
>>>> +{
>>>> + struct param p = { .dpll_id = dpll_id, .dpll_status = 0 };
>>>> +
>>>> + return dpll_send_event(DPLL_EVENT_STATUS_UNLOCKED, &p);
>>>> +}
>>>> +
>>>> +int dpll_notify_source_change(int dpll_id, int source_id, int source_type)
>>>> +{
>>>> + struct param p = { .dpll_id = dpll_id, .dpll_source_id = source_id,
>>>> + .dpll_source_type = source_type };
>>>> +
>>>> + return dpll_send_event(DPLL_EVENT_SOURCE_CHANGE, &p);
>>>> +}
>>>> +
>>>> +int dpll_notify_output_change(int dpll_id, int output_id, int output_type)
>>>> +{
>>>> + struct param p = { .dpll_id = dpll_id, .dpll_output_id = output_id,
>>>> + .dpll_output_type = output_type };
>>>> +
>>>> + return dpll_send_event(DPLL_EVENT_OUTPUT_CHANGE, &p);
>>>> +}
>>>> +
>>>> int __init dpll_netlink_init(void)
>>>> {
>>>> return genl_register_family(&dpll_gnl_family);
>>>> diff --git a/drivers/dpll/dpll_netlink.h b/drivers/dpll/dpll_netlink.h
>>>> index e2d100f59dd6..0dc81320f982 100644
>>>> --- a/drivers/dpll/dpll_netlink.h
>>>> +++ b/drivers/dpll/dpll_netlink.h
>>>> @@ -3,5 +3,12 @@
>>>> * Copyright (c) 2021 Meta Platforms, Inc. and affiliates
>>>> */
>>>>
>>>> +int dpll_notify_device_create(int dpll_id, const char *name);
>>>> +int dpll_notify_device_delete(int dpll_id);
>>>> +int dpll_notify_status_locked(int dpll_id);
>>>> +int dpll_notify_status_unlocked(int dpll_id);
>>>> +int dpll_notify_source_change(int dpll_id, int source_id, int source_type);
>>>> +int dpll_notify_output_change(int dpll_id, int output_id, int output_type);
>>>
>>> Only dpll_notify_device_create is actually used, rest is not.
>>> I am getting confused a bit, who should call those "notify" functions?
>>> It is straightforward for create/delete, dpll subsystem shall do it, but what
>>> about the rest?
>>> I would say notifications about status or source/output change shall originate
>>> in the driver implementing dpll interface, thus they shall be exported and
>>> defined in the header included by the driver.
>>>
>>
>>I was thinking about driver too, because device can have different interfaces to
>>configure source/output, and different notifications to update status. I will
>>update ptp_ocp driver to implement this logic. And it will also cover question
>>of exporting these functions and their definitions.
>>
>
>Great!
>
>Thank,
>Arkadiusz
>>>> +
>>>> int __init dpll_netlink_init(void);
>>>> void dpll_netlink_finish(void);
>>>> --
>>>> 2.27.0
>>>>
>>
>
Good day Vadim,
I just wanted to make sure I didn't miss anything through the spam filters or
something? We are still waiting for that github repo, or you were on
vacation/busy, right?
Thanks,
Arkadiusz
Powered by blists - more mailing lists