DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart
@ 2021-09-01  8:55 Dmitry Kozlyuk
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 1/2] ethdev: add capability to keep flow rules on restart Dmitry Kozlyuk
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-09-01  8:55 UTC (permalink / raw)
  To: dev

It is currently unspecified if flow rules and indirect actions persist
across port restart and reconfiguration. Actual behavior differs between
PMDs, for example, i40e and mlx5. There are patches that try to fix
particular behavior [1] or at least document it for specific PMDs [2].
We propose to codify the least demanding behavior, i.e. no persistence,
and to add device capabilities for persistence of rules and actions.
This continues the discussion thread [3].

[1]: http://patchwork.dpdk.org/project/dpdk/list/?series=18065
[2]: http://patchwork.dpdk.org/project/dpdk/list/?series=17939
[3]: http://inbox.dpdk.org/dev/20210727073121.895620-5-dkozlyuk@nvidia.com/

Dmitry Kozlyuk (2):
  ethdev: add capability to keep flow rules on restart
  ethdev: add capability to keep indirect actions on restart

 doc/guides/prog_guide/rte_flow.rst | 21 +++++++++++++++++++++
 lib/ethdev/rte_ethdev.h            |  7 +++++++
 2 files changed, 28 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [RFC PATCH 1/2] ethdev: add capability to keep flow rules on restart
  2021-09-01  8:55 [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Dmitry Kozlyuk
@ 2021-09-01  8:55 ` Dmitry Kozlyuk
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions " Dmitry Kozlyuk
  2021-10-05 17:23 ` [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Thomas Monjalon
  2 siblings, 0 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-09-01  8:55 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Ori Kam, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko

Currently, it is not specified what happens to the flow rules when
the device is stopped, possibly reconfigured, then started.
If flow rules were kept, it can be convenient for application
developers, because they wouldn't need to save and restore them.
However, due to the number of flows and possible creation rate it is
impractical to save all flow rules in DPDK layer. This means that flow
rules persistence really depends on whether PMD and HW can implement it
efficiently. It is proposed for PMDs to advertise this capability
if supported using a new flag.

If the device is being reconfigured in a way that is incompatible with
existing flow rules, 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 the
action of a flow rule specifies queues that are going away, the user
must update or remove the flow rule before removing the queues.

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 | 9 +++++++++
 lib/ethdev/rte_ethdev.h            | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 2b42d5ec8c..0a03097a7c 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -87,6 +87,15 @@ To avoid resource leaks on the PMD side, handles must be explicitly
 destroyed by the application before releasing associated resources such as
 queues and ports.
 
+By default flow rules are implicitly destroyed when the device is stopped.
+If the device advertises ``RTE_DEV_CAPA_FLOW_RULE_KEEP``, flow rules persist
+across device stop and start with possible reconfiguration in between.
+Some configuration changes may be incompatible with existing flow rules,
+in this case ``rte_eth_dev_configure()`` 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_create()`` if the new
+configuration was active.
+
 The following sections cover:
 
 - **Attributes** (represented by ``struct rte_flow_attr``): properties of a
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..1616bdf2dd 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1448,6 +1448,8 @@ struct rte_eth_conf {
 #define RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP 0x00000001
 /** Device supports Tx queue setup after device started. */
 #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
 /**@}*/
 
 /*
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-09-01  8:55 [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Dmitry Kozlyuk
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 1/2] ethdev: add capability to keep flow rules on restart Dmitry Kozlyuk
@ 2021-09-01  8:55 ` Dmitry Kozlyuk
  2021-09-27 11:21   ` Dmitry Kozlyuk
  2021-10-06 17:12   ` Ajit Khaparde
  2021-10-05 17:23 ` [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Thomas Monjalon
  2 siblings, 2 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-09-01  8:55 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Ori Kam, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko

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(+)

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions " Dmitry Kozlyuk
@ 2021-09-27 11:21   ` Dmitry Kozlyuk
  2021-10-06 17:12   ` Ajit Khaparde
  1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-09-27 11:21 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Matan Azrad, Ori Kam, Thomas Monjalon, Ferruh Yigit,
	Andrew Rybchenko

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/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart
  2021-09-01  8:55 [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Dmitry Kozlyuk
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 1/2] ethdev: add capability to keep flow rules on restart Dmitry Kozlyuk
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions " Dmitry Kozlyuk
@ 2021-10-05 17:23 ` Thomas Monjalon
  2 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2021-10-05 17:23 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, orika, ferruh.yigit, andrew.rybchenko, ajit.khaparde, jerinj

> Dmitry Kozlyuk (2):
>   ethdev: add capability to keep flow rules on restart
>   ethdev: add capability to keep indirect actions on restart
> 
>  doc/guides/prog_guide/rte_flow.rst | 21 +++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h            |  7 +++++++
>  2 files changed, 28 insertions(+)

We should update this:

 * Please note that some configuration is not stored between calls to
 * rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration will
 * be retained:
 *
 *     - MTU
 *     - flow control settings
 *     - receive mode configuration (promiscuous mode, all-multicast mode,
 *       hardware checksum mode, RSS/VMDQ settings etc.)
 *     - VLAN filtering configuration
 *     - default MAC address
 *     - MAC addresses supplied to MAC address array
 *     - flow director filtering mode (but not filtering rules)
 *     - NIC queue statistics mappings




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions " Dmitry Kozlyuk
  2021-09-27 11:21   ` Dmitry Kozlyuk
@ 2021-10-06 17:12   ` Ajit Khaparde
  2021-10-07  8:16     ` Dmitry Kozlyuk
  1 sibling, 1 reply; 14+ messages in thread
From: Ajit Khaparde @ 2021-10-06 17:12 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dpdk-dev, Matan Azrad, Ori Kam, Thomas Monjalon, Ferruh Yigit,
	Andrew Rybchenko

[-- Attachment #1: Type: text/plain, Size: 4079 bytes --]

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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-06 17:12   ` Ajit Khaparde
@ 2021-10-07  8:16     ` Dmitry Kozlyuk
  2021-10-11 13:58       ` Andrew Rybchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-10-07  8:16 UTC (permalink / raw)
  To: Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, Ori Kam, NBU-Contact-Thomas Monjalon,
	Ferruh Yigit, Andrew Rybchenko

> -----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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-07  8:16     ` Dmitry Kozlyuk
@ 2021-10-11 13:58       ` Andrew Rybchenko
  2021-10-11 15:53         ` Ori Kam
  2021-10-11 15:57         ` Dmitry Kozlyuk
  0 siblings, 2 replies; 14+ messages in thread
From: Andrew Rybchenko @ 2021-10-11 13:58 UTC (permalink / raw)
  To: Dmitry Kozlyuk, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, Ori Kam, NBU-Contact-Thomas Monjalon,
	Ferruh Yigit

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-11 13:58       ` Andrew Rybchenko
@ 2021-10-11 15:53         ` Ori Kam
  2021-10-12  9:15           ` Andrew Rybchenko
  2021-10-11 15:57         ` Dmitry Kozlyuk
  1 sibling, 1 reply; 14+ messages in thread
From: Ori Kam @ 2021-10-11 15:53 UTC (permalink / raw)
  To: Andrew Rybchenko, Dmitry Kozlyuk, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, NBU-Contact-Thomas Monjalon, Ferruh Yigit

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-11 13:58       ` Andrew Rybchenko
  2021-10-11 15:53         ` Ori Kam
@ 2021-10-11 15:57         ` Dmitry Kozlyuk
  1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-10-11 15:57 UTC (permalink / raw)
  To: Andrew Rybchenko, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, Ori Kam, NBU-Contact-Thomas Monjalon,
	Ferruh Yigit

> -----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?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-11 15:53         ` Ori Kam
@ 2021-10-12  9:15           ` Andrew Rybchenko
  2021-10-12 10:26             ` Ori Kam
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Rybchenko @ 2021-10-12  9:15 UTC (permalink / raw)
  To: Ori Kam, Dmitry Kozlyuk, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, NBU-Contact-Thomas Monjalon, Ferruh Yigit

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-12  9:15           ` Andrew Rybchenko
@ 2021-10-12 10:26             ` Ori Kam
  2021-10-12 10:41               ` Andrew Rybchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Ori Kam @ 2021-10-12 10:26 UTC (permalink / raw)
  To: Andrew Rybchenko, Dmitry Kozlyuk, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, NBU-Contact-Thomas Monjalon, Ferruh Yigit

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-12 10:26             ` Ori Kam
@ 2021-10-12 10:41               ` Andrew Rybchenko
  2021-10-13  8:36                 ` Dmitry Kozlyuk
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Rybchenko @ 2021-10-12 10:41 UTC (permalink / raw)
  To: Ori Kam, Dmitry Kozlyuk, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, NBU-Contact-Thomas Monjalon, Ferruh Yigit

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?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
  2021-10-12 10:41               ` Andrew Rybchenko
@ 2021-10-13  8:36                 ` Dmitry Kozlyuk
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Kozlyuk @ 2021-10-13  8:36 UTC (permalink / raw)
  To: Andrew Rybchenko, Ori Kam, Ajit Khaparde
  Cc: dpdk-dev, Matan Azrad, NBU-Contact-Thomas Monjalon, Ferruh Yigit

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?

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-10-13  8:36 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01  8:55 [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Dmitry Kozlyuk
2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 1/2] ethdev: add capability to keep flow rules on restart Dmitry Kozlyuk
2021-09-01  8:55 ` [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions " Dmitry Kozlyuk
2021-09-27 11:21   ` Dmitry Kozlyuk
2021-10-06 17:12   ` Ajit Khaparde
2021-10-07  8:16     ` Dmitry Kozlyuk
2021-10-11 13:58       ` Andrew Rybchenko
2021-10-11 15:53         ` Ori Kam
2021-10-12  9:15           ` Andrew Rybchenko
2021-10-12 10:26             ` Ori Kam
2021-10-12 10:41               ` Andrew Rybchenko
2021-10-13  8:36                 ` Dmitry Kozlyuk
2021-10-11 15:57         ` Dmitry Kozlyuk
2021-10-05 17:23 ` [dpdk-dev] [RFC PATCH 0/2] Flow entities behavior across port restart Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).