[dpdk-dev] [PATCH 0/3] ethdev: fail if requested offload is not supported

[dpdk-dev] [PATCH 0/3] ethdev: fail if requested offload is not supported

[Andrew Rybchenko]

The series has fixes for problems discussed in [1].

Basically it does not allow unsupported offloads to pass.

If fixes regressions for PMDs which carefully check offloads before,
but these checks are removed now in favor of checks in ethdev.

It may break applications which request some offload which is not
supported by underlying PMD, but does not actually used.

Depending on discussion results it should be either dropped or applied.

[1] http://dpdk.org/ml/archives/dev/2018-May/101261.html

Andrew Rybchenko (3):
  ethdev: fail configure if requested offload is not supported
  ethdev: fail if Tx queue offload is not supported at all
  ethdev: fail if Rx queue offload is not supported

 lib/librte_ethdev/rte_ethdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.17.0

[dpdk-dev] [PATCH 1/3] ethdev: fail configure if requested offload is not supported

[Andrew Rybchenko]

Do not allow allow unsupported offload to be passed to PMD since
in can result in inconsistent NIC configuration and processing
in the driver.

All PMDs are converted to the new offload API and must report its
capabilties correctly. Both device and queue offloads are listed
in [rt]x_offload_capa so the check should pass despite of which
level the offload is supported on.

Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 3ddf3accb..dd36e6270 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1171,6 +1171,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 				local_conf.rxmode.offloads,
 				dev_info.rx_offload_capa,
 				__func__);
+		return -EINVAL;
 	}
 	if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
 	     local_conf.txmode.offloads) {
@@ -1181,6 +1182,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 				local_conf.txmode.offloads,
 				dev_info.tx_offload_capa,
 				__func__);
+		return -EINVAL;
 	}
 
 	/* Check that device supports requested rss hash functions. */
-- 
2.17.0

[dpdk-dev] [PATCH 2/3] ethdev: fail if Tx queue offload is not supported at all

[Andrew Rybchenko]

Do not allow to request unsupported Tx offload since all checks
are removed from PMDs because of consistency check in ethdev.
Otherwise application may rely on offload which is not actually
supported and send traffic with, for example, wrong checksums,
truncated packets or packets with garbage.

Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index dd36e6270..60577efcf 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1744,6 +1744,16 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
 				local_conf.offloads,
 				dev_info.tx_queue_offload_capa,
 				__func__);
+		/*
+		 * Applications which are not converted yet to the new
+		 * Tx offload API may request device level offloads on
+		 * queue level (and nothing is requested on device level).
+		 * However, if the offload is not supported at all Tx
+		 * queue setup must fail.
+		 */
+		if ((local_conf.offloads & dev_info.tx_offload_capa) !=
+		    local_conf.offloads)
+			return -EINVAL;
 	}
 
 	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
-- 
2.17.0

Re: [dpdk-dev] [PATCH 2/3] ethdev: fail if Tx queue offload is not supported at all

[Shahaf Shuler]

--Shahaf


> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Friday, May 11, 2018 7:26 PM
> To: dev@dpdk.org
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Shahaf Shuler <shahafs@mellanox.com>; Wei Dai
> <wei.dai@intel.com>
> Subject: [PATCH 2/3] ethdev: fail if Tx queue offload is not supported at all
> 
> Do not allow to request unsupported Tx offload since all checks are removed
> from PMDs because of consistency check in ethdev.
> Otherwise application may rely on offload which is not actually supported
> and send traffic with, for example, wrong checksums, truncated packets or
> packets with garbage.
> 
> Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index dd36e6270..60577efcf 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1744,6 +1744,16 @@ rte_eth_tx_queue_setup(uint16_t port_id,
> uint16_t tx_queue_id,
>  				local_conf.offloads,
>  				dev_info.tx_queue_offload_capa,
>  				__func__);
> +		/*
> +		 * Applications which are not converted yet to the new
> +		 * Tx offload API may request device level offloads on
> +		 * queue level (and nothing is requested on device level).
> +		 * However, if the offload is not supported at all Tx
> +		 * queue setup must fail.
> +		 */
> +		if ((local_conf.offloads & dev_info.tx_offload_capa) !=
> +		    local_conf.offloads)
> +			return -EINVAL;

