On Tue, 11 Aug 2020 14:46:14 +0200 Pavel Machek <pavel@xxxxxxx> wrote: > > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > > @@ -449,16 +450,19 @@ static int i40e_config_iwarp_qvlist(stru > > "Incorrect number of iwarp vectors %u. > > Maximum %u allowed.\n", qvlist_info->num_vectors, > > msix_vf); > > - goto err; > > + ret = -EINVAL; > > + goto err_out; > > } > > And it is no longer freeing data qvlist_info() in this path. Is that > correct? Should it goto err_free instead? Hi Pavel, thanks for the review. I believe it is still correct, the logic is a bit convoluted, but tracing back, I see that the caller in i40e_main.c allocates a buffer, calls this function (eventually) with that memory cast to the *input* variable qvlist_info, and then the top caller frees the original buffer. One thing that I'll admit is confusing here is that the *input* struct qvlist_info is different than the vf->qvlist_info struct managed by this function. Maybe that they have the same name was confusing to you? It confused me for a moment while I investigated, but I believe there is no actual problem. The reason for the function working this way is that the input data is from the VF message, and the driver data structures in the PF (i40e) driver representing state of the VF are managed separately. Jesse
