NACK/Cmnt: [SRU][N][PULL][PATCH V3 0/10] ixgbe: Add support for E610 in Noble
Matthew Ruffell
matthew.ruffell at canonical.com
Tue Jan 6 02:15:35 UTC 2026
Hi Michael,
Sorry to NACK your patch, but Jay and I found a few things during review that
probably should be addressed.
Now, Jay and I would like to point out these are real nit-picks, and the code
does work "just fine" as-is, but they could cause problems later on maintaining
these older stable kernels into the future with unnecessary merge conflicts,
or mis-matching type compile errors.
The first is in patch 1, ixgbe: Add support for E610 FW Admin Command Interface:
In drivers/net/ethernet/intel/ixgbe/Makefile:
-ixgbe-objs := 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-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
you change the target label from ixgbe-objs to ixgbe-y to match upstream. This
only changed tree-wide in the driver in
a2fe35df41c4cfce44f9f87d417cbd44a27b7362 net: intel: Use *-y instead
of *-objs in Makefile
which landed in 6.11-rc1.
For 6.8, we would prefer it to be left as ixgbe-objs to match the rest of the
driver in 6.8.
The next is in patch 6, ixgbe: Add ixgbe_x540 multiple header
inclusion protection
In drivers/net/ethernet/intel/ixgbe/ixgbe_x540.h:
s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask);
void ixgbe_release_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask);
void ixgbe_init_swfw_sync_X540(struct ixgbe_hw *hw);
-s32 ixgbe_init_eeprom_params_X540(struct ixgbe_hw *hw);
+int ixgbe_init_eeprom_params_X540(struct ixgbe_hw *hw);
+
+#endif /* _IXGBE_X540_H_ */
you change the declared type of ixgbe_init_eeprom_params_X540 from s32 to int.
While s32 is pretty much the same as int on most architectures, I don't think
you checked ppc64el or s390x or riscv, so we can't really be sure.
This change was a part of
8f76c0f4c3ce489a7616f712fc08c032582145d9 ixgbe: Convert ret val type
from s32 to int
which landed in 6.9-rc1.
The actual declaration did not change either, so there would be a type mis-match
if anyone is compiling their own kernel with strict type checking or forward
or backward edge CFI enabled.
This needs to stay s32 in the older 6.8 kernel.
The next is in patch 8, ixgbe: Enable link management in E610 device
In drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c:
- **/
-s32 ixgbe_reset_hw_X540(struct ixgbe_hw *hw)
+ *
+ * Return: 0 on success or negative value on failure
+ */
+int ixgbe_reset_hw_X540(struct ixgbe_hw *hw)
there is the same type confusion, changing s32 to int, without changing the
forward declaration elsewhere.
This also needs to stay as s32 in the older 6.8 kernel.
If you make these small changes and push a V4, Jay and I will be happy to
approve. Or if this is a rush, then the Kernel Team can make these changes
when they apply the patches, but a V4 would be preferred.
Thanks,
Matthew
On Fri, 19 Dec 2025 at 16:04, Michael Reed <michael.reed at canonical.com> wrote:
>
> From: Michael Reed <Michael.Reed at canonical.com>
>
> SRU Justification
>
> BugLink: https://launchpad.net/bugs/2131265
> These patches are already in Plucky and Questing.
>
> [Impact]
> Add initial support for Intel(R) E610 Series of network devices. The E610
> is based on X550 but adds firmware managed link, enhanced security
> capabilities and support for updated server manageability.
>
> Additionally, this is needed for Charmed Openstack Jammy/Caracal deployment. It requires servers to be on 22.04 Jammy (HWE kernel 6.8). Some servers f.e. Dell PowerEdge uses Intel NIC E610 and if this nic is planned to be used for PXE boot and to access the internet this missing functionality will effectively disqualify those servers from the approved HW for Openstack since NIC will not be detected by the OS.
>
> [Fix]
>
> This patch series adds low level support for the following features and
> enables link management.
>
> b1e44b4 ixgbe: fix media cage present detection for E610 device
> 4020659 ixgbe: fix media type detection for E610 device
> 4600cdf ixgbe: Enable link management in E610 device
> 34b4157 ixgbe: Clean up the E610 link management related code
> a0834bd ixgbe: Add ixgbe_x540 multiple header inclusion protection
> e5b132b ixgbe: Add support for EEPROM dump in E610 device
> d2483eb ixgbe: Add support for NVM handling in E610 device
> 23c0e5a ixgbe: Add link management support for E610 device
> 7c3aa0f ixgbe: Add support for E610 device capabilities detection
> 46761fd ixgbe: Add support for E610 FW Admin Command Interface
>
> [Test Case]
>
> 1. Install the test kernel and reboot
>
> 2. Verify the Intel E610 is available on the system
> - Verify the E610 is in lspci network output
>
> 3. Configure the ports and ping an external IP address
> 4. Run Iperf from the SUT to an external iperf server
>
> [Where problems could occur]
>
> The regression risk is low. There are no core kernel changes. Changes are primarily to the ixgbe driver (drivers/net/ethernet/intel/ixgbe/) adding support for the Intel E610 device. The majority of the changes are in newly added files (ixgbe_e610.c ixgbe_type_e610.h). There are some minor refactoring to existing ixgbe code involving the x540 and x550 nics but existing device behavior isn't changed it's just shared with E610.
>
> [Other Info]
>
> Noble
> https://code.launchpad.net/~mreed8855/ubuntu/+source/linux/+git/noble/+ref/intel_e610_lp_2131265_3
>
> The following changes since commit ec94ee9fbaf301e392923b4715f5615df1a96e1f:
>
> UBUNTU: SAUCE: perf/core: Allow CAP_PERFMON for paranoid level 4 (2025-11-14 09:41:26 +0100)
>
> are available in the Git repository at:
>
> https://code.launchpad.net/~mreed8855/ubuntu/+source/linux/+git/noble/+ref/intel_e610_lp_2131265_3
>
> for you to fetch changes up to 6ab4955ff048213b7d632449bef9c6404194468a:
>
> ixgbe: fix media type detection for E610 device (2025-12-04 15:46:20 -0600)
>
> ----------------------------------------------------------------
> Piotr Kwapulinski (10):
> ixgbe: Add support for E610 FW Admin Command Interface
> ixgbe: Add support for E610 device capabilities detection
> ixgbe: Add link management support for E610 device
> ixgbe: Add support for NVM handling in E610 device
> ixgbe: Add support for EEPROM dump in E610 device
> ixgbe: Add ixgbe_x540 multiple header inclusion protection
> ixgbe: Clean up the E610 link management related code
> ixgbe: Enable link management in E610 device
> ixgbe: fix media cage present detection for E610 device
> ixgbe: fix media type detection for E610 device
>
> drivers/net/ethernet/intel/ixgbe/Makefile | 10 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 13 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c | 3 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 25 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c | 3 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 2660 ++++++++++++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 81 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 6 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 3 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 436 +++-
> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c | 4 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 5 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 72 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 1074 ++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 14 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_x540.h | 9 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 29 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.h | 20 +
> 18 files changed, 4415 insertions(+), 52 deletions(-)
> create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
> create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
> create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.h
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list