Re: [dpdk-dev] [PATCH v3 1/3] ethdev: support metadata as flow rule criteria

[Dekel Peled <dekelp@mellanox.com>]

Thanks, PSB

From: Andrew Rybchenko <arybchenko@solarflare.com> 
Sent: Friday, October 5, 2018 4:40 PM
To: Ferruh Yigit <ferruh.yigit@intel.com>; Dekel Peled <dekelp@mellanox.com>; wenzhuo.lu@intel.com; jingjing.wu@intel.com; bernard.iremonger@intel.com; dev@dpdk.org; olivier.matz@6wind.com; Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon <thomas@monjalon.net>
Cc: Shahaf Shuler <shahafs@mellanox.com>; Ori Kam <orika@mellanox.com>
Subject: Re: [PATCH v3 1/3] ethdev: support metadata as flow rule criteria

On 10/5/18 4:31 PM, Ferruh Yigit wrote:
On 9/27/2018 2:57 PM, Dekel Peled wrote:
As described in [1], a new rte_flow item is added to support metadata
to use as flow rule match pattern.
The metadata is an opaque item, fully controlled by the application.

The use of metadata is relevant for egress rules only.
It can be set in the flow rule using the RTE_FLOW_ITEM_META.

In order to avoid change in mbuf API, exisitng field buf.hash.fdir.hi
is used to carry the metadata item. This field is used only in
ingress packets, so using it for egress metadata will not cause
conflicts.

Application should set the packet metadata in the mbuf dedicated field,
and set the PKT_TX_METADATA flag in the mbuf->ol_flags.
The NIC will use the packet metadata as match criteria for relevant
flow rules.

This patch introduces metadata item type for rte_flow RTE_FLOW_ITEM_META,
along with corresponding struct rte_flow_item_meta and ol_flag
PKT_TX_METADATA.

[1] "[RFC,v2] ethdev: support metadata as flow rule criteria"
    https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2018-August%2F110194.html&data=02%7C01%7Cdekelp%40mellanox.com%7Ccc5ab6f21cd847d4b45008d62ac82949%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636743436510709804&sdata=AtPp27zdfM8G%2BbvgiLW47qSWDJElDrP2l8%2FHpNP04G0%3D&reserved=0

Signed-off-by: Dekel Peled mailto:dekelp@mellanox.com

<...>

@@ -526,6 +532,12 @@ struct rte_mbuf {
 			uint32_t hi;
 			/**< First 4 flexible bytes or FD ID, dependent on
 			     PKT_RX_FDIR_* flag in ol_flags. */
+			/**
+			 * Above member has optional use on egress:
+			 * Application specific metadata value
+			 * for flow rule match.
+			 * Valid if PKT_TX_METADATA is set.
+			 */
 		} fdir;           /**< Filter identifier if FDIR enabled */

Any objection/comment to use hash.fdir.hi for this new "metadata" meaning? Olivier?

As for me, I'd prefer to see dedicated union member something like
it was suggested in [1].

Andrew.

[1] https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2018-September%2F111954.html&data=02%7C01%7Cdekelp%40mellanox.com%7Ccc5ab6f21cd847d4b45008d62ac82949%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636743436510709804&sdata=S1iSzBG4NFNSwF%2B12Vu1b2VfXuwcYLha2Vxlybu8tXQ%3D&reserved=0


I will change it as suggested.
Dekel