Re: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an offload

[Ori Kam <orika@mellanox.com>]

Hi Pavan,

Sorry for jumping in late.

I don't understand why we need this feature. If the user didn't set any flow with MARK
then the user doesn't need to check it.

Also it breaks compatibility. 

If my understanding is correct the MARK field is going to be moved to dynamic field, and this 
will be way to control the use of MARK.

Other option is inside the PMD if one flow uses mark only than copy the mark to mbuf.

Please also not that we have deprecation notice on the FDIR.

Thanks,
Ori



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of pbhagavatula@marvell.com
> Sent: Friday, October 25, 2019 6:22 PM
> To: ferruh.yigit@intel.com; arybchenko@solarflare.com; jerinj@marvell.com;
> John McNamara <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an
> offload
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be used to
> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi` and
> `rte_mbuf::ol_flags` when flow actions `RTE_FLOW_ACTION_MARK` and
> `RTE_FLOW_ACTION_FLAG` are enabled.
> 
> PMDs notify the validity of `rte_mbuf::hash:fdir::hi` to the applcation
> by enabling `PKT_RX_FDIR_ID` flag in `rte_mbuf::ol_flags`.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  doc/guides/nics/features.rst           | 12 ++++++++++++
>  doc/guides/rel_notes/release_19_11.rst |  9 +++++++++
>  lib/librte_ethdev/rte_ethdev.c         |  1 +
>  lib/librte_ethdev/rte_ethdev.h         |  1 +
>  lib/librte_ethdev/rte_flow.h           |  6 ++++--
>  5 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index d96696801..0d8d08c18 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -588,6 +588,18 @@ Supports packet type parsing and returns a list of
> supported types.
>  * **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``.
> 
> 
> +.. _nic_features_flow_flag_mark:
> +
> +Flow flag/mark update
> +---------------------
> +
> +Supports flow action type update to ``mbuf.ol_flags`` and ``mbuf.hash.fdir.hi``.
> +
> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
> ``offloads:DEV_RX_OFFLOAD_FLOW_MARK``.
> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
> ``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
> +  ``mbuf.hash.fdir.hi``
> +
> +
>  .. _nic_features_timesync:
> 
>  Timesync
> diff --git a/doc/guides/rel_notes/release_19_11.rst
> b/doc/guides/rel_notes/release_19_11.rst
> index faf69b0d9..8593fea0e 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -185,6 +185,15 @@ New Features
>    * Added a console command to testpmd app, ``show port (port_id) ptypes``
> which
>      gives ability to print port supported ptypes in different protocol layers.
> 
> +* **Added Rx offload flag to enable or disable flow action type update**
> +
> +  *  Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be
> used to
> +     enable/disable PMDs write to `rte_mbuf::hash::fdir::hi` and
> +     `rte_mbuf::ol_flags` when flow actions `RTE_FLOW_ACTION_MARK` and
> +     `RTE_FLOW_ACTION_FLAG` are enabled.
> +  *  PMDs notify the validity of `rte_mbuf::hash:fdir::hi` to the application
> +     by enabling `PKT_RX_FDIR_ID` flag in `rte_mbuf::ol_flags`.
> +
> 
>  Removed Items
>  -------------
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 7743205d3..f70065043 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -129,6 +129,7 @@ static const struct {
>  	RTE_RX_OFFLOAD_BIT2STR(KEEP_CRC),
>  	RTE_RX_OFFLOAD_BIT2STR(SCTP_CKSUM),
>  	RTE_RX_OFFLOAD_BIT2STR(OUTER_UDP_CKSUM),
> +	RTE_RX_OFFLOAD_BIT2STR(FLOW_MARK),
>  };
> 
>  #undef RTE_RX_OFFLOAD_BIT2STR
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index c36c1b631..c4e626da7 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1048,6 +1048,7 @@ struct rte_eth_conf {
>  #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
>  #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>  #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
> 
>  #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>  				 DEV_RX_OFFLOAD_UDP_CKSUM | \
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 4fee10559..6698640fb 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -1538,7 +1538,8 @@ enum rte_flow_action_type {
> 
>  	/**
>  	 * Attaches an integer value to packets and sets PKT_RX_FDIR and
> -	 * PKT_RX_FDIR_ID mbuf flags.
> +	 * PKT_RX_FDIR_ID mbuf flags when
> +	 * `rx_mode:offloads:DEV_RX_OFFLOAD_FLOW_MARK` is enabled.
>  	 *
>  	 * See struct rte_flow_action_mark.
>  	 */
> @@ -1546,7 +1547,8 @@ enum rte_flow_action_type {
> 
>  	/**
>  	 * Flags packets. Similar to MARK without a specific value; only
> -	 * sets the PKT_RX_FDIR mbuf flag.
> +	 * sets the PKT_RX_FDIR mbuf flag when
> +	 * `rx_mode:offloads:DEV_RX_OFFLOAD_FLOW_MARK` is enabled.
>  	 *
>  	 * No associated configuration structure.
>  	 */
> --
> 2.17.1