* [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage
@ 2020-02-18 10:46 Ciara Power
2020-02-18 11:59 ` Ferruh Yigit
2020-02-18 13:39 ` [dpdk-dev] [PATCH v2] " Ciara Power
0 siblings, 2 replies; 6+ messages in thread
From: Ciara Power @ 2020-02-18 10:46 UTC (permalink / raw)
To: thomas, ferruh.yigit, arybchenko; +Cc: dev, Ciara Power
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. */
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage
2020-02-18 10:46 [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage Ciara Power
@ 2020-02-18 11:59 ` Ferruh Yigit
2020-02-18 13:13 ` Andrew Rybchenko
2020-02-18 13:39 ` [dpdk-dev] [PATCH v2] " Ciara Power
1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2020-02-18 11:59 UTC (permalink / raw)
To: Ciara Power, thomas, arybchenko; +Cc: dev
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage
2020-02-18 11:59 ` Ferruh Yigit
@ 2020-02-18 13:13 ` Andrew Rybchenko
2020-02-18 14:20 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Rybchenko @ 2020-02-18 13:13 UTC (permalink / raw)
To: Ferruh Yigit, Ciara Power, thomas, arybchenko; +Cc: dev
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage
2020-02-18 13:13 ` Andrew Rybchenko
@ 2020-02-18 14:20 ` Thomas Monjalon
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2020-02-18 14:20 UTC (permalink / raw)
To: Ciara Power; +Cc: Ferruh Yigit, Andrew Rybchenko, dev
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.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>
In the same idea as Andrew, I would suggest some uppercases,
maybe "ABOVE" and "BELOW".
Acked-by: Thomas Monjalon <thomas@monjalon.net>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v2] ethdev: add comment to warn of ABI breakage
2020-02-18 10:46 [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage Ciara Power
2020-02-18 11:59 ` Ferruh Yigit
@ 2020-02-18 13:39 ` Ciara Power
2020-02-18 18:40 ` Ferruh Yigit
1 sibling, 1 reply; 6+ messages in thread
From: Ciara Power @ 2020-02-18 13:39 UTC (permalink / raw)
To: thomas, ferruh.yigit, arybchenko; +Cc: dev, Ciara Power
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>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
V2: Added empty first line in comment
---
lib/librte_ethdev/rte_ethdev_core.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 7bf97e24e..9933ffdc1 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -667,6 +667,10 @@ 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. */
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH v2] ethdev: add comment to warn of ABI breakage
2020-02-18 13:39 ` [dpdk-dev] [PATCH v2] " Ciara Power
@ 2020-02-18 18:40 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2020-02-18 18:40 UTC (permalink / raw)
To: Ciara Power, thomas, arybchenko; +Cc: dev
On 2/18/2020 1:39 PM, 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>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
Applied to dpdk-next-net/master, thanks.
(uppercase ABOVE/BELOW as suggested applied while merging)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-18 18:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-18 10:46 [dpdk-dev] [PATCH] ethdev: add comment to warn of ABI breakage Ciara Power
2020-02-18 11:59 ` Ferruh Yigit
2020-02-18 13:13 ` Andrew Rybchenko
2020-02-18 14:20 ` Thomas Monjalon
2020-02-18 13:39 ` [dpdk-dev] [PATCH v2] " Ciara Power
2020-02-18 18:40 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).