Not converted application doesn't have a clue what are per-queue offloads, and this is the error they will get when the Tx queue configuration will fail.

How about using ETH_TXQ_FLAGS_IGNORE flag, which explicitly says "application converted to the new Tx offloads API". and have 2 different checks:
1. for converted application the already exist check[1] with the related error.
2. for not converted application your check with a related error. 


[1]
if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
     local_conf.offloads) {                                  

>  	}
> 
>  	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
> --
> 2.17.0

Re: [dpdk-dev] [PATCH 2/3] ethdev: fail if Tx queue offload is not supported at all

[Shahaf Shuler]

Sunday, May 13, 2018 8:38 AM, Shahaf Shuler:
> Subject: Re: [dpdk-dev] [PATCH 2/3] ethdev: fail if Tx queue offload is not
> supported at all
> > -----Original Message-----
> > From: Andrew Rybchenko <arybchenko@solarflare.com>
> > Sent: Friday, May 11, 2018 7:26 PM
> > To: dev@dpdk.org
> > Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Shahaf Shuler <shahafs@mellanox.com>; Wei
> Dai
> > <wei.dai@intel.com>
> > Subject: [PATCH 2/3] ethdev: fail if Tx queue offload is not supported
> > at all
> >
> > Do not allow to request unsupported Tx offload since all checks are
> > removed from PMDs because of consistency check in ethdev.
> > Otherwise application may rely on offload which is not actually
> > supported and send traffic with, for example, wrong checksums,
> > truncated packets or packets with garbage.
> >
> > Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")
> >
> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > ---
> >  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.c
> > b/lib/librte_ethdev/rte_ethdev.c index dd36e6270..60577efcf 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -1744,6 +1744,16 @@ rte_eth_tx_queue_setup(uint16_t port_id,
> > uint16_t tx_queue_id,
> >  				local_conf.offloads,
> >  				dev_info.tx_queue_offload_capa,
> >  				__func__);
> > +		/*
> > +		 * Applications which are not converted yet to the new
> > +		 * Tx offload API may request device level offloads on
> > +		 * queue level (and nothing is requested on device level).
> > +		 * However, if the offload is not supported at all Tx
> > +		 * queue setup must fail.
> > +		 */
> > +		if ((local_conf.offloads & dev_info.tx_offload_capa) !=
> > +		    local_conf.offloads)
> > +			return -EINVAL;
> 
> Not converted application doesn't have a clue what are per-queue offloads,
> and this is the error they will get when the Tx queue configuration will fail.
> 
> How about using ETH_TXQ_FLAGS_IGNORE flag, which explicitly says
> "application converted to the new Tx offloads API". and have 2 different
> checks:
> 1. for converted application the already exist check[1] with the related error.
> 2. for not converted application your check with a related error.
> 
> 

As a suggestion maybe the below diff can be squashed into this one:

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index b3dac067c5..4763718b9c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1727,25 +1727,29 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
         */
        local_conf.offloads &= ~dev->data->dev_conf.txmode.offloads;
 
