diff mbox series

[net] ice: fix vf->num_mac count with port representors

Message ID 20250410-jk-fix-v-num-mac-count-v1-1-19b3bf8fe55a@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [net] ice: fix vf->num_mac count with port representors | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 2 blamed authors not CCed: wojciech.drewek@intel.com michal.swiatkowski@linux.intel.com; 7 maintainers not CCed: wojciech.drewek@intel.com edumazet@google.com kuba@kernel.org pabeni@redhat.com andrew+netdev@lunn.ch michal.swiatkowski@linux.intel.com przemyslaw.kitszel@intel.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 140 this patch: 140
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2025-04-11--03-00 (tests: 900)

Commit Message

Jacob Keller April 10, 2025, 6:13 p.m. UTC
The ice_vc_repr_add_mac() function indicates that it does not store the MAC
address filters in the firmware. However, it still increments vf->num_mac.
This is incorrect, as vf->num_mac should represent the number of MAC
filters currently programmed to firmware.

Indeed, we only perform this increment if the requested filter is a unicast
address that doesn't match the existing vf->hw_lan_addr. In addition,
ice_vc_repr_del_mac() does not decrement the vf->num_mac counter. This
results in the counter becoming out of sync with the actual count.

As it turns out, vf->num_mac is currently only used in legacy made without
port representors. The single place where the value is checked is for
enforcing a filter limit on untrusted VFs.

Upcoming patches to support VF Live Migration will use this value when
determining the size of the TLV for MAC address filters. Fix the
representor mode function to stop incrementing the counter incorrectly.

Fixes: ac19e03ef780 ("ice: allow process VF opcodes in different ways")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
I am not certain if there is currently a way to trigger a bug from
userspace due to this incorrect count, but I think it still warrants a net
fix.
---
 drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 -
 1 file changed, 1 deletion(-)


---
base-commit: a9843689e2de1a3727d58b4225e4f8664937aefd
change-id: 20250410-jk-fix-v-num-mac-count-55acd188162b

Best regards,

Comments

Michal Swiatkowski April 11, 2025, 5:52 a.m. UTC | #1
On Thu, Apr 10, 2025 at 11:13:52AM -0700, Jacob Keller wrote:
> The ice_vc_repr_add_mac() function indicates that it does not store the MAC
> address filters in the firmware. However, it still increments vf->num_mac.
> This is incorrect, as vf->num_mac should represent the number of MAC
> filters currently programmed to firmware.
> 
> Indeed, we only perform this increment if the requested filter is a unicast
> address that doesn't match the existing vf->hw_lan_addr. In addition,
> ice_vc_repr_del_mac() does not decrement the vf->num_mac counter. This
> results in the counter becoming out of sync with the actual count.
> 
> As it turns out, vf->num_mac is currently only used in legacy made without
> port representors. The single place where the value is checked is for
> enforcing a filter limit on untrusted VFs.
> 
> Upcoming patches to support VF Live Migration will use this value when
> determining the size of the TLV for MAC address filters. Fix the
> representor mode function to stop incrementing the counter incorrectly.
> 
> Fixes: ac19e03ef780 ("ice: allow process VF opcodes in different ways")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> I am not certain if there is currently a way to trigger a bug from
> userspace due to this incorrect count, but I think it still warrants a net
> fix.
> ---
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> index 7c3006eb68dd071ab76e62d8715dc2729610586a..6446d0fcc0528656054e506c9208880ce1e417ea 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
> @@ -4275,7 +4275,6 @@ static int ice_vc_repr_add_mac(struct ice_vf *vf, u8 *msg)
>  		}
>  
>  		ice_vfhw_mac_add(vf, &al->list[i]);
> -		vf->num_mac++;
>  		break;
>  	}
>  

Right, thanks for fixing it.
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> 
> ---
> base-commit: a9843689e2de1a3727d58b4225e4f8664937aefd
> change-id: 20250410-jk-fix-v-num-mac-count-55acd188162b
> 
> Best regards,
> -- 
> Jacob Keller <jacob.e.keller@intel.com>
Simon Horman April 12, 2025, 6:51 p.m. UTC | #2
On Thu, Apr 10, 2025 at 11:13:52AM -0700, Jacob Keller wrote:
> The ice_vc_repr_add_mac() function indicates that it does not store the MAC
> address filters in the firmware. However, it still increments vf->num_mac.
> This is incorrect, as vf->num_mac should represent the number of MAC
> filters currently programmed to firmware.
> 
> Indeed, we only perform this increment if the requested filter is a unicast
> address that doesn't match the existing vf->hw_lan_addr. In addition,
> ice_vc_repr_del_mac() does not decrement the vf->num_mac counter. This
> results in the counter becoming out of sync with the actual count.
> 
> As it turns out, vf->num_mac is currently only used in legacy made without
> port representors. The single place where the value is checked is for
> enforcing a filter limit on untrusted VFs.
> 
> Upcoming patches to support VF Live Migration will use this value when
> determining the size of the TLV for MAC address filters. Fix the
> representor mode function to stop incrementing the counter incorrectly.
> 
> Fixes: ac19e03ef780 ("ice: allow process VF opcodes in different ways")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> I am not certain if there is currently a way to trigger a bug from
> userspace due to this incorrect count, but I think it still warrants a net
> fix.

Reviewed-by: Simon Horman <horms@kernel.org>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 7c3006eb68dd071ab76e62d8715dc2729610586a..6446d0fcc0528656054e506c9208880ce1e417ea 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -4275,7 +4275,6 @@  static int ice_vc_repr_add_mac(struct ice_vf *vf, u8 *msg)
 		}
 
 		ice_vfhw_mac_add(vf, &al->list[i]);
-		vf->num_mac++;
 		break;
 	}