Re: [PATCH net 2/2] ice: fix accounting if a VLAN already exists

[Simon Horman <horms@kernel.org>]

On Thu, May 23, 2024 at 10:45:30AM -0700, Jacob Keller wrote:
> The ice_vsi_add_vlan() function is used to add a VLAN filter for the target
> VSI. This function prepares a filter in the switch table for the given VSI.
> If it succeeds, the vsi->num_vlan counter is incremented.
> 
> It is not considered an error to add a VLAN which already exists in the
> switch table, so the function explicitly checks and ignores -EEXIST. The
> vsi->num_vlan counter is still incremented.
> 
> This seems incorrect, as it means we can double-count in the case where the
> same VLAN is added twice by the caller. The actual table will have one less
> filter than the count.
> 
> The ice_vsi_del_vlan() function similarly checks and handles the -ENOENT
> condition for when deleting a filter that doesn't exist. This flow only
> decrements the vsi->num_vlan if it actually deleted a filter.
> 
> The vsi->num_vlan counter is used only in a few places, primarily related
> to tracking the number of non-zero VLANs. If the vsi->num_vlans gets out of
> sync, then ice_vsi_num_non_zero_vlans() will incorrectly report more VLANs
> than are present, and ice_vsi_has_non_zero_vlans() could return true
> potentially in cases where there are only VLAN 0 filters left.
> 
> Fix this by only incrementing the vsi->num_vlan in the case where we
> actually added an entry, and not in the case where the entry already
> existed.
> 
> Fixes: a1ffafb0b4a4 ("ice: Support configuring the device to Double VLAN Mode")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com>

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