Re: [dpdk-dev] [PATCH v2 2/4] eal: modify device event callback process func

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 9/30/18 5:16 PM, Jeff Guo wrote:
> This patch modify the device event callback process function name to be
> more explicit, change the variable to be const and exposure the API out
> from private eal. The bus drivers and eal device would directly use this
> API to process device event callback.

Sorry, but from the description it is unclear what has changed to
make it necessary.

> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v2->v1:
> change the rte_dev_event_callback_prcess from internal to external api
> for bus or app usage.
> ---
>   app/test-pmd/testpmd.c                  |  4 ++--
>   lib/librte_eal/bsdapp/eal/eal_dev.c     |  8 ++++++++
>   lib/librte_eal/common/eal_common_dev.c  |  5 +++--
>   lib/librte_eal/common/eal_private.h     | 12 ------------
>   lib/librte_eal/common/include/rte_dev.h | 18 +++++++++++++++++-
>   lib/librte_eal/linuxapp/eal/eal_dev.c   |  2 +-
>   lib/librte_eal/rte_eal_version.map      |  1 +
>   7 files changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index bfef483..1313100 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -431,7 +431,7 @@ static void check_all_ports_link_status(uint32_t port_mask);
>   static int eth_event_callback(portid_t port_id,
>   			      enum rte_eth_event_type type,
>   			      void *param, void *ret_param);
> -static void eth_dev_event_callback(char *device_name,
> +static void eth_dev_event_callback(const char *device_name,
>   				enum rte_dev_event_type type,
>   				void *param);
>   static int eth_dev_event_callback_register(void);
> @@ -2249,7 +2249,7 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
>   
>   /* This function is used by the interrupt thread */
>   static void
> -eth_dev_event_callback(char *device_name, enum rte_dev_event_type type,
> +eth_dev_event_callback(const char *device_name, enum rte_dev_event_type type,
>   			     __rte_unused void *arg)
>   {
>   	uint16_t port_id;
> diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c b/lib/librte_eal/bsdapp/eal/eal_dev.c
> index ae1c558..374e737 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_dev.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c
> @@ -33,3 +33,11 @@ rte_dev_hotplug_handle_disable(void)
>   	RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n");
>   	return -1;
>   }
> +
> +void  __rte_experimental
> +rte_dev_event_callback_process(const char *device_name,
> +			       enum rte_dev_event_type event)
> +{
> +	RTE_LOG(ERR, EAL, "Device event callback process is not supported "
> +		"for FreeBSD\n");

Consider to avoid split of log message to simplify search using grep.

> +}
> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> index 678dbca..2d610a4 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -342,8 +342,9 @@ rte_dev_event_callback_unregister(const char *device_name,
>   	return ret;
>   }
>   
> -void
> -dev_callback_process(char *device_name, enum rte_dev_event_type event)
> +void __rte_experimental
> +rte_dev_event_callback_process(const char *device_name,
> +			       enum rte_dev_event_type event)
>   {
>   	struct dev_event_callback *cb_lst;
>   
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 637f20d..47e8a33 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -259,18 +259,6 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str);
>   int rte_mp_channel_init(void);
>   
>   /**
> - * Internal Executes all the user application registered callbacks for
> - * the specific device. It is for DPDK internal user only. User
> - * application should not call it directly.
> - *
> - * @param device_name
> - *  The device name.
> - * @param event
> - *  the device event type.
> - */
> -void dev_callback_process(char *device_name, enum rte_dev_event_type event);
> -
> -/**
>    * @internal
>    * Parse a device string and store its information in an
>    * rte_devargs structure.
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index ff580a0..58fab43 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -39,7 +39,7 @@ struct rte_dev_event {
>   	char *devname;			/**< device name */
>   };
>   
> -typedef void (*rte_dev_event_cb_fn)(char *device_name,
> +typedef void (*rte_dev_event_cb_fn)(const char *device_name,
>   					enum rte_dev_event_type event,
>   					void *cb_arg);
>   
> @@ -438,6 +438,22 @@ rte_dev_event_callback_unregister(const char *device_name,
>    * @warning
>    * @b EXPERIMENTAL: this API may change without prior notice
>    *
> + * Executes all the user application registered callbacks for
> + * the specific device.
> + *
> + * @param device_name
> + *  The device name.
> + * @param event
> + *  the device event type.
> + */
> +void  __rte_experimental
> +rte_dev_event_callback_process(const char *device_name,
> +			       enum rte_dev_event_type event);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
>    * Start the device event monitoring.
>    *
>    * @return
> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c
> index 9f9e1cf..01e3a04 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_dev.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c
> @@ -271,7 +271,7 @@ dev_uev_handler(__rte_unused void *param)
>   				return;
>   			}
>   		}
> -		dev_callback_process(uevent.devname, uevent.type);
> +		rte_dev_event_callback_process(uevent.devname, uevent.type);
>   	}
>   }
>   
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index a3255aa..b96da50 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -277,6 +277,7 @@ EXPERIMENTAL {
>   	rte_class_register;
>   	rte_class_unregister;
>   	rte_ctrl_thread_create;
> +	rte_dev_event_callback_process;
>   	rte_dev_event_callback_register;
>   	rte_dev_event_callback_unregister;
>   	rte_dev_event_monitor_start;