-       /*
-        * New added offloadings for this queue are those not enabled in
-        * rte_eth_dev_configure( ) and they must be per-queue type.
-        * A pure per-port offloading can't be enabled on a queue while
-        * disabled on another queue. A pure per-port offloading can't
-        * be enabled for any queue as new added one if it hasn't been
-        * enabled in rte_eth_dev_configure( ).
-        */
-       if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
-            local_conf.offloads) {
-               ethdev_log(ERR, "Ethdev port_id=%d tx_queue_id=%d, new "
-                               "added offloads 0x%" PRIx64 " must be "
-                               "within pre-queue offload capabilities 0x%"
-                               PRIx64 " in %s( )\n",
-                               port_id,
-                               tx_queue_id,
-                               local_conf.offloads,
-                               dev_info.tx_queue_offload_capa,
-                               __func__);
+       if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) {
+               /*
+                * New added offloadings for this queue are those not enabled in
+                * rte_eth_dev_configure( ) and they must be per-queue type.
+                * A pure per-port offloading can't be enabled on a queue while
+                * disabled on another queue. A pure per-port offloading can't
+                * be enabled for any queue as new added one if it hasn't been
+                * enabled in rte_eth_dev_configure( ).
+                */
+               if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
+                               local_conf.offloads) {
+                       ethdev_log(ERR, "Ethdev port_id=%d tx_queue_id=%d, new "
+                                       "added offloads 0x%" PRIx64 " must be "
+                                       "within pre-queue offload capabilities "
+                                       " 0x%" PRIx64 " in %s( )\n",
+                                       port_id,
+                                       tx_queue_id,
+                                       local_conf.offloads,
+                                       dev_info.tx_queue_offload_capa,
+                                       __func__);
+                       return -EINVAL;
+               }
+       } else {
                /*
                 * Applications which are not converted yet to the new
                 * Tx offload API may request device level offloads on
@@ -1754,8 +1758,18 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
                 * queue setup must fail.

Re: [dpdk-dev] [PATCH 2/3] ethdev: fail if Tx queue offload is not supported at all

[Andrew Rybchenko]

On 05/13/2018 08:37 AM, Shahaf Shuler wrote:
>> Do not allow to request unsupported Tx offload since all checks are removed
>> from PMDs because of consistency check in ethdev.
>> Otherwise application may rely on offload which is not actually supported
>> and send traffic with, for example, wrong checksums, truncated packets or
>> packets with garbage.
>>
>> Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index dd36e6270..60577efcf 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -1744,6 +1744,16 @@ rte_eth_tx_queue_setup(uint16_t port_id,
>> uint16_t tx_queue_id,
>>   				local_conf.offloads,
>>   				dev_info.tx_queue_offload_capa,
>>   				__func__);
>> +		/*
>> +		 * Applications which are not converted yet to the new
>> +		 * Tx offload API may request device level offloads on
>> +		 * queue level (and nothing is requested on device level).
>> +		 * However, if the offload is not supported at all Tx
>> +		 * queue setup must fail.
>> +		 */
>> +		if ((local_conf.offloads & dev_info.tx_offload_capa) !=
>> +		    local_conf.offloads)
>> +			return -EINVAL;
> Not converted application doesn't have a clue what are per-queue offloads, and this is the error they will get when the Tx queue configuration will fail.
>
> How about using ETH_TXQ_FLAGS_IGNORE flag, which explicitly says "application converted to the new Tx offloads API". and have 2 different checks:
> 1. for converted application the already exist check[1] with the related error.
> 2. for not converted application your check with a related error.

Yes, I like the idea. Many thanks. I'll send v2 shortly.

> [1]
> if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
>       local_conf.offloads) {
>
>>   	}
>>
>>   	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
>> --
>> 2.17.0

[dpdk-dev] [PATCH 3/3] ethdev: fail if Rx queue offload is not supported

[Andrew Rybchenko]

Return of error was removed to mitigate possible breakage of old
applications which are not converted to the new offload API yet.
However, old Rx offload API has no per queue controls and
Rx queue offloads are derived from the device Rx mode bitfields
exactly in the same way as it is done on configure. Device
level Rx offloads are removed from queue offloads, so Rx queue
offloads should be empty if old Rx offload API is used.

Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 60577efcf..938ec5638 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1580,6 +1580,7 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 				local_conf.offloads,
 				dev_info.rx_queue_offload_capa,
 				__func__);
+		return -EINVAL;
 	}
 
 	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
-- 
2.17.0

[dpdk-dev] [PATCH v2 0/3] ethdev: fail if requested offload is not supported

[Andrew Rybchenko]

The series has fixes for problems discussed in [1].

Basically it does not allow unsupported offloads to pass.

If fixes regressions for PMDs which carefully check offloads before,
but these checks are removed now in favor of checks in ethdev.

It may break applications which request some offload which is not
supported by underlying PMD, but does not actually used.

Depending on discussion results it should be either dropped or applied.

[1] http://dpdk.org/ml/archives/dev/2018-May/101261.html

v1 -> v2:
 - use IGNORE flag to separate old and new offload API cases on
   Tx queue setup and avoid expected errors if old API is used
   as suggested by Shahaf
 - remove convertion of rxmode bits on Rx queue setup since exactly
   these bits are removed

