[dpdk-dev] ethdev: force offloading API rules
Checks
Commit Message
The error path was disabled in previous release to let apps to be more
flexible.
But this release they are enabled, applications have to obey offload API
rules otherwise they will get errors from following APIs:
rte_eth_dev_configure
rte_eth_rx_queue_setup
rte_eth_tx_queue_setup
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Shahaf Shuler <shahafs@mellanox.com>
Cc: Wei Dai <wei.dai@intel.com>
Cc: Qi Zhang <qi.z.zhang@intel.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
---
lib/librte_ethdev/rte_ethdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
On 5/31/2018 1:44 PM, Ferruh Yigit wrote:
> The error path was disabled in previous release to let apps to be more
> flexible.
>
> But this release they are enabled, applications have to obey offload API
> rules otherwise they will get errors from following APIs:
> rte_eth_dev_configure
> rte_eth_rx_queue_setup
> rte_eth_tx_queue_setup
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Shahaf Shuler <shahafs@mellanox.com>
> Cc: Wei Dai <wei.dai@intel.com>
> Cc: Qi Zhang <qi.z.zhang@intel.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Any objection to this patch?
I really would like to get this early to catch any possible issue.
08/06/2018 21:51, Ferruh Yigit:
> On 5/31/2018 1:44 PM, Ferruh Yigit wrote:
> > The error path was disabled in previous release to let apps to be more
> > flexible.
> >
> > But this release they are enabled, applications have to obey offload API
> > rules otherwise they will get errors from following APIs:
> > rte_eth_dev_configure
> > rte_eth_rx_queue_setup
> > rte_eth_tx_queue_setup
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > Cc: Shahaf Shuler <shahafs@mellanox.com>
> > Cc: Wei Dai <wei.dai@intel.com>
> > Cc: Qi Zhang <qi.z.zhang@intel.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>
> Any objection to this patch?
> I really would like to get this early to catch any possible issue.
Yes you're right.
Let's merge it.
On Thu, 31 May 2018 13:44:30 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> The error path was disabled in previous release to let apps to be more
> flexible.
>
> But this release they are enabled, applications have to obey offload API
> rules otherwise they will get errors from following APIs:
> rte_eth_dev_configure
> rte_eth_rx_queue_setup
> rte_eth_tx_queue_setup
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Rather than always returning -EINVAL, why not propagate error code that the driver
returns. This would allow driver to give a more detailed error return.
The problem with that is you probably need to do a review of each drivers
configure and setup routines to see what they return on error (if any).
On 06/08/2018 10:51 PM, Ferruh Yigit wrote:
> On 5/31/2018 1:44 PM, Ferruh Yigit wrote:
>> The error path was disabled in previous release to let apps to be more
>> flexible.
>>
>> But this release they are enabled, applications have to obey offload API
>> rules otherwise they will get errors from following APIs:
>> rte_eth_dev_configure
>> rte_eth_rx_queue_setup
>> rte_eth_tx_queue_setup
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: Wei Dai <wei.dai@intel.com>
>> Cc: Qi Zhang <qi.z.zhang@intel.com>
>> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Any objection to this patch?
> I really would like to get this early to catch any possible issue.
I think it should be pushed just after old offload API removal since
if it is pushed before, it should take old offload API into account in
Tx queue setup function.
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Friday, June 8, 2018 11:02 PM, Stephen Hemminger:
> Subject: Re: [dpdk-dev] [PATCH] ethdev: force offloading API rules
>
> On Thu, 31 May 2018 13:44:30 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> > The error path was disabled in previous release to let apps to be more
> > flexible.
> >
> > But this release they are enabled, applications have to obey offload
> > API rules otherwise they will get errors from following APIs:
> > rte_eth_dev_configure
> > rte_eth_rx_queue_setup
> > rte_eth_tx_queue_setup
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
I am OK with returning EINVAL in case of configuration error.
>
> Rather than always returning -EINVAL, why not propagate error code that
> the driver returns. This would allow driver to give a more detailed error
> return.
>
> The problem with that is you probably need to do a review of each drivers
> configure and setup routines to see what they return on error (if any).
Per my understanding the error is in-depended on driver specific implementation, it is rather to enforce the offloading API rules of ethdev layer, which are:
1. PMD reports offloads capabilities
2. application sets it's offloads based on the supported capabilities.
When the configuration fail, no need to dig into driver, just to look on the error log and see which unsupported offloads were set.
08/06/2018 22:01, Thomas Monjalon:
> 08/06/2018 21:51, Ferruh Yigit:
> > On 5/31/2018 1:44 PM, Ferruh Yigit wrote:
> > > The error path was disabled in previous release to let apps to be more
> > > flexible.
> > >
> > > But this release they are enabled, applications have to obey offload API
> > > rules otherwise they will get errors from following APIs:
> > > rte_eth_dev_configure
> > > rte_eth_rx_queue_setup
> > > rte_eth_tx_queue_setup
> > >
> > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > ---
> > > Cc: Shahaf Shuler <shahafs@mellanox.com>
> > > Cc: Wei Dai <wei.dai@intel.com>
> > > Cc: Qi Zhang <qi.z.zhang@intel.com>
> > > Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> >
> > Any objection to this patch?
> > I really would like to get this early to catch any possible issue.
>
> Yes you're right.
> Let's merge it.
Acked-by: Thomas Monjalon <thomas@monjalon.net>
On 6/13/2018 4:16 PM, Thomas Monjalon wrote:
> 08/06/2018 22:01, Thomas Monjalon:
>> 08/06/2018 21:51, Ferruh Yigit:
>>> On 5/31/2018 1:44 PM, Ferruh Yigit wrote:
>>>> The error path was disabled in previous release to let apps to be more
>>>> flexible.
>>>>
>>>> But this release they are enabled, applications have to obey offload API
>>>> rules otherwise they will get errors from following APIs:
>>>> rte_eth_dev_configure
>>>> rte_eth_rx_queue_setup
>>>> rte_eth_tx_queue_setup
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> ---
>>>> Cc: Shahaf Shuler <shahafs@mellanox.com>
>>>> Cc: Wei Dai <wei.dai@intel.com>
>>>> Cc: Qi Zhang <qi.z.zhang@intel.com>
>>>> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>>>
>>> Any objection to this patch?
>>> I really would like to get this early to catch any possible issue.
>>
>> Yes you're right.
>> Let's merge it.
>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
Applied to dpdk-next-net/master, thanks.
@@ -1171,7 +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__);
- /* Will return -EINVAL in the next release */
+ return -EINVAL;
}
if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
local_conf.txmode.offloads) {
@@ -1182,7 +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__);
- /* Will return -EINVAL in the next release */
+ return -EINVAL;
}
/* Check that device supports requested rss hash functions. */
@@ -1580,7 +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__);
- /* Will return -EINVAL in the next release */
+ return -EINVAL;
}
ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
@@ -1745,7 +1745,7 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
local_conf.offloads,
dev_info.tx_queue_offload_capa,
__func__);
- /* Will return -EINVAL in the next release */
+ return -EINVAL;
}
return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,