Re: [dpdk-dev] [EXT] Re: [PATCH v1 1/3] eal: add API for bus close

[Rohit Raj <rohit.raj@nxp.com>]

Yes, I agree.
I will rework and send this patch series again.

Regards,
Rohit Raj
Software Engineer | NXP India Private Limited
Email: rohit.raj@nxp.com


-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org> 
Sent: Tuesday, July 28, 2020 10:54 PM
To: Rohit Raj <rohit.raj@nxp.com>
Cc: Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; dev@dpdk.org
Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/3] eal: add API for bus close

Caution: EXT Email

On Tue, 28 Jul 2020 22:12:50 +0530
rohit.raj@nxp.com wrote:

> From: Rohit Raj <rohit.raj@nxp.com>
>
> As per the current code we have API for bus probe, but the bus close 
> API is missing. This breaks the multi process scenarios as objects are 
> not cleaned while terminating the secondary processes.
>
> This patch adds a new API rte_bus_close() for cleanup of bus objects 
> which were acquired during probe.
>
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
> ---
>  lib/librte_eal/common/eal_common_bus.c | 34 +++++++++++++++++++++++++-
>  lib/librte_eal/include/rte_bus.h       | 25 ++++++++++++++++++-
>  lib/librte_eal/rte_eal_version.map     |  3 +++
>  3 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c 
> b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..4c8c7eecc 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>   */
>
>  #include <stdio.h>
> @@ -56,6 +56,38 @@ rte_bus_scan(void)
>       return 0;
>  }
>
> +int
> +rte_bus_close(void)
> +{
> +     int ret;
> +     struct rte_bus *bus, *vbus = NULL;
> +
> +     TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +             if (!strcmp(bus->name, "vdev")) {
> +                     vbus = bus;
> +                     continue;
> +             }
> +
> +             if (bus->close) {
> +                     ret = bus->close();
> +                     if (ret)
> +                             RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n",
> +                                     bus->name);
> +             }
> +     }
> +
> +     if (vbus) {
> +             if (vbus->close) {
> +                     ret = vbus->close();
> +                     if (ret)
> +                             RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n",
> +                                     vbus->name);
> +             }
> +     }
> +
> +     return 0;
> +}
> +
>  /* Probe all devices of all buses */
>  int
>  rte_bus_probe(void)
> diff --git a/lib/librte_eal/include/rte_bus.h 
> b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..af4787b18 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>   */
>
>  #ifndef _RTE_BUS_H_
> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>   */
>  typedef int (*rte_bus_probe_t)(void);
>
> +/**
> + * Implementation specific close function which is responsible for 
> +closing
> + * devices on that bus.
> + *
> + * This is called while iterating over each registered bus.
> + *
> + * @return
> + *   0 for successful close
> + *   !0 for any error while closing
> + */
> +typedef int (*rte_bus_close_t)(void);
> +
>  /**
>   * Device iterator to find a device on a bus.
>   *
> @@ -248,6 +260,7 @@ struct rte_bus {
>       const char *name;            /**< Name of the bus */
>       rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
>       rte_bus_probe_t probe;       /**< Probe devices on bus */
> +     rte_bus_close_t close;       /**< Close devices on bus */
>       rte_bus_find_device_t find_device; /**< Find a device on the bus */
>       rte_bus_plug_t plug;         /**< Probe single device for drivers */
>       rte_bus_unplug_t unplug;     /**< Remove single device from driver */
> @@ -301,6 +314,16 @@ int rte_bus_scan(void);
>   */
>  int rte_bus_probe(void);
>
> +/**
> + * For each device on the buses, call the device specific close.
> + *
> + * @return
> + *    0 for successful close
> + *   !0 otherwise
> + */
> +__rte_experimental
> +int rte_bus_close(void);
> +
>  /**
>   * Dump information of all the buses registered with EAL.
>   *
> diff --git a/lib/librte_eal/rte_eal_version.map 
> b/lib/librte_eal/rte_eal_version.map
> index bf0c17c23..6a6d993a1 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -403,6 +403,9 @@ EXPERIMENTAL {
>       rte_mp_disable;
>       rte_thread_register;
>       rte_thread_unregister;
> +
> +     # added in 20.11
> +     rte_bus_close;
>  };
>
>  INTERNAL {

This be called in the rte_eal_cleanup path?