Re: [PATCH] net/iavf: fix crash on closing representor ports

[David Marchand <david.marchand@redhat.com>]

On Thu, Oct 26, 2023 at 12:01 PM Mingjin Ye <mingjinx.ye@intel.com> wrote:
>

Afaics, the patch title is wrong, this is a net/ice fix.


> Since the representor port needs to access the resources of the
> associated DCF when it is closed. Therefore, the correct close
> port operation is to close all the representor ports first, and
> then close the associated DCF port.
>
> If the DCF port is closed before the representor port on pmd exit.
> This will result in accessing freed resources and eventually a
> core dump will occur.
>
> This patch fixes this issue by notifying all presentor ports
> that DCF is not accessible when the DCF port is closed.
> And when the presentor port is closed, it determines if the DCF
> resources are accessible. If it can't be accessed, it will
> report an error and return.
>
> Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> Fixes: 295968d17407 ("ethdev: add namespace")

This Fixes: tag above is very likely wrong: this 295968d17407 commit
only prefixed ethdev macros and functions with the right namespace.
Please remove.


> Fixes: da9cdcd1f372 ("net/ice: fix crash on representor port closing")
> Cc: stable@dpdk.org
>

I got pinged by Song Jiale about a ice DCF crash.

Is this the same issue?
If so, if a bugzilla was opened, it must be referenced here.
And please confirm the issue is fixed.


> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
>  drivers/net/ice/ice_dcf_ethdev.h         |  1 +
>  drivers/net/ice/ice_dcf_vf_representor.c | 12 +++++++-----
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ice/ice_dcf_ethdev.h b/drivers/net/ice/ice_dcf_ethdev.h
> index 4baaec4b8b..d94ef10244 100644
> --- a/drivers/net/ice/ice_dcf_ethdev.h
> +++ b/drivers/net/ice/ice_dcf_ethdev.h
> @@ -60,6 +60,7 @@ struct ice_dcf_vf_repr {
>         struct rte_ether_addr mac_addr;
>         uint16_t switch_domain_id;
>         uint16_t vf_id;
> +       bool dcf_valid;
>
>         struct ice_dcf_vlan outer_vlan_info; /* DCF always handle outer VLAN */
>  };
> diff --git a/drivers/net/ice/ice_dcf_vf_representor.c b/drivers/net/ice/ice_dcf_vf_representor.c
> index b9fcfc80ad..b4a94427df 100644
> --- a/drivers/net/ice/ice_dcf_vf_representor.c
> +++ b/drivers/net/ice/ice_dcf_vf_representor.c
> @@ -45,6 +45,8 @@ ice_dcf_vf_repr_dev_start(struct rte_eth_dev *dev)
>  static int
>  ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
>  {
> +       struct ice_dcf_vf_repr *repr = dev->data->dev_private;

Newline missing.

> +       repr->dcf_valid = false;
>         dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN;
>
>         return 0;
> @@ -111,14 +113,13 @@ ice_dcf_vf_repr_link_update(__rte_unused struct rte_eth_dev *ethdev,
>  static __rte_always_inline struct ice_dcf_hw *
>  ice_dcf_vf_repr_hw(struct ice_dcf_vf_repr *repr)
>  {
> -       struct ice_dcf_adapter *dcf_adapter =
> -                       repr->dcf_eth_dev->data->dev_private;
> -
> -       if (!dcf_adapter) {
> +       struct ice_dcf_adapter *dcf_adapter;

Newline missing.

> +       if (!repr->dcf_valid) {
>                 PMD_DRV_LOG(ERR, "DCF for VF representor has been released\n");
>                 return NULL;
>         }
> -
> +dcf_adapter =

Indent is wrong.

> +                       repr->dcf_eth_dev->data->dev_private;

And no need for extra newline.


>         return &dcf_adapter->real_hw;
>  }
>
> @@ -414,6 +415,7 @@ ice_dcf_vf_repr_init(struct rte_eth_dev *vf_rep_eth_dev, void *init_param)
>         repr->dcf_eth_dev = param->dcf_eth_dev;
>         repr->switch_domain_id = param->switch_domain_id;
>         repr->vf_id = param->vf_id;
> +       repr->dcf_valid = true;
>         repr->outer_vlan_info.port_vlan_ena = false;
>         repr->outer_vlan_info.stripping_ena = false;
>         repr->outer_vlan_info.tpid = RTE_ETHER_TYPE_VLAN;
> --
> 2.25.1
>

Thanks.


-- 
David Marchand