[net PATCH 0/2] ice bug fixes

[net PATCH 0/2] ice bug fixes

[Alice Michael]

From: Tony Nguyen <anthony.l.nguyen@intel.com>

There were a couple of bugs that have been found and
fixed by Anatolii in the ice driver.  First he fixed
a bug on ring creation by setting the default value
for the teid.  Anatolli also fixed a bug with deleting
queues in ice_vc_dis_qs_msg based on their enablement.

Anatolii Gerasymenko:
  ice: Set txq_teid to ICE_INVAL_TEID on ring creation
  ice: Do not skip not enabled queues in ice_vc_dis_qs_msg

 drivers/net/ethernet/intel/ice/ice_lib.c      | 1 +
 drivers/net/ethernet/intel/ice/ice_virtchnl.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.31.1

[net PATCH 1/2] ice: Set txq_teid to ICE_INVAL_TEID on ring creation

[Alice Michael]

From: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>

When VF is freshly created, but not brought up, ring->txq_teid
value is by default set to 0.
But 0 is a valid TEID. On some platforms the Root Node of
Tx scheduler has a TEID = 0. This can cause issues as shown below.

The proper way is to set ring->txq_teid to ICE_INVAL_TEID (0xFFFFFFFF).

Testing Hints:
echo 1 > /sys/class/net/ens785f0/device/sriov_numvfs
ip link set dev ens785f0v0 up
ip link set dev ens785f0v0 down

If we have freshly created VF and quickly turn it on and off, so there
would be no time to reach VIRTCHNL_OP_CONFIG_VSI_QUEUES stage, then
VIRTCHNL_OP_DISABLE_QUEUES stage will fail with error:
[  639.531454] disable queue 89 failed 14
[  639.532233] Failed to disable LAN Tx queues, error: ICE_ERR_AQ_ERROR
[  639.533107] ice 0000:02:00.0: Failed to stop Tx ring 0 on VSI 5

The reason for the fail is that we are trying to send AQ command to
delete queue 89, which has never been created and receive an "invalid
argument" error from firmware.

As this queue has never been created, it's teid and ring->txq_teid
have default value 0.
ice_dis_vsi_txq has a check against non-existent queues:

node = ice_sched_find_node_by_teid(pi->root, q_teids[i]);
if (!node)
	continue;

But on some platforms the Root Node of Tx scheduler has a teid = 0.
Hence, ice_sched_find_node_by_teid finds a node with teid = 0 (it is
pi->root), and we go further to submit an erroneous request to firmware.

Fixes: 37bb83901286 ("ice: Move common functions out of ice_main.c part 7/7")

Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Alice Michael <alice.michael@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 6d6233204388..2774cbd5b12a 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1480,6 +1480,7 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
 		ring->tx_tstamps = &pf->ptp.port.tx;
 		ring->dev = dev;
 		ring->count = vsi->num_tx_desc;
+		ring->txq_teid = ICE_INVAL_TEID;
 		if (dvm_ena)
 			ring->flags |= ICE_TX_FLAGS_RING_VLAN_L2TAG2;
 		else
-- 
2.31.1

Re: [net PATCH 1/2] ice: Set txq_teid to ICE_INVAL_TEID on ring creation

[Jakub Kicinski]

On Fri,  1 Apr 2022 05:14:52 -0700 Alice Michael wrote:
> Fixes: 37bb83901286 ("ice: Move common functions out of ice_main.c part 7/7")
> 
> Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>

No empty lines between tags, please.

[net PATCH 2/2] ice: Do not skip not enabled queues in ice_vc_dis_qs_msg

[Alice Michael]

From: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>

Disable check for queue being enabled in ice_vc_dis_qs_msg, because
there could be a case when queues were created, but were not enabled.
We still need to delete those queues.

Normal workflow for VF looks like:
Enable path:
VIRTCHNL_OP_ADD_ETH_ADDR (opcode 10)
VIRTCHNL_OP_CONFIG_VSI_QUEUES (opcode 6)
VIRTCHNL_OP_ENABLE_QUEUES (opcode 8)

Disable path:
VIRTCHNL_OP_DISABLE_QUEUES (opcode 9)
VIRTCHNL_OP_DEL_ETH_ADDR (opcode 11)

The issue appears only in stress conditions when VF is enabled and
disabled very fast.
Eventually there will be a case, when queues are created by
VIRTCHNL_OP_CONFIG_VSI_QUEUES, but are not enabled by
VIRTCHNL_OP_ENABLE_QUEUES.
In turn, these queues are not deleted by VIRTCHNL_OP_DISABLE_QUEUES,
because there is a check whether queues are enabled in
ice_vc_dis_qs_msg.

When we bring up the VF again, we will see the "Failed to set LAN Tx queue
context" error during VIRTCHNL_OP_CONFIG_VSI_QUEUES step. This
happens because old 16 queues were not deleted and VF requests to create
16 more, but ice_sched_get_free_qparent in ice_ena_vsi_txq would fail to
find a parent node for first newly requested queue (because all nodes
are allocated to 16 old queues).

Testing Hints:

Just enable and disable VF fast enough, so it would be disabled before
reaching VIRTCHNL_OP_ENABLE_QUEUES.

while true; do
        ip link set dev ens785f0v0 up
        sleep 0.065 # adjust delay value for your machine
        ip link set dev ens785f0v0 down
done

Fixes: 77ca27c41705 ("ice: add support for virtchnl_queue_select.[tx|rx]_queues bitmap")

Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Alice Michael <alice.michael@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_virtchnl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 3f1a63815bac..69ff4b929772 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -1358,9 +1358,9 @@ static int ice_vc_dis_qs_msg(struct ice_vf *vf, u8 *msg)
 				goto error_param;
 			}
 
-			/* Skip queue if not enabled */
 			if (!test_bit(vf_q_id, vf->txq_ena))
-				continue;
+				dev_dbg(ice_pf_to_dev(vsi->back), "Queue %u on VSI %u is not enabled, but stopping it anyway\n",
+					vf_q_id, vsi->vsi_num);
 
 			ice_fill_txq_meta(vsi, ring, &txq_meta);
 
-- 
2.31.1

RE: [net PATCH 0/2] ice bug fixes

[Michael, Alice]

> -----Original Message-----
> From: Michael, Alice <alice.michael@intel.com>
> Sent: Friday, April 1, 2022 5:15 AM
> To: Michael, Alice <alice.michael@intel.com>; davem@davemloft.net;
> kuba@kernel.org; pabeni@redhat.com
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> netdev@vger.kernel.org
> Subject: [net PATCH 0/2] ice bug fixes
> 
> From: Tony Nguyen <anthony.l.nguyen@intel.com>
I had permission to use Tony's machine while formatting patches as his 
coverage, but it was my work in sending this series.

If the maintainers would like me to resend with an updated cover letter
I can resend this series or the cover letter as well.
Thanks,
Alice

> 
> There were a couple of bugs that have been found and fixed by Anatolii in
> the ice driver.  First he fixed a bug on ring creation by setting the default
> value for the teid.  Anatolli also fixed a bug with deleting queues in
> ice_vc_dis_qs_msg based on their enablement.
> 
> Anatolii Gerasymenko:
>   ice: Set txq_teid to ICE_INVAL_TEID on ring creation
>   ice: Do not skip not enabled queues in ice_vc_dis_qs_msg
> 
>  drivers/net/ethernet/intel/ice/ice_lib.c      | 1 +
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> --
> 2.31.1