Message ID | 20230109061325.21395-1-hkelam@marvell.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 53da7aec32982f5ee775b69dce06d63992ce4af3 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net] octeontx2-pf: Fix resource leakage in VF driver unbind | expand |
On Mon, Jan 09, 2023 at 11:43:25AM +0530, Hariprasad Kelam wrote: > resources allocated like mcam entries to support the Ntuple feature > and hash tables for the tc feature are not getting freed in driver > unbind. This patch fixes the issue. It is not clear where in otx2vf_probe() these resource are allocated. Please add the stack trace to the commit message. Thanks > > Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count") > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com> > --- > drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c > index 86653bb8e403..7f8ffbf79cf7 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c > @@ -758,6 +758,8 @@ static void otx2vf_remove(struct pci_dev *pdev) > if (vf->otx2_wq) > destroy_workqueue(vf->otx2_wq); > otx2_ptp_destroy(vf); > + otx2_mcam_flow_del(vf); > + otx2_shutdown_tc(vf); > otx2vf_disable_mbox_intr(vf); > otx2_detach_resources(&vf->mbox); > if (test_bit(CN10K_LMTST, &vf->hw.cap_flag)) > -- > 2.17.1 >
Hello: This patch was applied to netdev/net.git (master) by Paolo Abeni <pabeni@redhat.com>: On Mon, 9 Jan 2023 11:43:25 +0530 you wrote: > resources allocated like mcam entries to support the Ntuple feature > and hash tables for the tc feature are not getting freed in driver > unbind. This patch fixes the issue. > > Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count") > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com> > > [...] Here is the summary with links: - [net] octeontx2-pf: Fix resource leakage in VF driver unbind https://git.kernel.org/netdev/net/c/53da7aec3298 You are awesome, thank you!
On Tue, Jan 10, 2023 at 10:20:15AM +0000, patchwork-bot+netdevbpf@kernel.org wrote: > Hello: > > This patch was applied to netdev/net.git (master) > by Paolo Abeni <pabeni@redhat.com>: > > On Mon, 9 Jan 2023 11:43:25 +0530 you wrote: > > resources allocated like mcam entries to support the Ntuple feature > > and hash tables for the tc feature are not getting freed in driver > > unbind. This patch fixes the issue. > > > > Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count") > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com> > > > > [...] > > Here is the summary with links: > - [net] octeontx2-pf: Fix resource leakage in VF driver unbind > https://git.kernel.org/netdev/net/c/53da7aec3298 Paolo, I don't think that this patch should be applied. It looks like wrong Fixes to me and I don't see clearly how structures were allocated on VF which were cleared in this patch. Thanks > > You are awesome, thank you! > -- > Deet-doot-dot, I am a bot. > https://korg.docs.kernel.org/patchwork/pwbot.html > >
Hello, On Tue, 2023-01-10 at 12:29 +0200, Leon Romanovsky wrote: > On Tue, Jan 10, 2023 at 10:20:15AM +0000, patchwork-bot+netdevbpf@kernel.org wrote: > > Hello: > > > > This patch was applied to netdev/net.git (master) > > by Paolo Abeni <pabeni@redhat.com>: > > > > On Mon, 9 Jan 2023 11:43:25 +0530 you wrote: > > > resources allocated like mcam entries to support the Ntuple feature > > > and hash tables for the tc feature are not getting freed in driver > > > unbind. This patch fixes the issue. > > > > > > Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count") > > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > > > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com> > > > > > > [...] > > > > Here is the summary with links: > > - [net] octeontx2-pf: Fix resource leakage in VF driver unbind > > https://git.kernel.org/netdev/net/c/53da7aec3298 > > I don't think that this patch should be applied. > > It looks like wrong Fixes to me and I don't see clearly how structures > were allocated on VF which were cleared in this patch. My understanding is that the resource allocation happens via: otx2_dl_mcam_count_set() which is registered as the devlink parameter write operation on the vf by the fixes commit - the patch looks legit to me. Did I miss something? Thanks! Paolo
On Tue, Jan 10, 2023 at 11:43:28AM +0100, Paolo Abeni wrote: > Hello, > > On Tue, 2023-01-10 at 12:29 +0200, Leon Romanovsky wrote: > > On Tue, Jan 10, 2023 at 10:20:15AM +0000, patchwork-bot+netdevbpf@kernel.org wrote: > > > Hello: > > > > > > This patch was applied to netdev/net.git (master) > > > by Paolo Abeni <pabeni@redhat.com>: > > > > > > On Mon, 9 Jan 2023 11:43:25 +0530 you wrote: > > > > resources allocated like mcam entries to support the Ntuple feature > > > > and hash tables for the tc feature are not getting freed in driver > > > > unbind. This patch fixes the issue. > > > > > > > > Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count") > > > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > > > > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com> > > > > > > > > [...] > > > > > > Here is the summary with links: > > > - [net] octeontx2-pf: Fix resource leakage in VF driver unbind > > > https://git.kernel.org/netdev/net/c/53da7aec3298 > > > > I don't think that this patch should be applied. > > > > It looks like wrong Fixes to me and I don't see clearly how structures > > were allocated on VF which were cleared in this patch. > > My understanding is that the resource allocation happens via: > > otx2_dl_mcam_count_set() > > which is registered as the devlink parameter write operation on the vf > by the fixes commit - the patch looks legit to me. > > Did I miss something? No, you are right. I'm not sure if I would be able to see that OTX2_FLAG_MCAM_ENTRIES_ALLOC flag without your hint. Thanks
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c index 86653bb8e403..7f8ffbf79cf7 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c @@ -758,6 +758,8 @@ static void otx2vf_remove(struct pci_dev *pdev) if (vf->otx2_wq) destroy_workqueue(vf->otx2_wq); otx2_ptp_destroy(vf); + otx2_mcam_flow_del(vf); + otx2_shutdown_tc(vf); otx2vf_disable_mbox_intr(vf); otx2_detach_resources(&vf->mbox); if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))