[dpdk-dev] [PATCH] vhost: add doxygen comment to vDPA header

[dpdk-dev] [PATCH] vhost: add doxygen comment to vDPA header

[Xiaolong Ye]

As APIs in rte_vdpa.h are public, we need to add doxygen comments
to all APIs and structures.

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 doc/api/doxy-api-index.md   |  1 +
 lib/librte_vhost/rte_vdpa.h | 54 +++++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 9584ccb4c..a3039d168 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -37,6 +37,7 @@ The public API headers are grouped by topics:
   [softnic]            (@ref rte_eth_softnic.h),
   [bond]               (@ref rte_eth_bond.h),
   [vhost]              (@ref rte_vhost.h),
+  [vdpa]               (@ref rte_vdpa.h),
   [KNI]                (@ref rte_kni.h),
   [ixgbe]              (@ref rte_pmd_ixgbe.h),
   [i40e]               (@ref rte_pmd_i40e.h),
diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
index b8223e337..bdbb2ae18 100644
--- a/lib/librte_vhost/rte_vdpa.h
+++ b/lib/librte_vhost/rte_vdpa.h
@@ -21,6 +21,9 @@ enum vdpa_addr_type {
 	VDPA_ADDR_MAX
 };
 
+/**
+ * vdpa device address
+ */
 struct rte_vdpa_dev_addr {
 	enum vdpa_addr_type type;
 	union {
@@ -29,6 +32,9 @@ struct rte_vdpa_dev_addr {
 	};
 };
 
+/**
+ * vdpa device operations
+ */
 struct rte_vdpa_dev_ops {
 	/* Get capabilities of this device */
 	int (*get_queue_num)(int did, uint32_t *queue_num);
@@ -62,29 +68,67 @@ struct rte_vdpa_dev_ops {
 	void *reserved[5];
 };
 
+/**
+ * vdpa device structure includes device address and device operations.
+ */
 struct rte_vdpa_device {
 	struct rte_vdpa_dev_addr addr;
 	struct rte_vdpa_dev_ops *ops;
 } __rte_cache_aligned;
 
-/* Register a vdpa device, return did if successful, -1 on failure */
+/**
+ * Register a vdpa device
+ *
+ * @param addr
+ *  the vdpa device address
+ * @parm ops
+ *  the vdpa device operations
+ * @return
+ *  device id on success, -1 on failure
+ */
 int __rte_experimental
 rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
 		struct rte_vdpa_dev_ops *ops);
 
-/* Unregister a vdpa device, return -1 on failure */
+/**
+ * Unregister a vdpa device
+ *
+ * @param did
+ *  vdpa device id
+ * @return
+ *  device id on success, -1 on failure
+ */
 int __rte_experimental
 rte_vdpa_unregister_device(int did);
 
-/* Find did of a vdpa device, return -1 on failure */
+/**
+ * Find the device id of a vdpa device
+ *
+ * @param addr
+ *  the vdpa device address
+ * @return
+ *  device id on success, -1 on failure
+ */
 int __rte_experimental
 rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr);
 
-/* Find a vdpa device based on did */
+/**
+ * Find a vdpa device based on device id
+ *
+ * @param did
+ *  device id
+ * @return
+ *  rte_vdpa_device on success, NULL on failure
+ */
 struct rte_vdpa_device * __rte_experimental
 rte_vdpa_get_device(int did);
 
-/* Get current available vdpa device number */
+/**
+ * Get current available vdpa device number
+ *
+ * @return
+ *  available vdpa device number
+ */
 int __rte_experimental
 rte_vdpa_get_device_num(void);
 #endif /* _RTE_VDPA_H_ */
-- 
2.17.1

Re: [dpdk-dev] [PATCH] vhost: add doxygen comment to vDPA header

[Maxime Coquelin]

On 10/10/2018 11:14 AM, Xiaolong Ye wrote:
> As APIs in rte_vdpa.h are public, we need to add doxygen comments
> to all APIs and structures.
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> ---
>   doc/api/doxy-api-index.md   |  1 +
>   lib/librte_vhost/rte_vdpa.h | 54 +++++++++++++++++++++++++++++++++----
>   2 files changed, 50 insertions(+), 5 deletions(-)
> 

Looks good to me:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH] vhost: add doxygen comment to vDPA header

[Ferruh Yigit]

On 10/10/2018 10:14 AM, Xiaolong Ye wrote:
> As APIs in rte_vdpa.h are public, we need to add doxygen comments
> to all APIs and structures.
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

<...>

> @@ -29,6 +32,9 @@ struct rte_vdpa_dev_addr {
>  	};
>  };
>  
> +/**
> + * vdpa device operations
> + */
>  struct rte_vdpa_dev_ops {
>  	/* Get capabilities of this device */
>  	int (*get_queue_num)(int did, uint32_t *queue_num);

Can you please document all fields of the structs?
This is part of public API and needs to be documented properly, if possible more
detail on struct documentation will be good.

<...>

> -/* Register a vdpa device, return did if successful, -1 on failure */
> +/**
> + * Register a vdpa device
> + *
> + * @param addr
> + *  the vdpa device address
> + * @parm ops
> + *  the vdpa device operations
> + * @return
> + *  device id on success, -1 on failure
> + */
>  int __rte_experimental
>  rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
>  		struct rte_vdpa_dev_ops *ops);

For experimental APIs we tend to add following into function comment:
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice

Please check lib/librte_member/rte_member.h for samples.

Re: [dpdk-dev] [PATCH] vhost: add doxygen comment to vDPA header

[Ye Xiaolong]

On 10/11, Ferruh Yigit wrote:
>On 10/10/2018 10:14 AM, Xiaolong Ye wrote:
>> As APIs in rte_vdpa.h are public, we need to add doxygen comments
>> to all APIs and structures.
>> 
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>
><...>
>
>> @@ -29,6 +32,9 @@ struct rte_vdpa_dev_addr {
>>  	};
>>  };
>>  
>> +/**
>> + * vdpa device operations
>> + */
>>  struct rte_vdpa_dev_ops {
>>  	/* Get capabilities of this device */
>>  	int (*get_queue_num)(int did, uint32_t *queue_num);
>
>Can you please document all fields of the structs?
>This is part of public API and needs to be documented properly, if possible more
>detail on struct documentation will be good.
>

Got it, I'll add more descriptions.

><...>
>
>> -/* Register a vdpa device, return did if successful, -1 on failure */
>> +/**
>> + * Register a vdpa device
>> + *
>> + * @param addr
>> + *  the vdpa device address
>> + * @parm ops
>> + *  the vdpa device operations
>> + * @return
>> + *  device id on success, -1 on failure
>> + */
>>  int __rte_experimental
>>  rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
>>  		struct rte_vdpa_dev_ops *ops);
>
>For experimental APIs we tend to add following into function comment:
>  * @warning
>  * @b EXPERIMENTAL: this API may change without prior notice
>
>Please check lib/librte_member/rte_member.h for samples.

Got it.

Thanks,
Xiaolong