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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <343e2638d2e9b3d13216235f85c2d1dae2634881.camel@redhat.com>
Date: Mon, 12 Jun 2023 11:17:23 +0200
From: Petr Oros <poros@...hat.com>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>, kuba@...nel.org, 
 jiri@...nulli.us, vadfed@...a.com, jonathan.lemon@...il.com,
 pabeni@...hat.com
Cc: corbet@....net, davem@...emloft.net, edumazet@...gle.com, vadfed@...com,
  jesse.brandeburg@...el.com, anthony.l.nguyen@...el.com, saeedm@...dia.com,
  leon@...nel.org, richardcochran@...il.com, sj@...nel.org,
 javierm@...hat.com,  ricardo.canuelo@...labora.com, mst@...hat.com,
 tzimmermann@...e.de,  michal.michalik@...el.com,
 gregkh@...uxfoundation.org,  jacek.lawrynowicz@...ux.intel.com,
 airlied@...hat.com, ogabbay@...nel.org,  arnd@...db.de,
 nipun.gupta@....com, axboe@...nel.dk, linux@...y.sk,  masahiroy@...nel.org,
 benjamin.tissoires@...hat.com, geert+renesas@...der.be, 
 milena.olech@...el.com, kuniyu@...zon.com, liuhangbin@...il.com, 
 hkallweit1@...il.com, andy.ren@...cruise.com, razor@...ckwall.org, 
 idosch@...dia.com, lucien.xin@...il.com, nicolas.dichtel@...nd.com,
 phil@....cc,  claudiajkang@...il.com, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org,  netdev@...r.kernel.org,
 intel-wired-lan@...ts.osuosl.org,  linux-rdma@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,  mschmidt@...hat.com,
 linux-clk@...r.kernel.org, vadim.fedorenko@...ux.dev, Jiri Pirko
 <jiri@...dia.com>
Subject: Re: [RFC PATCH v8 06/10] netdev: expose DPLL pin handle for
 netdevice

Arkadiusz Kubalewski píše v Pá 09. 06. 2023 v 14:18 +0200:
> From: Jiri Pirko <jiri@...dia.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@...dia.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.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;
>  

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