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: <Zl25Rioa4K2BmYe6@nanopsycho.orion>
Date: Mon, 3 Jun 2024 14:38:30 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	jacob.e.keller@...el.com, michal.kubiak@...el.com,
	maciej.fijalkowski@...el.com, sridhar.samudrala@...el.com,
	przemyslaw.kitszel@...el.com, wojciech.drewek@...el.com,
	pio.raczynski@...il.com, jiri@...dia.com,
	mateusz.polchlopek@...el.com, shayd@...dia.com,
	kalesh-anakkur.purayil@...adcom.com, horms@...nel.org
Subject: Re: [iwl-next v4 05/15] ice: allocate devlink for subfunction

Mon, Jun 03, 2024 at 11:50:15AM CEST, michal.swiatkowski@...ux.intel.com wrote:
>From: Piotr Raczynski <piotr.raczynski@...el.com>
>
>Make devlink allocation function generic to use it for PF and for SF.
>
>Add function for SF devlink port creation. It will be used in next
>patch.
>
>Create header file for subfunction device. Define subfunction device
>structure there as it is needed for devlink allocation and port
>creation.
>
>Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>Signed-off-by: Piotr Raczynski <piotr.raczynski@...el.com>
>Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
>---
> .../net/ethernet/intel/ice/devlink/devlink.c  | 33 +++++++++++++++
> .../net/ethernet/intel/ice/devlink/devlink.h  |  1 +
> .../ethernet/intel/ice/devlink/devlink_port.c | 41 +++++++++++++++++++
> .../ethernet/intel/ice/devlink/devlink_port.h |  3 ++
> drivers/net/ethernet/intel/ice/ice_sf_eth.h   | 21 ++++++++++
> 5 files changed, 99 insertions(+)
> create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.h
>
>diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>index bfb3d5b59a62..00f549daca57 100644
>--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>@@ -10,6 +10,7 @@
> #include "ice_eswitch.h"
> #include "ice_fw_update.h"
> #include "ice_dcb_lib.h"
>+#include "ice_sf_eth.h"
> 
> /* context for devlink info version reporting */
> struct ice_info_ctx {
>@@ -1282,6 +1283,8 @@ static const struct devlink_ops ice_devlink_ops = {
> 	.port_new = ice_devlink_port_new,
> };
> 
>+static const struct devlink_ops ice_sf_devlink_ops;
>+
> static int
> ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
> 			    struct devlink_param_gset_ctx *ctx)
>@@ -1422,6 +1425,7 @@ static void ice_devlink_free(void *devlink_ptr)
>  * Allocate a devlink instance for this device and return the private area as
>  * the PF structure. The devlink memory is kept track of through devres by
>  * adding an action to remove it when unwinding.
>+ *
>  */
> struct ice_pf *ice_allocate_pf(struct device *dev)
> {
>@@ -1438,6 +1442,35 @@ struct ice_pf *ice_allocate_pf(struct device *dev)
> 	return devlink_priv(devlink);
> }
> 
>+/**
>+ * ice_allocate_sf - Allocate devlink and return SF structure pointer
>+ * @dev: the device to allocate for
>+ * @pf: pointer to the PF structure
>+ *
>+ * Allocate a devlink instance for SF.
>+ *
>+ * Return: ice_sf_priv pointer to allocated memory or ERR_PTR in case of error
>+ */
>+struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf)
>+{
>+	struct devlink *devlink;
>+	int err;
>+
>+	devlink = devlink_alloc_ns(&ice_sf_devlink_ops,
>+				   sizeof(struct ice_sf_priv),
>+				   devlink_net(priv_to_devlink(pf)), dev);

I don't think this is correct. This is devlink instance for the actual
SF. It is probed on auxiliary bus. I don't see any reason why the
devlink instance netns should be determined by the PF devlink netns.
For VFs, you also don't do it. In mlx5, the only SF implementation, SF
devlink instances are created in initial netns. Please follow that.



>+	if (!devlink)
>+		return ERR_PTR(-ENOMEM);
>+
>+	err = devl_nested_devlink_set(priv_to_devlink(pf), devlink);
>+	if (err) {
>+		devlink_free(devlink);
>+		return ERR_PTR(err);
>+	}
>+
>+	return devlink_priv(devlink);
>+}
>+
> /**
>  * ice_devlink_register - Register devlink interface for this PF
>  * @pf: the PF to register the devlink for.
>diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.h b/drivers/net/ethernet/intel/ice/devlink/devlink.h
>index d291c0e2e17b..1af3b0763fbb 100644
>--- a/drivers/net/ethernet/intel/ice/devlink/devlink.h
>+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.h
>@@ -5,6 +5,7 @@
> #define _ICE_DEVLINK_H_
> 
> struct ice_pf *ice_allocate_pf(struct device *dev);
>+struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf);
> 
> void ice_devlink_register(struct ice_pf *pf);
> void ice_devlink_unregister(struct ice_pf *pf);
>diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
>index 5d1fe08e4bab..f06baabd0112 100644
>--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
>+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
>@@ -489,6 +489,47 @@ void ice_devlink_destroy_vf_port(struct ice_vf *vf)
> 	devl_port_unregister(&vf->devlink_port);
> }
> 
>+/**
>+ * ice_devlink_create_sf_dev_port - Register virtual port for a subfunction
>+ * @sf_dev: the subfunction device to create a devlink port for
>+ *
>+ * Register virtual flavour devlink port for the subfunction auxiliary device
>+ * created after activating a dynamically added devlink port.
>+ *
>+ * Return: zero on success or an error code on failure.
>+ */
>+int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev)
>+{
>+	struct devlink_port_attrs attrs = {};
>+	struct ice_dynamic_port *dyn_port;
>+	struct devlink_port *devlink_port;
>+	struct devlink *devlink;
>+	struct ice_vsi *vsi;
>+
>+	dyn_port = sf_dev->dyn_port;
>+	vsi = dyn_port->vsi;
>+
>+	devlink_port = &sf_dev->priv->devlink_port;
>+
>+	attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
>+
>+	devlink_port_attrs_set(devlink_port, &attrs);
>+	devlink = priv_to_devlink(sf_dev->priv);
>+
>+	return devl_port_register(devlink, devlink_port, vsi->idx);
>+}
>+
>+/**
>+ * ice_devlink_destroy_sf_dev_port - Destroy virtual port for a subfunction
>+ * @sf_dev: the subfunction device to create a devlink port for
>+ *
>+ * Unregisters the virtual port associated with this subfunction.
>+ */
>+void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
>+{
>+	devl_port_unregister(&sf_dev->priv->devlink_port);
>+}
>+
> /**
>  * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
>  * @dyn_port: dynamic port instance to deallocate
>diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
>index 08ebf56664a5..97b21b58c300 100644
>--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
>+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
>@@ -5,6 +5,7 @@
> #define _DEVLINK_PORT_H_
> 
> #include "../ice.h"
>+#include "../ice_sf_eth.h"
> 
> /**
>  * struct ice_dynamic_port - Track dynamically added devlink port instance
>@@ -34,6 +35,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf);
> void ice_devlink_destroy_vf_port(struct ice_vf *vf);
> int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port);
> void ice_devlink_destroy_sf_port(struct ice_dynamic_port *dyn_port);
>+int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev);
>+void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev);
> 
> #define ice_devlink_port_to_dyn(port) \
> 	container_of(port, struct ice_dynamic_port, devlink_port)
>diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
>new file mode 100644
>index 000000000000..a08f8b2bceef
>--- /dev/null
>+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
>@@ -0,0 +1,21 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/* Copyright (c) 2024, Intel Corporation. */
>+
>+#ifndef _ICE_SF_ETH_H_
>+#define _ICE_SF_ETH_H_
>+
>+#include <linux/auxiliary_bus.h>
>+#include "ice.h"
>+
>+struct ice_sf_dev {
>+	struct auxiliary_device adev;
>+	struct ice_dynamic_port *dyn_port;
>+	struct ice_sf_priv *priv;
>+};
>+
>+struct ice_sf_priv {
>+	struct ice_sf_dev *dev;
>+	struct devlink_port devlink_port;
>+};
>+
>+#endif /* _ICE_SF_ETH_H_ */
>-- 
>2.42.0
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