From: Jiri Pirko <j...@resnulli.us>
Sent: Wednesday, February 12, 2025 4:09 PM
>Wed, Feb 12, 2025 at 02:14:01PM +0100, jedrzej.jagiel...@intel.com wrote:
>>Add an initial support for devlink interface to ixgbe driver.
>>
>>Similarly to i40e driver the implementation doesn't enable
>>devlink to manage device-wide configuration. Devlink instance
>>is created for each physical function of PCIe device.
>>
>>Create separate directory for devlink related ixgbe files
>>and use naming scheme similar to the one used in the ice driver.
>>
>>Add a stub for Documentation, to be extended by further patches.
>>
>>Reviewed-by: Mateusz Polchlopek <mateusz.polchlo...@intel.com>
>>Signed-off-by: Jedrzej Jagielski <jedrzej.jagiel...@intel.com>
>>---
>>v2: fix error patch in probe; minor tweaks
>>---
>> Documentation/networking/devlink/index.rst | 1 +
>> Documentation/networking/devlink/ixgbe.rst | 8 ++
>> drivers/net/ethernet/intel/Kconfig | 1 +
>> drivers/net/ethernet/intel/ixgbe/Makefile | 3 +-
>> .../ethernet/intel/ixgbe/devlink/devlink.c | 80 +++++++++++++++++++
>> .../ethernet/intel/ixgbe/devlink/devlink.h | 10 +++
>> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 8 ++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 22 +++++
>> 8 files changed, 132 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/networking/devlink/ixgbe.rst
>> create mode 100644 drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
>> create mode 100644 drivers/net/ethernet/intel/ixgbe/devlink/devlink.h
>>
>>diff --git a/Documentation/networking/devlink/index.rst
>>b/Documentation/networking/devlink/index.rst
>>index 948c8c44e233..8319f43b5933 100644
>>--- a/Documentation/networking/devlink/index.rst
>>+++ b/Documentation/networking/devlink/index.rst
>>@@ -84,6 +84,7 @@ parameters, info versions, and other features it supports.
>> i40e
>> ionic
>> ice
>>+ ixgbe
>> mlx4
>> mlx5
>> mlxsw
>>diff --git a/Documentation/networking/devlink/ixgbe.rst
>>b/Documentation/networking/devlink/ixgbe.rst
>>new file mode 100644
>>index 000000000000..c04ac51c6d85
>>--- /dev/null
>>+++ b/Documentation/networking/devlink/ixgbe.rst
>>@@ -0,0 +1,8 @@
>>+.. SPDX-License-Identifier: GPL-2.0
>>+
>>+=====================
>>+ixgbe devlink support
>>+=====================
>>+
>>+This document describes the devlink features implemented by the ``ixgbe``
>>+device driver.
>>diff --git a/drivers/net/ethernet/intel/Kconfig
>>b/drivers/net/ethernet/intel/Kconfig
>>index 1640d2f27833..56ee58c9df21 100644
>>--- a/drivers/net/ethernet/intel/Kconfig
>>+++ b/drivers/net/ethernet/intel/Kconfig
>>@@ -147,6 +147,7 @@ config IXGBE
>> depends on PCI
>> depends on PTP_1588_CLOCK_OPTIONAL
>> select MDIO
>>+ select NET_DEVLINK
>> select PHYLIB
>> help
>> This driver supports Intel(R) 10GbE PCI Express family of
>>diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile
>>b/drivers/net/ethernet/intel/ixgbe/Makefile
>>index b456d102655a..11f37140c0a3 100644
>>--- a/drivers/net/ethernet/intel/ixgbe/Makefile
>>+++ b/drivers/net/ethernet/intel/ixgbe/Makefile
>>@@ -4,12 +4,13 @@
>> # Makefile for the Intel(R) 10GbE PCI Express ethernet driver
>> #
>>
>>+subdir-ccflags-y += -I$(src)
>> obj-$(CONFIG_IXGBE) += ixgbe.o
>>
>> ixgbe-y := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \
>> ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \
>> ixgbe_mbx.o ixgbe_x540.o ixgbe_x550.o ixgbe_lib.o ixgbe_ptp.o \
>>- ixgbe_xsk.o ixgbe_e610.o
>>+ ixgbe_xsk.o ixgbe_e610.o devlink/devlink.o
>>
>> ixgbe-$(CONFIG_IXGBE_DCB) += ixgbe_dcb.o ixgbe_dcb_82598.o \
>> ixgbe_dcb_82599.o ixgbe_dcb_nl.o
>>diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
>>b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
>>new file mode 100644
>>index 000000000000..c052e95c9496
>>--- /dev/null
>>+++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
>>@@ -0,0 +1,80 @@
>>+// SPDX-License-Identifier: GPL-2.0
>>+/* Copyright (c) 2025, Intel Corporation. */
>>+
>>+#include "ixgbe.h"
>>+#include "devlink.h"
>>+
>>+static const struct devlink_ops ixgbe_devlink_ops = {
>>+};
>>+
>>+/**
>>+ * ixgbe_allocate_devlink - Allocate devlink instance
>>+ * @adapter: pointer to the device adapter structure
>>+ *
>>+ * Allocate a devlink instance for this physical function.
>>+ *
>>+ * Return: 0 on success, -ENOMEM when allocation failed.
>>+ */
>>+int ixgbe_allocate_devlink(struct ixgbe_adapter *adapter)
>>+{
>>+ struct ixgbe_devlink_priv *devlink_private;
>>+ struct device *dev = &adapter->pdev->dev;
>>+ struct devlink *devlink;
>>+
>>+ devlink = devlink_alloc(&ixgbe_devlink_ops,
>>+ sizeof(*devlink_private), dev);
>>+ if (!devlink)
>>+ return -ENOMEM;
>>+
>>+ devlink_private = devlink_priv(devlink);
>>+ devlink_private->adapter = adapter;
>
>struct ixgbe_adapter * should be returned by devlink_priv(), that is the
>idea, to let devlink allocate the driver private for you.
Using ixgbe_devlink_priv here is a workaround which i decided to introduce
to mitigate the fact that ixgbe_adapter is used to alloc netdev with
alloc_etherdev_mq().
This would require general ixgbe refactoring.
>
>
>
>>+
>>+ adapter->devlink = devlink;
>>+
>>+ return 0;
>>+}
>>+
>>+/**
>>+ * ixgbe_devlink_set_switch_id - Set unique switch ID based on PCI DSN
>>+ * @adapter: pointer to the device adapter structure
>>+ * @ppid: struct with switch id information
>>+ */
>>+static void ixgbe_devlink_set_switch_id(struct ixgbe_adapter *adapter,
>>+ struct netdev_phys_item_id *ppid)
>>+{
>>+ u64 id = pci_get_dsn(adapter->pdev);
>>+
>>+ ppid->id_len = sizeof(id);
>>+ put_unaligned_be64(id, &ppid->id);
>>+}
>>+
>>+/**
>>+ * ixgbe_devlink_register_port - Register devlink port
>>+ * @adapter: pointer to the device adapter structure
>>+ *
>>+ * Create and register a devlink_port for this physical function.
>>+ *
>>+ * Return: 0 on success, error code on failure.
>>+ */
>>+int ixgbe_devlink_register_port(struct ixgbe_adapter *adapter)
>>+{
>>+ struct devlink_port *devlink_port = &adapter->devlink_port;
>>+ struct devlink *devlink = adapter->devlink;
>>+ struct device *dev = &adapter->pdev->dev;
>>+ struct devlink_port_attrs attrs = {};
>>+ int err;
>>+
>>+ attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
>>+ attrs.phys.port_number = adapter->hw.bus.func;
>>+ ixgbe_devlink_set_switch_id(adapter, &attrs.switch_id);
>>+
>>+ devlink_port_attrs_set(devlink_port, &attrs);
>>+
>>+ err = devl_port_register(devlink, devlink_port, 0);
>>+ if (err) {
>>+ dev_err(dev,
>>+ "devlink port registration failed, err %d\n", err);
>>+ }
>
>Don't use "{}" for single statement.
Sure, will be changed.
>
>
>>+
>>+ return err;
>>+}
>>diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.h
>>b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.h
>>new file mode 100644
>>index 000000000000..d73c57164aef
>>--- /dev/null
>>+++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.h
>>@@ -0,0 +1,10 @@
>>+/* SPDX-License-Identifier: GPL-2.0 */
>>+/* Copyright (c) 2025, Intel Corporation. */
>>+
>>+#ifndef _IXGBE_DEVLINK_H_
>>+#define _IXGBE_DEVLINK_H_
>>+
>>+int ixgbe_allocate_devlink(struct ixgbe_adapter *adapter);
>>+int ixgbe_devlink_register_port(struct ixgbe_adapter *adapter);
>>+
>>+#endif /* _IXGBE_DEVLINK_H_ */
>>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>index e6a380d4929b..37d761f8c409 100644
>>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>@@ -17,6 +17,8 @@
>> #include <linux/net_tstamp.h>
>> #include <linux/ptp_clock_kernel.h>
>>
>>+#include <net/devlink.h>
>>+
>> #include "ixgbe_type.h"
>> #include "ixgbe_common.h"
>> #include "ixgbe_dcb.h"
>>@@ -612,6 +614,8 @@ struct ixgbe_adapter {
>> struct bpf_prog *xdp_prog;
>> struct pci_dev *pdev;
>> struct mii_bus *mii_bus;
>>+ struct devlink *devlink;
>>+ struct devlink_port devlink_port;
>>
>> unsigned long state;
>>
>>@@ -830,6 +834,10 @@ struct ixgbe_adapter {
>> spinlock_t vfs_lock;
>> };
>>
>>+struct ixgbe_devlink_priv {
>>+ struct ixgbe_adapter *adapter;
>>+};
>>+
>> static inline int ixgbe_determine_xdp_q_idx(int cpu)
>> {
>> if (static_key_enabled(&ixgbe_xdp_locking_key))
>>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>index 7236f20c9a30..1617ece95f1f 100644
>>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>@@ -49,6 +49,7 @@
>> #include "ixgbe_sriov.h"
>> #include "ixgbe_model.h"
>> #include "ixgbe_txrx_common.h"
>>+#include "devlink/devlink.h"
>>
>> char ixgbe_driver_name[] = "ixgbe";
>> static const char ixgbe_driver_string[] =
>>@@ -11275,6 +11276,10 @@ static int ixgbe_probe(struct pci_dev *pdev, const
>>struct pci_device_id *ent)
>> hw->back = adapter;
>> adapter->msg_enable = netif_msg_init(debug, DEFAULT_MSG_ENABLE);
>>
>>+ err = ixgbe_allocate_devlink(adapter);
>>+ if (err)
>>+ goto err_devlink;
>>+
>> hw->hw_addr = ioremap(pci_resource_start(pdev, 0),
>> pci_resource_len(pdev, 0));
>> adapter->io_addr = hw->hw_addr;
>>@@ -11613,6 +11618,11 @@ static int ixgbe_probe(struct pci_dev *pdev, const
>>struct pci_device_id *ent)
>> }
>> strcpy(netdev->name, "eth%d");
>> pci_set_drvdata(pdev, adapter);
>>+
>>+ devl_lock(adapter->devlink);
>>+ ixgbe_devlink_register_port(adapter);
>>+ SET_NETDEV_DEVLINK_PORT(adapter->netdev, &adapter->devlink_port);
>>+
>> err = register_netdev(netdev);
>> if (err)
>> goto err_register;
>>@@ -11667,11 +11677,15 @@ static int ixgbe_probe(struct pci_dev *pdev, const
>>struct pci_device_id *ent)
>> if (err)
>> goto err_netdev;
>>
>>+ devl_register(adapter->devlink);
>>+ devl_unlock(adapter->devlink);
>> return 0;
>>
>> err_netdev:
>> unregister_netdev(netdev);
>> err_register:
>>+ devl_port_unregister(&adapter->devlink_port);
>>+ devl_unlock(adapter->devlink);
>> ixgbe_release_hw_control(adapter);
>> ixgbe_clear_interrupt_scheme(adapter);
>> if (hw->mac.type == ixgbe_mac_e610)
>>@@ -11685,6 +11699,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const
>>struct pci_device_id *ent)
>> kfree(adapter->rss_key);
>> bitmap_free(adapter->af_xdp_zc_qps);
>> err_ioremap:
>>+ devlink_free(adapter->devlink);
>>+err_devlink:
>> disable_dev = !test_and_set_bit(__IXGBE_DISABLED, &adapter->state);
>> free_netdev(netdev);
>> err_alloc_etherdev:
>>@@ -11717,6 +11733,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
>> return;
>>
>> netdev = adapter->netdev;
>>+ devl_lock(adapter->devlink);
>>+ devl_unregister(adapter->devlink);
>> ixgbe_dbg_adapter_exit(adapter);
>>
>> set_bit(__IXGBE_REMOVING, &adapter->state);
>>@@ -11752,6 +11770,10 @@ static void ixgbe_remove(struct pci_dev *pdev)
>> if (netdev->reg_state == NETREG_REGISTERED)
>> unregister_netdev(netdev);
>>
>>+ devl_port_unregister(&adapter->devlink_port);
>>+ devl_unlock(adapter->devlink);
>>+ devlink_free(adapter->devlink);
>>+
>> ixgbe_stop_ipsec_offload(adapter);
>> ixgbe_clear_interrupt_scheme(adapter);
>>
>>--
>>2.31.1
>>
