[dpdk-dev] doc: update new ethdev offload API description
Checks
Commit Message
Don't mandate API to pass port offload configuration during queue setup,
this is unnecessary for devices that support only port level offloads.
Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
Cc: shahafs@mellanox.com
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Patil, Harish <harish.patil@cavium.com>
Btw, this expectation from API should be clear from source code and API
documentation (doxygen comments in header file) instead of
documentation. Am I missing something or we are doing something wrong
here?
---
doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
Comments
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com>
Date: Friday, March 16, 2018 at 8:51 AM
To: John McNamara <john.mcnamara@intel.com>, Marko Kovacevic
<marko.kovacevic@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Ferruh Yigit <ferruh.yigit@intel.com>,
Thomas Monjalon <thomas@monjalon.net>, "shahafs@mellanox.com"
<shahafs@mellanox.com>, <Patil>, Harish Patil <Harish.Patil@cavium.com>
Subject: [PATCH] doc: update new ethdev offload API description
>Don't mandate API to pass port offload configuration during queue setup,
>this is unnecessary for devices that support only port level offloads.
>
>Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
>Cc: shahafs@mellanox.com
>
>Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>---
>Cc: Patil, Harish <harish.patil@cavium.com>
>
>Btw, this expectation from API should be clear from source code and API
>documentation (doxygen comments in header file) instead of
>documentation. Am I missing something or we are doing something wrong
>here?
>---
> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/doc/guides/prog_guide/poll_mode_drv.rst
>b/doc/guides/prog_guide/poll_mode_drv.rst
>index e5d01874e..3247f309f 100644
>--- a/doc/guides/prog_guide/poll_mode_drv.rst
>+++ b/doc/guides/prog_guide/poll_mode_drv.rst
>@@ -303,9 +303,7 @@ Supported offloads can be either per-port or
>per-queue.
> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or
>``DEV_RX_OFFLOAD_*`` flags.
> Per-port offload configuration is set using ``rte_eth_dev_configure``.
> Per-queue offload configuration is set using ``rte_eth_rx_queue_setup``
>and ``rte_eth_tx_queue_setup``.
>-To enable per-port offload, the offload should be set on both device
>configuration and queue setup.
>-In case of a mixed configuration the queue setup shall return with an
>error.
>-To enable per-queue offload, the offload can be set only on the queue
>setup.
>+Per-port offloads should be set on the port configuration. Queue
>offloads should be set on the queue configuration.
> Offloads which are not enabled are disabled by default.
>
> For an application to use the Tx offloads API it should set the
>``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in
>``rte_eth_txconf`` struct.
>--
>2.13.6
>
Acked-by: Harish Patil <harish.patil@cavium.com>
>
Friday, March 16, 2018 5:52 PM, Ferruh Yigit:
> Don't mandate API to pass port offload configuration during queue setup,
> this is unnecessary for devices that support only port level offloads.
>
> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
> Cc: shahafs@mellanox.com
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Patil, Harish <harish.patil@cavium.com>
>
> Btw, this expectation from API should be clear from source code and API
> documentation (doxygen comments in header file) instead of
> documentation. Am I missing something or we are doing something wrong
> here?
> ---
> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst
> b/doc/guides/prog_guide/poll_mode_drv.rst
> index e5d01874e..3247f309f 100644
> --- a/doc/guides/prog_guide/poll_mode_drv.rst
> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> @@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-
> queue.
> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or
> ``DEV_RX_OFFLOAD_*`` flags.
> Per-port offload configuration is set using ``rte_eth_dev_configure``.
> Per-queue offload configuration is set using ``rte_eth_rx_queue_setup``
> and ``rte_eth_tx_queue_setup``.
> -To enable per-port offload, the offload should be set on both device
> configuration and queue setup.
> -In case of a mixed configuration the queue setup shall return with an error.
> -To enable per-queue offload, the offload can be set only on the queue
> setup.
> +Per-port offloads should be set on the port configuration. Queue offloads
> should be set on the queue configuration.
> Offloads which are not enabled are disabled by default.
>
> For an application to use the Tx offloads API it should set the
> ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in
> ``rte_eth_txconf`` struct.
I am OK with such change.
However, while documentation is good, most of the customers learn on the API usage from the existing examples.
Currently both examples and testpmd behave according to the old approach, see example from testpmd[1] before the rx_queue_setup.
I think a modification there is needed if we are going to change the API.
[1]
/* Apply Rx offloads configuration */
port->rx_conf.offloads = port->dev_conf.rxmode.offloads;
> --
> 2.13.6
On 03/16/2018 06:51 PM, Ferruh Yigit wrote:
> Don't mandate API to pass port offload configuration during queue setup,
> this is unnecessary for devices that support only port level offloads.
>
> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
> Cc: shahafs@mellanox.com
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Patil, Harish <harish.patil@cavium.com>
>
> Btw, this expectation from API should be clear from source code and API
> documentation (doxygen comments in header file) instead of
> documentation. Am I missing something or we are doing something wrong
> here?
> ---
> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
> index e5d01874e..3247f309f 100644
> --- a/doc/guides/prog_guide/poll_mode_drv.rst
> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> @@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-queue.
> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or ``DEV_RX_OFFLOAD_*`` flags.
> Per-port offload configuration is set using ``rte_eth_dev_configure``.
> Per-queue offload configuration is set using ``rte_eth_rx_queue_setup`` and ``rte_eth_tx_queue_setup``.
> -To enable per-port offload, the offload should be set on both device configuration and queue setup.
> -In case of a mixed configuration the queue setup shall return with an error.
> -To enable per-queue offload, the offload can be set only on the queue setup.
> +Per-port offloads should be set on the port configuration. Queue offloads should be set on the queue configuration.
> Offloads which are not enabled are disabled by default.
>
> For an application to use the Tx offloads API it should set the ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in ``rte_eth_txconf`` struct.
net/sfc has code which double-checks old behaviour. So, it is not just
documentation update. We can provide patches if the behaviour
change is accepted.
IMHO, it should be allowed to specify queue offloads on port level.
It should simply enable these offloads on all queues. Also it will
match dev_info [rt]x_offload_capa which include both port and queue
offloads.
Yes, we lose possibility to enable on port level, but disable on queue
level by suggested changes, but I think it is OK - if you don't need
it for all queues, just control separately on queue level.
On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> On 03/16/2018 06:51 PM, Ferruh Yigit wrote:
>> Don't mandate API to pass port offload configuration during queue setup,
>> this is unnecessary for devices that support only port level offloads.
>>
>> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
>> Cc: shahafs@mellanox.com
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Patil, Harish <harish.patil@cavium.com>
>>
>> Btw, this expectation from API should be clear from source code and API
>> documentation (doxygen comments in header file) instead of
>> documentation. Am I missing something or we are doing something wrong
>> here?
>> ---
>> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
>> index e5d01874e..3247f309f 100644
>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
>> @@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-queue.
>> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or ``DEV_RX_OFFLOAD_*`` flags.
>> Per-port offload configuration is set using ``rte_eth_dev_configure``.
>> Per-queue offload configuration is set using ``rte_eth_rx_queue_setup`` and ``rte_eth_tx_queue_setup``.
>> -To enable per-port offload, the offload should be set on both device configuration and queue setup.
>> -In case of a mixed configuration the queue setup shall return with an error.
>> -To enable per-queue offload, the offload can be set only on the queue setup.
>> +Per-port offloads should be set on the port configuration. Queue offloads should be set on the queue configuration.
>> Offloads which are not enabled are disabled by default.
>>
>> For an application to use the Tx offloads API it should set the ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in ``rte_eth_txconf`` struct.
>
> net/sfc has code which double-checks old behaviour. So, it is not just
> documentation update. We can provide patches if the behaviour
> change is accepted.
Not definitely just doc update, PMDs needs to be modified. This patch is just to
agree on the behavior.
>
> IMHO, it should be allowed to specify queue offloads on port level.
> It should simply enable these offloads on all queues. Also it will
> match dev_info [rt]x_offload_capa which include both port and queue
> offloads.
>
> Yes, we lose possibility to enable on port level, but disable on queue
> level by suggested changes, but I think it is OK - if you don't need
> it for all queues, just control separately on queue level.
What I understand was queue offload can only enable more, but it seems it can
both enable or disable.
My concern was, even PMD reports no [rt]x_offload_capa at all, API forces
application to send at least port offloads during queue setup.
As long as application only allowed to send queue offloads within the boundaries
of the "queue offload capabilities", I am OK.
This will work fine for devices that support queue level offloads to enable -
disable queue specific offloads on top of port offloads. Will this make sense?
On 03/21/2018 01:54 PM, Ferruh Yigit wrote:
> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
>> On 03/16/2018 06:51 PM, Ferruh Yigit wrote:
>>> Don't mandate API to pass port offload configuration during queue setup,
>>> this is unnecessary for devices that support only port level offloads.
>>>
>>> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
>>> Cc: shahafs@mellanox.com
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>> Cc: Patil, Harish <harish.patil@cavium.com>
>>>
>>> Btw, this expectation from API should be clear from source code and API
>>> documentation (doxygen comments in header file) instead of
>>> documentation. Am I missing something or we are doing something wrong
>>> here?
>>> ---
>>> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
>>> index e5d01874e..3247f309f 100644
>>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
>>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
>>> @@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-queue.
>>> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or ``DEV_RX_OFFLOAD_*`` flags.
>>> Per-port offload configuration is set using ``rte_eth_dev_configure``.
>>> Per-queue offload configuration is set using ``rte_eth_rx_queue_setup`` and ``rte_eth_tx_queue_setup``.
>>> -To enable per-port offload, the offload should be set on both device configuration and queue setup.
>>> -In case of a mixed configuration the queue setup shall return with an error.
>>> -To enable per-queue offload, the offload can be set only on the queue setup.
>>> +Per-port offloads should be set on the port configuration. Queue offloads should be set on the queue configuration.
>>> Offloads which are not enabled are disabled by default.
>>>
>>> For an application to use the Tx offloads API it should set the ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in ``rte_eth_txconf`` struct.
>> net/sfc has code which double-checks old behaviour. So, it is not just
>> documentation update. We can provide patches if the behaviour
>> change is accepted.
> Not definitely just doc update, PMDs needs to be modified. This patch is just to
> agree on the behavior.
>
>> IMHO, it should be allowed to specify queue offloads on port level.
>> It should simply enable these offloads on all queues. Also it will
>> match dev_info [rt]x_offload_capa which include both port and queue
>> offloads.
>>
>> Yes, we lose possibility to enable on port level, but disable on queue
>> level by suggested changes, but I think it is OK - if you don't need
>> it for all queues, just control separately on queue level.
> What I understand was queue offload can only enable more, but it seems it can
> both enable or disable.
>
> My concern was, even PMD reports no [rt]x_offload_capa at all, API forces
> application to send at least port offloads during queue setup.
I guess you mean [rt]x_queue_offload_capa above.
> As long as application only allowed to send queue offloads within the boundaries
> of the "queue offload capabilities", I am OK.
If so, queue offloads should not be included in [rt]x_offload_capa.
But I'm afraid it is too restrictive for apps.
> This will work fine for devices that support queue level offloads to enable -
> disable queue specific offloads on top of port offloads. Will this make sense?
IMHO, disable on queue level is not required for enabled on port level.
If app always wants some offloads, just check [rt]x_offload_capa and
enable on
port level (regardless if it is actually per-port or per-queue).
If app wants to some offload per queue, check [rt]x_queue_offload_capa, do
not enable on port level and control on queue level.
Wednesday, March 21, 2018 1:09 PM, Andrew Rybchenko
> On 03/21/2018 01:54 PM, Ferruh Yigit wrote:
> > On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> >> On 03/16/2018 06:51 PM, Ferruh Yigit wrote:
> >>> Don't mandate API to pass port offload configuration during queue
> >>> setup, this is unnecessary for devices that support only port level
> offloads.
> >>>
> >>> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
> >>> Cc: shahafs@mellanox.com
> >>>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> ---
> >>> Cc: Patil, Harish <harish.patil@cavium.com>
> >>>
> >>> Btw, this expectation from API should be clear from source code and
> >>> API documentation (doxygen comments in header file) instead of
> >>> documentation. Am I missing something or we are doing something
> >>> wrong here?
> >>> ---
> >>> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
> >>> 1 file changed, 1 insertion(+), 3 deletions(-)
> >>>
> >>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst
> >>> b/doc/guides/prog_guide/poll_mode_drv.rst
> >>> index e5d01874e..3247f309f 100644
> >>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
> >>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> >>> @@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-
> queue.
> >>> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or
> ``DEV_RX_OFFLOAD_*`` flags.
> >>> Per-port offload configuration is set using ``rte_eth_dev_configure``.
> >>> Per-queue offload configuration is set using
> ``rte_eth_rx_queue_setup`` and ``rte_eth_tx_queue_setup``.
> >>> -To enable per-port offload, the offload should be set on both device
> configuration and queue setup.
> >>> -In case of a mixed configuration the queue setup shall return with an
> error.
> >>> -To enable per-queue offload, the offload can be set only on the queue
> setup.
> >>> +Per-port offloads should be set on the port configuration. Queue
> offloads should be set on the queue configuration.
> >>> Offloads which are not enabled are disabled by default.
> >>>
> >>> For an application to use the Tx offloads API it should set the
> ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in
> ``rte_eth_txconf`` struct.
> >> net/sfc has code which double-checks old behaviour. So, it is not
> >> just documentation update. We can provide patches if the behaviour
> >> change is accepted.
> > Not definitely just doc update, PMDs needs to be modified. This patch
> > is just to agree on the behavior.
> >
> >> IMHO, it should be allowed to specify queue offloads on port level.
> >> It should simply enable these offloads on all queues. Also it will
> >> match dev_info [rt]x_offload_capa which include both port and queue
> >> offloads.
> >>
> >> Yes, we lose possibility to enable on port level, but disable on
> >> queue level by suggested changes, but I think it is OK - if you don't
> >> need it for all queues, just control separately on queue level.
> > What I understand was queue offload can only enable more, but it seems
> > it can both enable or disable.
> >
> > My concern was, even PMD reports no [rt]x_offload_capa at all, API
> > forces application to send at least port offloads during queue setup.
>
> I guess you mean [rt]x_queue_offload_capa above.
>
> > As long as application only allowed to send queue offloads within the
> > boundaries of the "queue offload capabilities", I am OK.
>
> If so, queue offloads should not be included in [rt]x_offload_capa.
> But I'm afraid it is too restrictive for apps.
>
> > This will work fine for devices that support queue level offloads to
> > enable - disable queue specific offloads on top of port offloads. Will this
> make sense?
>
> IMHO, disable on queue level is not required for enabled on port level.
> If app always wants some offloads, just check [rt]x_offload_capa and enable
> on port level (regardless if it is actually per-port or per-queue).
> If app wants to some offload per queue, check [rt]x_queue_offload_capa,
> do not enable on port level and control on queue level.
+1.
And I think Ferruh this is the suggestion by this patch, isn't it?
On 03/21/2018 02:10 PM, Shahaf Shuler wrote:
> Wednesday, March 21, 2018 1:09 PM, Andrew Rybchenko
>> On 03/21/2018 01:54 PM, Ferruh Yigit wrote:
>>> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
>>>> On 03/16/2018 06:51 PM, Ferruh Yigit wrote:
>>>>> Don't mandate API to pass port offload configuration during queue
>>>>> setup, this is unnecessary for devices that support only port level
>> offloads.
>>>>> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
>>>>> Cc: shahafs@mellanox.com
>>>>>
>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> ---
>>>>> Cc: Patil, Harish <harish.patil@cavium.com>
>>>>>
>>>>> Btw, this expectation from API should be clear from source code and
>>>>> API documentation (doxygen comments in header file) instead of
>>>>> documentation. Am I missing something or we are doing something
>>>>> wrong here?
>>>>> ---
>>>>> doc/guides/prog_guide/poll_mode_drv.rst | 4 +---
>>>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst
>>>>> b/doc/guides/prog_guide/poll_mode_drv.rst
>>>>> index e5d01874e..3247f309f 100644
>>>>> --- a/doc/guides/prog_guide/poll_mode_drv.rst
>>>>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
>>>>> @@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-
>> queue.
>>>>> Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or
>> ``DEV_RX_OFFLOAD_*`` flags.
>>>>> Per-port offload configuration is set using ``rte_eth_dev_configure``.
>>>>> Per-queue offload configuration is set using
>> ``rte_eth_rx_queue_setup`` and ``rte_eth_tx_queue_setup``.
>>>>> -To enable per-port offload, the offload should be set on both device
>> configuration and queue setup.
>>>>> -In case of a mixed configuration the queue setup shall return with an
>> error.
>>>>> -To enable per-queue offload, the offload can be set only on the queue
>> setup.
>>>>> +Per-port offloads should be set on the port configuration. Queue
>> offloads should be set on the queue configuration.
>>>>> Offloads which are not enabled are disabled by default.
>>>>>
>>>>> For an application to use the Tx offloads API it should set the
>> ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in
>> ``rte_eth_txconf`` struct.
>>>> net/sfc has code which double-checks old behaviour. So, it is not
>>>> just documentation update. We can provide patches if the behaviour
>>>> change is accepted.
>>> Not definitely just doc update, PMDs needs to be modified. This patch
>>> is just to agree on the behavior.
>>>
>>>> IMHO, it should be allowed to specify queue offloads on port level.
>>>> It should simply enable these offloads on all queues. Also it will
>>>> match dev_info [rt]x_offload_capa which include both port and queue
>>>> offloads.
>>>>
>>>> Yes, we lose possibility to enable on port level, but disable on
>>>> queue level by suggested changes, but I think it is OK - if you don't
>>>> need it for all queues, just control separately on queue level.
>>> What I understand was queue offload can only enable more, but it seems
>>> it can both enable or disable.
>>>
>>> My concern was, even PMD reports no [rt]x_offload_capa at all, API
>>> forces application to send at least port offloads during queue setup.
>> I guess you mean [rt]x_queue_offload_capa above.
>>
>>> As long as application only allowed to send queue offloads within the
>>> boundaries of the "queue offload capabilities", I am OK.
>> If so, queue offloads should not be included in [rt]x_offload_capa.
>> But I'm afraid it is too restrictive for apps.
>>
>>> This will work fine for devices that support queue level offloads to
>>> enable - disable queue specific offloads on top of port offloads. Will this
>> make sense?
>>
>> IMHO, disable on queue level is not required for enabled on port level.
>> If app always wants some offloads, just check [rt]x_offload_capa and enable
>> on port level (regardless if it is actually per-port or per-queue).
>> If app wants to some offload per queue, check [rt]x_queue_offload_capa,
>> do not enable on port level and control on queue level.
> +1.
>
> And I think Ferruh this is the suggestion by this patch, isn't it?
Not exactly. We should add statement to allow to enable queue offloads
on port level (to enable on all queues).
Wednesday, March 21, 2018 1:20 PM, :Andrew Rybchenko
>Not exactly. We should add statement to allow to enable queue offloads
>on port level (to enable on all queues).
Why it is needed ?
Queue offload is also a port offload, for the simple case it is enabled on each of the queues.
PMDs should report rx[tx]_offload_capa = port_offloads | queue_offloads
So from the application side it enables a **port** offload which, by definition, will set the offload to each of the queues.
it is not “enabling queue offload on the port”.
On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
>
> Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
>
> >Not exactly. We should add statement to allow to enable queue offloads
> >on port level (to enable on all queues).
>
> Why it is needed ?
>
May be just a paranoia to avoid misreading/misunderstanding.
> Queue offload is also a port offload, for the simple case it is
> enabled on each of the queues.
>
> PMDs should report rx[tx]_offload_capa = port_offloads | queue_offloads
>
> So from the application side it enables a **port** offload which, by
> definition, will set the offload to each of the queues.
>
> it is not “enabling queue offload on the port”.
>
I think it would be really useful for understanding to highlight
that what is enabled on port level is enabled on all queues
regardless queue conf.
Wednesday, March 21, 2018 1:37 PM, Andrew Rybchenko:
> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
> >
> > Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
> >
> > >Not exactly. We should add statement to allow to enable queue
> > >offloads on port level (to enable on all queues).
> >
> > Why it is needed ?
> >
>
> May be just a paranoia to avoid misreading/misunderstanding.
>
> > Queue offload is also a port offload, for the simple case it is
> > enabled on each of the queues.
> >
> > PMDs should report rx[tx]_offload_capa = port_offloads |
> > queue_offloads
> >
> > So from the application side it enables a **port** offload which, by
> > definition, will set the offload to each of the queues.
> >
> > it is not “enabling queue offload on the port”.
> >
>
> I think it would be really useful for understanding to highlight that what is
> enabled on port level is enabled on all queues regardless queue conf.
So I think the extra wording should explain that queue offload is also a port offload, and not to mix between the queue and port offload configuration.
Hi everyone,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew Rybchenko
> Sent: Wednesday, March 21, 2018 11:37 AM
> To: Shahaf Shuler <shahafs@mellanox.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Patil@dpdk.org; Harish <harish.patil@cavium.com>; Ivan Malov
> <Ivan.Malov@oktetlabs.ru>
> Subject: Re: [dpdk-dev] [PATCH] doc: update new ethdev offload API description
>
> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
> >
> > Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
> >
> > >Not exactly. We should add statement to allow to enable queue offloads
> > >on port level (to enable on all queues).
> >
> > Why it is needed ?
> >
>
> May be just a paranoia to avoid misreading/misunderstanding.
>
> > Queue offload is also a port offload, for the simple case it is
> > enabled on each of the queues.
> >
> > PMDs should report rx[tx]_offload_capa = port_offloads | queue_offloads
> >
> > So from the application side it enables a **port** offload which, by
> > definition, will set the offload to each of the queues.
> >
> > it is not “enabling queue offload on the port”.
> >
>
> I think it would be really useful for understanding to highlight
> that what is enabled on port level is enabled on all queues
> regardless queue conf.
Why not to allow queue offloads to override port offload for given queue?
Konstantin
Wednesday, March 21, 2018 2:04 PM, Ananyev, Konstantin:
> Hi everyone,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew
> Rybchenko
> > Sent: Wednesday, March 21, 2018 11:37 AM
> > To: Shahaf Shuler <shahafs@mellanox.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > Kovacevic, Marko <marko.kovacevic@intel.com>
> > Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>;
> > Patil@dpdk.org; Harish <harish.patil@cavium.com>; Ivan Malov
> > <Ivan.Malov@oktetlabs.ru>
> > Subject: Re: [dpdk-dev] [PATCH] doc: update new ethdev offload API
> > description
> >
> > On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
> > >
> > > Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
> > >
> > > >Not exactly. We should add statement to allow to enable queue
> > > >offloads on port level (to enable on all queues).
> > >
> > > Why it is needed ?
> > >
> >
> > May be just a paranoia to avoid misreading/misunderstanding.
> >
> > > Queue offload is also a port offload, for the simple case it is
> > > enabled on each of the queues.
> > >
> > > PMDs should report rx[tx]_offload_capa = port_offloads |
> > > queue_offloads
> > >
> > > So from the application side it enables a **port** offload which, by
> > > definition, will set the offload to each of the queues.
> > >
> > > it is not “enabling queue offload on the port”.
> > >
> >
> > I think it would be really useful for understanding to highlight that
> > what is enabled on port level is enabled on all queues regardless
> > queue conf.
>
> Why not to allow queue offloads to override port offload for given queue?
> Konstantin
What is the use case for that? Why would application want to enable offload on the port and then disable it on some of the queues?
Why not just enable it on the needed queues as part of the queue level configuration?
>
On 03/21/2018 03:03 PM, Ananyev, Konstantin wrote:
> Hi everyone,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew Rybchenko
>> Sent: Wednesday, March 21, 2018 11:37 AM
>> To: Shahaf Shuler <shahafs@mellanox.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
>> Kovacevic, Marko <marko.kovacevic@intel.com>
>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Patil@dpdk.org; Harish <harish.patil@cavium.com>; Ivan Malov
>> <Ivan.Malov@oktetlabs.ru>
>> Subject: Re: [dpdk-dev] [PATCH] doc: update new ethdev offload API description
>>
>> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
>>> Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
>>>
>>>> Not exactly. We should add statement to allow to enable queue offloads
>>>> on port level (to enable on all queues).
>>> Why it is needed ?
>>>
>> May be just a paranoia to avoid misreading/misunderstanding.
>>
>>> Queue offload is also a port offload, for the simple case it is
>>> enabled on each of the queues.
>>>
>>> PMDs should report rx[tx]_offload_capa = port_offloads | queue_offloads
>>>
>>> So from the application side it enables a **port** offload which, by
>>> definition, will set the offload to each of the queues.
>>>
>>> it is not “enabling queue offload on the port”.
>>>
>> I think it would be really useful for understanding to highlight
>> that what is enabled on port level is enabled on all queues
>> regardless queue conf.
> Why not to allow queue offloads to override port offload for given queue?
Basically it returns us to the initial point made by Ferruh:
If device has no queue offloads, but application still has to repeat
port offloads in queue offloads.
> -----Original Message-----
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Wednesday, March 21, 2018 12:34 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Shahaf Shuler <shahafs@mellanox.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Mcnamara, John <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Patil@dpdk.org; Harish <harish.patil@cavium.com>; Ivan Malov
> <Ivan.Malov@oktetlabs.ru>
> Subject: Re: [dpdk-dev] [PATCH] doc: update new ethdev offload API description
>
> On 03/21/2018 03:03 PM, Ananyev, Konstantin wrote:
> > Hi everyone,
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew Rybchenko
> >> Sent: Wednesday, March 21, 2018 11:37 AM
> >> To: Shahaf Shuler <shahafs@mellanox.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> >> Kovacevic, Marko <marko.kovacevic@intel.com>
> >> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Patil@dpdk.org; Harish <harish.patil@cavium.com>; Ivan Malov
> >> <Ivan.Malov@oktetlabs.ru>
> >> Subject: Re: [dpdk-dev] [PATCH] doc: update new ethdev offload API description
> >>
> >> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
> >>> Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
> >>>
> >>>> Not exactly. We should add statement to allow to enable queue offloads
> >>>> on port level (to enable on all queues).
> >>> Why it is needed ?
> >>>
> >> May be just a paranoia to avoid misreading/misunderstanding.
> >>
> >>> Queue offload is also a port offload, for the simple case it is
> >>> enabled on each of the queues.
> >>>
> >>> PMDs should report rx[tx]_offload_capa = port_offloads | queue_offloads
> >>>
> >>> So from the application side it enables a **port** offload which, by
> >>> definition, will set the offload to each of the queues.
> >>>
> >>> it is not “enabling queue offload on the port”.
> >>>
> >> I think it would be really useful for understanding to highlight
> >> that what is enabled on port level is enabled on all queues
> >> regardless queue conf.
> > Why not to allow queue offloads to override port offload for given queue?
>
> Basically it returns us to the initial point made by Ferruh:
> If device has no queue offloads, but application still has to repeat
> port offloads in queue offloads.
If device doesn't have per queue offloads (only per port) then there should be nothing
to enable/disable per queue, no?
Or you'd like to allow at queue_setup() to enable/disable port offloads too?
On 3/21/2018 11:40 AM, Shahaf Shuler wrote:
> Wednesday, March 21, 2018 1:37 PM, Andrew Rybchenko:
>> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
>>>
>>> Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
>>>
>>>> Not exactly. We should add statement to allow to enable queue
>>>> offloads on port level (to enable on all queues).
>>>
>>> Why it is needed ?
>>>
>>
>> May be just a paranoia to avoid misreading/misunderstanding.
>>
>>> Queue offload is also a port offload, for the simple case it is
>>> enabled on each of the queues.
>>>
>>> PMDs should report rx[tx]_offload_capa = port_offloads |
>>> queue_offloads
>>>
>>> So from the application side it enables a **port** offload which, by
>>> definition, will set the offload to each of the queues.
>>>
>>> it is not “enabling queue offload on the port”.
>>>
>>
>> I think it would be really useful for understanding to highlight that what is
>> enabled on port level is enabled on all queues regardless queue conf.
>
> So I think the extra wording should explain that queue offload is also a port offload, and not to mix between the queue and port offload configuration.
+1 for more details, the sentences was the outcome of the previous discussion
but not clear enough. Perhaps some sample values can be also good.
Shahaf do you want to give a try?
And is following correct based on latest :
1- Port capability is always covers queue capability
P_cap = A, B, C, D
Q_cap = B, C, D
2- Requested port offloads should be subset of port capabilities, they will be
applied to all queues:
P_req = A, B
Q1: A, B
Q2: A, B
3- Later, requested queue offloads should be subset of queue capabilities, they
will be applied to specific queue:
Q_req = 1:B, C
Q1: A, B, C
Q2: A, B
Q_req = 2:D
Q1: A, B, C
Q2: A, D
Scenario 2:
1-
P_cap = A, B, C, D
Q_cap = ""
2-
P_req = A, B
Q1: A, B
Q2: A, B
3-
Q_req = ""
Q1: A, B
Q2: A, B
Wednesday, March 21, 2018 2:52 PM, Ferruh Yigit:
> On 3/21/2018 11:40 AM, Shahaf Shuler wrote:
> > Wednesday, March 21, 2018 1:37 PM, Andrew Rybchenko:
> >> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
> >>>
> >>> Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
> >>>
> >>>> Not exactly. We should add statement to allow to enable queue
> >>>> offloads on port level (to enable on all queues).
> >>>
> >>> Why it is needed ?
> >>>
> >>
> >> May be just a paranoia to avoid misreading/misunderstanding.
> >>
> >>> Queue offload is also a port offload, for the simple case it is
> >>> enabled on each of the queues.
> >>>
> >>> PMDs should report rx[tx]_offload_capa = port_offloads |
> >>> queue_offloads
> >>>
> >>> So from the application side it enables a **port** offload which, by
> >>> definition, will set the offload to each of the queues.
> >>>
> >>> it is not “enabling queue offload on the port”.
> >>>
> >>
> >> I think it would be really useful for understanding to highlight that
> >> what is enabled on port level is enabled on all queues regardless queue
> conf.
> >
> > So I think the extra wording should explain that queue offload is also a port
> offload, and not to mix between the queue and port offload configuration.
>
> +1 for more details, the sentences was the outcome of the previous
> +discussion
> but not clear enough. Perhaps some sample values can be also good.
> Shahaf do you want to give a try?
IMO what is missing is:
1. the syntax of this patch
2. explicitly write that queue offloads are always subset of the port offload
3. explicitly write that when port offload is enabled it applies to all queues (that one is already written AFAIR).
>
> And is following correct based on latest :
> 1- Port capability is always covers queue capability P_cap = A, B, C, D Q_cap =
> B, C, D
>
> 2- Requested port offloads should be subset of port capabilities, they will be
> applied to all queues:
> P_req = A, B
> Q1: A, B
> Q2: A, B
>
> 3- Later, requested queue offloads should be subset of queue capabilities,
> they will be applied to specific queue:
> Q_req = 1:B, C
> Q1: A, B, C
> Q2: A, B
>
> Q_req = 2:D
> Q1: A, B, C
> Q2: A, D
>
>
> Scenario 2:
> 1-
> P_cap = A, B, C, D
> Q_cap = ""
>
> 2-
> P_req = A, B
> Q1: A, B
> Q2: A, B
>
> 3-
> Q_req = ""
> Q1: A, B
> Q2: A, B
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Wednesday, March 21, 2018 1:06 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew Rybchenko <arybchenko@solarflare.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Patil@dpdk.org; Harish <harish.patil@cavium.com>; Ivan Malov
> <Ivan.Malov@oktetlabs.ru>
> Subject: Re: [dpdk-dev] [PATCH] doc: update new ethdev offload API description
>
> Wednesday, March 21, 2018 2:52 PM, Ferruh Yigit:
> > On 3/21/2018 11:40 AM, Shahaf Shuler wrote:
> > > Wednesday, March 21, 2018 1:37 PM, Andrew Rybchenko:
> > >> On 03/21/2018 02:23 PM, Shahaf Shuler wrote:
> > >>>
> > >>> Wednesday, March 21, 2018 1:20 PM, *:*Andrew Rybchenko
> > >>>
> > >>>> Not exactly. We should add statement to allow to enable queue
> > >>>> offloads on port level (to enable on all queues).
> > >>>
> > >>> Why it is needed ?
> > >>>
> > >>
> > >> May be just a paranoia to avoid misreading/misunderstanding.
> > >>
> > >>> Queue offload is also a port offload, for the simple case it is
> > >>> enabled on each of the queues.
> > >>>
> > >>> PMDs should report rx[tx]_offload_capa = port_offloads |
> > >>> queue_offloads
> > >>>
> > >>> So from the application side it enables a **port** offload which, by
> > >>> definition, will set the offload to each of the queues.
> > >>>
> > >>> it is not “enabling queue offload on the port”.
> > >>>
> > >>
> > >> I think it would be really useful for understanding to highlight that
> > >> what is enabled on port level is enabled on all queues regardless queue
> > conf.
> > >
> > > So I think the extra wording should explain that queue offload is also a port
> > offload, and not to mix between the queue and port offload configuration.
> >
> > +1 for more details, the sentences was the outcome of the previous
> > +discussion
> > but not clear enough. Perhaps some sample values can be also good.
> > Shahaf do you want to give a try?
>
> IMO what is missing is:
> 1. the syntax of this patch
> 2. explicitly write that queue offloads are always subset of the port offload
> 3. explicitly write that when port offload is enabled it applies to all queues (that one is already written AFAIR).
For me examples below seems quite handy.
It clearly shows that queue_setup() can do both: enable and disable any per-queue offload.
Konstantin
>
> >
> > And is following correct based on latest :
> > 1- Port capability is always covers queue capability P_cap = A, B, C, D Q_cap =
> > B, C, D
> >
> > 2- Requested port offloads should be subset of port capabilities, they will be
> > applied to all queues:
> > P_req = A, B
> > Q1: A, B
> > Q2: A, B
> >
> > 3- Later, requested queue offloads should be subset of queue capabilities,
> > they will be applied to specific queue:
> > Q_req = 1:B, C
> > Q1: A, B, C
> > Q2: A, B
> >
> > Q_req = 2:D
> > Q1: A, B, C
> > Q2: A, D
> >
> >
> > Scenario 2:
> > 1-
> > P_cap = A, B, C, D
> > Q_cap = ""
> >
> > 2-
> > P_req = A, B
> > Q1: A, B
> > Q2: A, B
> >
> > 3-
> > Q_req = ""
> > Q1: A, B
> > Q2: A, B
21/03/2018 11:54, Ferruh Yigit:
> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> > IMHO, it should be allowed to specify queue offloads on port level.
> > It should simply enable these offloads on all queues. Also it will
> > match dev_info [rt]x_offload_capa which include both port and queue
> > offloads.
> >
> > Yes, we lose possibility to enable on port level, but disable on queue
> > level by suggested changes, but I think it is OK - if you don't need
> > it for all queues, just control separately on queue level.
>
> What I understand was queue offload can only enable more, but it seems it can
> both enable or disable.
Yes, queue offload should only enable more.
An offload enabled at port level, cannot be disabled at queue level.
A port offload can be repeated in queue configuration.
If a port offload is not repeated in queue configuration, there should be
no impact: it is still in the port configuration, thus applying to all queues.
About capabilities, the queue offloads must be a subset of port offloads.
The queue capabilities show which offloads can be enabled per queue.
On 3/21/2018 2:08 PM, Thomas Monjalon wrote:
> 21/03/2018 11:54, Ferruh Yigit:
>> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
>>> IMHO, it should be allowed to specify queue offloads on port level.
>>> It should simply enable these offloads on all queues. Also it will
>>> match dev_info [rt]x_offload_capa which include both port and queue
>>> offloads.
>>>
>>> Yes, we lose possibility to enable on port level, but disable on queue
>>> level by suggested changes, but I think it is OK - if you don't need
>>> it for all queues, just control separately on queue level.
>>
>> What I understand was queue offload can only enable more, but it seems it can
>> both enable or disable.
>
> Yes, queue offload should only enable more.
> An offload enabled at port level, cannot be disabled at queue level.
Agree an offload enabled at port level can't be disabled at queue level, but why
not have the ability to disable a queue level offload with another queue setup call.
> A port offload can be repeated in queue configuration.
> If a port offload is not repeated in queue configuration, there should be
> no impact: it is still in the port configuration, thus applying to all queues.
This was a requirement, this patch targets removing the requirement to repeat
the port offload in queue config.
>
> About capabilities, the queue offloads must be a subset of port offloads.
> The queue capabilities show which offloads can be enabled per queue.
>
>
21/03/2018 15:28, Ferruh Yigit:
> On 3/21/2018 2:08 PM, Thomas Monjalon wrote:
> > 21/03/2018 11:54, Ferruh Yigit:
> >> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> >>> IMHO, it should be allowed to specify queue offloads on port level.
> >>> It should simply enable these offloads on all queues. Also it will
> >>> match dev_info [rt]x_offload_capa which include both port and queue
> >>> offloads.
> >>>
> >>> Yes, we lose possibility to enable on port level, but disable on queue
> >>> level by suggested changes, but I think it is OK - if you don't need
> >>> it for all queues, just control separately on queue level.
> >>
> >> What I understand was queue offload can only enable more, but it seems it can
> >> both enable or disable.
> >
> > Yes, queue offload should only enable more.
> > An offload enabled at port level, cannot be disabled at queue level.
>
> Agree an offload enabled at port level can't be disabled at queue level, but why
> not have the ability to disable a queue level offload with another queue setup call.
Yes it should be possible to disable a queue offload:
1/ enable offload in queue configuration
2/ later disable this offload in queue configuration
So, when I say "only enable more", I mean queue config should only enable
more than port config. In other words, a port-level offload cannot be
disabled at queue level.
> > A port offload can be repeated in queue configuration.
> > If a port offload is not repeated in queue configuration, there should be
> > no impact: it is still in the port configuration, thus applying to all queues.
>
> This was a requirement, this patch targets removing the requirement to repeat
> the port offload in queue config.
Understood, and I agree with this change if it is well explained
in sphinx and doxygen.
It is important to say that queue configuration has no impact on offloads
already enabled at port level.
> > About capabilities, the queue offloads must be a subset of port offloads.
> > The queue capabilities show which offloads can be enabled per queue.
On Wed, Mar 21, 2018 at 03:40:43PM +0100, Thomas Monjalon wrote:
> 21/03/2018 15:28, Ferruh Yigit:
> > On 3/21/2018 2:08 PM, Thomas Monjalon wrote:
> > > 21/03/2018 11:54, Ferruh Yigit:
> > >> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> > >>> IMHO, it should be allowed to specify queue offloads on port level.
> > >>> It should simply enable these offloads on all queues. Also it will
> > >>> match dev_info [rt]x_offload_capa which include both port and queue
> > >>> offloads.
> > >>>
> > >>> Yes, we lose possibility to enable on port level, but disable on queue
> > >>> level by suggested changes, but I think it is OK - if you don't need
> > >>> it for all queues, just control separately on queue level.
> > >>
> > >> What I understand was queue offload can only enable more, but it seems it can
> > >> both enable or disable.
> > >
> > > Yes, queue offload should only enable more.
> > > An offload enabled at port level, cannot be disabled at queue level.
> >
> > Agree an offload enabled at port level can't be disabled at queue level, but why
> > not have the ability to disable a queue level offload with another queue setup call.
>
> Yes it should be possible to disable a queue offload:
> 1/ enable offload in queue configuration
> 2/ later disable this offload in queue configuration
>
> So, when I say "only enable more", I mean queue config should only enable
> more than port config. In other words, a port-level offload cannot be
> disabled at queue level.
>
> > > A port offload can be repeated in queue configuration.
> > > If a port offload is not repeated in queue configuration, there should be
> > > no impact: it is still in the port configuration, thus applying to all queues.
> >
> > This was a requirement, this patch targets removing the requirement to repeat
> > the port offload in queue config.
>
> Understood, and I agree with this change if it is well explained
> in sphinx and doxygen.
> It is important to say that queue configuration has no impact on offloads
> already enabled at port level.
>
> > > About capabilities, the queue offloads must be a subset of port offloads.
> > > The queue capabilities show which offloads can be enabled per queue.
>
>
>
Why not abandon port-level config entirely? Then you just have queue-level
configs, with the restriction that on some NICs all queues must be
configured the same way. It can be up to the NIC drivers - or possibly
ethdev layer - to identify and report an error in such cases.
/Bruce
Wednesday, March 21, 2018 5:27 PM, Bruce Richardson
> On Wed, Mar 21, 2018 at 03:40:43PM +0100, Thomas Monjalon wrote:
> > 21/03/2018 15:28, Ferruh Yigit:
> > > On 3/21/2018 2:08 PM, Thomas Monjalon wrote:
> > > > 21/03/2018 11:54, Ferruh Yigit:
> > > >> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> > > >>> IMHO, it should be allowed to specify queue offloads on port level.
> > > >>> It should simply enable these offloads on all queues. Also it
> > > >>> will match dev_info [rt]x_offload_capa which include both port
> > > >>> and queue offloads.
> >
>
> Why not abandon port-level config entirely? Then you just have queue-level
> configs, with the restriction that on some NICs all queues must be configured
> the same way. It can be up to the NIC drivers - or possibly ethdev layer - to
> identify and report an error in such cases.
I would love that. And this was part of the original proposal when we first modified the offloads API.
However Konstantin explained to me it will not work with Intel devices. There are cases were port configuration should be set on the PF w/o any queues created to enable offload on the VF.
>
> /Bruce
On Wed, Mar 21, 2018 at 03:29:57PM +0000, Shahaf Shuler wrote:
> Wednesday, March 21, 2018 5:27 PM, Bruce Richardson
> > On Wed, Mar 21, 2018 at 03:40:43PM +0100, Thomas Monjalon wrote:
> > > 21/03/2018 15:28, Ferruh Yigit:
> > > > On 3/21/2018 2:08 PM, Thomas Monjalon wrote:
> > > > > 21/03/2018 11:54, Ferruh Yigit:
> > > > >> On 3/21/2018 9:47 AM, Andrew Rybchenko wrote:
> > > > >>> IMHO, it should be allowed to specify queue offloads on port level.
> > > > >>> It should simply enable these offloads on all queues. Also it
> > > > >>> will match dev_info [rt]x_offload_capa which include both port
> > > > >>> and queue offloads.
> > >
> >
> > Why not abandon port-level config entirely? Then you just have queue-level
> > configs, with the restriction that on some NICs all queues must be configured
> > the same way. It can be up to the NIC drivers - or possibly ethdev layer - to
> > identify and report an error in such cases.
>
> I would love that. And this was part of the original proposal when we first modified the offloads API.
>
> However Konstantin explained to me it will not work with Intel devices. There are cases were port configuration should be set on the PF w/o any queues created to enable offload on the VF.
>
Apologies so, my bad for not having followed the whole discussion too
closely. Never mind. I'll go back to pretending I have something meaningful
to contribute on other threads instead. :-)
/Bruce
On 3/16/2018 3:51 PM, Ferruh Yigit wrote:
> Don't mandate API to pass port offload configuration during queue setup,
> this is unnecessary for devices that support only port level offloads.
>
> Fixes: 81ac560dc1b4 ("doc: add details on ethdev offloads API")
> Cc: shahafs@mellanox.com
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Superseded by [1] which has both doc update and implementation.
[1]
https://dpdk.org/dev/patchwork/patch/39457/
@@ -303,9 +303,7 @@ Supported offloads can be either per-port or per-queue.
Offloads are enabled using the existing ``DEV_TX_OFFLOAD_*`` or ``DEV_RX_OFFLOAD_*`` flags.
Per-port offload configuration is set using ``rte_eth_dev_configure``.
Per-queue offload configuration is set using ``rte_eth_rx_queue_setup`` and ``rte_eth_tx_queue_setup``.
-To enable per-port offload, the offload should be set on both device configuration and queue setup.
-In case of a mixed configuration the queue setup shall return with an error.
-To enable per-queue offload, the offload can be set only on the queue setup.
+Per-port offloads should be set on the port configuration. Queue offloads should be set on the queue configuration.
Offloads which are not enabled are disabled by default.
For an application to use the Tx offloads API it should set the ``ETH_TXQ_FLAGS_IGNORE`` flag in the ``txq_flags`` field located in ``rte_eth_txconf`` struct.