diff mbox series

[v2,6/9] ice: enable initial devlink support

Message ID 20200312012726.973301-7-jacob.e.keller@intel.com
State Superseded
Headers show
Series ice devlink support | expand

Commit Message

Jacob Keller March 12, 2020, 1:27 a.m. UTC
Begin implementing support for the devlink interface with the ice
driver.

The pf structure is currently memory managed through devres, via
a devm_alloc. To mimic this behavior, after allocating the devlink
pointer, use devm_add_action to add a teardown action for releasing the
devlink memory on exit.

The ice hardware is a multi-function PCIe device. Thus, each physical
function will get its own devlink instance. This means that each
function will be treated independently, with its own parameters and
configuration. This is done because the ice driver loads a separate
instance for each function.

Due to this, the implementation does not enable devlink to manage
device-wide resources or configuration, as each physical function will
be treated independently. This is done for simplicity, as managing
a devlink instance across multiple driver instances would significantly
increase the complexity for minimal gain.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 drivers/net/ethernet/intel/Kconfig           |   1 +
 drivers/net/ethernet/intel/ice/Makefile      |   1 +
 drivers/net/ethernet/intel/ice/ice.h         |   4 +
 drivers/net/ethernet/intel/ice/ice_devlink.c | 117 +++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_devlink.h |  14 +++
 drivers/net/ethernet/intel/ice/ice_main.c    |  21 +++-
 6 files changed, 157 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/intel/ice/ice_devlink.c
 create mode 100644 drivers/net/ethernet/intel/ice/ice_devlink.h

Comments

Jacob Keller March 12, 2020, 1:34 a.m. UTC | #1
Ugh sorry for the thrash, Jeff, please do not apply this to IWL yet,
looks like I need a v3.

