Re: [Intel-wired-lan] [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter index

2025-03-06 Thread Jacob Keller



On 3/6/2025 1:11 PM, Przemek Kitszel wrote:
> Use Device Serial Number instead of PCI bus/device/function for
> index of struct ice_adapter.
> Functions on the same physical device should point to the very same
> ice_adapter instance.
> 
> This is not only simplification, but also fixes things up when PF
> is passed to VM (and thus has a random BDF).
> 
> Suggested-by: Jacob Keller 
> Suggested-by: Jakub Kicinski 
> Suggested-by: Jiri Pirko 
> Reviewed-by: Aleksandr Loktionov 
> Signed-off-by: Przemek Kitszel 
> ---

The only caution I have here is that we might run into issues with
pre-production or poorly flashed boards which don't have DSN properly
flashed. This shouldn't be an impact outside of early testing or
mistakes by devs. I think there is a default ID which is almost all 0s
we could check and log a warning to help prevent confusion in such a case?

A couple systems I've seen have serial numbers like:

  serial_number 00-00-00-00-00-00-00-00
  serial_number 00-00-00-00-00-00-00-00

or

  serial_number 00-01-00-ff-ff-00-00-00
  serial_number 00-01-00-ff-ff-00-00-00


In practice I'm not sure how big a deal breaker this is. Properly
initialized boards should have unique IDs, and if you update via
devlink, or any of our standard update tools, it will maintain the ID
across flash. However, during early development, boards were often
flashed manually which could lead to such non-unique IDs.

> CC: Karol Kolacinski 
> CC: Grzegorz Nitka 
> CC: Michal Schmidt 
> CC: Sergey Temerkhanov 
> ---
>  drivers/net/ethernet/intel/ice/ice_adapter.h |  4 +--
>  drivers/net/ethernet/intel/ice/ice_adapter.c | 29 +++-
>  2 files changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h 
> b/drivers/net/ethernet/intel/ice/ice_adapter.h
> index e233225848b3..1935163bd32f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adapter.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
> @@ -42,7 +42,7 @@ struct ice_adapter {
>   struct ice_port_list ports;
>  };
>  
> -struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev);
> -void ice_adapter_put(const struct pci_dev *pdev);
> +struct ice_adapter *ice_adapter_get(struct pci_dev *pdev);
> +void ice_adapter_put(struct pci_dev *pdev);
>  
>  #endif /* _ICE_ADAPTER_H */
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c 
> b/drivers/net/ethernet/intel/ice/ice_adapter.c
> index 01a08cfd0090..b668339ed0ef 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adapter.c
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
> @@ -1,7 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  // SPDX-FileCopyrightText: Copyright Red Hat
>  
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -14,29 +13,9 @@
>  static DEFINE_XARRAY(ice_adapters);
>  static DEFINE_MUTEX(ice_adapters_mutex);
>  
> -/* PCI bus number is 8 bits. Slot is 5 bits. Domain can have the rest. */
> -#define INDEX_FIELD_DOMAIN GENMASK(BITS_PER_LONG - 1, 13)
> -#define INDEX_FIELD_DEVGENMASK(31, 16)
> -#define INDEX_FIELD_BUSGENMASK(12, 5)
> -#define INDEX_FIELD_SLOT   GENMASK(4, 0)
> -
> -static unsigned long ice_adapter_index(const struct pci_dev *pdev)
> +static unsigned long ice_adapter_index(struct pci_dev *pdev)
>  {
> - unsigned int domain = pci_domain_nr(pdev->bus);
> -
> - WARN_ON(domain > FIELD_MAX(INDEX_FIELD_DOMAIN));
> -
> - switch (pdev->device) {
> - case ICE_DEV_ID_E825C_BACKPLANE:
> - case ICE_DEV_ID_E825C_QSFP:
> - case ICE_DEV_ID_E825C_SFP:
> - case ICE_DEV_ID_E825C_SGMII:
> - return FIELD_PREP(INDEX_FIELD_DEV, pdev->device);
> - default:
> - return FIELD_PREP(INDEX_FIELD_DOMAIN, domain) |
> -FIELD_PREP(INDEX_FIELD_BUS,pdev->bus->number) |
> -FIELD_PREP(INDEX_FIELD_SLOT,   PCI_SLOT(pdev->devfn));
> - }
> + return (unsigned long)pci_get_dsn(pdev);

Much simpler :D

>  }
>  
>  static struct ice_adapter *ice_adapter_new(void)
> @@ -77,7 +56,7 @@ static void ice_adapter_free(struct ice_adapter *adapter)
>   * Return:  Pointer to ice_adapter on success.
>   *  ERR_PTR() on error. -ENOMEM is the only possible error.
>   */
> -struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev)
> +struct ice_adapter *ice_adapter_get(struct pci_dev *pdev)
>  {
>   unsigned long index = ice_adapter_index(pdev);
>   struct ice_adapter *adapter;
> @@ -110,7 +89,7 @@ struct ice_adapter *ice_adapter_get(const struct pci_dev 
> *pdev)
>   *
>   * Context: Process, may sleep.
>   */
> -void ice_adapter_put(const struct pci_dev *pdev)
> +void ice_adapter_put(struct pci_dev *pdev)
>  {

A bit of a shame that this needs to be non const now.. Could
pci_get_dsn() be made const? Or does it do something which might modify
the device somehow?

>   unsigned long index = ice_adapter_index(pdev);
>   struct ice_adapter *adapter;



[Intel-wired-lan] [tnguy-net-queue:dev-queue] BUILD SUCCESS 15f8b9eed0baa7b18679e888d21099704d0cb662

2025-03-06 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git dev-queue
branch HEAD: 15f8b9eed0baa7b18679e888d21099704d0cb662  ice: fix fwlog after 
driver reinit

elapsed time: 1448m

configs tested: 84
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfiggcc-14.2.0
arc   allnoconfiggcc-13.2.0
arcnsim_700_defconfiggcc-13.2.0
arc   randconfig-001-20250306gcc-13.2.0
arc   randconfig-002-20250306gcc-13.2.0
arm   allnoconfigclang-17
arm   h3600_defconfiggcc-14.2.0
arm   randconfig-001-20250306gcc-14.2.0
arm   randconfig-002-20250306gcc-14.2.0
arm   randconfig-003-20250306gcc-14.2.0
arm   randconfig-004-20250306clang-18
arm wpcm450_defconfiggcc-14.2.0
arm64 allnoconfiggcc-14.2.0
arm64 randconfig-001-20250306gcc-14.2.0
arm64 randconfig-002-20250306gcc-14.2.0
arm64 randconfig-003-20250306gcc-14.2.0
arm64 randconfig-004-20250306gcc-14.2.0
csky  allnoconfiggcc-14.2.0
csky  randconfig-001-20250306gcc-14.2.0
csky  randconfig-002-20250306gcc-14.2.0
hexagon   allnoconfigclang-21
hexagon   randconfig-001-20250306clang-21
hexagon   randconfig-002-20250306clang-19
i386buildonly-randconfig-001-20250306clang-19
i386buildonly-randconfig-002-20250306clang-19
i386buildonly-randconfig-003-20250306clang-19
i386buildonly-randconfig-004-20250306gcc-12
i386buildonly-randconfig-005-20250306gcc-12
i386buildonly-randconfig-006-20250306clang-19
loongarch allnoconfiggcc-14.2.0
loongarch randconfig-001-20250306gcc-14.2.0
loongarch randconfig-002-20250306gcc-14.2.0
m68k  allnoconfiggcc-14.2.0
microblazeallnoconfiggcc-14.2.0
mips  allnoconfiggcc-14.2.0
mips rt305x_defconfigclang-19
nios2 allnoconfiggcc-14.2.0
nios2 randconfig-001-20250306gcc-14.2.0
nios2 randconfig-002-20250306gcc-14.2.0
openrisc  allnoconfiggcc-14.2.0
pariscallnoconfiggcc-14.2.0
pariscrandconfig-001-20250306gcc-14.2.0
pariscrandconfig-002-20250306gcc-14.2.0
powerpc   allnoconfiggcc-14.2.0
powerpc   motionpro_defconfigclang-17
powerpc   randconfig-001-20250306clang-21
powerpc   randconfig-002-20250306clang-18
powerpc   randconfig-003-20250306gcc-14.2.0
powerpc tqm8555_defconfiggcc-14.2.0
powerpc64 randconfig-001-20250306clang-18
powerpc64 randconfig-002-20250306clang-21
powerpc64 randconfig-003-20250306clang-18
riscv allnoconfiggcc-14.2.0
riscv randconfig-001-20250306clang-18
riscv randconfig-002-20250306gcc-14.2.0
s390 allmodconfigclang-19
s390  allnoconfigclang-15
s390 allyesconfiggcc-14.2.0
s390  randconfig-001-20250306gcc-14.2.0
s390  randconfig-002-20250306clang-19
sh   allmodconfiggcc-14.2.0
shallnoconfiggcc-14.2.0
sh   allyesconfiggcc-14.2.0
shrandconfig-001-20250306gcc-14.2.0
shrandconfig-002-20250306gcc-14.2.0
sh   se7206_defconfiggcc-14.2.0
sh   se7780_defconfiggcc-14.2.0
sparc allnoconfiggcc-14.2.0
sparc randconfig-001-20250306gcc-14.2.0
sparc randconfig-002-20250306gcc-14.2.0
sparc64   randconfig-001-20250306gcc-14.2.0
sparc64   randconfig-002-20250306gcc-14.2.0
umallnoconfigclang-18
umrandconfig-001-20250306gcc-12
umrandconfig-002-20250306clang-16
x86_64  buildonly-randconfig-001-20250306gcc-11
x86_64  buildonly-randconfig-002-20250306clang-19
x86_64  buildonly-randconfig-003-20250306clang-19
x86_64  buildonly

[Intel-wired-lan] [PATCH iwl-net] idpf: fix adapter NULL pointer dereference on reboot

2025-03-06 Thread Emil Tantilov
Driver calls idpf_remove() from idpf_shutdown(), which can end up
calling idpf_remove() again when disabling SRIOV.

echo 1 > /sys/class/net//device/sriov_numvfs
reboot

BUG: kernel NULL pointer dereference, address: 0020
...
RIP: 0010:idpf_remove+0x22/0x1f0 [idpf]
...
? idpf_remove+0x22/0x1f0 [idpf]
? idpf_remove+0x1e4/0x1f0 [idpf]
pci_device_remove+0x3f/0xb0
device_release_driver_internal+0x19f/0x200
pci_stop_bus_device+0x6d/0x90
pci_stop_and_remove_bus_device+0x12/0x20
pci_iov_remove_virtfn+0xbe/0x120
sriov_disable+0x34/0xe0
idpf_sriov_configure+0x58/0x140 [idpf]
idpf_remove+0x1b9/0x1f0 [idpf]
idpf_shutdown+0x12/0x30 [idpf]
pci_device_shutdown+0x35/0x60
device_shutdown+0x156/0x200
...

Replace the direct idpf_remove() call in idpf_shutdown() with
idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform
the bulk of the cleanup, such as stopping the init task, freeing IRQs,
destroying the vports and freeing the mailbox.

Reported-by: Yuying Ma 
Fixes: e850efed5e15 ("idpf: add module register and probe functionality")
Reviewed-by: Madhu Chittim 
Signed-off-by: Emil Tantilov 
---
 drivers/net/ethernet/intel/idpf/idpf_main.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c 
b/drivers/net/ethernet/intel/idpf/idpf_main.c
index b6c515d14cbf..bec4a02c5373 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -87,7 +87,11 @@ static void idpf_remove(struct pci_dev *pdev)
  */
 static void idpf_shutdown(struct pci_dev *pdev)
 {
-   idpf_remove(pdev);
+   struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+
+   cancel_delayed_work_sync(&adapter->vc_event_task);
+   idpf_vc_core_deinit(adapter);
+   idpf_deinit_dflt_mbx(adapter);
 
if (system_state == SYSTEM_POWER_OFF)
pci_set_power_state(pdev, PCI_D3hot);
-- 
2.17.2



Re: [Intel-wired-lan] [PATCH] i40e: fix MMIO write access to an invalid page in i40e_clear_hw

2025-03-06 Thread Kyungwook Boo
On 25. 3. 6. 20:13, Loktionov, Aleksandr wrote:
> 
> 
>> -Original Message-
>> From: Kyungwook Boo 
>> Sent: Thursday, March 6, 2025 6:26 AM
>> To: Loktionov, Aleksandr ; Kitszel,
>> Przemyslaw ; Nguyen, Anthony L
>> 
>> Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org
>> Subject: [PATCH] i40e: fix MMIO write access to an invalid page in
>> i40e_clear_hw
> Please follow the rules, add v2 to the patch

Hi, Loktionov,

Thank you for reviewing the patch.

Following your guidance, I will update the patch with the correct format and
also include v2.

>>
>> In i40e_clear_hw(), when the device sends a specific input(e.g., 0), an 
>> integer
>> underflow in the num_{pf,vf}_int variables can occur, leading to MMIO write
>> access to an invalid page.
>>
>> To fix this, we change the type of the unsigned integer variables
>> num_{pf,vf}_int to signed integers. Additionally, in the for-loop where the
>> integer underflow occurs, we also change the type of the loop variable i to a
>> signed integer.
> Please do follow the linux kernel 

If you are referring to the tone of the patch description, I will rewrite it in
the imperative mood.

>>
>> Signed-off-by: Kyungwook Boo 
>> Signed-off-by: Loktionov, Aleksandr 
>> Signed-off-by: Przemek Kitszel 
>> Link: https://lore.kernel.org/lkml/ffc91764-1142-4ba2-91b6-
>> 8c773f6f7...@gmail.com/T/
>> ---
> Please up here versions history.

I have noted your request and will add the version history in the next update.

>>  drivers/net/ethernet/intel/i40e/i40e_common.c | 10 +-
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c
>> b/drivers/net/ethernet/intel/i40e/i40e_common.c
>> index 370b4bddee44..9a73cb94dc5e 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_common.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
>> @@ -817,8 +817,8 @@ int i40e_pf_reset(struct i40e_hw *hw)  void
>> i40e_clear_hw(struct i40e_hw *hw)  {
>>  u32 num_queues, base_queue;
>> -u32 num_pf_int;
>> -u32 num_vf_int;
>> +s32 num_pf_int;
>> +s32 num_vf_int;
>>  u32 num_vfs;
>>  u32 i, j;
> What about this vars? Are they used?

i, j are both used.
I think the relevant line to be considered is as follows:

if (val & I40E_PF_VT_PFALLOC_VALID_MASK && j >= i)
num_vfs = (j - i) + 1;

After this, j is not used and 
i is used as index of several loops.

My current plan was to change only i to s32 since it is related to the bug.
However, i is also used outside the loop, as in the code above.

Should I proceed with the original plan, or would it be better to separate the
loop variable for clarity? I would appreciate your opinion on this.

>>  u32 val;
>> @@ -848,18 +848,18 @@ void i40e_clear_hw(struct i40e_hw *hw)
>>  /* stop all the interrupts */
>>  wr32(hw, I40E_PFINT_ICR0_ENA, 0);
>>  val = 0x3 << I40E_PFINT_DYN_CTLN_ITR_INDX_SHIFT;
>> -for (i = 0; i < num_pf_int - 2; i++)
>> +for (s32 i = 0; i < num_pf_int - 2; i++)
>>  wr32(hw, I40E_PFINT_DYN_CTLN(i), val);
>>
>>  /* Set the FIRSTQ_INDX field to 0x7FF in PFINT_LNKLSTx */
>>  val = eol << I40E_PFINT_LNKLST0_FIRSTQ_INDX_SHIFT;
>>  wr32(hw, I40E_PFINT_LNKLST0, val);
>> -for (i = 0; i < num_pf_int - 2; i++)
>> +for (s32 i = 0; i < num_pf_int - 2; i++)
>>  wr32(hw, I40E_PFINT_LNKLSTN(i), val);
>>  val = eol << I40E_VPINT_LNKLST0_FIRSTQ_INDX_SHIFT;
>>  for (i = 0; i < num_vfs; i++)
>>  wr32(hw, I40E_VPINT_LNKLST0(i), val);
>> -for (i = 0; i < num_vf_int - 2; i++)
>> +for (s32 i = 0; i < num_vf_int - 2; i++)
>>  wr32(hw, I40E_VPINT_LNKLSTN(i), val);
>>
>>  /* warn the HW of the coming Tx disables */
>> --
>> 2.25.1

Best,
Kyungwook Boo


Re: [Intel-wired-lan] [PATCH intel-net v2] ice: fix reservation of resources for RDMA when disabled

2025-03-06 Thread Loktionov, Aleksandr



> -Original Message-
> From: Intel-wired-lan  On Behalf Of
> Jesse Brandeburg
> Sent: Thursday, March 6, 2025 6:57 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: Brandeburg, Jesse ;
> net...@vger.kernel.org; kernel-t...@cloudflare.com; jbran...@kernel.org;
> l...@kernel.org; Kitszel, Przemyslaw ;
> Ertman, David M 
> Subject: [Intel-wired-lan] [PATCH intel-net v2] ice: fix reservation of 
> resources
> for RDMA when disabled
> 
> From: Jesse Brandeburg 
> 
> If the CONFIG_INFINIBAND_IRDMA symbol is not enabled as a module or a
> built-in, then don't let the driver reserve resources for RDMA. The result of 
> this
> change is a large savings in resources for older kernels, and a cleaner driver
> configuration for the IRDMA=n case for old and new kernels.
> 
> Implement this by avoiding enabling the RDMA capability when scanning
> hardware capabilities.
> 
> Note: Loading the out-of-tree irdma driver in connection to the in-kernel ice
> driver, is not supported, and should not be attempted, especially when
> disabling IRDMA in the kernel config.
> 
> Fixes: d25a0fc41c1f ("ice: Initialize RDMA support")
> Signed-off-by: Jesse Brandeburg 
> Acked-by: Dave Ertman 
Reviewed-by: Aleksandr Loktionov 

> ---
> v2: resend with acks, add note about oot irdma (Leon), reword commit
> message
> v1: https://lore.kernel.org/netdev/20241114000105.703740-1-
> jbran...@kernel.org/
> ---
>  drivers/net/ethernet/intel/ice/ice_common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> b/drivers/net/ethernet/intel/ice/ice_common.c
> index 7a2a2e8da8fa..1e801300310e 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -2271,7 +2271,8 @@ ice_parse_common_caps(struct ice_hw *hw, struct
> ice_hw_common_caps *caps,
> caps->nvm_unified_update);
>   break;
>   case ICE_AQC_CAPS_RDMA:
> - caps->rdma = (number == 1);
> + if (IS_ENABLED(CONFIG_INFINIBAND_IRDMA))
> + caps->rdma = (number == 1);
>   ice_debug(hw, ICE_DBG_INIT, "%s: rdma = %d\n", prefix, caps-
> >rdma);
>   break;
>   case ICE_AQC_CAPS_MAX_MTU:
> --
> 2.43.0



Re: [Intel-wired-lan] [PATCH net-next 11/16] idpf: prepare structures to support XDP

2025-03-06 Thread Jakub Kicinski
On Wed,  5 Mar 2025 17:21:27 +0100 Alexander Lobakin wrote:
> +/**
> + * idpf_xdp_is_prog_ena - check if there is an XDP program on adapter
> + * @vport: vport to check
> + */
> +static inline bool idpf_xdp_is_prog_ena(const struct idpf_vport *vport)
> +{
> + return vport->adapter && vport->xdp_prog;
> +}

drivers/net/ethernet/intel/idpf/idpf.h:624: warning: No description found for 
return value of 'idpf_xdp_is_prog_ena'

The documentation doesn't add much info, just remove it ?
-- 
pw-bot: cr


Re: [Intel-wired-lan] [PATCH iwl-net] idpf: fix adapter NULL pointer dereference on reboot

2025-03-06 Thread Michal Swiatkowski
On Thu, Mar 06, 2025 at 04:39:56PM -0800, Emil Tantilov wrote:
> Driver calls idpf_remove() from idpf_shutdown(), which can end up
> calling idpf_remove() again when disabling SRIOV.
> 

The same is done in other drivers (ice, iavf). Why here it is a problem?
I am asking because heaving one function to remove is pretty handy.
Maybe the problem can be fixed by some changes in idpf_remove() instead?

> echo 1 > /sys/class/net//device/sriov_numvfs
> reboot
> 
> BUG: kernel NULL pointer dereference, address: 0020
> ...
> RIP: 0010:idpf_remove+0x22/0x1f0 [idpf]
> ...
> ? idpf_remove+0x22/0x1f0 [idpf]
> ? idpf_remove+0x1e4/0x1f0 [idpf]
> pci_device_remove+0x3f/0xb0
> device_release_driver_internal+0x19f/0x200
> pci_stop_bus_device+0x6d/0x90
> pci_stop_and_remove_bus_device+0x12/0x20
> pci_iov_remove_virtfn+0xbe/0x120
> sriov_disable+0x34/0xe0
> idpf_sriov_configure+0x58/0x140 [idpf]
> idpf_remove+0x1b9/0x1f0 [idpf]
> idpf_shutdown+0x12/0x30 [idpf]
> pci_device_shutdown+0x35/0x60
> device_shutdown+0x156/0x200
> ...
> 
> Replace the direct idpf_remove() call in idpf_shutdown() with
> idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform
> the bulk of the cleanup, such as stopping the init task, freeing IRQs,
> destroying the vports and freeing the mailbox.
> 
> Reported-by: Yuying Ma 
> Fixes: e850efed5e15 ("idpf: add module register and probe functionality")
> Reviewed-by: Madhu Chittim 
> Signed-off-by: Emil Tantilov 
> ---
>  drivers/net/ethernet/intel/idpf/idpf_main.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c 
> b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index b6c515d14cbf..bec4a02c5373 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -87,7 +87,11 @@ static void idpf_remove(struct pci_dev *pdev)
>   */
>  static void idpf_shutdown(struct pci_dev *pdev)
>  {
> - idpf_remove(pdev);
> + struct idpf_adapter *adapter = pci_get_drvdata(pdev);
> +
> + cancel_delayed_work_sync(&adapter->vc_event_task);
> + idpf_vc_core_deinit(adapter);
> + idpf_deinit_dflt_mbx(adapter);
>  
>   if (system_state == SYSTEM_POWER_OFF)
>   pci_set_power_state(pdev, PCI_D3hot);
> -- 
> 2.17.2


Re: [Intel-wired-lan] [PATCH iwl-next v8 11/11] igc: add support to get frame preemption statistics via ethtool

2025-03-06 Thread Abdul Rahim, Faizal




On 6/3/2025 8:48 am, Vladimir Oltean wrote:

On Wed, Mar 05, 2025 at 08:00:26AM -0500, Faizal Rahim wrote:

+/* Received out of order packets with SMD-C */
+#define IGC_PRMEXCPRCNT_OOO_SMDC   0x00FF
+/* Received out of order packets with SMD-C and wrong Frame CNT */
+#define IGC_PRMEXCPRCNT_OOO_FRAME_CNT  0xFF00
+/* Received out of order packets with SMD-C and wrong Frag CNT */
+#define IGC_PRMEXCPRCNT_OOO_FRAG_CNT   0x00FF
+/* Received packets with SMD-S and wrong Frag CNT and Frame CNT */
+#define IGC_PRMEXCPRCNT_MISS_FRAME_FRAG_CNT0xFF00
  
+/**

+ * igc_ethtool_get_frame_ass_error - Get the frame assembly error count.
+ * @reg_value: Register value for IGC_PRMEXCPRCNT
+ * Return: The count of frame assembly errors.
+ */
+static u64 igc_ethtool_get_frame_ass_error(u32 reg_value)
+{
+   u32 ooo_frame_cnt, ooo_frag_cnt; /* Out of order statistics */
+   u32 miss_frame_frag_cnt;
+
+   ooo_frame_cnt = FIELD_GET(IGC_PRMEXCPRCNT_OOO_FRAME_CNT, reg_value);
+   ooo_frag_cnt = FIELD_GET(IGC_PRMEXCPRCNT_OOO_FRAG_CNT, reg_value);
+   miss_frame_frag_cnt = FIELD_GET(IGC_PRMEXCPRCNT_MISS_FRAME_FRAG_CNT, 
reg_value);
+
+   return ooo_frame_cnt + ooo_frag_cnt + miss_frame_frag_cnt;
+}


These counters are quite small (8 bits each). What is their behavior
once they reach 255? Saturate? Truncate? Do they clear on read?


Hi Vladimir,

These are part of the statistic registers, which in IGC, reset upon read. 
When they reach their maximum value, each field remain at 0xFF.





Re: [Intel-wired-lan] [PATCH iwl-next v5 12/15] ixgbe: add support for devlink reload

2025-03-06 Thread Simon Horman
On Fri, Feb 21, 2025 at 12:51:13PM +0100, Jedrzej Jagielski wrote:
> The E610 adapters contain an embedded chip with firmware which can be
> updated using devlink flash. The firmware which runs on this chip is
> referred to as the Embedded Management Processor firmware (EMP
> firmware).
> 
> Activating the new firmware image currently requires that the system be
> rebooted. This is not ideal as rebooting the system can cause unwanted
> downtime.
> 
> The EMP firmware itself can be reloaded by issuing a special update
> to the device called an Embedded Management Processor reset (EMP
> reset). This reset causes the device to reset and reload the EMP
> firmware.
> 
> Implement support for devlink reload with the "fw_activate" flag. This
> allows user space to request the firmware be activated immediately.
> 
> Reviewed-by: Mateusz Polchlopek 
> Co-developed-by: Slawomir Mrozowicz 
> Signed-off-by: Slawomir Mrozowicz 
> Co-developed-by: Piotr Kwapulinski 
> Signed-off-by: Piotr Kwapulinski 
> Co-developed-by: Stefan Wegrzyn 
> Signed-off-by: Stefan Wegrzyn 
> Signed-off-by: Jedrzej Jagielski 

...

> diff --git a/Documentation/networking/devlink/ixgbe.rst 
> b/Documentation/networking/devlink/ixgbe.rst
> index 41aedf4b8017..e5fef951c6f5 100644
> --- a/Documentation/networking/devlink/ixgbe.rst
> +++ b/Documentation/networking/devlink/ixgbe.rst
> @@ -88,3 +88,18 @@ combined flash image that contains the ``fw.mgmt``, 
> ``fw.undi``, and
> and device serial number. It is expected that this combination be 
> used with an
> image customized for the specific device.
>  
> +Reload
> +==
> +
> +The ``ixgbe`` driver supports activating new firmware after a flash update
> +using ``DEVLINK_CMD_RELOAD`` with the ``DEVLINK_RELOAD_ACTION_FW_ACTIVATE``
> +action.
> +
> +.. code:: shell
> +$ devlink dev reload pci/:01:00.0 reload action fw_activate
> +The new firmware is activated by issuing a device specific Embedded
> +Management Processor reset which requests the device to reset and reload the
> +EMP firmware image.
> +
> +The driver does not currently support reloading the driver via
> +``DEVLINK_RELOAD_ACTION_DRIVER_REINIT``.

Hi Jedrzej, all,

This is not a proper review. And I didn't look into this, but make htmldocs
complains that:

 .../ixgbe.rst:98: ERROR: Error in "code" directive:
 maximum 1 argument(s) allowed, 9 supplied.
 
 .. code:: shell
 $ devlink dev reload pci/:01:00.0 reload action fw_activate

...


Re: [Intel-wired-lan] [PATCH] i40e: Disable i40e PCIe AER on system reboot

2025-03-06 Thread Yue Zhao
Hi Tony,

I am resending as I realized I sent in Rich Text instead of Plain Text.
I am sorry if any of you got this duplicate email.

Our DELL servers are all out of warranty, so I cannot provide more
useful information from the communication with the vendor side.
Is there any possible fix via upgrading firmware or other components?

Thanks,
Best Regards

Yue


On Thu, Mar 6, 2025 at 12:28 PM Yue Zhao  wrote:
>
> Hi Tony,
>
> Our DELL servers are all out of warranty, so I cannot provide more
> useful information from the communication with the vendor side.
> Is there any possible fix via upgrading firmware or other components?
>
> Thanks,
> Best Regards
>
> Yue
>
> On Thu, Mar 6, 2025 at 8:47 AM Tony Nguyen  wrote:
>>
>> On 12/26/2024 7:54 PM, Yue Zhao wrote:
>> > Disable PCIe AER on the i40e device on system reboot on a limited
>> > list of Dell PowerEdge systems. This prevents a fatal PCIe AER event
>> > on the i40e device during the ACPI _PTS (prepare to sleep) method for
>> > S5 on those systems. The _PTS is invoked by acpi_enter_sleep_state_prep()
>> > as part of the kernel's reboot sequence as a result of commit
>> > 38f34dba806a ("PM: ACPI: reboot: Reinstate S5 for reboot").
>>
>> Hi Yue,
>>
>> We've contacted Dell to try to root cause the issue and find the proper
>> fix. It would help if we could provide more information about the
>> problem and circumstances. Have you reported the issue to Dell? If so,
>> could you provide that to me (here or privately) so that we can pass
>> that along to help the investigation?
>>
>> Thank you,
>> Tony
>>
>> > We first noticed this abnormal reboot issue in tg3 device, and there
>> > is a similar patch about disable PCIe AER to fix hardware error during
>> > reboot. The hardware error in tg3 device has gone after we apply this
>> > patch below.
>> >
>> > https://lore.kernel.org/lkml/20241129203640.54492-1-lszub...@redhat.com/T/
>> >
>> > So we try to disable PCIe AER on the i40e device in the similar way.
>> >
>> > hardware crash dmesg log:
>> >
>> > ACPI: PM: Preparing to enter system sleep state S5
>> > {1}[Hardware Error]: Hardware error from APEI Generic Hardware Error 
>> > Source: 5
>> > {1}[Hardware Error]: event severity: fatal
>> > {1}[Hardware Error]:  Error 0, type: fatal
>> > {1}[Hardware Error]:   section_type: PCIe error
>> > {1}[Hardware Error]:   port_type: 0, PCIe end point
>> > {1}[Hardware Error]:   version: 3.0
>> > {1}[Hardware Error]:   command: 0x0006, status: 0x0010
>> > {1}[Hardware Error]:   device_id: :05:00.1
>> > {1}[Hardware Error]:   slot: 0
>> > {1}[Hardware Error]:   secondary_bus: 0x00
>> > {1}[Hardware Error]:   vendor_id: 0x8086, device_id: 0x1572
>> > {1}[Hardware Error]:   class_code: 02
>> > {1}[Hardware Error]:   aer_uncor_status: 0x0010, aer_uncor_mask: 
>> > 0x00018000
>> > {1}[Hardware Error]:   aer_uncor_severity: 0x000ef030
>> > {1}[Hardware Error]:   TLP Header: 4001 000f 90028090 
>> > Kernel panic - not syncing: Fatal hardware error!
>> > Hardware name: Dell Inc. PowerEdge C4140/08Y2GR, BIOS 2.21.1 12/12/2023
>> > Call Trace:
>> >   
>> >   dump_stack_lvl+0x48/0x70
>> >   dump_stack+0x10/0x20
>> >   panic+0x1b4/0x3a0
>> >   __ghes_panic+0x6c/0x70
>> >   ghes_in_nmi_queue_one_entry.constprop.0+0x1ee/0x2c0
>> >   ghes_notify_nmi+0x5e/0xe0
>> >   nmi_handle+0x62/0x160
>> >   default_do_nmi+0x4c/0x150
>> >   exc_nmi+0x140/0x1f0
>> >   end_repeat_nmi+0x16/0x67
>> > RIP: 0010:intel_idle_irq+0x70/0xf0
>> >   
>> >   
>> >   cpuidle_enter_state+0x91/0x6f0
>> >   cpuidle_enter+0x2e/0x50
>> >   call_cpuidle+0x23/0x60
>> >   cpuidle_idle_call+0x11d/0x190
>> >   do_idle+0x82/0xf0
>> >   cpu_startup_entry+0x2a/0x30
>> >   rest_init+0xc2/0xf0
>> >   arch_call_rest_init+0xe/0x30
>> >   start_kernel+0x34f/0x440
>> >   x86_64_start_reservations+0x18/0x30
>> >   x86_64_start_kernel+0xbf/0x110
>> >   secondary_startup_64_no_verify+0x18f/0x19b
>> >   
>> >
>> > Fixes: 38f34dba806a ("PM: ACPI: reboot: Reinstate S5 for reboot")
>> > Signed-off-by: Yue Zhao 
>> > ---
>> >   drivers/net/ethernet/intel/i40e/i40e_main.c | 64 +
>> >   1 file changed, 64 insertions(+)
>> >
>> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
>> > b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > index 0e1d9e2fbf38..80e66e4e90f7 100644
>> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> > @@ -8,6 +8,7 @@
>> >   #include 
>> >   #include 
>> >   #include 
>> > +#include 
>> >
>> >   /* Local includes */
>> >   #include "i40e.h"
>> > @@ -16608,6 +16609,56 @@ static void i40e_pci_error_resume(struct pci_dev 
>> > *pdev)
>> >   i40e_io_resume(pf);
>> >   }
>> >
>> > +/* Systems where ACPI _PTS (Prepare To Sleep) S5 will result in a fatal
>> > + * PCIe AER event on the i40e device if the i40e device is not, or cannot
>> > + * be, powered down.
>> > + */
>> > +static const struct dmi_system_id i40e_restart_aer_quirk_table[] = {
>> > + {
>> 

Re: [Intel-wired-lan] [PATCH] i40e: fix MMIO write access to an invalid page in i40e_clear_hw

2025-03-06 Thread Przemek Kitszel

On 3/6/25 06:25, Kyungwook Boo wrote:

In i40e_clear_hw(), when the device sends a specific input(e.g., 0),
an integer underflow in the num_{pf,vf}_int variables can occur,
leading to MMIO write access to an invalid page.

To fix this, we change the type of the unsigned integer variables
num_{pf,vf}_int to signed integers. Additionally, in the for-loop where the
integer underflow occurs, we also change the type of the loop variable i to
a signed integer.

Signed-off-by: Kyungwook Boo 
Signed-off-by: Loktionov, Aleksandr 


when Alex said "make sure I signed too" he meant:
"make sure the variable @i is signed too", not the Sign-off ;)

(please wait 24h for the next submission, and also put "iwl-next" after
the "PATCH" word)


Signed-off-by: Przemek Kitszel 


I didn't signed that either


Link: 
https://lore.kernel.org/lkml/ffc91764-1142-4ba2-91b6-8c773f6f7...@gmail.com/T/
---
  drivers/net/ethernet/intel/i40e/i40e_common.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c 
b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 370b4bddee44..9a73cb94dc5e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -817,8 +817,8 @@ int i40e_pf_reset(struct i40e_hw *hw)
  void i40e_clear_hw(struct i40e_hw *hw)
  {
u32 num_queues, base_queue;
-   u32 num_pf_int;
-   u32 num_vf_int;
+   s32 num_pf_int;
+   s32 num_vf_int;
u32 num_vfs;
u32 i, j;


It's fine to move the declaration of @i into the for loop, but
you have to remove it here, otherwise it's shadowing, which we
avoid.


u32 val;
@@ -848,18 +848,18 @@ void i40e_clear_hw(struct i40e_hw *hw)
/* stop all the interrupts */
wr32(hw, I40E_PFINT_ICR0_ENA, 0);
val = 0x3 << I40E_PFINT_DYN_CTLN_ITR_INDX_SHIFT;
-   for (i = 0; i < num_pf_int - 2; i++)
+   for (s32 i = 0; i < num_pf_int - 2; i++)
wr32(hw, I40E_PFINT_DYN_CTLN(i), val);
  
  	/* Set the FIRSTQ_INDX field to 0x7FF in PFINT_LNKLSTx */

val = eol << I40E_PFINT_LNKLST0_FIRSTQ_INDX_SHIFT;
wr32(hw, I40E_PFINT_LNKLST0, val);
-   for (i = 0; i < num_pf_int - 2; i++)
+   for (s32 i = 0; i < num_pf_int - 2; i++)
wr32(hw, I40E_PFINT_LNKLSTN(i), val);
val = eol << I40E_VPINT_LNKLST0_FIRSTQ_INDX_SHIFT;
for (i = 0; i < num_vfs; i++)
wr32(hw, I40E_VPINT_LNKLST0(i), val);
-   for (i = 0; i < num_vf_int - 2; i++)
+   for (s32 i = 0; i < num_vf_int - 2; i++)
wr32(hw, I40E_VPINT_LNKLSTN(i), val);
  
  	/* warn the HW of the coming Tx disables */




Re: [Intel-wired-lan] [PATCH] i40e: fix MMIO write access to an invalid page in i40e_clear_hw

2025-03-06 Thread Loktionov, Aleksandr


> -Original Message-
> From: Kyungwook Boo 
> Sent: Thursday, March 6, 2025 6:26 AM
> To: Loktionov, Aleksandr ; Kitszel,
> Przemyslaw ; Nguyen, Anthony L
> 
> Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org
> Subject: [PATCH] i40e: fix MMIO write access to an invalid page in
> i40e_clear_hw
Please follow the rules, add v2 to the patch

> 
> In i40e_clear_hw(), when the device sends a specific input(e.g., 0), an 
> integer
> underflow in the num_{pf,vf}_int variables can occur, leading to MMIO write
> access to an invalid page.
> 
> To fix this, we change the type of the unsigned integer variables
> num_{pf,vf}_int to signed integers. Additionally, in the for-loop where the
> integer underflow occurs, we also change the type of the loop variable i to a
> signed integer.
Please do follow the linux kernel 

> 
> Signed-off-by: Kyungwook Boo 
> Signed-off-by: Loktionov, Aleksandr 
> Signed-off-by: Przemek Kitszel 
> Link: https://lore.kernel.org/lkml/ffc91764-1142-4ba2-91b6-
> 8c773f6f7...@gmail.com/T/
> ---
Please up here versions history.

>  drivers/net/ethernet/intel/i40e/i40e_common.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c
> b/drivers/net/ethernet/intel/i40e/i40e_common.c
> index 370b4bddee44..9a73cb94dc5e 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_common.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
> @@ -817,8 +817,8 @@ int i40e_pf_reset(struct i40e_hw *hw)  void
> i40e_clear_hw(struct i40e_hw *hw)  {
>   u32 num_queues, base_queue;
> - u32 num_pf_int;
> - u32 num_vf_int;
> + s32 num_pf_int;
> + s32 num_vf_int;
>   u32 num_vfs;
>   u32 i, j;
What about this vars? Are they used?

>   u32 val;
> @@ -848,18 +848,18 @@ void i40e_clear_hw(struct i40e_hw *hw)
>   /* stop all the interrupts */
>   wr32(hw, I40E_PFINT_ICR0_ENA, 0);
>   val = 0x3 << I40E_PFINT_DYN_CTLN_ITR_INDX_SHIFT;
> - for (i = 0; i < num_pf_int - 2; i++)
> + for (s32 i = 0; i < num_pf_int - 2; i++)
>   wr32(hw, I40E_PFINT_DYN_CTLN(i), val);
> 
>   /* Set the FIRSTQ_INDX field to 0x7FF in PFINT_LNKLSTx */
>   val = eol << I40E_PFINT_LNKLST0_FIRSTQ_INDX_SHIFT;
>   wr32(hw, I40E_PFINT_LNKLST0, val);
> - for (i = 0; i < num_pf_int - 2; i++)
> + for (s32 i = 0; i < num_pf_int - 2; i++)
>   wr32(hw, I40E_PFINT_LNKLSTN(i), val);
>   val = eol << I40E_VPINT_LNKLST0_FIRSTQ_INDX_SHIFT;
>   for (i = 0; i < num_vfs; i++)
>   wr32(hw, I40E_VPINT_LNKLST0(i), val);
> - for (i = 0; i < num_vf_int - 2; i++)
> + for (s32 i = 0; i < num_vf_int - 2; i++)
>   wr32(hw, I40E_VPINT_LNKLSTN(i), val);
> 
>   /* warn the HW of the coming Tx disables */
> --
> 2.25.1


Re: [Intel-wired-lan] [PATCH iwl-next v5 12/15] ixgbe: add support for devlink reload

2025-03-06 Thread Jagielski, Jedrzej
From: Simon Horman  
>Sent: Thursday, March 6, 2025 10:41 AM
>To: Jagielski, Jedrzej 
>Cc: intel-wired-...@lists.osuosl.org; Nguyen, Anthony L 
>; net...@vger.kernel.org; j...@nvidia.com; 
>Polchlopek, Mateusz ; Mrozowicz, SlawomirX 
>; Kwapulinski, Piotr 
>; Wegrzyn, Stefan 
>Subject: Re: [PATCH iwl-next v5 12/15] ixgbe: add support for devlink reload
>
>On Fri, Feb 21, 2025 at 12:51:13PM +0100, Jedrzej Jagielski wrote:
>> The E610 adapters contain an embedded chip with firmware which can be
>> updated using devlink flash. The firmware which runs on this chip is
>> referred to as the Embedded Management Processor firmware (EMP
>> firmware).
>> 
>> Activating the new firmware image currently requires that the system be
>> rebooted. This is not ideal as rebooting the system can cause unwanted
>> downtime.
>> 
>> The EMP firmware itself can be reloaded by issuing a special update
>> to the device called an Embedded Management Processor reset (EMP
>> reset). This reset causes the device to reset and reload the EMP
>> firmware.
>> 
>> Implement support for devlink reload with the "fw_activate" flag. This
>> allows user space to request the firmware be activated immediately.
>> 
>> Reviewed-by: Mateusz Polchlopek 
>> Co-developed-by: Slawomir Mrozowicz 
>> Signed-off-by: Slawomir Mrozowicz 
>> Co-developed-by: Piotr Kwapulinski 
>> Signed-off-by: Piotr Kwapulinski 
>> Co-developed-by: Stefan Wegrzyn 
>> Signed-off-by: Stefan Wegrzyn 
>> Signed-off-by: Jedrzej Jagielski 
>
>...
>
>> diff --git a/Documentation/networking/devlink/ixgbe.rst 
>> b/Documentation/networking/devlink/ixgbe.rst
>> index 41aedf4b8017..e5fef951c6f5 100644
>> --- a/Documentation/networking/devlink/ixgbe.rst
>> +++ b/Documentation/networking/devlink/ixgbe.rst
>> @@ -88,3 +88,18 @@ combined flash image that contains the ``fw.mgmt``, 
>> ``fw.undi``, and
>> and device serial number. It is expected that this combination be 
>> used with an
>> image customized for the specific device.
>>  
>> +Reload
>> +==
>> +
>> +The ``ixgbe`` driver supports activating new firmware after a flash update
>> +using ``DEVLINK_CMD_RELOAD`` with the ``DEVLINK_RELOAD_ACTION_FW_ACTIVATE``
>> +action.
>> +
>> +.. code:: shell
>> +$ devlink dev reload pci/:01:00.0 reload action fw_activate
>> +The new firmware is activated by issuing a device specific Embedded
>> +Management Processor reset which requests the device to reset and reload the
>> +EMP firmware image.
>> +
>> +The driver does not currently support reloading the driver via
>> +``DEVLINK_RELOAD_ACTION_DRIVER_REINIT``.
>
>Hi Jedrzej, all,
>
>This is not a proper review. And I didn't look into this, but make htmldocs
>complains that:
>
> .../ixgbe.rst:98: ERROR: Error in "code" directive:
> maximum 1 argument(s) allowed, 9 supplied.
> 
> .. code:: shell
> $ devlink dev reload pci/:01:00.0 reload action fw_activate
>
>...

Hi Simon

looks like blank lines might be missing there.
I will fix that in the next revision then.




Re: [Intel-wired-lan] [PATCH iwl-net] igc: Fix XSK queue NAPI ID mapping

2025-03-06 Thread florian

Hi Joe,

On 2025-03-05 19:09, Joe Damato wrote:

In commit b65969856d4f ("igc: Link queues to NAPI instances"), the XSK
queues were incorrectly unmapped from their NAPI instances. After
discussion on the mailing list and the introduction of a test to codify
the expected behavior, we can see that the unmapping causes the
check_xsk test to fail:

NETIF=enp86s0 ./tools/testing/selftests/drivers/net/queues.py

[...]
  # Check| ksft_eq(q.get('xsk', None), {},
  # Check failed None != {} xsk attr on queue we configured
  not ok 4 queues.check_xsk

After this commit, the test passes:

  ok 4 queues.check_xsk

Note that the test itself is only in net-next, so I tested this change
by applying it to my local net-next tree, booting, and running the 
test.


Cc: sta...@vger.kernel.org
Fixes: b65969856d4f ("igc: Link queues to NAPI instances")
Signed-off-by: Joe Damato 
---
 drivers/net/ethernet/intel/igc/igc_xdp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_xdp.c
b/drivers/net/ethernet/intel/igc/igc_xdp.c
index 13bbd3346e01..869815f48ac1 100644
--- a/drivers/net/ethernet/intel/igc/igc_xdp.c
+++ b/drivers/net/ethernet/intel/igc/igc_xdp.c
@@ -86,7 +86,6 @@ static int igc_xdp_enable_pool(struct igc_adapter 
*adapter,

napi_disable(napi);
}

-   igc_set_queue_napi(adapter, queue_id, NULL);
set_bit(IGC_RING_FLAG_AF_XDP_ZC, &rx_ring->flags);
set_bit(IGC_RING_FLAG_AF_XDP_ZC, &tx_ring->flags);

@@ -136,7 +135,6 @@ static int igc_xdp_disable_pool(struct igc_adapter
*adapter, u16 queue_id)
xsk_pool_dma_unmap(pool, IGC_RX_DMA_ATTR);
clear_bit(IGC_RING_FLAG_AF_XDP_ZC, &rx_ring->flags);
clear_bit(IGC_RING_FLAG_AF_XDP_ZC, &tx_ring->flags);
-   igc_set_queue_napi(adapter, queue_id, napi);

if (needs_reset) {
napi_enable(napi);


That doesn't look correct to me. You removed both invocations of
igc_set_queue_napi() from igc_xdp.c. Where is the napi mapping now
done (in case XDP is enabled)?

To me it seems flipped. igc_xdp_enable_pool() should do the mapping
(previously did the unmapping) and igc_xdp_disable_pool() should do
the unmapping (previously did the mapping). No?

Btw: I got this patch via stable. It doesn't make sense to send it
to stable where this patch does not apply.



base-commit: 3c9231ea6497dfc50ac0ef69fff484da27d0df66


[Intel-wired-lan] [tnguy-next-queue:for-next-fixes] BUILD SUCCESS f798c4bce163411673e49b7d07587aff7896ded4

2025-03-06 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git 
for-next-fixes
branch HEAD: f798c4bce163411673e49b7d07587aff7896ded4  irdma: free iwdev->rf 
after removing MSI-X

elapsed time: 1457m

configs tested: 190
configs skipped: 1

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfiggcc-14.2.0
arc   allnoconfiggcc-14.2.0
arc   randconfig-001-20250306gcc-13.2.0
arc   randconfig-001-20250307clang-15
arc   randconfig-002-20250306gcc-13.2.0
arc   randconfig-002-20250307clang-15
arcvdk_hs38_smp_defconfiggcc-14.2.0
arm   allnoconfiggcc-14.2.0
arm  exynos_defconfiggcc-14.2.0
armmvebu_v7_defconfiggcc-14.2.0
arm   randconfig-001-20250306gcc-14.2.0
arm   randconfig-001-20250307clang-15
arm   randconfig-002-20250306gcc-14.2.0
arm   randconfig-002-20250307clang-15
arm   randconfig-003-20250306gcc-14.2.0
arm   randconfig-003-20250307clang-15
arm   randconfig-004-20250306clang-18
arm   randconfig-004-20250307clang-15
arm64 allnoconfiggcc-14.2.0
arm64 randconfig-001-20250306gcc-14.2.0
arm64 randconfig-001-20250307clang-15
arm64 randconfig-002-20250306gcc-14.2.0
arm64 randconfig-002-20250307clang-15
arm64 randconfig-003-20250306gcc-14.2.0
arm64 randconfig-003-20250307clang-15
arm64 randconfig-004-20250306gcc-14.2.0
arm64 randconfig-004-20250307clang-15
csky  allnoconfiggcc-14.2.0
csky  randconfig-001-20250306gcc-14.2.0
csky  randconfig-001-20250307gcc-14.2.0
csky  randconfig-002-20250306gcc-14.2.0
csky  randconfig-002-20250307gcc-14.2.0
hexagon   allnoconfiggcc-14.2.0
hexagon   randconfig-001-20250306clang-21
hexagon   randconfig-001-20250307gcc-14.2.0
hexagon   randconfig-002-20250306clang-19
hexagon   randconfig-002-20250307gcc-14.2.0
i386 allmodconfigclang-19
i386  allnoconfigclang-19
i386 allyesconfigclang-19
i386buildonly-randconfig-001-20250306clang-19
i386buildonly-randconfig-001-20250307clang-19
i386buildonly-randconfig-002-20250306clang-19
i386buildonly-randconfig-002-20250307clang-19
i386buildonly-randconfig-003-20250306clang-19
i386buildonly-randconfig-003-20250307clang-19
i386buildonly-randconfig-004-20250306gcc-12
i386buildonly-randconfig-004-20250307clang-19
i386buildonly-randconfig-005-20250306gcc-12
i386buildonly-randconfig-005-20250307clang-19
i386buildonly-randconfig-006-20250306clang-19
i386buildonly-randconfig-006-20250307clang-19
i386defconfigclang-19
i386  randconfig-001-20250307clang-19
i386  randconfig-002-20250307clang-19
i386  randconfig-003-20250307clang-19
i386  randconfig-004-20250307clang-19
i386  randconfig-005-20250307clang-19
i386  randconfig-006-20250307clang-19
i386  randconfig-007-20250307clang-19
i386  randconfig-011-20250307gcc-11
i386  randconfig-012-20250307gcc-11
i386  randconfig-013-20250307gcc-11
i386  randconfig-014-20250307gcc-11
i386  randconfig-015-20250307gcc-11
i386  randconfig-016-20250307gcc-11
i386  randconfig-017-20250307gcc-11
loongarch allnoconfiggcc-14.2.0
loongarch randconfig-001-20250306gcc-14.2.0
loongarch randconfig-001-20250307gcc-14.2.0
loongarch randconfig-002-20250306gcc-14.2.0
loongarch randconfig-002-20250307gcc-14.2.0
m68k  allnoconfiggcc-14.2.0
m68k  amiga_defconfiggcc-14.2.0
m68km5407c3_defconfiggcc-14.2.0
microblaze   alldefconfiggcc-14.2.0
microblazeallnoconfiggcc-14.2.0
mips  allnoconfiggcc-14.2.0
mips   ip32_defcon

[Intel-wired-lan] [tnguy-net-queue:100GbE] BUILD SUCCESS 2a3e89a14864090ee4804fcec655ffc15fabf45c

2025-03-06 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git 100GbE
branch HEAD: 2a3e89a14864090ee4804fcec655ffc15fabf45c  ice: register devlink 
prior to creating health reporters

elapsed time: 1454m

configs tested: 108
configs skipped: 1

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alphaallyesconfiggcc-14.2.0
arc  allmodconfiggcc-13.2.0
arc  allyesconfiggcc-13.2.0
arc   randconfig-001-20250306gcc-13.2.0
arc   randconfig-002-20250306gcc-13.2.0
arm  allmodconfiggcc-14.2.0
arm  allyesconfiggcc-14.2.0
arm   randconfig-001-20250306gcc-14.2.0
arm   randconfig-002-20250306gcc-14.2.0
arm   randconfig-003-20250306gcc-14.2.0
arm   randconfig-004-20250306clang-18
arm64 randconfig-001-20250306gcc-14.2.0
arm64 randconfig-002-20250306gcc-14.2.0
arm64 randconfig-003-20250306gcc-14.2.0
arm64 randconfig-004-20250306gcc-14.2.0
csky  randconfig-001-20250306gcc-14.2.0
csky  randconfig-002-20250306gcc-14.2.0
hexagon  allmodconfigclang-21
hexagon  allyesconfigclang-18
hexagon   randconfig-001-20250306clang-21
hexagon   randconfig-002-20250306clang-19
i386buildonly-randconfig-001-20250306clang-19
i386buildonly-randconfig-001-20250307clang-19
i386buildonly-randconfig-002-20250306clang-19
i386buildonly-randconfig-002-20250307clang-19
i386buildonly-randconfig-003-20250306clang-19
i386buildonly-randconfig-003-20250307clang-19
i386buildonly-randconfig-004-20250306gcc-12
i386buildonly-randconfig-004-20250307clang-19
i386buildonly-randconfig-005-20250306gcc-12
i386buildonly-randconfig-005-20250307clang-19
i386buildonly-randconfig-006-20250306clang-19
i386buildonly-randconfig-006-20250307clang-19
i386  randconfig-011-20250307gcc-11
i386  randconfig-012-20250307gcc-11
i386  randconfig-013-20250307gcc-11
i386  randconfig-014-20250307gcc-11
i386  randconfig-015-20250307gcc-11
i386  randconfig-016-20250307gcc-11
i386  randconfig-017-20250307gcc-11
loongarch randconfig-001-20250306gcc-14.2.0
loongarch randconfig-002-20250306gcc-14.2.0
m68k  allnoconfiggcc-14.2.0
microblazeallnoconfiggcc-14.2.0
mips  allnoconfiggcc-14.2.0
nios2 allnoconfiggcc-14.2.0
nios2 randconfig-001-20250306gcc-14.2.0
nios2 randconfig-002-20250306gcc-14.2.0
openrisc  allnoconfiggcc-14.2.0
pariscallnoconfiggcc-14.2.0
pariscrandconfig-001-20250306gcc-14.2.0
pariscrandconfig-002-20250306gcc-14.2.0
powerpc   allnoconfiggcc-14.2.0
powerpc   randconfig-001-20250306clang-21
powerpc   randconfig-002-20250306clang-18
powerpc   randconfig-003-20250306gcc-14.2.0
powerpc64 randconfig-001-20250306clang-18
powerpc64 randconfig-002-20250306clang-21
powerpc64 randconfig-003-20250306clang-18
riscv allnoconfiggcc-14.2.0
riscv randconfig-001-20250306clang-18
riscv randconfig-002-20250306gcc-14.2.0
s390 allmodconfigclang-19
s390  allnoconfigclang-15
s390 allyesconfiggcc-14.2.0
s390  randconfig-001-20250306gcc-14.2.0
s390  randconfig-002-20250306clang-19
sh   allmodconfiggcc-14.2.0
shallnoconfiggcc-14.2.0
sh   allyesconfiggcc-14.2.0
shrandconfig-001-20250306gcc-14.2.0
shrandconfig-002-20250306gcc-14.2.0
sparcallmodconfiggcc-14.2.0
sparc allnoconfiggcc-14.2.0
sparc randconfig-001-20250306gcc-14.2.0
sparc randconfig-002-20250306gcc-14.2.0
sparc64   randconfig-001-20250306gcc-14.2.0
sparc64   randconfig-002-20250306gcc-14.2.0
um

Re: [Intel-wired-lan] [PATCH iwl-net] igc: Fix XSK queue NAPI ID mapping

2025-03-06 Thread Joe Damato
On Thu, Mar 06, 2025 at 05:27:38PM +0100, flor...@bezdeka.de wrote:
> Hi Joe,
> 
> On 2025-03-05 19:09, Joe Damato wrote:
> > In commit b65969856d4f ("igc: Link queues to NAPI instances"), the XSK
> > queues were incorrectly unmapped from their NAPI instances. After
> > discussion on the mailing list and the introduction of a test to codify
> > the expected behavior, we can see that the unmapping causes the
> > check_xsk test to fail:
> > 
> > NETIF=enp86s0 ./tools/testing/selftests/drivers/net/queues.py
> > 
> > [...]
> >   # Check| ksft_eq(q.get('xsk', None), {},
> >   # Check failed None != {} xsk attr on queue we configured
> >   not ok 4 queues.check_xsk
> > 
> > After this commit, the test passes:
> > 
> >   ok 4 queues.check_xsk
> > 
> > Note that the test itself is only in net-next, so I tested this change
> > by applying it to my local net-next tree, booting, and running the test.
> > 
> > Cc: sta...@vger.kernel.org
> > Fixes: b65969856d4f ("igc: Link queues to NAPI instances")
> > Signed-off-by: Joe Damato 
> > ---
> >  drivers/net/ethernet/intel/igc/igc_xdp.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/igc/igc_xdp.c
> > b/drivers/net/ethernet/intel/igc/igc_xdp.c
> > index 13bbd3346e01..869815f48ac1 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_xdp.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_xdp.c
> > @@ -86,7 +86,6 @@ static int igc_xdp_enable_pool(struct igc_adapter
> > *adapter,
> > napi_disable(napi);
> > }
> > 
> > -   igc_set_queue_napi(adapter, queue_id, NULL);
> > set_bit(IGC_RING_FLAG_AF_XDP_ZC, &rx_ring->flags);
> > set_bit(IGC_RING_FLAG_AF_XDP_ZC, &tx_ring->flags);
> > 
> > @@ -136,7 +135,6 @@ static int igc_xdp_disable_pool(struct igc_adapter
> > *adapter, u16 queue_id)
> > xsk_pool_dma_unmap(pool, IGC_RX_DMA_ATTR);
> > clear_bit(IGC_RING_FLAG_AF_XDP_ZC, &rx_ring->flags);
> > clear_bit(IGC_RING_FLAG_AF_XDP_ZC, &tx_ring->flags);
> > -   igc_set_queue_napi(adapter, queue_id, napi);
> > 
> > if (needs_reset) {
> > napi_enable(napi);
> 
> That doesn't look correct to me. You removed both invocations of
> igc_set_queue_napi() from igc_xdp.c. Where is the napi mapping now
> done (in case XDP is enabled)?

igc_set_queue_napi is called when the queues are created (igc_up,
__igc_open). When the queues are created they'll be linked. Whether
or not XDP is enabled does not affect the queues being linked.

The test added for this (which I mentioned in the commit message)
confirms that this is the correct behavior, as does the
documentation in Documentation/netlink/specs/netdev.yaml.

See commit df524c8f5771 ("netdev-genl: Add an XSK attribute to
queues").

> To me it seems flipped. igc_xdp_enable_pool() should do the mapping
> (previously did the unmapping) and igc_xdp_disable_pool() should do
> the unmapping (previously did the mapping). No?

In igc, all queues get their NAPIs mapped in igc_up or __igc_open. I
had mistakenly added code to remove the mapping for XDP because I
was under the impression that NAPIs should not be mapped for XDP
queues. See the commit under fixes.

This was incorrect, so this commit removes the unmapping and
corrects the behavior.

With this change, all queues have their NAPIs mapped (whether or not
they are used for AF_XDP) and is the agreed upon behavior based on
prior conversations on the list and the documentation I mentioned
above.

> Btw: I got this patch via stable. It doesn't make sense to send it
> to stable where this patch does not apply.

Maybe I made a mistake, but as far as I can tell the commit under
fixes is in 6.14-rc*:

$ git tag --contains b65969856d4f
v6.14-rc1
v6.14-rc2
v6.14-rc3
v6.14-rc4

So, I think this change is:
  - Correct
  - Definitely a "fixes" and should go to iwl-net
  - But maybe does not need to CC stable ?

If the Intel folks would like me to resend with some change please
let me know.


[Intel-wired-lan] [PATCH intel-net v2] ice: fix reservation of resources for RDMA when disabled

2025-03-06 Thread Jesse Brandeburg
From: Jesse Brandeburg 

If the CONFIG_INFINIBAND_IRDMA symbol is not enabled as a module or a
built-in, then don't let the driver reserve resources for RDMA. The result
of this change is a large savings in resources for older kernels, and a
cleaner driver configuration for the IRDMA=n case for old and new kernels.

Implement this by avoiding enabling the RDMA capability when scanning
hardware capabilities.

Note: Loading the out-of-tree irdma driver in connection to the in-kernel
ice driver, is not supported, and should not be attempted, especially when
disabling IRDMA in the kernel config.

Fixes: d25a0fc41c1f ("ice: Initialize RDMA support")
Signed-off-by: Jesse Brandeburg 
Acked-by: Dave Ertman 
---
v2: resend with acks, add note about oot irdma (Leon), reword commit
message
v1: https://lore.kernel.org/netdev/20241114000105.703740-1-jbran...@kernel.org/
---
 drivers/net/ethernet/intel/ice/ice_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c 
b/drivers/net/ethernet/intel/ice/ice_common.c
index 7a2a2e8da8fa..1e801300310e 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -2271,7 +2271,8 @@ ice_parse_common_caps(struct ice_hw *hw, struct 
ice_hw_common_caps *caps,
  caps->nvm_unified_update);
break;
case ICE_AQC_CAPS_RDMA:
-   caps->rdma = (number == 1);
+   if (IS_ENABLED(CONFIG_INFINIBAND_IRDMA))
+   caps->rdma = (number == 1);
ice_debug(hw, ICE_DBG_INIT, "%s: rdma = %d\n", prefix, 
caps->rdma);
break;
case ICE_AQC_CAPS_MAX_MTU:
-- 
2.43.0



Re: [Intel-wired-lan] [PATCH iwl-net] igc: Fix XSK queue NAPI ID mapping

2025-03-06 Thread Tony Nguyen




On 3/6/2025 8:45 AM, Joe Damato wrote:

On Thu, Mar 06, 2025 at 05:27:38PM +0100, flor...@bezdeka.de wrote:


...


Btw: I got this patch via stable. It doesn't make sense to send it
to stable where this patch does not apply.


Maybe I made a mistake, but as far as I can tell the commit under
fixes is in 6.14-rc*:

$ git tag --contains b65969856d4f
v6.14-rc1
v6.14-rc2
v6.14-rc3
v6.14-rc4

So, I think this change is:
   - Correct
   - Definitely a "fixes" and should go to iwl-net
   - But maybe does not need to CC stable ?

If the Intel folks would like me to resend with some change please
let me know.


Seems like the only change needed is the omission of stable. If so, no 
changes need. I can take care of that when sending on to netdev.


Thanks,
Tony