diff mbox series

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

Message ID 20240523-net-2024-05-23-intel-net-fixes-v1-2-17a923e0bb5f@intel.com (mailing list archive)
State Accepted
Commit 82617b9a04649e83ee8731918aeadbb6e6d7cbc7
Delegated to: Netdev Maintainers
Headers show
Series Intel Wired LAN Driver Updates 2024-05-23 (ice, idpf) | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
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: 905 this patch: 905
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 3 blamed authors not CCed: brett.creeley@intel.com dan.nowlin@intel.com anthony.l.nguyen@intel.com; 7 maintainers not CCed: pabeni@redhat.com edumazet@google.com anthony.l.nguyen@intel.com brett.creeley@intel.com dan.nowlin@intel.com jesse.brandeburg@intel.com intel-wired-lan@lists.osuosl.org
netdev/build_clang success Errors and warnings before: 909 this patch: 909
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: 909 this patch: 909
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 15 this patch: 15
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-24--15-00 (tests: 1037)

Commit Message

Jacob Keller May 23, 2024, 5:45 p.m. UTC
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>
---
 drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Simon Horman May 23, 2024, 7:46 p.m. UTC | #1
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>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
index 2e9ad27cb9d1..6e8f2aab6080 100644
--- a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
@@ -45,14 +45,15 @@  int ice_vsi_add_vlan(struct ice_vsi *vsi, struct ice_vlan *vlan)
 		return -EINVAL;
 
 	err = ice_fltr_add_vlan(vsi, vlan);
-	if (err && err != -EEXIST) {
+	if (!err)
+		vsi->num_vlan++;
+	else if (err == -EEXIST)
+		err = 0;
+	else
 		dev_err(ice_pf_to_dev(vsi->back), "Failure Adding VLAN %d on VSI %i, status %d\n",
 			vlan->vid, vsi->vsi_num, err);
-		return err;
-	}
 
-	vsi->num_vlan++;
-	return 0;
+	return err;
 }
 
 /**