Re: [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 2/18/20 2:59 PM, Ferruh Yigit wrote:
> On 2/18/2020 10:46 AM, Ciara Power wrote:
>> If a function is added to the eth_dev_ops struct before
>> tx_descriptor_status function, this will cause ABI breakage. This is due
>> to static inline functions using this function, and some other functions
>> above it in the struct, so they cannot change position. A comment is
>> added to inform developers of this possible breakage.
>>
>> Signed-off-by: Ciara Power <ciara.power@intel.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev_core.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
>> index 7bf97e24e..1ad04a129 100644
>> --- a/lib/librte_ethdev/rte_ethdev_core.h
>> +++ b/lib/librte_ethdev/rte_ethdev_core.h
>> @@ -667,6 +667,9 @@ struct eth_dev_ops {
>>  	/**< Check the status of a Rx descriptor. */
>>  	eth_tx_descriptor_status_t tx_descriptor_status;
>>  	/**< Check the status of a Tx descriptor. */
>> +	/* Static inline functions use functions above this comment.
>> +	 * New dev_ops functions should be added below to avoid breaking ABI.
>> +	 */
>>  	eth_rx_enable_intr_t       rx_queue_intr_enable;  /**< Enable Rx queue interrupt. */
>>  	eth_rx_disable_intr_t      rx_queue_intr_disable; /**< Disable Rx queue interrupt. */
>>  	eth_tx_queue_setup_t       tx_queue_setup;/**< Set up device TX queue. */
>>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

LGTM, but may I suggest to make the first line in the comment
empty to make it easier to notice. I.e.
  /*
   * Static inline functions use functions above this comment.
   * ...
   */

Or even add empty lines before and after the comment.

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>