On Tue, 27 Aug 2024 09:16:51 +0200 Przemek Kitszel <[email protected]> wrote:
> On 8/26/24 12:17, Thomas Bogendoerfer wrote: > > On Mon, 26 Aug 2024 11:41:19 +0200 > > Jiri Pirko <[email protected]> wrote: > > > >> Mon, Aug 26, 2024 at 10:58:30AM CEST, [email protected] wrote: > >>> For PFs, which don't support SRIOV_LAG, there is no pf->lag struct > >>> allocated. So before accessing pf->lag a NULL pointer check is needed. > >>> > >>> Signed-off-by: Thomas Bogendoerfer <[email protected]> > >> > >> You need to add a "fixes" tag blaming the commit that introduced the > >> bug. > > Would be also good to CC the author. sure, I'm using get_maintainer for building address line and looks like it only adds the author, if there is a Fixes tag, which IMHO makes more sense than mailing all possible authors of file (in this case it would work, but there are other files). > > Fixes: 1e0f9881ef79 ("ice: Flesh out implementation of support for > > SRIOV on bonded interface") > > the bug was introduced later, the tag should be: > Fixes: ec5a6c5f79ed ("ice: process events created by lag netdev event > handler") I'd like to disagree, ec5a6c5f79ed adds an empty ice_lag_move_new_vf_nodes(), which will do no harm if pf->lag is NULL. Commit 1e0f9881ef79 introduces the access to pf->lag without checking for NULL. > > The mentioned commit extracted code into ice_lag_move_new_vf_nodes(), > and there is just one call to this function by now, just after > releasing lag_mutex, so would be good to change the semantics of > ice_lag_move_new_vf_nodes() to "only for lag-enabled flows, with > lag_mutex held", and fix the call to it to reflect that. I could do that for sure, but IMHO this is about fixing a bug, which crashes the kernel. Making the code better should be done after fixing. Thomas. -- SUSE Software Solutions Germany GmbH HRB 36809 (AG Nürnberg) Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
