| Message ID | 20250212131413.91787-3-jedrzej.jagielski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ixgbe: Add basic devlink support | expand |
Wed, Feb 12, 2025 at 02:14:01PM +0100, jedrzej.jagielski@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.polchlopek@intel.com> >Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@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. >+ >+ 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. >+ >+ 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 >
From: Jiri Pirko <jiri@resnulli.us> Sent: Wednesday, February 12, 2025 4:09 PM >Wed, Feb 12, 2025 at 02:14:01PM +0100, jedrzej.jagielski@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.polchlopek@intel.com> >>Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@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 >>
On 2/12/25 16:47, Jagielski, Jedrzej wrote: > From: Jiri Pirko <jiri@resnulli.us> > Sent: Wednesday, February 12, 2025 4:09 PM >> Wed, Feb 12, 2025 at 02:14:01PM +0100, jedrzej.jagielski@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.polchlopek@intel.com> >>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com> >>> +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. We will try to do that, pending a retest before for new submission ;)
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; + + 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); + } + + 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);