| Message ID | 20240731-filter-return-eexist-v1-1-5b03d444a423@intel.com |
|---|---|
| State | Under Review |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [iwl-net] ice: fix accounting for filters shared by multiple VSIs | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jacob > Keller > Sent: Wednesday, July 31, 2024 6:56 PM > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Nguyen, Anthony L > <anthony.l.nguyen@intel.com> > Cc: Keller, Jacob E <jacob.e.keller@intel.com> > Subject: [Intel-wired-lan] [PATCH iwl-net] ice: fix accounting for filters shared by > multiple VSIs > > When adding a switch filter (such as a MAC or VLAN filter), it is expected that the > driver will detect the case where the filter already exists, and return -EEXIST. This > is used by calling code such as ice_vc_add_mac_addr, and ice_vsi_add_vlan to > avoid incrementing the accounting fields such as > vsi->num_vlan or vf->num_mac. > > This logic works correctly for the case where only a single VSI has added a given > switch filter. > > When a second VSI adds the same switch filter, the driver converts the existing > filter from an ICE_FWD_TO_VSI filter into an ICE_FWD_TO_VSI_LIST filter. This > saves switch resources, by ensuring that multiple VSIs can re-use the same filter. > > The ice_add_update_vsi_list() function is responsible for doing this conversion. > When first converting a filter from the FWD_TO_VSI into FWD_TO_VSI_LIST, it > checks if the VSI being added is the same as the existing rule's VSI. In such a case > it returns -EEXIST. > > However, when the switch rule has already been converted to a > FWD_TO_VSI_LIST, the logic is different. Adding a new VSI in this case just > requires extending the VSI list entry. The logic for checking if the rule already > exists in this case returns 0 instead of -EEXIST. > > This breaks the accounting logic mentioned above, so the counters for how many > MAC and VLAN filters exist for a given VF or VSI no longer accurately reflect the > actual count. This breaks other code which relies on these counts. > > In typical usage this primarily affects such filters generally shared by multiple VSIs > such as VLAN 0, or broadcast and multicast MAC addresses. > > Fix this by correctly reporting -EEXIST in the case of adding the same VSI to a > switch rule already converted to ICE_FWD_TO_VSI_LIST. > > Fixes: 9daf8208dd4d ("ice: Add support for switch filter programming") > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > --- > --- > drivers/net/ethernet/intel/ice/ice_switch.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c > b/drivers/net/ethernet/intel/ice/ice_switch.c > index fe8847184cb1..0160f0bae8d6 100644 > --- a/drivers/net/ethernet/intel/ice/ice_switch.c > +++ b/drivers/net/ethernet/intel/ice/ice_switch.c > @@ -3194,7 +3194,7 @@ ice_add_update_vsi_list(struct ice_hw *hw, Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c index fe8847184cb1..0160f0bae8d6 100644 --- a/drivers/net/ethernet/intel/ice/ice_switch.c +++ b/drivers/net/ethernet/intel/ice/ice_switch.c @@ -3194,7 +3194,7 @@ ice_add_update_vsi_list(struct ice_hw *hw, /* A rule already exists with the new VSI being added */ if (test_bit(vsi_handle, m_entry->vsi_list_info->vsi_map)) - return 0; + return -EEXIST; /* Update the previously created VSI list set with * the new VSI ID passed in
When adding a switch filter (such as a MAC or VLAN filter), it is expected that the driver will detect the case where the filter already exists, and return -EEXIST. This is used by calling code such as ice_vc_add_mac_addr, and ice_vsi_add_vlan to avoid incrementing the accounting fields such as vsi->num_vlan or vf->num_mac. This logic works correctly for the case where only a single VSI has added a given switch filter. When a second VSI adds the same switch filter, the driver converts the existing filter from an ICE_FWD_TO_VSI filter into an ICE_FWD_TO_VSI_LIST filter. This saves switch resources, by ensuring that multiple VSIs can re-use the same filter. The ice_add_update_vsi_list() function is responsible for doing this conversion. When first converting a filter from the FWD_TO_VSI into FWD_TO_VSI_LIST, it checks if the VSI being added is the same as the existing rule's VSI. In such a case it returns -EEXIST. However, when the switch rule has already been converted to a FWD_TO_VSI_LIST, the logic is different. Adding a new VSI in this case just requires extending the VSI list entry. The logic for checking if the rule already exists in this case returns 0 instead of -EEXIST. This breaks the accounting logic mentioned above, so the counters for how many MAC and VLAN filters exist for a given VF or VSI no longer accurately reflect the actual count. This breaks other code which relies on these counts. In typical usage this primarily affects such filters generally shared by multiple VSIs such as VLAN 0, or broadcast and multicast MAC addresses. Fix this by correctly reporting -EEXIST in the case of adding the same VSI to a switch rule already converted to ICE_FWD_TO_VSI_LIST. Fixes: 9daf8208dd4d ("ice: Add support for switch filter programming") Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- --- drivers/net/ethernet/intel/ice/ice_switch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 0bf50cead4c4710d9f704778c32ab8af47ddf070 change-id: 20240724-filter-return-eexist-a2e4f205f05b Best regards,