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

[Jeff Guo <jia.guo@intel.com>]

andrew,

On 10/1/2018 5:46 PM, Andrew Rybchenko wrote:
> 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.
>

the reason is that not only eal device help will use the callback, but 
also vfio bus will use

the callback to handle hot-unplug. So exposure these API to let vfio 
call it. I will add detail

more at next version if need.


>> 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.
>

i think i got your point, but the 80 limitation a  line and the clearly 
meaning of the log should be the 1 prior to be considerate,

right? It should be considerate but it is inevitable.


>> +}
>> 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;
>