From: Suman Ghosh <sumang@marvell.com>
To: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
<pabeni@redhat.com>, <sgoutham@marvell.com>,
<sbhatta@marvell.com>, <jerinj@marvell.com>, <gakula@marvell.com>,
<hkelam@marvell.com>, <lcherian@marvell.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<horms@kernel.org>, <wojciech.drewek@intel.com>
Cc: Suman Ghosh <sumang@marvell.com>
Subject: [net-next PATCH v2] octeontx2-af: Fix multicast/mirror group lock/unlock issue
Date: Wed, 13 Dec 2023 15:23:49 +0530 [thread overview]
Message-ID: <20231213095349.69895-1-sumang@marvell.com> (raw)
As per the existing implementation, there exists a race between finding
a multicast/mirror group entry and deleting that entry. The group lock
was taken and released independently by rvu_nix_mcast_find_grp_elem()
function. Which is incorrect and group lock should be taken during the
entire operation of group updation/deletion. This patch fixes the same.
Fixes: 51b2804c19cd ("octeontx2-af: Add new mbox to support multicast/mirror offload")
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
Note: This is a follow up of
https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_netdev_net-2Dnext_c_51b2804c19cd&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=7si3Xn9Ly-Se1a655kvEPIYU0nQ9HPeN280sEUv5ROU&m=NjKPoTkYVlL5Dh4aSr3-dVo-AukiIperlvB0S4_Mqzkyl_VcYAAKrWhkGZE5Cx-p&s=AkBf0454Xm-0adqV0Os7ZE8peaCXtYyuNbCS5kit6Jk&e=
v2 changes:
- Addresed review comments from Simon about a missed unlock and re-org the code
with some goto labels.
.../ethernet/marvell/octeontx2/af/rvu_nix.c | 84 ++++++++++++-------
1 file changed, 54 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index b01503acd520..72e0a7717c3e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -6142,14 +6142,12 @@ static struct nix_mcast_grp_elem *rvu_nix_mcast_find_grp_elem(struct nix_mcast_g
struct nix_mcast_grp_elem *iter;
bool is_found = false;
- mutex_lock(&mcast_grp->mcast_grp_lock);
list_for_each_entry(iter, &mcast_grp->mcast_grp_head, list) {
if (iter->mcast_grp_idx == mcast_grp_idx) {
is_found = true;
break;
}
}
- mutex_unlock(&mcast_grp->mcast_grp_lock);
if (is_found)
return iter;
@@ -6162,7 +6160,7 @@ int rvu_nix_mcast_get_mce_index(struct rvu *rvu, u16 pcifunc, u32 mcast_grp_idx)
struct nix_mcast_grp_elem *elem;
struct nix_mcast_grp *mcast_grp;
struct nix_hw *nix_hw;
- int blkaddr;
+ int blkaddr, ret;
blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
nix_hw = get_nix_hw(rvu->hw, blkaddr);
@@ -6170,11 +6168,15 @@ int rvu_nix_mcast_get_mce_index(struct rvu *rvu, u16 pcifunc, u32 mcast_grp_idx)
return NIX_AF_ERR_INVALID_NIXBLK;
mcast_grp = &nix_hw->mcast_grp;
+ mutex_lock(&mcast_grp->mcast_grp_lock);
elem = rvu_nix_mcast_find_grp_elem(mcast_grp, mcast_grp_idx);
if (!elem)
- return NIX_AF_ERR_INVALID_MCAST_GRP;
+ ret = NIX_AF_ERR_INVALID_MCAST_GRP;
+ else
+ ret = elem->mce_start_index;
- return elem->mce_start_index;
+ mutex_unlock(&mcast_grp->mcast_grp_lock);
+ return ret;
}
void rvu_nix_mcast_flr_free_entries(struct rvu *rvu, u16 pcifunc)
@@ -6238,7 +6240,7 @@ int rvu_nix_mcast_update_mcam_entry(struct rvu *rvu, u16 pcifunc,
struct nix_mcast_grp_elem *elem;
struct nix_mcast_grp *mcast_grp;
struct nix_hw *nix_hw;
- int blkaddr;
+ int blkaddr, ret = 0;
blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
nix_hw = get_nix_hw(rvu->hw, blkaddr);
@@ -6246,13 +6248,15 @@ int rvu_nix_mcast_update_mcam_entry(struct rvu *rvu, u16 pcifunc,
return NIX_AF_ERR_INVALID_NIXBLK;
mcast_grp = &nix_hw->mcast_grp;
+ mutex_lock(&mcast_grp->mcast_grp_lock);
elem = rvu_nix_mcast_find_grp_elem(mcast_grp, mcast_grp_idx);
if (!elem)
- return NIX_AF_ERR_INVALID_MCAST_GRP;
-
- elem->mcam_index = mcam_index;
+ ret = NIX_AF_ERR_INVALID_MCAST_GRP;
+ else
+ elem->mcam_index = mcam_index;
- return 0;
+ mutex_unlock(&mcast_grp->mcast_grp_lock);
+ return ret;
}
int rvu_mbox_handler_nix_mcast_grp_create(struct rvu *rvu,
@@ -6297,18 +6301,27 @@ int rvu_mbox_handler_nix_mcast_grp_destroy(struct rvu *rvu,
struct npc_delete_flow_rsp uninstall_rsp = { 0 };
struct nix_mcast_grp_elem *elem;
struct nix_mcast_grp *mcast_grp;
+ int blkaddr, err, ret = 0;
struct nix_mcast *mcast;
struct nix_hw *nix_hw;
- int blkaddr, err;
err = nix_get_struct_ptrs(rvu, req->hdr.pcifunc, &nix_hw, &blkaddr);
if (err)
return err;
mcast_grp = &nix_hw->mcast_grp;
+
+ /* If AF is requesting for the deletion,
+ * then AF is already taking the lock
+ */
+ if (!req->is_af)
+ mutex_lock(&mcast_grp->mcast_grp_lock);
+
elem = rvu_nix_mcast_find_grp_elem(mcast_grp, req->mcast_grp_idx);
- if (!elem)
- return NIX_AF_ERR_INVALID_MCAST_GRP;
+ if (!elem) {
+ ret = NIX_AF_ERR_INVALID_MCAST_GRP;
+ goto unlock_grp;
+ }
/* If no mce entries are associated with the group
* then just remove it from the global list.
@@ -6333,19 +6346,15 @@ int rvu_mbox_handler_nix_mcast_grp_destroy(struct rvu *rvu,
mutex_unlock(&mcast->mce_lock);
delete_grp:
- /* If AF is requesting for the deletion,
- * then AF is already taking the lock
- */
- if (!req->is_af)
- mutex_lock(&mcast_grp->mcast_grp_lock);
-
list_del(&elem->list);
kfree(elem);
mcast_grp->count--;
+
+unlock_grp:
if (!req->is_af)
mutex_unlock(&mcast_grp->mcast_grp_lock);
- return 0;
+ return ret;
}
int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
@@ -6370,9 +6379,18 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
return err;
mcast_grp = &nix_hw->mcast_grp;
+
+ /* If AF is requesting for the updation,
+ * then AF is already taking the lock
+ */
+ if (!req->is_af)
+ mutex_lock(&mcast_grp->mcast_grp_lock);
+
elem = rvu_nix_mcast_find_grp_elem(mcast_grp, req->mcast_grp_idx);
- if (!elem)
- return NIX_AF_ERR_INVALID_MCAST_GRP;
+ if (!elem) {
+ ret = NIX_AF_ERR_INVALID_MCAST_GRP;
+ goto unlock_grp;
+ }
/* If any pcifunc matches the group's pcifunc, then we can
* delete the entire group.
@@ -6383,9 +6401,10 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
/* Delete group */
dreq.hdr.pcifunc = elem->pcifunc;
dreq.mcast_grp_idx = elem->mcast_grp_idx;
- dreq.is_af = req->is_af;
+ dreq.is_af = 1;
rvu_mbox_handler_nix_mcast_grp_destroy(rvu, &dreq, NULL);
- return 0;
+ ret = 0;
+ goto unlock_grp;
}
}
}
@@ -6410,7 +6429,7 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
npc_enable_mcam_entry(rvu, mcam, npc_blkaddr,
elem->mcam_index, true);
ret = NIX_AF_ERR_NON_CONTIG_MCE_LIST;
- goto done;
+ goto unlock_mce;
}
}
@@ -6426,7 +6445,7 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
npc_enable_mcam_entry(rvu, mcam, npc_blkaddr,
elem->mcam_index, true);
- goto done;
+ goto unlock_mce;
}
} else {
if (!prev_count || prev_count < req->num_mce_entry) {
@@ -6434,7 +6453,7 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
npc_enable_mcam_entry(rvu, mcam, npc_blkaddr,
elem->mcam_index, true);
ret = NIX_AF_ERR_INVALID_MCAST_DEL_REQ;
- goto done;
+ goto unlock_mce;
}
nix_free_mce_list(mcast, prev_count, elem->mce_start_index, elem->dir);
@@ -6450,14 +6469,14 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
elem->mcam_index,
true);
- goto done;
+ goto unlock_mce;
}
}
if (elem->mcam_index == -1) {
rsp->mce_start_index = elem->mce_start_index;
ret = 0;
- goto done;
+ goto unlock_mce;
}
nix_mcast_update_action(rvu, elem);
@@ -6465,7 +6484,12 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
rsp->mce_start_index = elem->mce_start_index;
ret = 0;
-done:
+unlock_mce:
mutex_unlock(&mcast->mce_lock);
+
+unlock_grp:
+ if (!req->is_af)
+ mutex_unlock(&mcast_grp->mcast_grp_lock);
+
return ret;
}
--
2.25.1
next reply other threads:[~2023-12-13 9:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-13 9:53 Suman Ghosh [this message]
2023-12-14 19:45 ` [net-next PATCH v2] octeontx2-af: Fix multicast/mirror group lock/unlock issue Simon Horman
2023-12-15 10:20 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231213095349.69895-1-sumang@marvell.com \
--to=sumang@marvell.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gakula@marvell.com \
--cc=hkelam@marvell.com \
--cc=horms@kernel.org \
--cc=jerinj@marvell.com \
--cc=kuba@kernel.org \
--cc=lcherian@marvell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sbhatta@marvell.com \
--cc=sgoutham@marvell.com \
--cc=wojciech.drewek@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.