18/02/2020 14:13, Andrew Rybchenko:
> 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.po...@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.yi...@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 <arybche...@solarflare.com>
In the same idea as Andrew, I would suggest some uppercases,
maybe "ABOVE" and "BELOW".
Acked-by: Thomas Monjalon <tho...@monjalon.net>
