From: Petr Oros <poros@redhat.com>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
kuba@kernel.org, jiri@resnulli.us, vadfed@meta.com,
jonathan.lemon@gmail.com, pabeni@redhat.com
Cc: corbet@lwn.net, davem@davemloft.net, edumazet@google.com,
vadfed@fb.com, jesse.brandeburg@intel.com,
anthony.l.nguyen@intel.com, saeedm@nvidia.com, leon@kernel.org,
richardcochran@gmail.com, sj@kernel.org, javierm@redhat.com,
ricardo.canuelo@collabora.com, mst@redhat.com,
tzimmermann@suse.de, michal.michalik@intel.com,
gregkh@linuxfoundation.org, jacek.lawrynowicz@linux.intel.com,
airlied@redhat.com, ogabbay@kernel.org, arnd@arndb.de,
nipun.gupta@amd.com, axboe@kernel.dk, linux@zary.sk,
masahiroy@kernel.org, benjamin.tissoires@redhat.com,
geert+renesas@glider.be, milena.olech@intel.com,
kuniyu@amazon.com, liuhangbin@gmail.com, hkallweit1@gmail.com,
andy.ren@getcruise.com, razor@blackwall.org, idosch@nvidia.com,
lucien.xin@gmail.com, nicolas.dichtel@6wind.com, phil@nwl.cc,
claudiajkang@gmail.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, mschmidt@redhat.com,
linux-clk@vger.kernel.org, vadim.fedorenko@linux.dev,
Jiri Pirko <jiri@nvidia.com>
Subject: Re: [RFC PATCH v8 06/10] netdev: expose DPLL pin handle for netdevice
Date: Mon, 12 Jun 2023 11:17:23 +0200 [thread overview]
Message-ID: <343e2638d2e9b3d13216235f85c2d1dae2634881.camel@redhat.com> (raw)
In-Reply-To: <20230609121853.3607724-7-arkadiusz.kubalewski@intel.com>
Arkadiusz Kubalewski píše v Pá 09. 06. 2023 v 14:18 +0200:
> From: Jiri Pirko <jiri@nvidia.com>
>
> In case netdevice represents a SyncE port, the user needs to
> understand
> the connection between netdevice and associated DPLL pin. There might
> me
> multiple netdevices pointing to the same pin, in case of VF/SF
> implementation.
>
> Add a IFLA Netlink attribute to nest the DPLL pin handle, similar to
> how it is implemented for devlink port. Add a struct dpll_pin pointer
> to netdev and protect access to it by RTNL. Expose
> netdev_dpll_pin_set()
> and netdev_dpll_pin_clear() helpers to the drivers so they can
> set/clear
> the DPLL pin relationship to netdev.
>
> Note that during the lifetime of struct dpll_pin the pin handle does
> not
> change. Therefore it is save to access it lockless. It is drivers
> responsibility to call netdev_dpll_pin_clear() before dpll_pin_put().
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> ---
> drivers/dpll/dpll_netlink.c | 28 ++++++++++++++++++--------
> include/linux/dpll.h | 20 +++++++++++++++++++
> include/linux/netdevice.h | 10 ++++++++++
> include/uapi/linux/if_link.h | 2 ++
> net/core/dev.c | 22 +++++++++++++++++++++
> net/core/rtnetlink.c | 38
> ++++++++++++++++++++++++++++++++++++
> 6 files changed, 112 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dpll/dpll_netlink.c
> b/drivers/dpll/dpll_netlink.c
> index 44d9699c9e6c..e6efc17aaf26 100644
> --- a/drivers/dpll/dpll_netlink.c
> +++ b/drivers/dpll/dpll_netlink.c
> @@ -214,10 +214,9 @@ dpll_msg_add_pin_parents(struct sk_buff *msg,
> struct dpll_pin *pin,
> nest = nla_nest_start(msg, DPLL_A_PIN_PARENT);
> if (!nest)
> return -EMSGSIZE;
> - if (nla_put_u32(msg, DPLL_A_PIN_ID, ppin->id)) {
> - ret = -EMSGSIZE;
> + ret = dpll_msg_add_pin_handle(msg, ppin);
> + if (ret)
> goto nest_cancel;
> - }
> if (nla_put_u8(msg, DPLL_A_PIN_STATE, state)) {
> ret = -EMSGSIZE;
> goto nest_cancel;
> @@ -274,8 +273,9 @@ dpll_cmd_pin_fill_details(struct sk_buff *msg,
> struct dpll_pin *pin,
> const struct dpll_pin_properties *prop = pin->prop;
> int ret;
>
> - if (nla_put_u32(msg, DPLL_A_PIN_ID, pin->id))
> - return -EMSGSIZE;
> + ret = dpll_msg_add_pin_handle(msg, pin);
> + if (ret)
> + return ret;
> if (nla_put_string(msg, DPLL_A_MODULE_NAME, module_name(pin-
> >module)))
> return -EMSGSIZE;
> if (nla_put_64bit(msg, DPLL_A_CLOCK_ID, sizeof(pin-
> >clock_id),
> @@ -301,6 +301,20 @@ dpll_cmd_pin_fill_details(struct sk_buff *msg,
> struct dpll_pin *pin,
> return 0;
> }
>
> +size_t dpll_msg_pin_handle_size(struct dpll_pin *pin)
> +{
> + return nla_total_size(4); /* DPLL_A_PIN_ID */
> +}
> +EXPORT_SYMBOL_GPL(dpll_msg_pin_handle_size);
> +
> +int dpll_msg_add_pin_handle(struct sk_buff *msg, struct dpll_pin
> *pin)
> +{
> + if (nla_put_u32(msg, DPLL_A_PIN_ID, pin->id))
> + return -EMSGSIZE;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dpll_msg_add_pin_handle);
> +
> static int
> __dpll_cmd_pin_dump_one(struct sk_buff *msg, struct dpll_pin *pin,
> struct netlink_ext_ack *extack)
> @@ -690,9 +704,7 @@ dpll_pin_find_from_nlattr(struct genl_info *info,
> struct sk_buff *skb)
> panel_label_attr, package_label_attr);
> if (!pin)
> return -EINVAL;
> - if (nla_put_u32(skb, DPLL_A_PIN_ID, pin->id))
> - return -EMSGSIZE;
> - return 0;
> + return dpll_msg_add_pin_handle(skb, pin);
> }
>
> int dpll_nl_pin_id_get_doit(struct sk_buff *skb, struct genl_info
> *info)
> diff --git a/include/linux/dpll.h b/include/linux/dpll.h
> index a18bcaa13553..8d085dc92cdd 100644
> --- a/include/linux/dpll.h
> +++ b/include/linux/dpll.h
> @@ -108,6 +108,26 @@ struct dpll_pin_properties {
> struct dpll_pin_frequency *freq_supported;
> };
>
> +#if IS_ENABLED(CONFIG_DPLL)
> +
> +size_t dpll_msg_pin_handle_size(struct dpll_pin *pin);
> +
> +int dpll_msg_add_pin_handle(struct sk_buff *msg, struct dpll_pin
> *pin);
> +
> +#else
> +
> +static inline size_t dpll_msg_pin_handle_size(struct dpll_pin *pin)
> +{
> + return 0;
> +}
> +
> +static inline int dpll_msg_add_pin_handle(struct sk_buff *msg,
> struct dpll_pin *pin)
> +{
> + return 0;
> +}
> +
> +#endif
> +
> struct dpll_device
> *dpll_device_get(u64 clock_id, u32 dev_driver_id, struct module
> *module);
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 08fbd4622ccf..c57723b12f75 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -34,6 +34,7 @@
> #include <linux/rculist.h>
> #include <linux/workqueue.h>
> #include <linux/dynamic_queue_limits.h>
> +#include <linux/dpll.h>
>
> #include <net/net_namespace.h>
> #ifdef CONFIG_DCB
> @@ -2055,6 +2056,9 @@ enum netdev_ml_priv_type {
> * SET_NETDEV_DEVLINK_PORT macro. This pointer
> is static
> * during the time netdevice is registered.
> *
> + * @dpll_pin: Pointer to the SyncE source pin of a DPLL
> subsystem,
> + * where the clock is recovered.
> + *
> * FIXME: cleanup struct net_device such that network protocol
> info
> * moves out.
> */
> @@ -2411,6 +2415,10 @@ struct net_device {
> struct rtnl_hw_stats64 *offload_xstats_l3;
>
> struct devlink_port *devlink_port;
> +
> +#if IS_ENABLED(CONFIG_DPLL)
> + struct dpll_pin *dpll_pin;
> +#endif
> };
> #define to_net_dev(d) container_of(d, struct net_device, dev)
>
> @@ -3954,6 +3962,8 @@ int dev_get_mac_address(struct sockaddr *sa,
> struct net *net, char *dev_name);
> int dev_get_port_parent_id(struct net_device *dev,
> struct netdev_phys_item_id *ppid, bool
> recurse);
> bool netdev_port_same_parent_id(struct net_device *a, struct
> net_device *b);
> +void netdev_dpll_pin_set(struct net_device *dev, struct dpll_pin
> *dpll_pin);
> +void netdev_dpll_pin_clear(struct net_device *dev);
> struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct
> net_device *dev, bool *again);
> struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct
> net_device *dev,
> struct netdev_queue *txq, int
> *ret);
> diff --git a/include/uapi/linux/if_link.h
> b/include/uapi/linux/if_link.h
> index 0f6a0fe09bdb..be03c8292cd7 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -377,6 +377,8 @@ enum {
> IFLA_GSO_IPV4_MAX_SIZE,
> IFLA_GRO_IPV4_MAX_SIZE,
>
> + IFLA_DPLL_PIN,
> +
> __IFLA_MAX
> };
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 99d99b247bc9..7ae0ce75a5c7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -8993,6 +8993,28 @@ bool netdev_port_same_parent_id(struct
> net_device *a, struct net_device *b)
> }
> EXPORT_SYMBOL(netdev_port_same_parent_id);
>
> +static void netdev_dpll_pin_assign(struct net_device *dev, struct
> dpll_pin *dpll_pin)
> +{
> +#if IS_ENABLED(CONFIG_DPLL)
> + rtnl_lock();
> + dev->dpll_pin = dpll_pin;
> + rtnl_unlock();
> +#endif
> +}
> +
> +void netdev_dpll_pin_set(struct net_device *dev, struct dpll_pin
> *dpll_pin)
> +{
> + WARN_ON(!dpll_pin);
> + netdev_dpll_pin_assign(dev, dpll_pin);
> +}
> +EXPORT_SYMBOL(netdev_dpll_pin_set);
> +
> +void netdev_dpll_pin_clear(struct net_device *dev)
> +{
> + netdev_dpll_pin_assign(dev, NULL);
> +}
> +EXPORT_SYMBOL(netdev_dpll_pin_clear);
> +
> /**
> * dev_change_proto_down - set carrier according to proto_down.
> *
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 41de3a2f29e1..ebe9ae8608fc 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1052,6 +1052,16 @@ static size_t rtnl_devlink_port_size(const
> struct net_device *dev)
> return size;
> }
>
> +static size_t rtnl_dpll_pin_size(const struct net_device *dev)
> +{
> + size_t size = nla_total_size(0); /* nest IFLA_DPLL_PIN */
> +
> + if (dev->dpll_pin)
> + size += dpll_msg_pin_handle_size(dev->dpll_pin);
Hi Arkadiusz,
net_device->dpll_pin is only valid if IS_ENABLED(CONFIG_DPLL)
But the code in net/core/rtnetlink.c doesn't respect that.
If CONFIG_DPLL is not set, net/core/rtnetlink.c cannot be compiled.
Regards,
Petr
> +
> + return size;
> +}
> +
> static noinline size_t if_nlmsg_size(const struct net_device *dev,
> u32 ext_filter_mask)
> {
> @@ -1108,6 +1118,7 @@ static noinline size_t if_nlmsg_size(const
> struct net_device *dev,
> + rtnl_prop_list_size(dev)
> + nla_total_size(MAX_ADDR_LEN) /* IFLA_PERM_ADDRESS */
> + rtnl_devlink_port_size(dev)
> + + rtnl_dpll_pin_size(dev)
> + 0;
> }
>
> @@ -1769,6 +1780,30 @@ static int rtnl_fill_devlink_port(struct
> sk_buff *skb,
> return ret;
> }
>
> +static int rtnl_fill_dpll_pin(struct sk_buff *skb,
> + const struct net_device *dev)
> +{
> + struct nlattr *dpll_pin_nest;
> + int ret;
> +
> + dpll_pin_nest = nla_nest_start(skb, IFLA_DPLL_PIN);
> + if (!dpll_pin_nest)
> + return -EMSGSIZE;
> +
> + if (dev->dpll_pin) {
> + ret = dpll_msg_add_pin_handle(skb, dev->dpll_pin);
> + if (ret < 0)
> + goto nest_cancel;
> + }
> +
> + nla_nest_end(skb, dpll_pin_nest);
> + return 0;
> +
> +nest_cancel:
> + nla_nest_cancel(skb, dpll_pin_nest);
> + return ret;
> +}
> +
> static int rtnl_fill_ifinfo(struct sk_buff *skb,
> struct net_device *dev, struct net
> *src_net,
> int type, u32 pid, u32 seq, u32 change,
> @@ -1911,6 +1946,9 @@ static int rtnl_fill_ifinfo(struct sk_buff
> *skb,
> if (rtnl_fill_devlink_port(skb, dev))
> goto nla_put_failure;
>
> + if (rtnl_fill_dpll_pin(skb, dev))
> + goto nla_put_failure;
> +
> nlmsg_end(skb, nlh);
> return 0;
>
next prev parent reply other threads:[~2023-06-12 9:24 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-09 12:18 [RFC PATCH v8 00/10] Create common DPLL configuration API Arkadiusz Kubalewski
2023-06-09 12:18 ` [RFC PATCH v8 01/10] dpll: documentation on DPLL subsystem interface Arkadiusz Kubalewski
2023-06-10 3:22 ` Bagas Sanjaya
2023-06-12 14:24 ` Kubalewski, Arkadiusz
2023-06-10 16:28 ` Jiri Pirko
2023-06-12 15:24 ` Kubalewski, Arkadiusz
2023-06-12 22:30 ` Jakub Kicinski
2023-06-12 15:30 ` Bart Van Assche
2023-06-12 16:01 ` Kubalewski, Arkadiusz
2023-06-12 22:43 ` Jakub Kicinski
2023-06-13 9:55 ` Jiri Pirko
2023-06-13 16:38 ` Jakub Kicinski
2023-06-14 9:27 ` Jiri Pirko
2023-06-14 12:21 ` Kubalewski, Arkadiusz
2023-06-14 19:15 ` Jakub Kicinski
2023-06-14 19:23 ` Jakub Kicinski
2023-06-15 10:18 ` Jiri Pirko
2023-06-15 13:44 ` Kubalewski, Arkadiusz
2023-06-15 16:31 ` Jakub Kicinski
2023-06-17 10:36 ` Jiri Pirko
2023-06-12 23:49 ` Jakub Kicinski
2023-06-14 12:23 ` Kubalewski, Arkadiusz
2023-06-09 12:18 ` [RFC PATCH v8 02/10] dpll: spec: Add Netlink spec in YAML Arkadiusz Kubalewski
2023-06-10 16:22 ` Jiri Pirko
2023-06-15 13:42 ` Kubalewski, Arkadiusz
2023-06-09 12:18 ` [RFC PATCH v8 03/10] dpll: core: Add DPLL framework base functions Arkadiusz Kubalewski
2023-06-10 17:38 ` Jiri Pirko
2023-06-21 16:28 ` Kubalewski, Arkadiusz
2023-06-21 18:55 ` Kubalewski, Arkadiusz
2023-06-22 7:05 ` Jiri Pirko
2023-07-17 19:03 ` Vadim Fedorenko
2023-06-11 9:36 ` Jiri Pirko
2023-06-12 7:25 ` Paolo Abeni
2023-06-21 20:38 ` Kubalewski, Arkadiusz
2023-06-11 10:01 ` Jiri Pirko
2023-06-12 23:45 ` Jakub Kicinski
2023-06-21 21:17 ` Kubalewski, Arkadiusz
2023-06-22 7:09 ` Jiri Pirko
2023-06-09 12:18 ` [RFC PATCH v8 04/10] dpll: netlink: " Arkadiusz Kubalewski
2023-06-11 11:42 ` Jiri Pirko
2023-06-23 1:01 ` Kubalewski, Arkadiusz
2023-06-21 11:18 ` Petr Oros
2023-06-21 11:53 ` Jiri Pirko
2023-06-21 13:07 ` Jiri Pirko
2023-06-23 0:56 ` Kubalewski, Arkadiusz
2023-06-23 7:48 ` Jiri Pirko
2023-06-23 0:56 ` Kubalewski, Arkadiusz
2023-06-09 12:18 ` [RFC PATCH v8 05/10] dpll: api header: " Arkadiusz Kubalewski
2023-06-10 7:25 ` Jiri Pirko
2023-06-10 7:29 ` Jiri Pirko
2023-06-12 15:00 ` Vadim Fedorenko
2023-06-10 7:32 ` Jiri Pirko
2023-06-09 12:18 ` [RFC PATCH v8 06/10] netdev: expose DPLL pin handle for netdevice Arkadiusz Kubalewski
2023-06-12 9:17 ` Petr Oros [this message]
2023-06-13 13:51 ` Jiri Pirko
2023-06-14 12:25 ` Kubalewski, Arkadiusz
2023-06-09 12:18 ` [RFC PATCH v8 07/10] ice: add admin commands to access cgu configuration Arkadiusz Kubalewski
2023-06-10 8:46 ` Jiri Pirko
2023-06-15 21:35 ` Kubalewski, Arkadiusz
2023-06-09 12:18 ` [RFC PATCH v8 08/10] ice: implement dpll interface to control cgu Arkadiusz Kubalewski
2023-06-10 9:57 ` Jiri Pirko
2023-06-19 18:08 ` Kubalewski, Arkadiusz
2023-06-21 12:28 ` Jiri Pirko
2023-06-10 16:36 ` Jiri Pirko
2023-06-19 20:34 ` Kubalewski, Arkadiusz
2023-06-21 12:29 ` Jiri Pirko
2023-06-29 6:14 ` Jiri Pirko
2023-07-03 12:37 ` Kubalewski, Arkadiusz
2023-07-10 8:23 ` Jiri Pirko
2023-06-09 12:18 ` [RFC PATCH v8 09/10] ptp_ocp: implement DPLL ops Arkadiusz Kubalewski
2023-06-10 8:06 ` Jiri Pirko
2023-06-09 12:18 ` [RFC PATCH v8 10/10] mlx5: Implement SyncE support using DPLL infrastructure Arkadiusz Kubalewski
2023-06-09 23:27 ` [RFC PATCH v8 00/10] Create common DPLL configuration API Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=343e2638d2e9b3d13216235f85c2d1dae2634881.camel@redhat.com \
--to=poros@redhat.com \
--cc=airlied@redhat.com \
--cc=andy.ren@getcruise.com \
--cc=anthony.l.nguyen@intel.com \
--cc=arkadiusz.kubalewski@intel.com \
--cc=arnd@arndb.de \
--cc=axboe@kernel.dk \
--cc=benjamin.tissoires@redhat.com \
--cc=claudiajkang@gmail.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=geert+renesas@glider.be \
--cc=gregkh@linuxfoundation.org \
--cc=hkallweit1@gmail.com \
--cc=idosch@nvidia.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacek.lawrynowicz@linux.intel.com \
--cc=javierm@redhat.com \
--cc=jesse.brandeburg@intel.com \
--cc=jiri@nvidia.com \
--cc=jiri@resnulli.us \
--cc=jonathan.lemon@gmail.com \
--cc=kuba@kernel.org \
--cc=kuniyu@amazon.com \
--cc=leon@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux@zary.sk \
--cc=liuhangbin@gmail.com \
--cc=lucien.xin@gmail.com \
--cc=masahiroy@kernel.org \
--cc=michal.michalik@intel.com \
--cc=milena.olech@intel.com \
--cc=mschmidt@redhat.com \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=nicolas.dichtel@6wind.com \
--cc=nipun.gupta@amd.com \
--cc=ogabbay@kernel.org \
--cc=pabeni@redhat.com \
--cc=phil@nwl.cc \
--cc=razor@blackwall.org \
--cc=ricardo.canuelo@collabora.com \
--cc=richardcochran@gmail.com \
--cc=saeedm@nvidia.com \
--cc=sj@kernel.org \
--cc=tzimmermann@suse.de \
--cc=vadfed@fb.com \
--cc=vadfed@meta.com \
--cc=vadim.fedorenko@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).