[dpdk-dev] [PATCH v4 1/3] ethdev: refine API to query supported packet types

2016-03-25 Thread Bruce Richardson
On Fri, Mar 25, 2016 at 08:47:45AM +0800, Jianfeng Tan wrote:
> Return 0 instead of -ENOTSUP for those which do not fill any packet types,
> with some note and doc updated.
> 
> Signed-off-by: Jianfeng Tan 
> Acked-by: Konstantin Ananyev 

Hi Jianfeng,

I think this is a good change to the API, as it should simplify app code - as
any driver which doesn't tell us what ptypes it supports should be counted as
not supporting any. It also eliminates the need for the vdevs to see about
exporting this function to say they don't support any types.

However, two comments:
1. I think the commit message for this change should include information as to
why we want to tweak the API of this new function i.e. put in the above reasons
plus any others.
2. Please separate out the doc change from the API change as they are unrelated.

Regards,
/Bruce



[dpdk-dev] [PATCH v4 1/3] ethdev: refine API to query supported packet types

2016-03-25 Thread Tan, Jianfeng
NACK.

I'll send an independent patchset for this.

Thanks,
Jianfeng

> -Original Message-
> From: Tan, Jianfeng
> Sent: Friday, March 25, 2016 8:48 AM
> To: dev at dpdk.org
> Cc: Tan, Jianfeng; Ananyev, Konstantin; Zhang, Helin; Richardson, Bruce
> Subject: [PATCH v4 1/3] ethdev: refine API to query supported packet types
> 
> Return 0 instead of -ENOTSUP for those which do not fill any packet types,
> with some note and doc updated.
> 
> Signed-off-by: Jianfeng Tan 
> Acked-by: Konstantin Ananyev 
> ---
>  doc/guides/nics/overview.rst  | 2 +-
>  lib/librte_ether/rte_ethdev.c | 3 +--
>  lib/librte_ether/rte_ethdev.h | 9 ++---
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/nics/overview.rst b/doc/guides/nics/overview.rst
> index 542479a..e7504da 100644
> --- a/doc/guides/nics/overview.rst
> +++ b/doc/guides/nics/overview.rst
> @@ -124,7 +124,7 @@ Most of these differences are summarized below.
> L4 checksum offload  X   X   X   X
> inner L3 checksumX   X   X
> inner L4 checksumX   X   X
> -   packet type parsing  X   X   X
> +   packet type parsing  X X X X X X   X X   X X 
> X X X X   X
> timesync X X
> basic stats  X   X   X X X X  
>  X X
> extended stats   X   X X X X
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index a328027..1ee79d2 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1636,8 +1636,7 @@ rte_eth_dev_get_supported_ptypes(uint8_t
> port_id, uint32_t ptype_mask,
> 
>   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>   dev = &rte_eth_devices[port_id];
> - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >dev_supported_ptypes_get,
> - -ENOTSUP);
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >dev_supported_ptypes_get, 0);
>   all_ptypes = (*dev->dev_ops->dev_supported_ptypes_get)(dev);
> 
>   if (!all_ptypes)
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index e7de34a..5167750 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -2326,6 +2326,9 @@ void rte_eth_dev_info_get(uint8_t port_id, struct
> rte_eth_dev_info *dev_info);
>   * @note
>   *   Better to invoke this API after the device is already started or rx 
> burst
>   *   function is decided, to obtain correct supported ptypes.
> + * @note
> + *   if a given PMD does not report what ptypes it supports, then the
> supported
> + *   ptype count is reported as 0.
>   * @param port_id
>   *   The port identifier of the Ethernet device.
>   * @param ptype_mask
> @@ -2335,9 +2338,9 @@ void rte_eth_dev_info_get(uint8_t port_id, struct
> rte_eth_dev_info *dev_info);
>   * @param num
>   *  Size of the array pointed by param ptypes.
>   * @return
> - *   - (>0) Number of supported ptypes. If it exceeds param num, exceeding
> - *  packet types will not be filled in the given array.
> - *   - (0 or -ENOTSUP) if PMD does not fill the specified ptype.
> + *   - (>=0) Number of supported ptypes. If the number of types exceeds
> num,
> + only num entries will be filled into the ptypes array, but the 
> full
> + count of supported ptypes will be returned.
>   *   - (-ENODEV) if *port_id* invalid.
>   */
>  int rte_eth_dev_get_supported_ptypes(uint8_t port_id, uint32_t
> ptype_mask,
> --
> 2.1.4



[dpdk-dev] [PATCH v4 1/3] ethdev: refine API to query supported packet types

2016-03-25 Thread Jianfeng Tan
Return 0 instead of -ENOTSUP for those which do not fill any packet types,
with some note and doc updated.

Signed-off-by: Jianfeng Tan 
Acked-by: Konstantin Ananyev 
---
 doc/guides/nics/overview.rst  | 2 +-
 lib/librte_ether/rte_ethdev.c | 3 +--
 lib/librte_ether/rte_ethdev.h | 9 ++---
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/overview.rst b/doc/guides/nics/overview.rst
index 542479a..e7504da 100644
--- a/doc/guides/nics/overview.rst
+++ b/doc/guides/nics/overview.rst
@@ -124,7 +124,7 @@ Most of these differences are summarized below.
L4 checksum offload  X   X   X   X
inner L3 checksumX   X   X
inner L4 checksumX   X   X
-   packet type parsing  X   X   X
+   packet type parsing  X X X X X X   X X   X X X 
X X X   X
timesync X X
basic stats  X   X   X X X X   
X X
extended stats   X   X X X X
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a328027..1ee79d2 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1636,8 +1636,7 @@ rte_eth_dev_get_supported_ptypes(uint8_t port_id, 
uint32_t ptype_mask,

RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
-   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_get,
-   -ENOTSUP);
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_get, 0);
all_ptypes = (*dev->dev_ops->dev_supported_ptypes_get)(dev);

if (!all_ptypes)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index e7de34a..5167750 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -2326,6 +2326,9 @@ void rte_eth_dev_info_get(uint8_t port_id, struct 
rte_eth_dev_info *dev_info);
  * @note
  *   Better to invoke this API after the device is already started or rx burst
  *   function is decided, to obtain correct supported ptypes.
+ * @note
+ *   if a given PMD does not report what ptypes it supports, then the supported
+ *   ptype count is reported as 0.
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param ptype_mask
@@ -2335,9 +2338,9 @@ void rte_eth_dev_info_get(uint8_t port_id, struct 
rte_eth_dev_info *dev_info);
  * @param num
  *  Size of the array pointed by param ptypes.
  * @return
- *   - (>0) Number of supported ptypes. If it exceeds param num, exceeding
- *  packet types will not be filled in the given array.
- *   - (0 or -ENOTSUP) if PMD does not fill the specified ptype.
+ *   - (>=0) Number of supported ptypes. If the number of types exceeds num,
+ only num entries will be filled into the ptypes array, but the 
full
+ count of supported ptypes will be returned.
  *   - (-ENODEV) if *port_id* invalid.
  */
 int rte_eth_dev_get_supported_ptypes(uint8_t port_id, uint32_t ptype_mask,
-- 
2.1.4