[dpdk-dev] [PATCH v2] vhost: mark vDPA driver API as internal

[dpdk-dev] [PATCH v2] vhost: mark vDPA driver API as internal

[Maxime Coquelin]

This patch marks the vDPA driver APIs as internal and
rename the corresponding header file to vdpa_driver.h.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---

Hi Techboard,

Please vote for an exception for this unannounced API
breakage.

Changes in v2:
==============
- Alphabetical ordering in version.map (David)
- Rename header to vdpa_driver.h (David)
- Add Techboard in Cc to vote for API breakage exception

 drivers/vdpa/ifc/ifcvf_vdpa.c               |  2 +-
 drivers/vdpa/mlx5/mlx5_vdpa.h               |  2 +-
 lib/vhost/meson.build                       |  4 +++-
 lib/vhost/vdpa.c                            |  2 +-
 lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} | 12 +++++++++---
 lib/vhost/version.map                       | 13 +++++++++----
 lib/vhost/vhost.h                           |  2 +-
 7 files changed, 25 insertions(+), 12 deletions(-)
 rename lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} (95%)

diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index dd5251d382..3853c4cf7e 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -17,7 +17,7 @@
 #include <rte_bus_pci.h>
 #include <rte_vhost.h>
 #include <rte_vdpa.h>
-#include <rte_vdpa_dev.h>
+#include <vdpa_driver.h>
 #include <rte_vfio.h>
 #include <rte_spinlock.h>
 #include <rte_log.h>
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h b/drivers/vdpa/mlx5/mlx5_vdpa.h
index cf4f384fa4..a6c9404cb0 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.h
@@ -12,7 +12,7 @@
 #pragma GCC diagnostic ignored "-Wpedantic"
 #endif
 #include <rte_vdpa.h>
-#include <rte_vdpa_dev.h>
+#include <vdpa_driver.h>
 #include <rte_vhost.h>
 #ifdef PEDANTIC
 #pragma GCC diagnostic error "-Wpedantic"
diff --git a/lib/vhost/meson.build b/lib/vhost/meson.build
index 2d8fe0239f..cdb37a4814 100644
--- a/lib/vhost/meson.build
+++ b/lib/vhost/meson.build
@@ -29,9 +29,11 @@ sources = files(
 )
 headers = files(
         'rte_vdpa.h',
-        'rte_vdpa_dev.h',
         'rte_vhost.h',
         'rte_vhost_async.h',
         'rte_vhost_crypto.h',
 )
+driver_sdk_headers = files(
+        'vdpa_driver.h',
+)
 deps += ['ethdev', 'cryptodev', 'hash', 'pci']
diff --git a/lib/vhost/vdpa.c b/lib/vhost/vdpa.c
index 6dd91859ac..09ad5d866e 100644
--- a/lib/vhost/vdpa.c
+++ b/lib/vhost/vdpa.c
@@ -17,7 +17,7 @@
 #include <rte_tailq.h>
 
 #include "rte_vdpa.h"
-#include "rte_vdpa_dev.h"
+#include "vdpa_driver.h"
 #include "vhost.h"
 
 /** Double linked list of vDPA devices. */
diff --git a/lib/vhost/rte_vdpa_dev.h b/lib/vhost/vdpa_driver.h
similarity index 95%
rename from lib/vhost/rte_vdpa_dev.h
rename to lib/vhost/vdpa_driver.h
index b0f494815f..fc2d6acedd 100644
--- a/lib/vhost/rte_vdpa_dev.h
+++ b/lib/vhost/vdpa_driver.h
@@ -2,11 +2,13 @@
  * Copyright(c) 2018 Intel Corporation
  */
 
-#ifndef _RTE_VDPA_H_DEV_
-#define _RTE_VDPA_H_DEV_
+#ifndef _VDPA_DRIVER_H_
+#define _VDPA_DRIVER_H_
 
 #include <stdbool.h>
 
+#include <rte_compat.h>
+
 #include "rte_vhost.h"
 #include "rte_vdpa.h"
 
@@ -88,6 +90,7 @@ struct rte_vdpa_device {
  * @return
  *  vDPA device pointer on success, NULL on failure
  */
+__rte_internal
 struct rte_vdpa_device *
 rte_vdpa_register_device(struct rte_device *rte_dev,
 		struct rte_vdpa_dev_ops *ops);
@@ -100,6 +103,7 @@ rte_vdpa_register_device(struct rte_device *rte_dev,
  * @return
  *  device id on success, -1 on failure
  */
+__rte_internal
 int
 rte_vdpa_unregister_device(struct rte_vdpa_device *dev);
 
@@ -115,6 +119,7 @@ rte_vdpa_unregister_device(struct rte_vdpa_device *dev);
  * @return
  *  0 on success, -1 on failure
  */
+__rte_internal
 int
 rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);
 
@@ -132,7 +137,8 @@ rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);
  * @return
  *  number of synced used entries on success, -1 on failure
  */
+__rte_internal
 int
 rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
 
