Re: [dpdk-stable] It there plan to queue this patch to stable release 19.11.9

Re: [dpdk-stable] It there plan to queue this patch to stable release 19.11.9

[Christian Ehrhardt]

On Wed, May 26, 2021 at 5:13 AM Yu, DapengX <dapengx.yu@intel.com> wrote:
>
> Hi Christian,
>

Hi Dapeng

> This patch: http://mails.dpdk.org/archives/stable/2021-March/029204.html

This patch actually was never merged in the main branch
On main we have:

commit f93949c3afc39b0f610e0661f7baa21060ada13a
Author: Haiyue Wang <haiyue.wang@intel.com>
net/ice: fix memory leak on dev closed

This one also refers to bd513ece3c40 for a fix and was in 19.11 already.
Then quite similar to what you referred to, but a more generic (all
drivers) fix on the main branch

commit 30410493759f4bae3f65497737661e27b93c2d0e
Author: Thomas Monjalon <thomas@monjalon.net>
drivers/net: check process type in close operation
See => https://github.com/DPDK/dpdk/commit/30410493759f4bae3f65497737661e27b93c2d0e

This one was in 20.11 but not targeted for stable@dpdk.org

The patch almost applies but since the change is mostly the same to
all drivers it seems backportable.

Maybe if you could sync with Thomas if this (general) solution is
applicable and prepare the backport we can add it.
It is close as I wanted to call for RC1 already, but if done soon it
should be fine.
I'll hold back 19.11.9-RC1 another day to give this a chance to be
clarified until tomorrow.

@Dapeng please talk to Thomas and consider providing a 19.11.9
backport of the fix that made it into the main branch.
@Thomas Monjalon was there a specific reason/prereq which made you not
targeting stable@ with the fix?