On 3/11/2020 6:27 PM, Jacob Keller wrote:
> +/**
> + * ice_devlink_create_port - Create a devlink port for this PF
> + * @pf: the PF to create a port for
> + *
> + * Create and register a devlink_port for this PF. Note that although each
> + * physical function is connected to a separate devlink instance, the port
> + * will still be numbered according to the physical function id.
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int ice_devlink_create_port(struct ice_pf *pf)
> +{
> +	struct devlink *devlink = priv_to_devlink(pf);
> +	struct ice_vsi *vsi = ice_get_main_vsi(pf);
> +	struct device *dev = ice_pf_to_dev(pf);
> +	int err;
> +
> +	if (!vsi) {
> +		dev_err(dev, "%s: unable to find main VSI\n", __func__);
> +		return -EIO;
> +	}
> +
> +	devlink_port_attrs_set(&pf->devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
> +			       pf->hw.pf_id, false, 0, NULL, 0);
> +	err = devlink_port_register(devlink, &pf->devlink_port, pf->hw.pf_id);
> +	if (err) {
> +		dev_err(dev, "devlink_port_register failed: %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}

<snip>

>  #define DRV_VERSION_MAJOR 0
>  #define DRV_VERSION_MINOR 8
> @@ -2426,6 +2427,8 @@ static int ice_cfg_netdev(struct ice_vsi *vsi)
>  	if (err)
>  		return err;
>  
> +	devlink_port_type_eth_set(&pf->devlink_port, vsi->netdev);
> +

Ugh. I just noticed this when I went to reboot my test system,
apparently this is called before ice_devlink_create_port and generates a
warning:

> [  186.682927] ------------[ cut here ]------------
> [  186.682934] WARNING: CPU: 28 PID: 731 at net/core/devlink.c:6936 __devlink_port_type_set+0x60/0x70
> [  186.682934] Modules linked in: ice(+) ip6t_REJECT nf_reject_ipv6 ip6t_rpfilter ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable_nat ebtable_broute ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat iptable_ma
> ngle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc vfat fat intel_rapl_msr intel_rapl_common isst_if_common skx_edac nfit
>  libnvdimm x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel iTCO_wdt iTCO_vendor_support intel_cstate qat_c62x ipmi_ssif intel_qat intel_uncore mei_me mei intel_rapl
> _perf ioatdma joydev authenc pcspkr i2c_i801 lpc_ich ipmi_si dca ipmi_devintf ipmi_msghandler acpi_power_meter acpi_pad ip_tables xfs libcrc32c ast i2c_algo_bit drm_vram_helper drm_ttm_helper ttm drm_kms_helper cec drm i40e crc32c_intel
> virtual_bus wmi pkcs8_key_parser [last unloaded: ice]
> [  186.682962] CPU: 28 PID: 731 Comm: kworker/28:1 Tainted: G        W         5.6.0-rc3+ #3
> [  186.682963] Hardware name: Intel Corporation S2600STQ/S2600STQ, BIOS SE5C620.86B.02.01.0010.010620200716 01/06/2020
> [  186.682969] Workqueue: events work_for_cpu_fn
> [  186.682971] RIP: 0010:__devlink_port_type_set+0x60/0x70
> [  186.682972] Code: 89 e7 e8 33 52 16 00 44 89 6d 34 4c 89 e7 48 89 5d 40 e8 93 52 16 00 48 89 ef 5b be 07 00 00 00 5d 41 5c 41 5d e9 90 fc ff ff <0f> 0b c3 66 66 2e 0f 1f 84 00 00 00 00 00 66 90 0f 1f 44 00 00 41
> [  186.682973] RSP: 0018:ffffa14c60567d70 EFLAGS: 00010246
> [  186.682974] RAX: 0000000000000000 RBX: ffffffffc05b86c0 RCX: 0000000000000000
> [  186.682974] RDX: ffff8bdffb72b000 RSI: 0000000000000002 RDI: ffff8bdff7568128
> [  186.682975] RBP: ffff8bdff7568128 R08: ffffffffb77f9820 R09: ffffa14c60567c68
> [  186.682975] R10: ffff8bdfa9d22a18 R11: ffff8bdfff16af38 R12: ffff8bdffb72b000
> [  186.682975] R13: ffff8bdff7568120 R14: ffff8bdff7568280 R15: 0000000000000000
> [  186.682976] FS:  0000000000000000(0000) GS:ffff8bdfff000000(0000) knlGS:0000000000000000
> [  186.682977] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  186.682977] CR2: 0000556b79c52c58 CR3: 0000000e5760a004 CR4: 00000000007606e0
> [  186.682978] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  186.682978] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [  186.682979] PKRU: 55555554
> [  186.682979] Call Trace:
> [  186.682983]  devlink_port_type_eth_set+0x7b/0xb0
> [  186.682993]  ice_probe+0x7dc/0xda0 [ice]
> [  186.682998]  local_pci_probe+0x42/0x80
> [  186.683002]  ? __schedule+0x2cf/0x740
> [  186.683003]  work_for_cpu_fn+0x16/0x20
> [  186.683006]  process_one_work+0x1b5/0x360
> [  186.683007]  worker_thread+0x1e2/0x3c0
> [  186.683010]  kthread+0xf9/0x130
> [  186.683012]  ? process_one_work+0x360/0x360
> [  186.683013]  ? kthread_park+0x90/0x90
> [  186.683014]  ret_from_fork+0x35/0x40
> [  186.683016] ---[ end trace 5fff949e65047c29 ]---

I'm fairly sure this is because we do not call ice_devlink_create_port
early enough, but I need to figure out the best place to put this :(

Regards,
Jake
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index ced9d2650da4..b421c0c5fc8b 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -296,6 +296,7 @@  config ICE
 	default n
 	depends on PCI_MSI
 	select VIRTUAL_BUS
+	select NET_DEVLINK
 	---help---
 	  This driver supports Intel(R) Ethernet Connection E800 Series of
 	  devices.  For more information on how to identify your adapter, go
diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index 85e2ba18cec2..41bfd5f21679 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -20,6 +20,7 @@  ice-y := ice_main.o	\
 	 ice_flex_pipe.o \
 	 ice_flow.o	\
 	 ice_idc.o	\
+	 ice_devlink.o	\
 	 ice_ethtool.o
 ice-$(CONFIG_PCI_IOV) += ice_virtchnl_pf.o ice_sriov.o
 ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index f237df923ea1..288b6d08eb3f 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -35,6 +35,7 @@ 
 #include <linux/bpf.h>
 #include <linux/virtual_bus.h>
 #include <linux/avf/virtchnl.h>
+#include <net/devlink.h>
 #include <net/ipv6.h>
 #include <net/xdp_sock.h>
 #include <net/geneve.h>
@@ -358,6 +359,9 @@  enum ice_pf_flags {
 struct ice_pf {
 	struct pci_dev *pdev;
 
+	/* devlink port data */
+	struct devlink_port devlink_port;
+
 	/* OS reserved IRQ details */
 	struct msix_entry *msix_entries;
 	struct ice_res_tracker *irq_tracker;
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
new file mode 100644
index 000000000000..cedd9d02299e
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -0,0 +1,117 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020, Intel Corporation. */
+
+#include "ice.h"
+#include "ice_devlink.h"
+
+static const struct devlink_ops ice_devlink_ops = {
+};
+
+static void ice_devlink_free(void *devlink_ptr)
+{
+	devlink_free((struct devlink *)devlink_ptr);
+}
+
+/**
+ * ice_allocate_pf - Allocate devlink and return PF structure pointer
+ * @dev: the device to allocate for
+ *
+ * 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)
+{
+	struct devlink *devlink;
+
+	devlink = devlink_alloc(&ice_devlink_ops, sizeof(struct ice_pf));
+	if (!devlink)
+		return NULL;
+
+	/* Add an action to teardown the devlink when unwinding the driver */
+	if (devm_add_action(dev, ice_devlink_free, devlink)) {
+		devlink_free(devlink);
+		return NULL;
+	}
+
+	return devlink_priv(devlink);
+}
+
+/**
+ * ice_devlink_register - Register devlink interface for this PF
+ * @pf: the PF to register the devlink for.
+ *
+ * Register the devlink instance associated with this physical function.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+int ice_devlink_register(struct ice_pf *pf)
+{
+	struct devlink *devlink = priv_to_devlink(pf);
+	struct device *dev = ice_pf_to_dev(pf);
+	int err;
+
+	err = devlink_register(devlink, dev);
+	if (err) {
+		dev_err(dev, "devlink registration failed: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+/**
+ * ice_devlink_unregister - Unregister devlink resources for this PF.
+ * @pf: the PF structure to cleanup
+ *
+ * Releases resources used by devlink and cleans up associated memory.
+ */
+void ice_devlink_unregister(struct ice_pf *pf)
+{
+	devlink_unregister(priv_to_devlink(pf));
+}
+
+/**
+ * ice_devlink_create_port - Create a devlink port for this PF
+ * @pf: the PF to create a port for
+ *
+ * Create and register a devlink_port for this PF. Note that although each
+ * physical function is connected to a separate devlink instance, the port
+ * will still be numbered according to the physical function id.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+int ice_devlink_create_port(struct ice_pf *pf)
+{
+	struct devlink *devlink = priv_to_devlink(pf);
+	struct ice_vsi *vsi = ice_get_main_vsi(pf);
+	struct device *dev = ice_pf_to_dev(pf);
+	int err;
+
+	if (!vsi) {
+		dev_err(dev, "%s: unable to find main VSI\n", __func__);
+		return -EIO;
+	}
+
+	devlink_port_attrs_set(&pf->devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
+			       pf->hw.pf_id, false, 0, NULL, 0);
+	err = devlink_port_register(devlink, &pf->devlink_port, pf->hw.pf_id);
+	if (err) {
+		dev_err(dev, "devlink_port_register failed: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+/**
+ * ice_devlink_destroy_port - Destroy the devlink_port for this PF
+ * @pf: the PF to cleanup
+ *
+ * Unregisters the devlink_port structure associated with this PF.
+ */
+void ice_devlink_destroy_port(struct ice_pf *pf)
+{
+	devlink_port_type_clear(&pf->devlink_port);
+	devlink_port_unregister(&pf->devlink_port);
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.h b/drivers/net/ethernet/intel/ice/ice_devlink.h
new file mode 100644
index 000000000000..f94dc93c24c5
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.h
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2019, Intel Corporation. */
+
+#ifndef _ICE_DEVLINK_H_
+#define _ICE_DEVLINK_H_
+
+struct ice_pf *ice_allocate_pf(struct device *dev);
+
+int ice_devlink_register(struct ice_pf *pf);
+void ice_devlink_unregister(struct ice_pf *pf);
+int ice_devlink_create_port(struct ice_pf *pf);
+void ice_devlink_destroy_port(struct ice_pf *pf);
+
+#endif /* _ICE_DEVLINK_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5e99b100fcb3..e4284084e7b5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -10,6 +10,7 @@ 
 #include "ice_lib.h"
 #include "ice_dcb_lib.h"
 #include "ice_dcb_nl.h"
+#include "ice_devlink.h"
 
 #define DRV_VERSION_MAJOR 0
 #define DRV_VERSION_MINOR 8
@@ -2426,6 +2427,8 @@  static int ice_cfg_netdev(struct ice_vsi *vsi)
 	if (err)
 		return err;
 
+	devlink_port_type_eth_set(&pf->devlink_port, vsi->netdev);
+
 	netif_carrier_off(vsi->netdev);
 
 	/* make sure transmit queues start off as stopped */
@@ -3224,7 +3227,7 @@  ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		return err;
 	}
 
