| Message ID | 20241030160643.9950-1-pegro@friiks.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [iwl-net] i40e: Fix handling changed priv flags | expand |
[Cc: +Przemek who succeeded Jesse] Dear Peter, Thank you very much for your patch. Some minor comments. Am 30.10.24 um 17:06 schrieb pegro@friiks.de: > From: Peter Große <pegro@friiks.de> > > After assembling the new private flags on a PF, the operation to determine > the changed flags uses the wrong bitmaps. Instead of xor-ing orig_flags with > new_flags, it uses the still unchanged pf->flags, thus changed_flags is always 0. It’d be great if you reflowed for 75 characters per line. > Fix it by using the corrent bitmaps. corre*c*t > The issue was discovered while debugging why disabling source pruning > stopped working with release 6.7. Although the new flags will be copied to > pf->flags later on in that function, source pruning requires a reset of the PF, > which was skipped due to this bug. If you have the actual commands handy to reproduce it, that’d be great to have in the commit message. > Fixes: 70756d0a4727 ("i40e: Use DECLARE_BITMAP for flags and hw_features fields in i40e_pf") > Signed-off-by: Peter Große <pegro@friiks.de> > --- > drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > index c841779713f6..016c0ae6b36f 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > @@ -5306,7 +5306,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) > } > > flags_complete: > - bitmap_xor(changed_flags, pf->flags, orig_flags, I40E_PF_FLAGS_NBITS); > + bitmap_xor(changed_flags, new_flags, orig_flags, I40E_PF_FLAGS_NBITS); > > if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags)) > reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG; With the style fixes above: Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> Kind regards, Paul
[Cc: -Jesse 550 #5.1.0 Address rejected.] Am 30.10.24 um 17:34 schrieb Paul Menzel: > [Cc: +Przemek who succeeded Jesse] > > Dear Peter, > > > Thank you very much for your patch. Some minor comments. > > Am 30.10.24 um 17:06 schrieb pegro@friiks.de: >> From: Peter Große <pegro@friiks.de> >> >> After assembling the new private flags on a PF, the operation to determine >> the changed flags uses the wrong bitmaps. Instead of xor-ing orig_flags with >> new_flags, it uses the still unchanged pf->flags, thus changed_flags is always 0. > > It’d be great if you reflowed for 75 characters per line. > >> Fix it by using the corrent bitmaps. > > corre*c*t > >> The issue was discovered while debugging why disabling source pruning >> stopped working with release 6.7. Although the new flags will be copied to >> pf->flags later on in that function, source pruning requires a reset of the PF, >> which was skipped due to this bug. > > If you have the actual commands handy to reproduce it, that’d be great > to have in the commit message. > >> Fixes: 70756d0a4727 ("i40e: Use DECLARE_BITMAP for flags and hw_features fields in i40e_pf") >> Signed-off-by: Peter Große <pegro@friiks.de> >> --- >> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/ >> net/ethernet/intel/i40e/i40e_ethtool.c >> index c841779713f6..016c0ae6b36f 100644 >> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c >> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c >> @@ -5306,7 +5306,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) >> } >> flags_complete: >> - bitmap_xor(changed_flags, pf->flags, orig_flags, I40E_PF_FLAGS_NBITS); >> + bitmap_xor(changed_flags, new_flags, orig_flags, I40E_PF_FLAGS_NBITS); >> if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags)) >> reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG; > > With the style fixes above: > > Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> > > > Kind regards, > > Paul
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c index c841779713f6..016c0ae6b36f 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c @@ -5306,7 +5306,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) } flags_complete: - bitmap_xor(changed_flags, pf->flags, orig_flags, I40E_PF_FLAGS_NBITS); + bitmap_xor(changed_flags, new_flags, orig_flags, I40E_PF_FLAGS_NBITS); if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags)) reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG;