Re: [PATCH v4 1/7] ethdev: support report register names and filter

[Jie Hai <haijie1@huawei.com>]

Hi, Thomas ,

Thanks for your review.
On 2024/2/29 17:52, Thomas Monjalon wrote:
> 26/02/2024 04:07, Jie Hai:
>> This patch adds "filter" and "names" fields to "rte_dev_reg_info"
>> structure. Names of registers in data fields can be reported and
>> the registers can be filtered by their names.
>>
>> The new API rte_eth_dev_get_reg_info_ext() is added to support
>> reporting names and filtering by names. And the original API
>> rte_eth_dev_get_reg_info() does not use the name and filter fields.
>> A local variable is used in rte_eth_dev_get_reg_info for
>> compatibility. If the drivers does not report the names, set them
>> to "offset_XXX".
> 
> Isn't it possible to implement filtering in the original function?
> What would it break?
> 
If we implement filtering in the original function
rte_eth_dev_get_reg_info(), ABI would be broken and
old app cannot run with linked to the new library.

Existing binary applications will have backwards compatibility with our 
current patch.
Although the ABI is modified, the old app can still behave normally in 
the case of dynamic linking with the new library.
And the new APP using rte_eth_dev_get_reg_info() works well with  the 
old library.
>> @@ -20,6 +25,12 @@ struct rte_dev_reg_info {
>>   	uint32_t length; /**< Number of registers to fetch */
>>   	uint32_t width; /**< Size of device register */
>>   	uint32_t version; /**< Device version */
>> +	/**
>> +	 * Filter for target subset of registers.
>> +	 * This field could affects register selection for data/length/names.
>> +	 */
>> +	const char *filter;
>> +	struct rte_eth_reg_name *names; /**< Registers name saver */
>>   };
> 
> I suppose this is an ABI break?
> Confirmed: http://mails.dpdk.org/archives/test-report/2024-February/587314.html
> 
I think  it is ABI change but not ABI break. please see above.
> 
> .
Best regards,
Jie Hai