-	pf = devm_kzalloc(dev, sizeof(*pf), GFP_KERNEL);
+	pf = ice_allocate_pf(dev);
 	if (!pf)
 		return -ENOMEM;
 
@@ -3262,6 +3265,12 @@  ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 
 	pf->msg_enable = netif_msg_init(debug, ICE_DFLT_NETIF_M);
 
+	err = ice_devlink_register(pf);
+	if (err) {
+		dev_err(dev, "ice_devlink_register failed: %d\n", err);
+		goto err_exit_unroll;
+	}
+
 #ifndef CONFIG_DYNAMIC_DEBUG
 	if (debug < -1)
 		hw->debug_mask = debug;
@@ -3353,6 +3362,11 @@  ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		goto err_alloc_sw_unroll;
 	}
 
+	err = ice_devlink_create_port(pf);
+	if (err)
+		goto err_alloc_sw_unroll;
+
+
 	clear_bit(__ICE_SERVICE_DIS, pf->state);
 
 	/* tell the firmware we are up */
@@ -3422,6 +3436,7 @@  ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 		}
 	}
 err_alloc_sw_unroll:
+	ice_devlink_destroy_port(pf);
 	set_bit(__ICE_SERVICE_DIS, pf->state);
 	set_bit(__ICE_DOWN, pf->state);
 	devm_kfree(dev, pf->first_sw);
@@ -3434,6 +3449,7 @@  ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
 	ice_deinit_pf(pf);
 	ice_deinit_hw(hw);
 err_exit_unroll:
+	ice_devlink_unregister(pf);
 	pci_disable_pcie_error_reporting(pdev);
 	return err;
 }
@@ -3469,6 +3485,7 @@  static void ice_remove(struct pci_dev *pdev)
 	}
 	set_bit(__ICE_DOWN, pf->state);
 
+	ice_devlink_destroy_port(pf);
 	ice_vsi_release_all(pf);
 	if (ice_is_peer_ena(pf)) {
 		ice_for_each_peer(pf, NULL, ice_unreg_peer_device);
@@ -3482,6 +3499,8 @@  static void ice_remove(struct pci_dev *pdev)
 	}
 	ice_deinit_pf(pf);
 	ice_deinit_hw(&pf->hw);
+	ice_devlink_unregister(pf);
+
 	/* Issue a PFR as part of the prescribed driver unload flow.  Do not
 	 * do it via ice_schedule_reset() since there is no need to rebuild
 	 * and the service task is already stopped.