[RFC,2/3] ethdev: add flow modify mark action
Checks
Commit Message
Mark ID can be modified when supporting multiple tables. Partial bit
alteration is supported to preserve some bit-fields set by previous match.
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
doc/guides/prog_guide/rte_flow.rst | 21 +++++++++++++++++++++
lib/librte_ethdev/rte_flow.h | 24 ++++++++++++++++++++++++
2 files changed, 45 insertions(+)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
> Sent: Tuesday, June 4, 2019 3:03 AM
> To: shahafs@mellanox.com; thomas@monjalon.net; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; adrien.mazarguil@6wind.com;
> olivier.matz@6wind.com
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [RFC 2/3] ethdev: add flow modify mark action
>
> Mark ID can be modified when supporting multiple tables. Partial bit
> alteration is supported to preserve some bit-fields set by previous match.
>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> doc/guides/prog_guide/rte_flow.rst | 21 +++++++++++++++++++++
> lib/librte_ethdev/rte_flow.h | 24 ++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 016cd90e52..2907edfff4 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1463,6 +1463,27 @@ depends on the underlying implementation. It is
> returned in the
> | ``id`` | integer value to return with packets |
> +--------+--------------------------------------+
>
> +Action: ``MODIFY_MARK``
> +^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Alter partial bits of mark ID set by ``MARK`` action.
> +
> +``mask`` indicates which bits are modified. For bits which have never
> +been set by ``MARK`` or ``MODIFY_MARK``, unpredictable value will be
> +seen depending on driver implementation.
> +
> +.. _table_rte_flow_action_modify_mark:
> +
> +.. table:: MODIFY_MARK
> +
> + +----------+--------------------------------------+
> + | Field | Value |
> + +==========+======================================+
> + | ``id`` | integer value to return with packets |
> + +----------+--------------------------------------+
> + | ``mask`` | bit-mask applies to "id" |
> + +----------+--------------------------------------+
> +
> Action: ``FLAG``
> ^^^^^^^^^^^^^^^^
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h index
> cda8628183..d811f8a06e 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -1316,6 +1316,13 @@ enum rte_flow_action_type {
> */
> RTE_FLOW_ACTION_TYPE_MARK,
>
> + /**
> + * Alter partial bits of mark ID set by
> RTE_FLOW_ACTION_TYPE_MARK.
> + *
> + * See struct rte_flow_action_modify_mark.
> + */
> + RTE_FLOW_ACTION_TYPE_MODIFY_MARK,
> +
I think, we need to define the case where application calls MODIFY_MARK first on given pattern before MARK
I think, either we can
# Introduce an error number for that?
# Treat first MODIFY_MARK as MARK
Just to understand, in this absence of this new action, an application needs
to destroy the given pattern with associated existing MARK action and
add the same pattern with updated value as MARK action? Right?
> On Jun 6, 2019, at 3:35 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
>> Sent: Tuesday, June 4, 2019 3:03 AM
>> To: shahafs@mellanox.com; thomas@monjalon.net; ferruh.yigit@intel.com;
>> arybchenko@solarflare.com; adrien.mazarguil@6wind.com;
>> olivier.matz@6wind.com
>> Cc: dev@dpdk.org
>> Subject: [dpdk-dev] [RFC 2/3] ethdev: add flow modify mark action
>>
>> Mark ID can be modified when supporting multiple tables. Partial bit
>> alteration is supported to preserve some bit-fields set by previous match.
>>
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> doc/guides/prog_guide/rte_flow.rst | 21 +++++++++++++++++++++
>> lib/librte_ethdev/rte_flow.h | 24 ++++++++++++++++++++++++
>> 2 files changed, 45 insertions(+)
>>
>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>> b/doc/guides/prog_guide/rte_flow.rst
>> index 016cd90e52..2907edfff4 100644
>> --- a/doc/guides/prog_guide/rte_flow.rst
>> +++ b/doc/guides/prog_guide/rte_flow.rst
>> @@ -1463,6 +1463,27 @@ depends on the underlying implementation. It is
>> returned in the
>> | ``id`` | integer value to return with packets |
>> +--------+--------------------------------------+
>>
>> +Action: ``MODIFY_MARK``
>> +^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +Alter partial bits of mark ID set by ``MARK`` action.
>> +
>> +``mask`` indicates which bits are modified. For bits which have never
>> +been set by ``MARK`` or ``MODIFY_MARK``, unpredictable value will be
>> +seen depending on driver implementation.
>> +
>> +.. _table_rte_flow_action_modify_mark:
>> +
>> +.. table:: MODIFY_MARK
>> +
>> + +----------+--------------------------------------+
>> + | Field | Value |
>> + +==========+======================================+
>> + | ``id`` | integer value to return with packets |
>> + +----------+--------------------------------------+
>> + | ``mask`` | bit-mask applies to "id" |
>> + +----------+--------------------------------------+
>> +
>> Action: ``FLAG``
>> ^^^^^^^^^^^^^^^^
>>
>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h index
>> cda8628183..d811f8a06e 100644
>> --- a/lib/librte_ethdev/rte_flow.h
>> +++ b/lib/librte_ethdev/rte_flow.h
>> @@ -1316,6 +1316,13 @@ enum rte_flow_action_type {
>> */
>> RTE_FLOW_ACTION_TYPE_MARK,
>>
>> + /**
>> + * Alter partial bits of mark ID set by
>> RTE_FLOW_ACTION_TYPE_MARK.
>> + *
>> + * See struct rte_flow_action_modify_mark.
>> + */
>> + RTE_FLOW_ACTION_TYPE_MODIFY_MARK,
>> +
>
> I think, we need to define the case where application calls MODIFY_MARK first on given pattern before MARK
Good input.
> I think, either we can
> # Introduce an error number for that?
Practically, it would be impossible to keep track of MARK action to check if it is set or not prior to MODIFY_MARK.
When creating flows with multiple tables, we can't say a flow having MODIFY_MARK action will have prior MARK action or not.
> # Treat first MODIFY_MARK as MARK
So, I took similar approach.
In the documentation above, unset bits would have arbitrary value depending on driver/device implementation.
User can't assume mark ID is initially zeroed but rather need to check it with vendors.
> Just to understand, in this absence of this new action, an application needs
> to destroy the given pattern with associated existing MARK action and
> add the same pattern with updated value as MARK action? Right?
Application would have to override it by second MARK action.
But it has to be validated by user anyway to check if device allows override.
Thanks,
Yongseok
@@ -1463,6 +1463,27 @@ depends on the underlying implementation. It is returned in the
| ``id`` | integer value to return with packets |
+--------+--------------------------------------+
+Action: ``MODIFY_MARK``
+^^^^^^^^^^^^^^^^^^^^^^^
+
+Alter partial bits of mark ID set by ``MARK`` action.
+
+``mask`` indicates which bits are modified. For bits which have never been set
+by ``MARK`` or ``MODIFY_MARK``, unpredictable value will be seen depending on
+driver implementation.
+
+.. _table_rte_flow_action_modify_mark:
+
+.. table:: MODIFY_MARK
+
+ +----------+--------------------------------------+
+ | Field | Value |
+ +==========+======================================+
+ | ``id`` | integer value to return with packets |
+ +----------+--------------------------------------+
+ | ``mask`` | bit-mask applies to "id" |
+ +----------+--------------------------------------+
+
Action: ``FLAG``
^^^^^^^^^^^^^^^^
@@ -1316,6 +1316,13 @@ enum rte_flow_action_type {
*/
RTE_FLOW_ACTION_TYPE_MARK,
+ /**
+ * Alter partial bits of mark ID set by RTE_FLOW_ACTION_TYPE_MARK.
+ *
+ * See struct rte_flow_action_modify_mark.
+ */
+ RTE_FLOW_ACTION_TYPE_MODIFY_MARK,
+
/**
* Flags packets. Similar to MARK without a specific value; only
* sets the PKT_RX_FDIR mbuf flag.
@@ -1681,6 +1688,23 @@ struct rte_flow_action_mark {
uint32_t id; /**< Integer value to return with packets. */
};
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_MODIFY_MARK
+ *
+ * Alter partial bits of mark ID set by RTE_FLOW_ACTION_TYPE_MARK.
+ *
+ * Provided mask indicates which bits are modified. For bits which have never
+ * been set by mark action or modify_mark action, unpredictable value will be
+ * seen depending on driver implementation.
+ */
+struct rte_flow_action_modify_mark {
+ uint32_t id; /**< Integer value to return with packets. */
+ uint32_t mask; /**< Mask of bits to modify. */
+};
+
/**
* @warning
* @b EXPERIMENTAL: this structure may change without prior notice