| Message ID | 20240105235439.28282-3-michael.chan@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fd7769798de8a3748c286da65d7e32437f9854bf |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bnxt_en: ntuple filter fixes | expand |
On Fri, Jan 05, 2024 at 03:54:38PM -0800, Michael Chan wrote: > After looking up an ntuple filter from a RCU hash list, the > rcu_read_unlock() call should be made after reading the structure, > or after determining that the filter cannot age out (by aRFS). > The existing code was calling rcu_read_unlock() too early in > bnxt_srxclsrldel(). > > As suggested by Simon Horman, change the code to handle the error > case of fltr_base not found in the if condition. The code looks > cleaner this way. > > Fixes: 8d7ba028aa9a ("bnxt_en: Add support for ntuple filter deletion by ethtool.") > Suggested-by: Simon Horman <horms@kernel.org> > Reported-by: Jakub Kicinski <kuba@kernel.org> > Link: https://lore.kernel.org/netdev/20240104145955.5a6df702@kernel.org/ > Signed-off-by: Michael Chan <michael.chan@broadcom.com> Thanks Michael, I agree that this addresses the issue flagged at the Link above. That it is a bug-fix, and should have a Fixes tag. And that as the cited commit has not propagated beyond net-next it is appropriate to target this patch at net-next. FWIIW, I might have separated the bug fix from the code re-arrangement that I suggested. But perhaps that is over doing things as the patch is for net-next anyway. Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index 5629ba9f4b2e..27b983c0a8a9 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -1345,25 +1345,26 @@ static int bnxt_srxclsrldel(struct bnxt *bp, struct ethtool_rxnfc *cmd) { struct ethtool_rx_flow_spec *fs = &cmd->fs; struct bnxt_filter_base *fltr_base; + struct bnxt_ntuple_filter *fltr; rcu_read_lock(); fltr_base = bnxt_get_one_fltr_rcu(bp, bp->ntp_fltr_hash_tbl, BNXT_NTP_FLTR_HASH_SIZE, fs->location); - if (fltr_base) { - struct bnxt_ntuple_filter *fltr; - - fltr = container_of(fltr_base, struct bnxt_ntuple_filter, base); + if (!fltr_base) { rcu_read_unlock(); - if (!(fltr->base.flags & BNXT_ACT_NO_AGING)) - return -EINVAL; - bnxt_hwrm_cfa_ntuple_filter_free(bp, fltr); - bnxt_del_ntp_filter(bp, fltr); - return 0; + return -ENOENT; } + fltr = container_of(fltr_base, struct bnxt_ntuple_filter, base); + if (!(fltr->base.flags & BNXT_ACT_NO_AGING)) { + rcu_read_unlock(); + return -EINVAL; + } rcu_read_unlock(); - return -ENOENT; + bnxt_hwrm_cfa_ntuple_filter_free(bp, fltr); + bnxt_del_ntp_filter(bp, fltr); + return 0; } static u64 get_ethtool_ipv4_rss(struct bnxt *bp)
After looking up an ntuple filter from a RCU hash list, the rcu_read_unlock() call should be made after reading the structure, or after determining that the filter cannot age out (by aRFS). The existing code was calling rcu_read_unlock() too early in bnxt_srxclsrldel(). As suggested by Simon Horman, change the code to handle the error case of fltr_base not found in the if condition. The code looks cleaner this way. Fixes: 8d7ba028aa9a ("bnxt_en: Add support for ntuple filter deletion by ethtool.") Suggested-by: Simon Horman <horms@kernel.org> Reported-by: Jakub Kicinski <kuba@kernel.org> Link: https://lore.kernel.org/netdev/20240104145955.5a6df702@kernel.org/ Signed-off-by: Michael Chan <michael.chan@broadcom.com> --- .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-)