Dear Dave,

Thank you for the patch. One nit for the summary/title, that the verb *clean up* is spelled with a space.

Am 26.08.25 um 13:25 schrieb Dave Ertman:
In a previous commit (see Fixes tag), code to handle the LAG part

Instead of “see Fixes tag” I’d add the hash and summary into the text to not have the reader jump to the bottom. Or:

Moving the code to handle the LAG part of a VF reset to helper functions deprecated the function ….

of a VF reset was refactored and moved into helper functions.
This deprecated the function ice_lag_move_new_vf_nodes().  The
cleanup missed a call to this function in the error path of
ice_vc_cfg_qs_msg().

In the case that would end in the error path, a NULL pointer would
be encountered due to the empty list of netdevs for members of the
aggregate.

Do you have the trace? If yes, please paste it.

Remove the unnecessary call to ice_lag_move_new_vf_nodes(), and since
this is the only call to this function, remove the function as well.

Reading the message and the diff, I’d use:

ice: Remove deprecated ice_lag_move_new_vf_nodes()

Fixes: 351d8d8ab6af ("ice: breakout common LAG code into helpers")
Reviewed-by: Przemek Kitszel <[email protected]>
Reviewed-by: Aleksandr Loktionov <[email protected]>
Signed-off-by: Dave Ertman <[email protected]>
---
This goes to -next as the code is not yet present in -net
---
  drivers/net/ethernet/intel/ice/ice_lag.c     | 55 --------------------
  drivers/net/ethernet/intel/ice/ice_lag.h     |  1 -
  drivers/net/ethernet/intel/ice/virt/queues.c |  2 -
  3 files changed, 58 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c 
b/drivers/net/ethernet/intel/ice/ice_lag.c
index 80312e1dcf7f..aebf8e08a297 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -789,61 +789,6 @@ ice_lag_move_single_vf_nodes(struct ice_lag *lag, u8 
oldport, u8 newport,
                ice_lag_move_vf_node_tc(lag, oldport, newport, vsi_num, tc);
  }
-/**
- * ice_lag_move_new_vf_nodes - Move Tx scheduling nodes for a VF if required
- * @vf: the VF to move Tx nodes for
- *
- * Called just after configuring new VF queues. Check whether the VF Tx
- * scheduling nodes need to be updated to fail over to the active port. If so,
- * move them now.
- */
-void ice_lag_move_new_vf_nodes(struct ice_vf *vf)
-{
-       struct ice_lag_netdev_list ndlist;
-       u8 pri_port, act_port;
-       struct ice_lag *lag;
-       struct ice_vsi *vsi;
-       struct ice_pf *pf;
-
-       vsi = ice_get_vf_vsi(vf);
-
-       if (WARN_ON(!vsi))
-               return;
-
-       if (WARN_ON(vsi->type != ICE_VSI_VF))
-               return;
-
-       pf = vf->pf;
-       lag = pf->lag;
-
-       mutex_lock(&pf->lag_mutex);
-       if (!lag->bonded)
-               goto new_vf_unlock;
-
-       pri_port = pf->hw.port_info->lport;
-       act_port = lag->active_port;
-
-       if (lag->upper_netdev)
-               ice_lag_build_netdev_list(lag, &ndlist);
-
-       if (lag->bonded && lag->primary && !list_empty(lag->netdev_head)) {
-               if (lag->bond_aa &&
-                   ice_is_feature_supported(pf, ICE_F_SRIOV_AA_LAG))
-                       ice_lag_aa_failover(lag, ICE_LAGS_IDX, NULL);
-
-               if (!lag->bond_aa &&
-                   ice_is_feature_supported(pf, ICE_F_SRIOV_LAG) &&
-                   pri_port != act_port)
-                       ice_lag_move_single_vf_nodes(lag, pri_port, act_port,
-                                                    vsi->idx);
-       }
-
-       ice_lag_destroy_netdev_list(lag, &ndlist);
-
-new_vf_unlock:
-       mutex_unlock(&pf->lag_mutex);
-}
-
  /**
   * ice_lag_move_vf_nodes - move Tx scheduling nodes for all VFs to new port
   * @lag: lag info struct
diff --git a/drivers/net/ethernet/intel/ice/ice_lag.h 
b/drivers/net/ethernet/intel/ice/ice_lag.h
index e2a0a782bdd7..f77ebcd61042 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.h
+++ b/drivers/net/ethernet/intel/ice/ice_lag.h
@@ -82,7 +82,6 @@ struct ice_lag_work {
        } info;
  };
-void ice_lag_move_new_vf_nodes(struct ice_vf *vf);
  void ice_lag_aa_failover(struct ice_lag *lag, u8 dest, struct ice_pf *e_pf);
  int ice_init_lag(struct ice_pf *pf);
  void ice_deinit_lag(struct ice_pf *pf);
diff --git a/drivers/net/ethernet/intel/ice/virt/queues.c 
b/drivers/net/ethernet/intel/ice/virt/queues.c
index 5eb34030426c..9c8daffb66bf 100644
--- a/drivers/net/ethernet/intel/ice/virt/queues.c
+++ b/drivers/net/ethernet/intel/ice/virt/queues.c
@@ -905,8 +905,6 @@ int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg)
        ice_lag_complete_vf_reset(pf->lag, act_prt);
        mutex_unlock(&pf->lag_mutex);
- ice_lag_move_new_vf_nodes(vf);
-
        /* send the response to the VF */
        return ice_vc_send_msg_to_vf(vf, VIRTCHNL_OP_CONFIG_VSI_QUEUES,
                                     VIRTCHNL_STATUS_ERR_PARAM, NULL, 0);

Reviewed-by: Paul Menzel <[email protected]>


Kind regards,

Paul

Reply via email to