[PATCH net 0/2] Intel Wired LAN Driver Updates 2024-05-23 (ice, idpf)

[PATCH net 0/2] Intel Wired LAN Driver Updates 2024-05-23 (ice, idpf)

[Jacob Keller]

This series contains two fixes which finished up testing.

First, Alexander fixes an issue in idpf caused by enabling NAPI and
interrupts prior to actually allocating the Rx buffers.

Second, Jacob fixes the ice driver VSI VLAN counting logic to ensure that
addition and deletion of VLANs properly manages the total VSI count.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Alexander Lobakin (1):
      idpf: don't enable NAPI and interrupts prior to allocating Rx buffers

Jacob Keller (1):
      ice: fix accounting if a VLAN already exists

 drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c | 11 ++++++-----
 drivers/net/ethernet/intel/idpf/idpf_lib.c        |  1 +
 drivers/net/ethernet/intel/idpf/idpf_txrx.c       | 12 +++++++-----
 drivers/net/ethernet/intel/idpf/idpf_txrx.h       |  1 +
 4 files changed, 15 insertions(+), 10 deletions(-)
---
base-commit: c71e3a5cffd5309d7f84444df03d5b72600cc417
change-id: 20240523-net-2024-05-23-intel-net-fixes-7941f33d62d5

Best regards,
-- 
Jacob Keller <jacob.e.keller@intel.com>

[PATCH net 1/2] idpf: don't enable NAPI and interrupts prior to allocating Rx buffers

[Jacob Keller]

From: Alexander Lobakin <aleksander.lobakin@intel.com>

Currently, idpf enables NAPI and interrupts prior to allocating Rx
buffers.
This may lead to frame loss (there are no buffers to place incoming
frames) and even crashes on quick ifup-ifdown. Interrupts must be
enabled only after all the resources are here and available.
Split interrupt init into two phases: initialization and enabling,
and perform the second only after the queues are fully initialized.
Note that we can't just move interrupt initialization down the init
process, as the queues must have correct a ::q_vector pointer set
and NAPI already added in order to allocate buffers correctly.
Also, during the deinit process, disable HW interrupts first and
only then disable NAPI. Otherwise, there can be a HW event leading
to napi_schedule(), but the NAPI will already be unavailable.

Fixes: d4d558718266 ("idpf: initialize interrupts and enable vport")
Reported-by: Michal Kubiak <michal.kubiak@intel.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_lib.c  |  1 +
 drivers/net/ethernet/intel/idpf/idpf_txrx.c | 12 +++++++-----
 drivers/net/ethernet/intel/idpf/idpf_txrx.h |  1 +
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 52ceda6306a3..f1ee5584e8fa 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1394,6 +1394,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool alloc_res)
 	}
 
 	idpf_rx_init_buf_tail(vport);
+	idpf_vport_intr_ena(vport);
 
 	err = idpf_send_config_queues_msg(vport);
 	if (err) {
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 285da2177ee4..b023704bbbda 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -3746,9 +3746,9 @@ static void idpf_vport_intr_ena_irq_all(struct idpf_vport *vport)
  */
 void idpf_vport_intr_deinit(struct idpf_vport *vport)
 {
+	idpf_vport_intr_dis_irq_all(vport);
 	idpf_vport_intr_napi_dis_all(vport);
 	idpf_vport_intr_napi_del_all(vport);
-	idpf_vport_intr_dis_irq_all(vport);
 	idpf_vport_intr_rel_irq(vport);
 }
 
@@ -4179,7 +4179,6 @@ int idpf_vport_intr_init(struct idpf_vport *vport)
 
 	idpf_vport_intr_map_vector_to_qs(vport);
 	idpf_vport_intr_napi_add_all(vport);
-	idpf_vport_intr_napi_ena_all(vport);
 
 	err = vport->adapter->dev_ops.reg_ops.intr_reg_init(vport);
 	if (err)
@@ -4193,17 +4192,20 @@ int idpf_vport_intr_init(struct idpf_vport *vport)
 	if (err)
 		goto unroll_vectors_alloc;
 
-	idpf_vport_intr_ena_irq_all(vport);
-
 	return 0;
 
 unroll_vectors_alloc:
-	idpf_vport_intr_napi_dis_all(vport);
 	idpf_vport_intr_napi_del_all(vport);
 
 	return err;
 }
 
