[dpdk-dev] [PATCH 0/2] Non-supported offloads are ignored

[dpdk-dev] [PATCH 0/2] Non-supported offloads are ignored

[Sunil Kumar Kori]

Currently DPAA/DPAA2 driver returns error if non-supported offloads
(Rx or Tx) are requested from user which causes init failures in
multiple application.

Some offloads may be enabled by default in many hardware and cannot
be disabled. These offloads cannot be part of capability as toggling
is not allowed (Like CRC STRIP). Thus DPAA/DPAA2 driver will ignore
such offloads and rather print warning message.

Sunil Kumar Kori (2):
  net/dpaa: non supported offloads are ignored with warning
  net/dpaa2: non supported offloads are ignored with warning

 drivers/net/dpaa/dpaa_ethdev.c   | 6 ++----
 drivers/net/dpaa2/dpaa2_ethdev.c | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH 1/2] net/dpaa: non supported offloads are ignored with warning

[Sunil Kumar Kori]

Fixes: 16e2c27f4fc7 ("net/dpaa: support new ethdev offload APIs")
Cc: sunil.kori@nxp.com

Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
---
 drivers/net/dpaa/dpaa_ethdev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index b2740b4..b368f7e 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -151,17 +151,15 @@ dpaa_eth_dev_configure(struct rte_eth_dev *dev)
 
 	dpaa_eth_dev_info(dev, &dev_info);
 	if (((~(dev_info.rx_offload_capa) & rx_offloads) != 0)) {
-		DPAA_PMD_ERR("Some Rx offloads are not supported "
+		DPAA_PMD_WARN("Some Rx offloads are not supported "
 			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
 			rx_offloads, dev_info.rx_offload_capa);
-		return -ENOTSUP;
 	}
 
 	if (((~(dev_info.tx_offload_capa) & tx_offloads) != 0)) {
-		DPAA_PMD_ERR("Some Tx offloads are not supported "
+		DPAA_PMD_WARN("Some Tx offloads are not supported "
 			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
 			tx_offloads, dev_info.tx_offload_capa);
-		return -ENOTSUP;
 	}
 
 	if (((rx_offloads & DEV_RX_OFFLOAD_IPV4_CKSUM) == 0) ||
-- 
2.9.3

[dpdk-dev] [PATCH 2/2] net/dpaa2: non supported offloads are ignored with warning

[Sunil Kumar Kori]

Fixes: 0ebce6129bc6 ("net/dpaa2: support new ethdev offload APIs")
Cc: sunil.kori@nxp.com

Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 54ab9eb..60c6076 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -290,17 +290,15 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
 
 	dpaa2_dev_info_get(dev, &dev_info);
 	if ((~(dev_info.rx_offload_capa) & rx_offloads) != 0) {
-		DPAA2_PMD_ERR("Some Rx offloads are not supported "
+		DPAA2_PMD_WARN("Some Rx offloads are not supported "
 			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
 			rx_offloads, dev_info.rx_offload_capa);
-		return -ENOTSUP;
 	}
 
 	if ((~(dev_info.tx_offload_capa) & tx_offloads) != 0) {
-		DPAA2_PMD_ERR("Some Tx offloads are not supported "
+		DPAA2_PMD_WARN("Some Tx offloads are not supported "
 			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
 			tx_offloads, dev_info.tx_offload_capa);
-		return -ENOTSUP;
 	}
 
 	if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
-- 
2.9.3

Re: [dpdk-dev] [PATCH 0/2] Non-supported offloads are ignored

[Ferruh Yigit]

On 4/20/2018 11:45 AM, Sunil Kumar Kori wrote:
> Currently DPAA/DPAA2 driver returns error if non-supported offloads
> (Rx or Tx) are requested from user which causes init failures in
> multiple application.
> 
> Some offloads may be enabled by default in many hardware and cannot
> be disabled. These offloads cannot be part of capability as toggling
> is not allowed (Like CRC STRIP). Thus DPAA/DPAA2 driver will ignore
> such offloads and rather print warning message.

Hi Sunil,

I understand the usecase, and some PMDs already do same thing.

But instead of ignoring all offloads provided by user, can it be possible to
keep another list of offloads that are free to ignore and return warning for them.

For rest of the offloads you can still return error.

This lets application to get error if a really wrong offload set, also add more
information into driver about offload support and expectations, what do you think?

> 
> Sunil Kumar Kori (2):
>   net/dpaa: non supported offloads are ignored with warning
>   net/dpaa2: non supported offloads are ignored with warning
> 
>  drivers/net/dpaa/dpaa_ethdev.c   | 6 ++----
>  drivers/net/dpaa2/dpaa2_ethdev.c | 6 ++----
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 

Re: [dpdk-dev] [PATCH 1/2] net/dpaa: non supported offloads are ignored with warning

[Stephen Hemminger]

On Fri, 20 Apr 2018 16:15:40 +0530
Sunil Kumar Kori <sunil.kori@nxp.com> wrote:

>  	dpaa_eth_dev_info(dev, &dev_info);
>  	if (((~(dev_info.rx_offload_capa) & rx_offloads) != 0)) {
> -		DPAA_PMD_ERR("Some Rx offloads are not supported "
> +		DPAA_PMD_WARN("Some Rx offloads are not supported "
>  			"requested 0x%" PRIx64 " supported 0x%" PRIx64,

The if statement has more parens than necessary.

Please don't break error messages across lines, it is better to have a long source
line since it allows users to use tools like grep to search for error messages.

[dpdk-dev] [PATCH v2 0/2] Non-supported offloads are ignored

[Sunil Kumar Kori]

Currently DPAA/DPAA2 driver returns error if non-supported offloads
(Rx or Tx) are requested from user which causes init failures in
multiple application.

Some offloads may be enabled by default in many hardware and cannot
be disabled. These offloads cannot be part of capability as toggling
is not allowed (Like CRC STRIP). Thus DPAA/DPAA2 driver will ignore
such offloads and rather print warning message.

Change History:
[v2] Incorporated review comments

Sunil Kumar Kori (2):
  net/dpaa: non supported offloads are ignored with warning
  net/dpaa2: non supported offloads are ignored with warning

 drivers/net/dpaa/dpaa_ethdev.c   | 34 +++++++++++++++++++++++++++-------
 drivers/net/dpaa2/dpaa2_ethdev.c | 37 +++++++++++++++++++++++++++++--------
 2 files changed, 56 insertions(+), 15 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH v2 1/2] net/dpaa: non supported offloads are ignored with warning

[Sunil Kumar Kori]

Fixes: 16e2c27f4fc7 ("net/dpaa: support new ethdev offload APIs")
Cc: shreyansh.jain@nxp.com

Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
---
 drivers/net/dpaa/dpaa_ethdev.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index b2740b4..81001cb 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -45,6 +45,25 @@
 #include <fsl_bman.h>
 #include <fsl_fman.h>
 
+/* Non-Supported Rx offloads */
+static uint64_t dev_rx_offloads_not_supported =
+		DEV_RX_OFFLOAD_TCP_LRO |
+		DEV_RX_OFFLOAD_MACSEC_STRIP |
+		DEV_RX_OFFLOAD_HEADER_SPLIT |
+		DEV_RX_OFFLOAD_VLAN_EXTEND |
+		DEV_RX_OFFLOAD_SECURITY;
+
+/* Non-Supported Tx offloads */
+static uint64_t dev_tx_offloads_not_supported =
+		DEV_TX_OFFLOAD_TCP_TSO |
+		DEV_TX_OFFLOAD_UDP_TSO |
+		DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
+		DEV_TX_OFFLOAD_GRE_TNL_TSO |
+		DEV_TX_OFFLOAD_IPIP_TNL_TSO |
+		DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
+		DEV_TX_OFFLOAD_MACSEC_INSERT |
+		DEV_TX_OFFLOAD_SECURITY;
+
 /* Keep track of whether QMAN and BMAN have been globally initialized */
 static int is_global_init;
 /* At present we only allow up to 4 push mode queues - as each of this queue
@@ -150,16 +169,17 @@ dpaa_eth_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	dpaa_eth_dev_info(dev, &dev_info);
-	if (((~(dev_info.rx_offload_capa) & rx_offloads) != 0)) {
-		DPAA_PMD_ERR("Some Rx offloads are not supported "
-			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
+	/* Rx offloads validation */
+	if (dev_rx_offloads_not_supported & rx_offloads) {
+		DPAA_PMD_ERR(
+		"Rx offloads not supported - Requested 0x%" PRIx64 " supported 0x%" PRIx64,
 			rx_offloads, dev_info.rx_offload_capa);
 		return -ENOTSUP;
 	}
-
-	if (((~(dev_info.tx_offload_capa) & tx_offloads) != 0)) {
-		DPAA_PMD_ERR("Some Tx offloads are not supported "
-			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
+	/* Tx offloads validation */
+	if (dev_tx_offloads_not_supported & tx_offloads) {
+		DPAA_PMD_ERR(
+		"Tx offloads not supported - Requested 0x%" PRIx64 " Supported 0x%" PRIx64,
 			tx_offloads, dev_info.tx_offload_capa);
 		return -ENOTSUP;
 	}
-- 
2.9.3

[dpdk-dev] [PATCH v2 2/2] net/dpaa2: non supported offloads are ignored with warning

[Sunil Kumar Kori]

Fixes: 0ebce6129bc6 ("net/dpaa2: support new ethdev offload APIs")
Cc: shreyansh.jain@nxp.com

Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 54ab9eb..a47c723 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -27,6 +27,25 @@
 #include "dpaa2_ethdev.h"
 #include <fsl_qbman_debug.h>
 
+/* Non-Supported Rx offloads */
+static uint64_t dev_rx_offloads_not_supported =
+		DEV_RX_OFFLOAD_TCP_LRO |
+		DEV_RX_OFFLOAD_MACSEC_STRIP |
+		DEV_RX_OFFLOAD_HEADER_SPLIT |
+		DEV_RX_OFFLOAD_VLAN_EXTEND |
+		DEV_RX_OFFLOAD_SECURITY;
+
+/* Non-Supported Tx offloads */
+static uint64_t dev_tx_offloads_not_supported =
+		DEV_TX_OFFLOAD_TCP_TSO |
+		DEV_TX_OFFLOAD_UDP_TSO |
+		DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
+		DEV_TX_OFFLOAD_GRE_TNL_TSO |
+		DEV_TX_OFFLOAD_IPIP_TNL_TSO |
+		DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
+		DEV_TX_OFFLOAD_MACSEC_INSERT |
+		DEV_TX_OFFLOAD_SECURITY;
+
 struct rte_dpaa2_xstats_name_off {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
 	uint8_t page_id; /* dpni statistics page id */
@@ -289,18 +308,20 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	dpaa2_dev_info_get(dev, &dev_info);
-	if ((~(dev_info.rx_offload_capa) & rx_offloads) != 0) {
-		DPAA2_PMD_ERR("Some Rx offloads are not supported "
-			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
+	/* Rx offloads validation */
+	if (dev_rx_offloads_not_supported & rx_offloads) {
+		DPAA2_PMD_ERR(
+		"Rx offloads not supported - Requested 0x%" PRIx64 " supported 0x%" PRIx64,
 			rx_offloads, dev_info.rx_offload_capa);
-		return -ENOTSUP;
+			return -ENOTSUP;
 	}
 
-	if ((~(dev_info.tx_offload_capa) & tx_offloads) != 0) {
-		DPAA2_PMD_ERR("Some Tx offloads are not supported "
-			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
+	/* Tx offloads validation */
+	if (dev_tx_offloads_not_supported & tx_offloads) {
+		DPAA2_PMD_ERR(
+		"Tx offloads not supported - Requested 0x%" PRIx64 " Supported 0x%" PRIx64,
 			tx_offloads, dev_info.tx_offload_capa);
-		return -ENOTSUP;
+			return -ENOTSUP;
 	}
 
 	if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
-- 
2.9.3

Re: [dpdk-dev] [PATCH v2 1/2] net/dpaa: non supported offloads are ignored with warning

[Ferruh Yigit]

On 4/23/2018 1:33 PM, Sunil Kumar Kori wrote:
> Fixes: 16e2c27f4fc7 ("net/dpaa: support new ethdev offload APIs")
> Cc: shreyansh.jain@nxp.com
> 
> Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
> ---
>  drivers/net/dpaa/dpaa_ethdev.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
> index b2740b4..81001cb 100644
> --- a/drivers/net/dpaa/dpaa_ethdev.c
> +++ b/drivers/net/dpaa/dpaa_ethdev.c
> @@ -45,6 +45,25 @@
>  #include <fsl_bman.h>
>  #include <fsl_fman.h>
>  
> +/* Non-Supported Rx offloads */
> +static uint64_t dev_rx_offloads_not_supported =
> +		DEV_RX_OFFLOAD_TCP_LRO |
> +		DEV_RX_OFFLOAD_MACSEC_STRIP |
> +		DEV_RX_OFFLOAD_HEADER_SPLIT |
> +		DEV_RX_OFFLOAD_VLAN_EXTEND |
> +		DEV_RX_OFFLOAD_SECURITY;
> +
> +/* Non-Supported Tx offloads */
> +static uint64_t dev_tx_offloads_not_supported =
> +		DEV_TX_OFFLOAD_TCP_TSO |
> +		DEV_TX_OFFLOAD_UDP_TSO |
> +		DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
> +		DEV_TX_OFFLOAD_GRE_TNL_TSO |
> +		DEV_TX_OFFLOAD_IPIP_TNL_TSO |
> +		DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
> +		DEV_TX_OFFLOAD_MACSEC_INSERT |
> +		DEV_TX_OFFLOAD_SECURITY;
> +
>  /* Keep track of whether QMAN and BMAN have been globally initialized */
>  static int is_global_init;
>  /* At present we only allow up to 4 push mode queues - as each of this queue
> @@ -150,16 +169,17 @@ dpaa_eth_dev_configure(struct rte_eth_dev *dev)
>  	PMD_INIT_FUNC_TRACE();
>  
>  	dpaa_eth_dev_info(dev, &dev_info);
> -	if (((~(dev_info.rx_offload_capa) & rx_offloads) != 0)) {
> -		DPAA_PMD_ERR("Some Rx offloads are not supported "
> -			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
> +	/* Rx offloads validation */
> +	if (dev_rx_offloads_not_supported & rx_offloads) {
> +		DPAA_PMD_ERR(
> +		"Rx offloads not supported - Requested 0x%" PRIx64 " supported 0x%" PRIx64,
>  			rx_offloads, dev_info.rx_offload_capa);
>  		return -ENOTSUP;
>  	}

There are three group of offloads from PMD point of view:
- Supported, reported via rx_offload_capa, setting these will work fine
- Not supported, listed in dev_rx_offloads_not_supported, application setting
this will get an error with and error log.
- Not configurable, supported by hardware but not configurable by application,
these goes silently, can you print a warning for this case?
And it can be easier to keep list of these offloads, otherwise you will need to
update your unsupported list when a new type of offloading introduced. And
developers may more interested to see the list of not configurable, and leaving
rest as unsupported.