> Is prepared for ice NIC PMD on 19.11.7, but not be merged.
>
> Do you have plan to merge this patch in 19.11.9?
>
>
>
> Without this patch, if there are primary and secondary process use ice PMD at the same time, the processes may crash.
>
>
>
> Best regards,
>
> Dapeng Yu
>
>
>
> Note: the patch details:
>
> ============================================================
>
> From: Dapeng Yu <dapengx.yu at intel.com>
>
>
>
> The secondary processes are not allowed to release shared resources.
>
> Only process-private resources should be freed in a secondary process.
>
> So the close operation is just forbidden in a secondary process.
>
>
>
> Fixes: bd513ece3c40 ("net/ice: release port upon close")
>
> Cc: stable at dpdk.org
>
>
>
> Signed-off-by: Dapeng Yu <dapengx.yu at intel.com>
>
> ---
>
> PATCH 19.11.7-rc1
>
>
>
> drivers/net/ice/ice_ethdev.c | 3 +++
>
> 1 file changed, 3 insertions(+)
>
>
>
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
>
> index c2e659303c..2552bf228c 100644
>
> --- a/drivers/net/ice/ice_ethdev.c
>
> +++ b/drivers/net/ice/ice_ethdev.c
>
> @@ -2394,6 +2394,9 @@ ice_dev_close(struct rte_eth_dev *dev)
>
>                struct ice_adapter *ad =
>
>                                ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>
>
>
> +             if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>
> +                             return;
>
> +
>
>                /* Since stop will make link down, then the link event will be
>
>                 * triggered, disable the irq firstly to avoid the port_infoe etc
>
>                 * resources deallocation causing the interrupt service thread
>
> --
>
> 2.27.0
>
>



-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

Re: [dpdk-stable] It there plan to queue this patch to stable release 19.11.9

[Yu, DapengX]

Hi Christian,
My comments is inline.

> -----Original Message-----
> From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Sent: Monday, May 31, 2021 5:22 PM
> To: Yu, DapengX <dapengx.yu@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Yu, PingX <pingx.yu@intel.com>; dpdk stable <stable@dpdk.org>
> Subject: Re: It there plan to queue this patch to stable release 19.11.9
> 
> On Wed, May 26, 2021 at 5:13 AM Yu, DapengX <dapengx.yu@intel.com>
> wrote:
> >
> > Hi Christian,
> >
> 
> Hi Dapeng
> 
> > This patch:
> > http://mails.dpdk.org/archives/stable/2021-March/029204.html
> 
> This patch actually was never merged in the main branch On main we have:
> 
> commit f93949c3afc39b0f610e0661f7baa21060ada13a
> Author: Haiyue Wang <haiyue.wang@intel.com>
> net/ice: fix memory leak on dev closed
> 
> This one also refers to bd513ece3c40 for a fix and was in 19.11 already.
> Then quite similar to what you referred to, but a more generic (all
> drivers) fix on the main branch
> 
> commit 30410493759f4bae3f65497737661e27b93c2d0e
> Author: Thomas Monjalon <thomas@monjalon.net>
> drivers/net: check process type in close operation See =>
> https://github.com/DPDK/dpdk/commit/30410493759f4bae3f65497737661e2
> 7b93c2d0e
> 
> This one was in 20.11 but not targeted for stable@dpdk.org
> 
> The patch almost applies but since the change is mostly the same to all
> drivers it seems backportable.
> 
> Maybe if you could sync with Thomas if this (general) solution is applicable
> and prepare the backport we can add it.
> It is close as I wanted to call for RC1 already, but if done soon it should be fine.
> I'll hold back 19.11.9-RC1 another day to give this a chance to be clarified until
> tomorrow.
> 
> @Dapeng please talk to Thomas and consider providing a 19.11.9 backport of
> the fix that made it into the main branch.
[Dapeng Yu] The patch from Thomas can replace my patch and it is better solution, suggest merging Thomas's patch into 19.11.9

> @Thomas Monjalon was there a specific reason/prereq which made you not
> targeting stable@ with the fix?
> 
> > Is prepared for ice NIC PMD on 19.11.7, but not be merged.
> >
> > Do you have plan to merge this patch in 19.11.9?
> >
> >
> >
> > Without this patch, if there are primary and secondary process use ice PMD
> at the same time, the processes may crash.
> >
> >
> >
> > Best regards,
> >
> > Dapeng Yu
> >
> >
> >
> > Note: the patch details:
> >
> >
> ==========================================================
> ==
> >
> > From: Dapeng Yu <dapengx.yu at intel.com>
> >
> >
> >
> > The secondary processes are not allowed to release shared resources.
> >
> > Only process-private resources should be freed in a secondary process.
> >
> > So the close operation is just forbidden in a secondary process.
> >
> >
> >
> > Fixes: bd513ece3c40 ("net/ice: release port upon close")
> >
> > Cc: stable at dpdk.org
> >
> >
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu at intel.com>
> >
> > ---
> >
> > PATCH 19.11.7-rc1
> >
> >
> >
> > drivers/net/ice/ice_ethdev.c | 3 +++
> >
> > 1 file changed, 3 insertions(+)
> >
> >
> >
> > diff --git a/drivers/net/ice/ice_ethdev.c
> > b/drivers/net/ice/ice_ethdev.c
> >
> > index c2e659303c..2552bf228c 100644
> >
> > --- a/drivers/net/ice/ice_ethdev.c
> >
> > +++ b/drivers/net/ice/ice_ethdev.c
> >
> > @@ -2394,6 +2394,9 @@ ice_dev_close(struct rte_eth_dev *dev)
> >
> >                struct ice_adapter *ad =
> >
> >
> > ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> >
> >
> >
> > +             if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >
> > +                             return;
> >
> > +
> >
> >                /* Since stop will make link down, then the link event
> > will be
> >
> >                 * triggered, disable the irq firstly to avoid the
> > port_infoe etc
> >
> >                 * resources deallocation causing the interrupt service
> > thread
> >
> > --
> >
> > 2.27.0
> >
> >
> 
> 
> 
> --
> Christian Ehrhardt
> Staff Engineer, Ubuntu Server
> Canonical Ltd

Re: [dpdk-stable] It there plan to queue this patch to stable release 19.11.9

[Thomas Monjalon]

31/05/2021 11:21, Christian Ehrhardt:
> On Wed, May 26, 2021 at 5:13 AM Yu, DapengX <dapengx.yu@intel.com> wrote:
> > This patch: http://mails.dpdk.org/archives/stable/2021-March/029204.html
> 
> This patch actually was never merged in the main branch
> On main we have:
> 
> commit f93949c3afc39b0f610e0661f7baa21060ada13a
> Author: Haiyue Wang <haiyue.wang@intel.com>
> net/ice: fix memory leak on dev closed
> 
> This one also refers to bd513ece3c40 for a fix and was in 19.11 already.
> Then quite similar to what you referred to, but a more generic (all
> drivers) fix on the main branch
> 
> commit 30410493759f4bae3f65497737661e27b93c2d0e
> Author: Thomas Monjalon <thomas@monjalon.net>
> drivers/net: check process type in close operation
> See => https://github.com/DPDK/dpdk/commit/30410493759f4bae3f65497737661e27b93c2d0e
> 
> This one was in 20.11 but not targeted for stable@dpdk.org
> 
> The patch almost applies but since the change is mostly the same to
> all drivers it seems backportable.
> 
> Maybe if you could sync with Thomas if this (general) solution is
> applicable and prepare the backport we can add it.
> It is close as I wanted to call for RC1 already, but if done soon it
> should be fine.
> I'll hold back 19.11.9-RC1 another day to give this a chance to be
> clarified until tomorrow.
> 
> @Dapeng please talk to Thomas and consider providing a 19.11.9
> backport of the fix that made it into the main branch.
> @Thomas Monjalon was there a specific reason/prereq which made you not
> targeting stable@ with the fix?

It was not considered as a fix but a behaviour improvement :)
I agree it could be backported.

Re: [dpdk-stable] It there plan to queue this patch to stable release 19.11.9

[Christian Ehrhardt]

On Mon, May 31, 2021 at 5:01 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 31/05/2021 11:21, Christian Ehrhardt:
> > On Wed, May 26, 2021 at 5:13 AM Yu, DapengX <dapengx.yu@intel.com> wrote:
> > > This patch: http://mails.dpdk.org/archives/stable/2021-March/029204.html
> >
> > This patch actually was never merged in the main branch
> > On main we have:
> >
> > commit f93949c3afc39b0f610e0661f7baa21060ada13a
> > Author: Haiyue Wang <haiyue.wang@intel.com>
> > net/ice: fix memory leak on dev closed
> >
> > This one also refers to bd513ece3c40 for a fix and was in 19.11 already.
> > Then quite similar to what you referred to, but a more generic (all
> > drivers) fix on the main branch
> >
> > commit 30410493759f4bae3f65497737661e27b93c2d0e
> > Author: Thomas Monjalon <thomas@monjalon.net>
> > drivers/net: check process type in close operation
> > See => https://github.com/DPDK/dpdk/commit/30410493759f4bae3f65497737661e27b93c2d0e
> >
> > This one was in 20.11 but not targeted for stable@dpdk.org
> >
> > The patch almost applies but since the change is mostly the same to
> > all drivers it seems backportable.
> >
> > Maybe if you could sync with Thomas if this (general) solution is
> > applicable and prepare the backport we can add it.
> > It is close as I wanted to call for RC1 already, but if done soon it
> > should be fine.
> > I'll hold back 19.11.9-RC1 another day to give this a chance to be
> > clarified until tomorrow.
> >
> > @Dapeng please talk to Thomas and consider providing a 19.11.9
> > backport of the fix that made it into the main branch.
> > @Thomas Monjalon was there a specific reason/prereq which made you not
> > targeting stable@ with the fix?
>
> It was not considered as a fix but a behaviour improvement :)
> I agree it could be backported.

FYI - by now it is backported and part of the WIP 19.11.9

>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd