diff mbox series

[iwl-net] i40e: Fix handling changed priv flags

Message ID 20241030160643.9950-1-pegro@friiks.de
State New
Headers show
Series [iwl-net] i40e: Fix handling changed priv flags | expand

Commit Message

pegro@friiks.de Oct. 30, 2024, 4:06 p.m. UTC
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.

Fix it by using the corrent bitmaps.

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.

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(-)

Comments

Paul Menzel Oct. 30, 2024, 4:34 p.m. UTC | #1
[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
Paul Menzel Oct. 30, 2024, 4:38 p.m. UTC | #2
[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 mbox series

Patch

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;