Andrew Rybchenko (3):
  ethdev: fail configure if requested offload is not supported
  ethdev: fail if Tx queue offload is not supported
  ethdev: fail if Rx queue offload is not supported

 lib/librte_ethdev/rte_ethdev.c | 62 +++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 27 deletions(-)

-- 
2.17.0

[dpdk-dev] [PATCH v2 1/3] ethdev: fail configure if requested offload is not supported

[Andrew Rybchenko]

Do not allow allow unsupported offload to be passed to PMD since
in can result in inconsistent NIC configuration and processing
in the driver.

All PMDs are converted to the new offload API and must report its
capabilties correctly. Both device and queue offloads are listed
in [rt]x_offload_capa so the check should pass despite of which
level the offload is supported on.

Fixes: 0330605295cf ("ethdev: new Rx/Tx offloads API")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 3528ba179..54e1ee771 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1171,6 +1171,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 				local_conf.rxmode.offloads,
 				dev_info.rx_offload_capa,
 				__func__);
+		return -EINVAL;
 	}
 	if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
 	     local_conf.txmode.offloads) {
@@ -1181,6 +1182,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 				local_conf.txmode.offloads,
 				dev_info.tx_offload_capa,
 				__func__);
+		return -EINVAL;
 	}
 
 	/* Check that device supports requested rss hash functions. */
-- 
2.17.0

[dpdk-dev] [PATCH v2 2/3] ethdev: fail if Tx queue offload is not supported

[Andrew Rybchenko]

Do not allow to request unsupported Tx offload since all checks
are removed from PMDs because of consistency check in ethdev.
Otherwise application may rely on offload which is not actually
supported and send traffic with, for example, wrong checksums,
truncated packets or packets with garbage.

If application is using the new offload API, queue offloads only
may be added on Tx queue setup.

If application is using the old offload API, it knows nothing
about device level Tx offloads and requests everything required
on Tx queue setup using txq_flags. So, both device level (from PMD
point of view) and queue level offloads may be requested.
It is assumed that no PMD yet strictly separate device and queue
level offloads. If any PMD does it, it was broken for applications
which use the old offload API at the moment of PMD convertion anyway.

Fixes: 0330605295cf ("ethdev: new Rx/Tx offloads API")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 51 +++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 54e1ee771..2b673013a 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1727,25 +1727,38 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
 	 */
 	local_conf.offloads &= ~dev->data->dev_conf.txmode.offloads;
 
-	/*
-	 * New added offloadings for this queue are those not enabled in
-	 * rte_eth_dev_configure( ) and they must be per-queue type.
-	 * A pure per-port offloading can't be enabled on a queue while
-	 * disabled on another queue. A pure per-port offloading can't
-	 * be enabled for any queue as new added one if it hasn't been
-	 * enabled in rte_eth_dev_configure( ).
-	 */
-	if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
-	     local_conf.offloads) {
-		ethdev_log(ERR, "Ethdev port_id=%d tx_queue_id=%d, new "
-				"added offloads 0x%" PRIx64 " must be "
-				"within pre-queue offload capabilities 0x%"
-				PRIx64 " in %s( )\n",
-				port_id,
-				tx_queue_id,
-				local_conf.offloads,
-				dev_info.tx_queue_offload_capa,
-				__func__);
+	if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) {
+		/*
+		 * New added offloadings for this queue are those not enabled in
+		 * rte_eth_dev_configure( ) and they must be per-queue type.
+		 * A pure per-port offloading can't be enabled on a queue while
+		 * disabled on another queue. A pure per-port offloading can't
+		 * be enabled for any queue as new added one if it hasn't been
+		 * enabled in rte_eth_dev_configure( ).
+		 */
+		if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
+		    local_conf.offloads) {
+			ethdev_log(ERR, "Ethdev port_id=%d tx_queue_id=%d, new "
+					"added offloads 0x%" PRIx64 " must be "
+					"within pre-queue offload capabilities 0x%"
+					PRIx64 " in %s( )\n",
+					port_id,
+					tx_queue_id,
+					local_conf.offloads,
+					dev_info.tx_queue_offload_capa,
+					__func__);
+		}
+	} else {
+		/*
+		 * Applications which are not converted yet to the new
+		 * Tx offload API may request device level offloads on
+		 * queue level (and nothing is requested on device level).
+		 * However, if the offload is not supported at all,
+		 * Tx queue setup must fail.
+		 */
+		if ((local_conf.offloads & dev_info.tx_offload_capa) !=
+		    local_conf.offloads)
+			return -EINVAL;
 	}
 
 	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
