[RFC,2/2] ethdev: add capability to keep indirect actions on restart
Checks
Commit Message
rte_flow_action_handle_create() did not mention what happens
with an indirect action when a device is stopped, possibly reconfigured,
and started again. It is natural for some indirect actions to be
persistent, like counters and meters; keeping others just saves
application time and complexity. However, not all PMDs can support it.
It is proposed to add a device capability to indicate if indirect actions
are kept across the above sequence or implicitly destroyed.
It may happen that in the future a PMD acquires support for a type of
indirect actions that it cannot keep across a restart. It is undesirable
to stop advertising the capability so that applications that don't use
actions of the problematic type can still take advantage of it.
This is why PMDs are allowed to keep only a subset of indirect actions
provided that the vendor mandatorily documents it.
If the device is being reconfigured in a way that is incompatible with
an existing indirect action, PMD is required to report an error.
This is mandatory, because flow API does not supply users with
capabilities, so this is the only way for a user to learn that
configuration is invalid. For example, if queue count changes and RSS
indirect action specifies queues that are going away, the user must
update the action before removing the queues or remove the action and
all flow rules that were using it.
Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
doc/guides/prog_guide/rte_flow.rst | 12 ++++++++++++
lib/ethdev/rte_ethdev.h | 5 +++++
2 files changed, 17 insertions(+)
Comments
2021-09-01 11:55 (UTC+0300), Dmitry Kozlyuk:
> rte_flow_action_handle_create() did not mention what happens
> with an indirect action when a device is stopped, possibly reconfigured,
> and started again. It is natural for some indirect actions to be
> persistent, like counters and meters; keeping others just saves
> application time and complexity. However, not all PMDs can support it.
> It is proposed to add a device capability to indicate if indirect actions
> are kept across the above sequence or implicitly destroyed.
>
> It may happen that in the future a PMD acquires support for a type of
> indirect actions that it cannot keep across a restart. It is undesirable
> to stop advertising the capability so that applications that don't use
> actions of the problematic type can still take advantage of it.
> This is why PMDs are allowed to keep only a subset of indirect actions
> provided that the vendor mandatorily documents it.
>
> If the device is being reconfigured in a way that is incompatible with
> an existing indirect action, PMD is required to report an error.
> This is mandatory, because flow API does not supply users with
> capabilities, so this is the only way for a user to learn that
> configuration is invalid. For example, if queue count changes and RSS
> indirect action specifies queues that are going away, the user must
> update the action before removing the queues or remove the action and
> all flow rules that were using it.
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
> doc/guides/prog_guide/rte_flow.rst | 12 ++++++++++++
> lib/ethdev/rte_ethdev.h | 5 +++++
> 2 files changed, 17 insertions(+)
>
> [...]
Hello, any opinions?
Just noticed that I forgot to Cc everyone in the cover letter with context:
http://inbox.dpdk.org/dev/20210901085516.3647814-1-dkozlyuk@nvidia.com/
On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
>
> rte_flow_action_handle_create() did not mention what happens
> with an indirect action when a device is stopped, possibly reconfigured,
> and started again. It is natural for some indirect actions to be
> persistent, like counters and meters; keeping others just saves
> application time and complexity. However, not all PMDs can support it.
> It is proposed to add a device capability to indicate if indirect actions
> are kept across the above sequence or implicitly destroyed.
>
> It may happen that in the future a PMD acquires support for a type of
> indirect actions that it cannot keep across a restart. It is undesirable
> to stop advertising the capability so that applications that don't use
> actions of the problematic type can still take advantage of it.
> This is why PMDs are allowed to keep only a subset of indirect actions
> provided that the vendor mandatorily documents it.
Sorry - I am seeing this late.
This could become confusing.
May be it is better for the PMDs to specify which actions are persistent.
How about adding a bit for the possible actions of interest.
And then PMDs can set bits for actions which can be persistent across
stop, start and reconfigurations?
>
> If the device is being reconfigured in a way that is incompatible with
> an existing indirect action, PMD is required to report an error.
> This is mandatory, because flow API does not supply users with
> capabilities, so this is the only way for a user to learn that
> configuration is invalid. For example, if queue count changes and RSS
> indirect action specifies queues that are going away, the user must
> update the action before removing the queues or remove the action and
> all flow rules that were using it.
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
> doc/guides/prog_guide/rte_flow.rst | 12 ++++++++++++
> lib/ethdev/rte_ethdev.h | 5 +++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 0a03097a7c..da90b52f48 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2794,6 +2794,18 @@ updated depend on the type of the ``action`` and different for every type.
> The indirect action specified data (e.g. counter) can be queried by
> ``rte_flow_action_handle_query()``.
>
> +By default indirect actions are destroyed when the device is stopped.
> +If the device advertises ``RTE_ETH_DEV_CAPA_FLOW_INDIRECT_ACTION_KEEP``,
> +indirect actions persist across the device stop and start with possible
> +reconfiguration in between. Some configuration changes may be incompatible
> +with existing indirect actions, in this case ``rte_eth_dev_configure()`` and/or
> +``rte_eth_rx/tx_queue_setup()`` will fail. At this point PMD developers
> +are encouraged to log errors identical to the ones that would be emitted by
> +``rte_flow_action_handle_create()`` if the new configuration was active.
> +Even if this capability is advertised, there may be kinds of indirect actions
> +that the device cannot keep. They are implicitly destroyed at device stop.
> +PMD developers must document such kinds of actions if applicable.
> +
> .. _table_rte_flow_action_handle:
>
> .. table:: INDIRECT
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 1616bdf2dd..c3be5afcb2 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1450,6 +1450,11 @@ struct rte_eth_conf {
> #define RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP 0x00000002
> /** Device keeps flow rules across restart and reconfiguration. */
> #define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP 0x00000004
> +/**
> + * Device keeps indirect actions across restart and reconfiguration.
> + * For a specific PMD this may not be applicable to certain action types.
> + */
> +#define RTE_ETH_DEV_CAPA_FLOW_INDIRECT_ACTION_KEEP 0x00000008
> /**@}*/
>
> /*
> --
> 2.25.1
>
> -----Original Message-----
> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Sent: 6 октября 2021 г. 20:13
> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori Kam
> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect
> actions on restart
>
> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
> >
> > rte_flow_action_handle_create() did not mention what happens
> > with an indirect action when a device is stopped, possibly reconfigured,
> > and started again. It is natural for some indirect actions to be
> > persistent, like counters and meters; keeping others just saves
> > application time and complexity. However, not all PMDs can support it.
> > It is proposed to add a device capability to indicate if indirect actions
> > are kept across the above sequence or implicitly destroyed.
> >
> > It may happen that in the future a PMD acquires support for a type of
> > indirect actions that it cannot keep across a restart. It is undesirable
> > to stop advertising the capability so that applications that don't use
> > actions of the problematic type can still take advantage of it.
> > This is why PMDs are allowed to keep only a subset of indirect actions
> > provided that the vendor mandatorily documents it.
> Sorry - I am seeing this late.
> This could become confusing.
> May be it is better for the PMDs to specify which actions are persistent.
> How about adding a bit for the possible actions of interest.
> And then PMDs can set bits for actions which can be persistent across
> stop, start and reconfigurations?
This approach was considered, but there is a risk of quickly running out of capability bits. Each action would consume one bit plus as many bits as there are special conditions for it in all the PMDs, because conditions are likely to be PMD-specific. And the application will anyway need to consider specific conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the other hand, PMDs are not expected to exercise this loophole unless absolutely needed.
On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
>> -----Original Message-----
>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Sent: 6 октября 2021 г. 20:13
>> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori Kam
>> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
>> Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect
>> actions on restart
>>
>> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
>>>
>>> rte_flow_action_handle_create() did not mention what happens
>>> with an indirect action when a device is stopped, possibly reconfigured,
>>> and started again. It is natural for some indirect actions to be
>>> persistent, like counters and meters; keeping others just saves
>>> application time and complexity. However, not all PMDs can support it.
>>> It is proposed to add a device capability to indicate if indirect actions
>>> are kept across the above sequence or implicitly destroyed.
>>>
>>> It may happen that in the future a PMD acquires support for a type of
>>> indirect actions that it cannot keep across a restart. It is undesirable
>>> to stop advertising the capability so that applications that don't use
>>> actions of the problematic type can still take advantage of it.
>>> This is why PMDs are allowed to keep only a subset of indirect actions
>>> provided that the vendor mandatorily documents it.
>> Sorry - I am seeing this late.
>> This could become confusing.
>> May be it is better for the PMDs to specify which actions are persistent.
>> How about adding a bit for the possible actions of interest.
>> And then PMDs can set bits for actions which can be persistent across
>> stop, start and reconfigurations?
>
> This approach was considered, but there is a risk of quickly running out of capability bits. Each action would consume one bit plus as many bits as there are special conditions for it in all the PMDs, because conditions are likely to be PMD-specific. And the application will anyway need to consider specific conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the other hand, PMDs are not expected to exercise this loophole unless absolutely needed.
>
May be we should separate at least transfer and non-transfer
rules? Transfer rules are less configuration dependent.
Hi Andrew and Ajit,
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, October 11, 2021 4:58 PM
> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
>
> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
> >> -----Original Message-----
> >> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >> Sent: 6 октября 2021 г. 20:13
> >> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> >> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori Kam
> >> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> >> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
> >> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >> keep indirect actions on restart
> >>
> >> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
> >>>
> >>> rte_flow_action_handle_create() did not mention what happens with an
> >>> indirect action when a device is stopped, possibly reconfigured, and
> >>> started again. It is natural for some indirect actions to be
> >>> persistent, like counters and meters; keeping others just saves
> >>> application time and complexity. However, not all PMDs can support it.
> >>> It is proposed to add a device capability to indicate if indirect
> >>> actions are kept across the above sequence or implicitly destroyed.
> >>>
> >>> It may happen that in the future a PMD acquires support for a type
> >>> of indirect actions that it cannot keep across a restart. It is
> >>> undesirable to stop advertising the capability so that applications
> >>> that don't use actions of the problematic type can still take advantage of it.
> >>> This is why PMDs are allowed to keep only a subset of indirect
> >>> actions provided that the vendor mandatorily documents it.
> >> Sorry - I am seeing this late.
> >> This could become confusing.
> >> May be it is better for the PMDs to specify which actions are persistent.
> >> How about adding a bit for the possible actions of interest.
> >> And then PMDs can set bits for actions which can be persistent across
> >> stop, start and reconfigurations?
> >
> > This approach was considered, but there is a risk of quickly running out of capability bits. Each action
> would consume one bit plus as many bits as there are special conditions for it in all the PMDs, because
> conditions are likely to be PMD-specific. And the application will anyway need to consider specific
> conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the other hand,
> PMDs are not expected to exercise this loophole unless absolutely needed.
> >
Right those bits should be considered as master bits and are not per actions.
If there is specific case for a PMD it should solve it by documation or other means.
>
> May be we should separate at least transfer and non-transfer rules? Transfer rules are less configuration
> dependent.
May be I'm missing something but jut like stated above those are master bits I don't see much use case where
the PMD can store transfer rules but not other rules. I assume that if the application uses the transfer mode
most of the flows will be in the transfer domain.
Best,
Ori
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: 11 октября 2021 г. 16:58
> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>
> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori Kam
> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect
> actions on restart
>
> External email: Use caution opening links or attachments
>
>
> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
> >> -----Original Message-----
> >> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >> Sent: 6 октября 2021 г. 20:13
> >> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> >> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori Kam
> >> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> >> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
> >> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >> keep indirect actions on restart
> >>
> >> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> wrote:
> >>>
> >>> rte_flow_action_handle_create() did not mention what happens with an
> >>> indirect action when a device is stopped, possibly reconfigured, and
> >>> started again. It is natural for some indirect actions to be
> >>> persistent, like counters and meters; keeping others just saves
> >>> application time and complexity. However, not all PMDs can support it.
> >>> It is proposed to add a device capability to indicate if indirect
> >>> actions are kept across the above sequence or implicitly destroyed.
> >>>
> >>> It may happen that in the future a PMD acquires support for a type
> >>> of indirect actions that it cannot keep across a restart. It is
> >>> undesirable to stop advertising the capability so that applications
> >>> that don't use actions of the problematic type can still take advantage of it.
> >>> This is why PMDs are allowed to keep only a subset of indirect
> >>> actions provided that the vendor mandatorily documents it.
> >> Sorry - I am seeing this late.
> >> This could become confusing.
> >> May be it is better for the PMDs to specify which actions are persistent.
> >> How about adding a bit for the possible actions of interest.
> >> And then PMDs can set bits for actions which can be persistent across
> >> stop, start and reconfigurations?
> >
> > This approach was considered, but there is a risk of quickly running out of
> capability bits. Each action would consume one bit plus as many bits as there are
> special conditions for it in all the PMDs, because conditions are likely to be PMD-
> specific. And the application will anyway need to consider specific conditions to
> know which bit to test, so the meaning of the bits will be PMD-specific. On the
> other hand, PMDs are not expected to exercise this loophole unless absolutely
> needed.
> >
>
> May be we should separate at least transfer and non-transfer rules? Transfer
> rules are less configuration dependent.
Do you suggest splitting the bit from patch 1/2 in two?
Or did you mean indirect actions with only "transfer" bit set
and suggest splitting the bit from this patch in two?
On 10/11/21 6:53 PM, Ori Kam wrote:
> Hi Andrew and Ajit,
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, October 11, 2021 4:58 PM
>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
>>
>> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
>>>> -----Original Message-----
>>>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>> Sent: 6 октября 2021 г. 20:13
>>>> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
>>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori Kam
>>>> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
>>>> Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
>>>> keep indirect actions on restart
>>>>
>>>> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
>>>>>
>>>>> rte_flow_action_handle_create() did not mention what happens with an
>>>>> indirect action when a device is stopped, possibly reconfigured, and
>>>>> started again. It is natural for some indirect actions to be
>>>>> persistent, like counters and meters; keeping others just saves
>>>>> application time and complexity. However, not all PMDs can support it.
>>>>> It is proposed to add a device capability to indicate if indirect
>>>>> actions are kept across the above sequence or implicitly destroyed.
>>>>>
>>>>> It may happen that in the future a PMD acquires support for a type
>>>>> of indirect actions that it cannot keep across a restart. It is
>>>>> undesirable to stop advertising the capability so that applications
>>>>> that don't use actions of the problematic type can still take advantage of it.
>>>>> This is why PMDs are allowed to keep only a subset of indirect
>>>>> actions provided that the vendor mandatorily documents it.
>>>> Sorry - I am seeing this late.
>>>> This could become confusing.
>>>> May be it is better for the PMDs to specify which actions are persistent.
>>>> How about adding a bit for the possible actions of interest.
>>>> And then PMDs can set bits for actions which can be persistent across
>>>> stop, start and reconfigurations?
>>>
>>> This approach was considered, but there is a risk of quickly running out of capability bits. Each action
>> would consume one bit plus as many bits as there are special conditions for it in all the PMDs, because
>> conditions are likely to be PMD-specific. And the application will anyway need to consider specific
>> conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the other hand,
>> PMDs are not expected to exercise this loophole unless absolutely needed.
>>>
> Right those bits should be considered as master bits and are not per actions.
> If there is specific case for a PMD it should solve it by documation or other means.
Documentation does not solve the problem since it can't be
automated. So, it just help to solve case-by-case.
>
>>
>> May be we should separate at least transfer and non-transfer rules? Transfer rules are less configuration
>> dependent.
>
> May be I'm missing something but jut like stated above those are master bits I don't see much use case where
> the PMD can store transfer rules but not other rules. I assume that if the application uses the transfer mode
> most of the flows will be in the transfer domain.
Most likely different HW blocks are responsible for transfer
and non-transfer rules. So, I can easily imagine that one
could be preserved across restart, but another can't.
Anyway, I'm just trying to understand. Not a blocker.
Also have you considered to make it controllable by the
application. I.e. PMD advertises a capability and it is
responsibility of the application to use it or not.
May be it is excessive. In theory application can check
the flag and do flush before or just after stop if it
does not want to preserve rules.
Andrew.
Hi
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, October 12, 2021 12:15 PM
> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
>
> On 10/11/21 6:53 PM, Ori Kam wrote:
> > Hi Andrew and Ajit,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, October 11, 2021 4:58 PM
> >> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >> keep indirect actions on restart
> >>
> >> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
> >>>> -----Original Message-----
> >>>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >>>> Sent: 6 октября 2021 г. 20:13
> >>>> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> >>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori
> >>>> Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> >>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> >>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >>>> keep indirect actions on restart
> >>>>
> >>>> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
> >>>>>
> >>>>> rte_flow_action_handle_create() did not mention what happens with
> >>>>> an indirect action when a device is stopped, possibly
> >>>>> reconfigured, and started again. It is natural for some indirect
> >>>>> actions to be persistent, like counters and meters; keeping others
> >>>>> just saves application time and complexity. However, not all PMDs can support it.
> >>>>> It is proposed to add a device capability to indicate if indirect
> >>>>> actions are kept across the above sequence or implicitly destroyed.
> >>>>>
> >>>>> It may happen that in the future a PMD acquires support for a type
> >>>>> of indirect actions that it cannot keep across a restart. It is
> >>>>> undesirable to stop advertising the capability so that
> >>>>> applications that don't use actions of the problematic type can still take advantage of it.
> >>>>> This is why PMDs are allowed to keep only a subset of indirect
> >>>>> actions provided that the vendor mandatorily documents it.
> >>>> Sorry - I am seeing this late.
> >>>> This could become confusing.
> >>>> May be it is better for the PMDs to specify which actions are persistent.
> >>>> How about adding a bit for the possible actions of interest.
> >>>> And then PMDs can set bits for actions which can be persistent
> >>>> across stop, start and reconfigurations?
> >>>
> >>> This approach was considered, but there is a risk of quickly running
> >>> out of capability bits. Each action
> >> would consume one bit plus as many bits as there are special
> >> conditions for it in all the PMDs, because conditions are likely to
> >> be PMD-specific. And the application will anyway need to consider
> >> specific conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the
> other hand, PMDs are not expected to exercise this loophole unless absolutely needed.
> >>>
> > Right those bits should be considered as master bits and are not per actions.
> > If there is specific case for a PMD it should solve it by documation or other means.
>
> Documentation does not solve the problem since it can't be automated. So, it just help to solve case-by-
> case.
I agree that documentation can't be automated, I think this is just like other edge cases that can't be checked
for example you can reconfigure the device after start except the queue number or queue size (just an example)
The metrix of actions/items/pmds I don't think we will ever be able to have an easy way to check capabilities.
Maybe we can say that if PMD reports that it supports keeping the actions, and it can't support just one of the actions
it can fail or issue a special error code when calling stop. To let the application know that something was incorrect.
In this case application can create a sample of the action it requires and then call the stop. If it fails it can try again until
he gets no error, and only then start. What do you think?
Another way is to assume that if the action was created before port start it will be kept after port stop.
And this bit is just for letting the application know if it is worth to check.
>
> >
> >>
> >> May be we should separate at least transfer and non-transfer rules?
> >> Transfer rules are less configuration dependent.
> >
> > May be I'm missing something but jut like stated above those are
> > master bits I don't see much use case where the PMD can store transfer
> > rules but not other rules. I assume that if the application uses the transfer mode most of the flows will be
> in the transfer domain.
>
> Most likely different HW blocks are responsible for transfer and non-transfer rules. So, I can easily imagine
> that one could be preserved across restart, but another can't.
>
I don't know, but in our case this is the same block.
since a lot of the action are the same between the eswitch and the ethdev I would expect that the limitation will be the same.
how is it in your case?
> Anyway, I'm just trying to understand. Not a blocker.
>
> Also have you considered to make it controllable by the application. I.e. PMD advertises a capability and it
> is responsibility of the application to use it or not.
> May be it is excessive. In theory application can check the flag and do flush before or just after stop if it
> does not want to preserve rules.
>
I'm not sure I understand this comment, The application is always free to use or not use a capability this is
just to let the application know that if it doesn't want to destroy the action before stop he doesn't have to
and the action will be saved.
> Andrew.
Ori.
On 10/12/21 1:26 PM, Ori Kam wrote:
> Hi
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Tuesday, October 12, 2021 12:15 PM
>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
>>
>> On 10/11/21 6:53 PM, Ori Kam wrote:
>>> Hi Andrew and Ajit,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Monday, October 11, 2021 4:58 PM
>>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
>>>> keep indirect actions on restart
>>>>
>>>> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
>>>>>> -----Original Message-----
>>>>>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>> Sent: 6 октября 2021 г. 20:13
>>>>>> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
>>>>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori
>>>>>> Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
>>>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
>>>>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
>>>>>> keep indirect actions on restart
>>>>>>
>>>>>> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
>>>>>>>
>>>>>>> rte_flow_action_handle_create() did not mention what happens with
>>>>>>> an indirect action when a device is stopped, possibly
>>>>>>> reconfigured, and started again. It is natural for some indirect
>>>>>>> actions to be persistent, like counters and meters; keeping others
>>>>>>> just saves application time and complexity. However, not all PMDs can support it.
>>>>>>> It is proposed to add a device capability to indicate if indirect
>>>>>>> actions are kept across the above sequence or implicitly destroyed.
>>>>>>>
>>>>>>> It may happen that in the future a PMD acquires support for a type
>>>>>>> of indirect actions that it cannot keep across a restart. It is
>>>>>>> undesirable to stop advertising the capability so that
>>>>>>> applications that don't use actions of the problematic type can still take advantage of it.
>>>>>>> This is why PMDs are allowed to keep only a subset of indirect
>>>>>>> actions provided that the vendor mandatorily documents it.
>>>>>> Sorry - I am seeing this late.
>>>>>> This could become confusing.
>>>>>> May be it is better for the PMDs to specify which actions are persistent.
>>>>>> How about adding a bit for the possible actions of interest.
>>>>>> And then PMDs can set bits for actions which can be persistent
>>>>>> across stop, start and reconfigurations?
>>>>>
>>>>> This approach was considered, but there is a risk of quickly running
>>>>> out of capability bits. Each action
>>>> would consume one bit plus as many bits as there are special
>>>> conditions for it in all the PMDs, because conditions are likely to
>>>> be PMD-specific. And the application will anyway need to consider
>>>> specific conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the
>> other hand, PMDs are not expected to exercise this loophole unless absolutely needed.
>>>>>
>>> Right those bits should be considered as master bits and are not per actions.
>>> If there is specific case for a PMD it should solve it by documation or other means.
>>
>> Documentation does not solve the problem since it can't be automated. So, it just help to solve case-by-
>> case.
>
> I agree that documentation can't be automated, I think this is just like other edge cases that can't be checked
> for example you can reconfigure the device after start except the queue number or queue size (just an example)
> The metrix of actions/items/pmds I don't think we will ever be able to have an easy way to check capabilities.
>
> Maybe we can say that if PMD reports that it supports keeping the actions, and it can't support just one of the actions
> it can fail or issue a special error code when calling stop. To let the application know that something was incorrect.
> In this case application can create a sample of the action it requires and then call the stop. If it fails it can try again until
> he gets no error, and only then start. What do you think?
It all sounds complicated. Do we really need it?
> Another way is to assume that if the action was created before port start it will be kept after port stop.
It does not sound like a solution. May be I simply don't know
target usecase.
> And this bit is just for letting the application know if it is worth to check.
>
>>
>>>
>>>>
>>>> May be we should separate at least transfer and non-transfer rules?
>>>> Transfer rules are less configuration dependent.
>>>
>>> May be I'm missing something but jut like stated above those are
>>> master bits I don't see much use case where the PMD can store transfer
>>> rules but not other rules. I assume that if the application uses the transfer mode most of the flows will be
>> in the transfer domain.
>>
>> Most likely different HW blocks are responsible for transfer and non-transfer rules. So, I can easily imagine
>> that one could be preserved across restart, but another can't.
>>
>
> I don't know, but in our case this is the same block.
> since a lot of the action are the same between the eswitch and the ethdev I would expect that the limitation will be the same.
> how is it in your case?
Actions or rules? QUEUE and RSS are non-transfer actions.
PORT_ID etc are transfer actions which do not make sense in
non-transfer case. DROP, COUNT and MARK make sense in both
cases. Packet edits make sense in both cases as well.
>
>> Anyway, I'm just trying to understand. Not a blocker.
>>
>> Also have you considered to make it controllable by the application. I.e. PMD advertises a capability and it
>> is responsibility of the application to use it or not.
>> May be it is excessive. In theory application can check the flag and do flush before or just after stop if it
>> does not want to preserve rules.
>>
>
> I'm not sure I understand this comment, The application is always free to use or not use a capability this is
> just to let the application know that if it doesn't want to destroy the action before stop he doesn't have to
> and the action will be saved.
Hm, who said that application must explicitly destroy
rules/actions before stop?
Please let's continue in the thread for the latest version of the patches:
http://inbox.dpdk.org/dev/CH0PR12MB5091792A77CBD1528DB7A005B9B79@CH0PR12MB5091.namprd12.prod.outlook.com/
Please see my comments there.
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: 12 октября 2021 г. 13:41
> To: Ori Kam <orika@nvidia.com>; Dmitry Kozlyuk <dkozlyuk@nvidia.com>; Ajit
> Khaparde <ajit.khaparde@broadcom.com>
> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; NBU-
> Contact-Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect
> actions on restart
>
> External email: Use caution opening links or attachments
>
>
> On 10/12/21 1:26 PM, Ori Kam wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Tuesday, October 12, 2021 12:15 PM
> >> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >> keep indirect actions on restart
> >>
> >> On 10/11/21 6:53 PM, Ori Kam wrote:
> >>> Hi Andrew and Ajit,
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Monday, October 11, 2021 4:58 PM
> >>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >>>> keep indirect actions on restart
> >>>>
> >>>> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >>>>>> Sent: 6 октября 2021 г. 20:13
> >>>>>> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> >>>>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori
> >>>>>> Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> >>>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> >>>>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >>>>>> keep indirect actions on restart
> >>>>>>
> >>>>>> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> wrote:
> >>>>>>>
> >>>>>>> rte_flow_action_handle_create() did not mention what happens
> >>>>>>> with an indirect action when a device is stopped, possibly
> >>>>>>> reconfigured, and started again. It is natural for some indirect
> >>>>>>> actions to be persistent, like counters and meters; keeping
> >>>>>>> others just saves application time and complexity. However, not all
> PMDs can support it.
> >>>>>>> It is proposed to add a device capability to indicate if
> >>>>>>> indirect actions are kept across the above sequence or implicitly
> destroyed.
> >>>>>>>
> >>>>>>> It may happen that in the future a PMD acquires support for a
> >>>>>>> type of indirect actions that it cannot keep across a restart.
> >>>>>>> It is undesirable to stop advertising the capability so that
> >>>>>>> applications that don't use actions of the problematic type can still take
> advantage of it.
> >>>>>>> This is why PMDs are allowed to keep only a subset of indirect
> >>>>>>> actions provided that the vendor mandatorily documents it.
> >>>>>> Sorry - I am seeing this late.
> >>>>>> This could become confusing.
> >>>>>> May be it is better for the PMDs to specify which actions are persistent.
> >>>>>> How about adding a bit for the possible actions of interest.
> >>>>>> And then PMDs can set bits for actions which can be persistent
> >>>>>> across stop, start and reconfigurations?
> >>>>>
> >>>>> This approach was considered, but there is a risk of quickly
> >>>>> running out of capability bits. Each action
> >>>> would consume one bit plus as many bits as there are special
> >>>> conditions for it in all the PMDs, because conditions are likely to
> >>>> be PMD-specific. And the application will anyway need to consider
> >>>> specific conditions to know which bit to test, so the meaning of
> >>>> the bits will be PMD-specific. On the
> >> other hand, PMDs are not expected to exercise this loophole unless
> absolutely needed.
> >>>>>
> >>> Right those bits should be considered as master bits and are not per actions.
> >>> If there is specific case for a PMD it should solve it by documation or other
> means.
> >>
> >> Documentation does not solve the problem since it can't be automated.
> >> So, it just help to solve case-by- case.
> >
> > I agree that documentation can't be automated, I think this is just
> > like other edge cases that can't be checked for example you can
> > reconfigure the device after start except the queue number or queue size (just
> an example) The metrix of actions/items/pmds I don't think we will ever be able
> to have an easy way to check capabilities.
> >
> > Maybe we can say that if PMD reports that it supports keeping the
> > actions, and it can't support just one of the actions it can fail or issue a special
> error code when calling stop. To let the application know that something was
> incorrect.
> > In this case application can create a sample of the action it requires
> > and then call the stop. If it fails it can try again until he gets no error, and only
> then start. What do you think?
>
> It all sounds complicated. Do we really need it?
>
> > Another way is to assume that if the action was created before port start it will
> be kept after port stop.
>
> It does not sound like a solution. May be I simply don't know target usecase.
>
> > And this bit is just for letting the application know if it is worth to check.
> >
> >>
> >>>
> >>>>
> >>>> May be we should separate at least transfer and non-transfer rules?
> >>>> Transfer rules are less configuration dependent.
> >>>
> >>> May be I'm missing something but jut like stated above those are
> >>> master bits I don't see much use case where the PMD can store
> >>> transfer rules but not other rules. I assume that if the
> >>> application uses the transfer mode most of the flows will be
> >> in the transfer domain.
> >>
> >> Most likely different HW blocks are responsible for transfer and
> >> non-transfer rules. So, I can easily imagine that one could be preserved
> across restart, but another can't.
> >>
> >
> > I don't know, but in our case this is the same block.
> > since a lot of the action are the same between the eswitch and the ethdev I
> would expect that the limitation will be the same.
> > how is it in your case?
>
> Actions or rules? QUEUE and RSS are non-transfer actions.
> PORT_ID etc are transfer actions which do not make sense in non-transfer case.
> DROP, COUNT and MARK make sense in both cases. Packet edits make sense in
> both cases as well.
>
> >
> >> Anyway, I'm just trying to understand. Not a blocker.
> >>
> >> Also have you considered to make it controllable by the application.
> >> I.e. PMD advertises a capability and it is responsibility of the application to
> use it or not.
> >> May be it is excessive. In theory application can check the flag and
> >> do flush before or just after stop if it does not want to preserve rules.
> >>
> >
> > I'm not sure I understand this comment, The application is always free
> > to use or not use a capability this is just to let the application
> > know that if it doesn't want to destroy the action before stop he doesn't have
> to and the action will be saved.
>
> Hm, who said that application must explicitly destroy rules/actions before stop?
@@ -2794,6 +2794,18 @@ updated depend on the type of the ``action`` and different for every type.
The indirect action specified data (e.g. counter) can be queried by
``rte_flow_action_handle_query()``.
+By default indirect actions are destroyed when the device is stopped.
+If the device advertises ``RTE_ETH_DEV_CAPA_FLOW_INDIRECT_ACTION_KEEP``,
+indirect actions persist across the device stop and start with possible
+reconfiguration in between. Some configuration changes may be incompatible
+with existing indirect actions, in this case ``rte_eth_dev_configure()`` and/or
+``rte_eth_rx/tx_queue_setup()`` will fail. At this point PMD developers
+are encouraged to log errors identical to the ones that would be emitted by
+``rte_flow_action_handle_create()`` if the new configuration was active.
+Even if this capability is advertised, there may be kinds of indirect actions
+that the device cannot keep. They are implicitly destroyed at device stop.
+PMD developers must document such kinds of actions if applicable.
+
.. _table_rte_flow_action_handle:
.. table:: INDIRECT
@@ -1450,6 +1450,11 @@ struct rte_eth_conf {
#define RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP 0x00000002
/** Device keeps flow rules across restart and reconfiguration. */
#define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP 0x00000004
+/**
+ * Device keeps indirect actions across restart and reconfiguration.
+ * For a specific PMD this may not be applicable to certain action types.
+ */
+#define RTE_ETH_DEV_CAPA_FLOW_INDIRECT_ACTION_KEEP 0x00000008
/**@}*/
/*