On 8/27/24 21:12, Thomas Bogendoerfer wrote:
On Tue, 27 Aug 2024 09:16:51 +0200
Przemek Kitszel <przemyslaw.kits...@intel.com> wrote:
On 8/26/24 12:17, Thomas Bogendoerfer wrote:
On Mon, 26 Aug 2024 11:41:19 +0200
Jiri Pirko <j...@resnulli.us> wrote:
Mon, Aug 26, 2024 at 10:58:30AM CEST, tbogendoer...@suse.de 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 <tbogendoer...@suse.de>
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.
Thanks for persistence, I do agree, will review v2.
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.