-- 
2.17.0

[dpdk-dev] [PATCH v2 3/3] ethdev: fail if Rx queue offload is not supported

[Andrew Rybchenko]

Return of error was removed to mitigate possible breakage of old
applications which are not converted to the new offload API yet.

Old Rx offload API has no per queue controls and Rx queue offloads
are derived from the device Rx mode bitfields exactly in the same
way as it is done on configure and, then, removed to pass queue
level offloads only. It is meaningless and should be removed.

So, the new offload API only may be used to request per-queue
Rx offloads and error should be returned if offload not supported
on queue level is requested.

Fixes: 0330605295cf ("ethdev: new Rx/Tx offloads API")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 2b673013a..d82962fae 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1542,14 +1542,6 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 		rx_conf = &dev_info.default_rxconf;
 
 	local_conf = *rx_conf;
-	if (dev->data->dev_conf.rxmode.ignore_offload_bitfield == 0) {
-		/**
-		 * Reflect port offloads to queue offloads in order for
-		 * offloads to not be discarded.
-		 */
-		rte_eth_convert_rx_offload_bitfield(&dev->data->dev_conf.rxmode,
-						    &local_conf.offloads);
-	}
 
 	/*
 	 * If an offloading has already been enabled in
@@ -1581,6 +1573,7 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 				local_conf.offloads,
 				dev_info.rx_queue_offload_capa,
 				__func__);
+		return -EINVAL;
 	}
 
 	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
-- 
2.17.0

Re: [dpdk-dev] [PATCH v2 0/3] ethdev: fail if requested offload is not supported

[Shahaf Shuler]

Monday, May 14, 2018 10:36 AM, Andrew Rybchenko:
> Subject: [PATCH v2 0/3] ethdev: fail if requested offload is not supported
> 
> The series has fixes for problems discussed in [1].
> 
> Basically it does not allow unsupported offloads to pass.
> 
> If fixes regressions for PMDs which carefully check offloads before, but these
> checks are removed now in favor of checks in ethdev.
> 
> It may break applications which request some offload which is not supported
> by underlying PMD, but does not actually used.
> 
> Depending on discussion results it should be either dropped or applied.
> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd
> k.org%2Fml%2Farchives%2Fdev%2F2018-
> May%2F101261.html&data=02%7C01%7Cshahafs%40mellanox.com%7C9a572
> e9d86b04c99854c08d5b96d75b5%7Ca652971c7d2e4d9ba6a4d149256f461b%7
> C0%7C0%7C636618802134607945&sdata=fQKCr%2FrFvakWVIFomy8iSD%2Bc
> VtSie8mvgx63Qqgq690%3D&reserved=0
> 
> v1 -> v2:
>  - use IGNORE flag to separate old and new offload API cases on
>    Tx queue setup and avoid expected errors if old API is used
>    as suggested by Shahaf
>  - remove convertion of rxmode bits on Rx queue setup since exactly
>    these bits are removed
> 
> Andrew Rybchenko (3):
>   ethdev: fail configure if requested offload is not supported
>   ethdev: fail if Tx queue offload is not supported
>   ethdev: fail if Rx queue offload is not supported
> 
>  lib/librte_ethdev/rte_ethdev.c | 62 +++++++++++++++++++---------------
>  1 file changed, 35 insertions(+), 27 deletions(-)

For the series -
Acked-by: Shahaf Shuler <shahafs@mellanox.com>

> 
> --
> 2.17.0

Re: [dpdk-dev] [PATCH v2 0/3] ethdev: fail if requested offload is not supported

[Ferruh Yigit]

On 5/14/2018 8:51 AM, Shahaf Shuler wrote:
> Monday, May 14, 2018 10:36 AM, Andrew Rybchenko:
>> Subject: [PATCH v2 0/3] ethdev: fail if requested offload is not supported
>>
>> The series has fixes for problems discussed in [1].
>>
>> Basically it does not allow unsupported offloads to pass.
>>
>> If fixes regressions for PMDs which carefully check offloads before, but these
>> checks are removed now in favor of checks in ethdev.
>>
>> It may break applications which request some offload which is not supported
>> by underlying PMD, but does not actually used.
>>
>> Depending on discussion results it should be either dropped or applied.
>>
>> [1]
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd
>> k.org%2Fml%2Farchives%2Fdev%2F2018-
>> May%2F101261.html&data=02%7C01%7Cshahafs%40mellanox.com%7C9a572
>> e9d86b04c99854c08d5b96d75b5%7Ca652971c7d2e4d9ba6a4d149256f461b%7
>> C0%7C0%7C636618802134607945&sdata=fQKCr%2FrFvakWVIFomy8iSD%2Bc
>> VtSie8mvgx63Qqgq690%3D&reserved=0
>>
>> v1 -> v2:
>>  - use IGNORE flag to separate old and new offload API cases on
>>    Tx queue setup and avoid expected errors if old API is used
>>    as suggested by Shahaf
>>  - remove convertion of rxmode bits on Rx queue setup since exactly
>>    these bits are removed
>>
>> Andrew Rybchenko (3):
>>   ethdev: fail configure if requested offload is not supported
>>   ethdev: fail if Tx queue offload is not supported
>>   ethdev: fail if Rx queue offload is not supported
>>
>>  lib/librte_ethdev/rte_ethdev.c | 62 +++++++++++++++++++---------------
>>  1 file changed, 35 insertions(+), 27 deletions(-)
> 
> For the series -
> Acked-by: Shahaf Shuler <shahafs@mellanox.com>

Updated status of the set as deferred in patchwork, lets continue to work on
this after release.

Re: [dpdk-dev] [PATCH v2 0/3] ethdev: fail if requested offload is not supported

[Ferruh Yigit]

On 5/14/2018 3:48 PM, Ferruh Yigit wrote:
> On 5/14/2018 8:51 AM, Shahaf Shuler wrote:
>> Monday, May 14, 2018 10:36 AM, Andrew Rybchenko:
>>> Subject: [PATCH v2 0/3] ethdev: fail if requested offload is not supported
>>>
>>> The series has fixes for problems discussed in [1].
>>>
>>> Basically it does not allow unsupported offloads to pass.
>>>
>>> If fixes regressions for PMDs which carefully check offloads before, but these
>>> checks are removed now in favor of checks in ethdev.
>>>
>>> It may break applications which request some offload which is not supported
>>> by underlying PMD, but does not actually used.
>>>
>>> Depending on discussion results it should be either dropped or applied.
>>>
>>> [1]
>>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd
>>> k.org%2Fml%2Farchives%2Fdev%2F2018-
>>> May%2F101261.html&data=02%7C01%7Cshahafs%40mellanox.com%7C9a572
>>> e9d86b04c99854c08d5b96d75b5%7Ca652971c7d2e4d9ba6a4d149256f461b%7
>>> C0%7C0%7C636618802134607945&sdata=fQKCr%2FrFvakWVIFomy8iSD%2Bc
>>> VtSie8mvgx63Qqgq690%3D&reserved=0
>>>
>>> v1 -> v2:
>>>  - use IGNORE flag to separate old and new offload API cases on
>>>    Tx queue setup and avoid expected errors if old API is used
>>>    as suggested by Shahaf
>>>  - remove convertion of rxmode bits on Rx queue setup since exactly
>>>    these bits are removed
>>>
>>> Andrew Rybchenko (3):
>>>   ethdev: fail configure if requested offload is not supported
>>>   ethdev: fail if Tx queue offload is not supported
>>>   ethdev: fail if Rx queue offload is not supported
>>>
>>>  lib/librte_ethdev/rte_ethdev.c | 62 +++++++++++++++++++---------------
>>>  1 file changed, 35 insertions(+), 27 deletions(-)
>>
>> For the series -
>> Acked-by: Shahaf Shuler <shahafs@mellanox.com>
> 
> Updated status of the set as deferred in patchwork, lets continue to work on
> this after release.

This set should not be valid anymore after adding error returns back and
removing old offloading API in this release. Updating patchwork according.