+void idpf_vport_intr_ena(struct idpf_vport *vport)
+{
+	idpf_vport_intr_napi_ena_all(vport);
+	idpf_vport_intr_ena_irq_all(vport);
+}
+
 /**
  * idpf_config_rss - Send virtchnl messages to configure RSS
  * @vport: virtual port
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index 3d046b81e507..551391e20464 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -990,6 +990,7 @@ int idpf_vport_intr_alloc(struct idpf_vport *vport);
 void idpf_vport_intr_update_itr_ena_irq(struct idpf_q_vector *q_vector);
 void idpf_vport_intr_deinit(struct idpf_vport *vport);
 int idpf_vport_intr_init(struct idpf_vport *vport);
+void idpf_vport_intr_ena(struct idpf_vport *vport);
 enum pkt_hash_types idpf_ptype_to_htype(const struct idpf_rx_ptype_decoded *decoded);
 int idpf_config_rss(struct idpf_vport *vport);
 int idpf_init_rss(struct idpf_vport *vport);

-- 
2.44.0.53.g0f9d4d28b7e6

[PATCH net 2/2] ice: fix accounting if a VLAN already exists

[Jacob Keller]

The ice_vsi_add_vlan() function is used to add a VLAN filter for the target
VSI. This function prepares a filter in the switch table for the given VSI.
If it succeeds, the vsi->num_vlan counter is incremented.

It is not considered an error to add a VLAN which already exists in the
switch table, so the function explicitly checks and ignores -EEXIST. The
vsi->num_vlan counter is still incremented.

This seems incorrect, as it means we can double-count in the case where the
same VLAN is added twice by the caller. The actual table will have one less
filter than the count.

The ice_vsi_del_vlan() function similarly checks and handles the -ENOENT
condition for when deleting a filter that doesn't exist. This flow only
decrements the vsi->num_vlan if it actually deleted a filter.

The vsi->num_vlan counter is used only in a few places, primarily related
to tracking the number of non-zero VLANs. If the vsi->num_vlans gets out of
sync, then ice_vsi_num_non_zero_vlans() will incorrectly report more VLANs
than are present, and ice_vsi_has_non_zero_vlans() could return true
potentially in cases where there are only VLAN 0 filters left.

Fix this by only incrementing the vsi->num_vlan in the case where we
actually added an entry, and not in the case where the entry already
existed.

Fixes: a1ffafb0b4a4 ("ice: Support configuring the device to Double VLAN Mode")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
index 2e9ad27cb9d1..6e8f2aab6080 100644
--- a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
@@ -45,14 +45,15 @@ int ice_vsi_add_vlan(struct ice_vsi *vsi, struct ice_vlan *vlan)
 		return -EINVAL;
 
 	err = ice_fltr_add_vlan(vsi, vlan);
-	if (err && err != -EEXIST) {
+	if (!err)
+		vsi->num_vlan++;
+	else if (err == -EEXIST)
+		err = 0;
+	else
 		dev_err(ice_pf_to_dev(vsi->back), "Failure Adding VLAN %d on VSI %i, status %d\n",
 			vlan->vid, vsi->vsi_num, err);
-		return err;
-	}
 
-	vsi->num_vlan++;
-	return 0;
+	return err;
 }
 
 /**

-- 
2.44.0.53.g0f9d4d28b7e6

Re: [PATCH net 2/2] ice: fix accounting if a VLAN already exists

[Simon Horman]

On Thu, May 23, 2024 at 10:45:30AM -0700, Jacob Keller wrote:
> The ice_vsi_add_vlan() function is used to add a VLAN filter for the target
> VSI. This function prepares a filter in the switch table for the given VSI.
> If it succeeds, the vsi->num_vlan counter is incremented.
> 
> It is not considered an error to add a VLAN which already exists in the
> switch table, so the function explicitly checks and ignores -EEXIST. The
> vsi->num_vlan counter is still incremented.
> 
> This seems incorrect, as it means we can double-count in the case where the
> same VLAN is added twice by the caller. The actual table will have one less
> filter than the count.
> 
> The ice_vsi_del_vlan() function similarly checks and handles the -ENOENT
> condition for when deleting a filter that doesn't exist. This flow only
> decrements the vsi->num_vlan if it actually deleted a filter.
> 
> The vsi->num_vlan counter is used only in a few places, primarily related
> to tracking the number of non-zero VLANs. If the vsi->num_vlans gets out of
> sync, then ice_vsi_num_non_zero_vlans() will incorrectly report more VLANs
> than are present, and ice_vsi_has_non_zero_vlans() could return true
> potentially in cases where there are only VLAN 0 filters left.
> 
> Fix this by only incrementing the vsi->num_vlan in the case where we
> actually added an entry, and not in the case where the entry already
> existed.
> 
> Fixes: a1ffafb0b4a4 ("ice: Support configuring the device to Double VLAN Mode")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 0/2] Intel Wired LAN Driver Updates 2024-05-23 (ice, idpf)

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 23 May 2024 10:45:28 -0700 you wrote:
> This series contains two fixes which finished up testing.
> 
> First, Alexander fixes an issue in idpf caused by enabling NAPI and
> interrupts prior to actually allocating the Rx buffers.
> 
> Second, Jacob fixes the ice driver VSI VLAN counting logic to ensure that
> addition and deletion of VLANs properly manages the total VSI count.
> 
> [...]

Here is the summary with links:
  - [net,1/2] idpf: don't enable NAPI and interrupts prior to allocating Rx buffers
    https://git.kernel.org/netdev/net/c/d514c8b54209
  - [net,2/2] ice: fix accounting if a VLAN already exists
    https://git.kernel.org/netdev/net/c/82617b9a0464

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html