> -----Original Message-----
> From: Paul Menzel <[email protected]>
> Sent: Tuesday, August 26, 2025 10:06 PM
> To: Ertman, David M <[email protected]>
> Cc: [email protected]; Kitszel, Przemyslaw
> <[email protected]>; Loktionov, Aleksandr
> <[email protected]>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] ice: Cleanup legacy code in VF
> reset error path
>
> Dear Dave,
>
>
> Thank you for the patch. One nit for the summary/title, that the verb
> *clean up* is spelled with a space.
Following your suggestion below, I will change this line to new text.
>
> 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 ….
>
Will change to form that does not refer to a commit - this went back and forth
In internal review, and this solution seems cleanest.
> > 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.
>
I do not have a trace. This was discovered through code inspection.
> > 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 -
...
> > 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
Thanks for the review - V2 will be incoming.
DaveE