-#endif /* _RTE_VDPA_DEV_H_ */
+#endif /* _VDPA_DRIVER_H_ */
diff --git a/lib/vhost/version.map b/lib/vhost/version.map
index 8ebde3f694..9de4df0886 100644
--- a/lib/vhost/version.map
+++ b/lib/vhost/version.map
@@ -8,10 +8,7 @@ DPDK_22 {
 	rte_vdpa_get_rte_device;
 	rte_vdpa_get_stats;
 	rte_vdpa_get_stats_names;
-	rte_vdpa_register_device;
-	rte_vdpa_relay_vring_used;
 	rte_vdpa_reset_stats;
-	rte_vdpa_unregister_device;
 	rte_vhost_avail_entries;
 	rte_vhost_clr_inflight_desc_packed;
 	rte_vhost_clr_inflight_desc_split;
@@ -52,7 +49,6 @@ DPDK_22 {
 	rte_vhost_get_vring_base_from_inflight;
 	rte_vhost_get_vring_num;
 	rte_vhost_gpa_to_vva;
-	rte_vhost_host_notifier_ctrl;
 	rte_vhost_log_used_vring;
 	rte_vhost_log_write;
 	rte_vhost_rx_queue_count;
@@ -86,3 +82,12 @@ EXPERIMENTAL {
 	rte_vhost_async_channel_unregister_thread_unsafe;
 	rte_vhost_clear_queue_thread_unsafe;
 };
+
+INTERNAL {
+	global;
+
+	rte_vdpa_register_device;
+	rte_vdpa_relay_vring_used;
+	rte_vdpa_unregister_device;
+	rte_vhost_host_notifier_ctrl;
+};
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 6c6a2da2c9..a373f3cf12 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -22,7 +22,7 @@
 
 #include "rte_vhost.h"
 #include "rte_vdpa.h"
-#include "rte_vdpa_dev.h"
+#include "vdpa_driver.h"
 
 #include "rte_vhost_async.h"
 
-- 
2.31.1

Re: [dpdk-dev] [dpdk-techboard] [PATCH v2] vhost: mark vDPA driver API as internal

[Thomas Monjalon]

28/10/2021 16:15, Maxime Coquelin:
> This patch marks the vDPA driver APIs as internal and
> rename the corresponding header file to vdpa_driver.h.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> 
> Hi Techboard,
> 
> Please vote for an exception for this unannounced API
> breakage.
[...]
>  lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} | 12 +++++++++---

Hiding more internal structs is a good breakage.

[...]
> --- a/lib/vhost/rte_vdpa_dev.h
> +++ b/lib/vhost/vdpa_driver.h
> +__rte_internal
>  struct rte_vdpa_device *
>  rte_vdpa_register_device(struct rte_device *rte_dev,
>  		struct rte_vdpa_dev_ops *ops);
[...]
> +__rte_internal
>  int
>  rte_vdpa_unregister_device(struct rte_vdpa_device *dev);
[...]
> +__rte_internal
>  int
>  rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);
[...]
> +__rte_internal
>  int
>  rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
[...]
> --- a/lib/vhost/version.map
> +++ b/lib/vhost/version.map
> -	rte_vdpa_register_device;
> -	rte_vdpa_relay_vring_used;
> -	rte_vdpa_unregister_device;
> -	rte_vhost_host_notifier_ctrl;

OK to remove these functions from the ABI
and mark them internal.

I suppose this breakage should not hurt too much,
as I don't see the need for out-of-tree vDPA drivers.
Of course it is always better to announce such change,
but it would be a pity to wait one more year for hiding this.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-dev] [PATCH v2] vhost: mark vDPA driver API as internal

[Xia, Chenbo]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, October 28, 2021 10:16 PM
> To: dev@dpdk.org; techboard@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> xuemingl@nvidia.com; Wang, Xiao W <xiao.w.wang@intel.com>;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2] vhost: mark vDPA driver API as internal
> 
> This patch marks the vDPA driver APIs as internal and
> rename the corresponding header file to vdpa_driver.h.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> 
> Hi Techboard,
> 
> Please vote for an exception for this unannounced API
> breakage.

More accurately, the four functions were announced :)

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

> 
> Changes in v2:
> ==============
> - Alphabetical ordering in version.map (David)
> - Rename header to vdpa_driver.h (David)
> - Add Techboard in Cc to vote for API breakage exception
> 
>  drivers/vdpa/ifc/ifcvf_vdpa.c               |  2 +-
>  drivers/vdpa/mlx5/mlx5_vdpa.h               |  2 +-
>  lib/vhost/meson.build                       |  4 +++-
>  lib/vhost/vdpa.c                            |  2 +-
>  lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} | 12 +++++++++---
>  lib/vhost/version.map                       | 13 +++++++++----
>  lib/vhost/vhost.h                           |  2 +-
>  7 files changed, 25 insertions(+), 12 deletions(-)
>  rename lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} (95%)

Re: [dpdk-dev] [PATCH v2] vhost: mark vDPA driver API as internal

[Maxime Coquelin]

On 11/1/21 07:50, Xia, Chenbo wrote:
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, October 28, 2021 10:16 PM
>> To: dev@dpdk.org; techboard@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
>> xuemingl@nvidia.com; Wang, Xiao W <xiao.w.wang@intel.com>;
>> david.marchand@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v2] vhost: mark vDPA driver API as internal
>>
>> This patch marks the vDPA driver APIs as internal and
>> rename the corresponding header file to vdpa_driver.h.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>
>> Hi Techboard,
>>
>> Please vote for an exception for this unannounced API
>> breakage.
> 
> More accurately, the four functions were announced :)

My bad, I forgot about it. Thanks for the reminder!

> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

Maxime

>>
>> Changes in v2:
>> ==============
>> - Alphabetical ordering in version.map (David)
>> - Rename header to vdpa_driver.h (David)
>> - Add Techboard in Cc to vote for API breakage exception
>>
>>   drivers/vdpa/ifc/ifcvf_vdpa.c               |  2 +-
>>   drivers/vdpa/mlx5/mlx5_vdpa.h               |  2 +-
>>   lib/vhost/meson.build                       |  4 +++-
>>   lib/vhost/vdpa.c                            |  2 +-
>>   lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} | 12 +++++++++---
>>   lib/vhost/version.map                       | 13 +++++++++----
>>   lib/vhost/vhost.h                           |  2 +-
>>   7 files changed, 25 insertions(+), 12 deletions(-)
>>   rename lib/vhost/{rte_vdpa_dev.h => vdpa_driver.h} (95%)
>