On 9/30/2021 3:38 PM, Ori Kam wrote:

Hi Jie,

You are missing updating the rte_flow.rst and release notes:

Please see more comments below.

Thanks,
Ori

quoted

-----Original Message-----
From: dev <redacted> On Behalf Of Jie Wang
Sent: Friday, September 24, 2021 6:17 PM
Subject: [dpdk-dev] [PATCH 3/4] ethdev: support PPPoL2TPv2oUDP RSS
Hash

Add flow pattern items, RSS offload types and header formats of L2TPv2 and
PPP.

Signed-off-by: Wenjun Wu <redacted>
Signed-off-by: Jie Wang <redacted>
---
  lib/ethdev/rte_flow.c |  2 +
  lib/ethdev/rte_flow.h | 99
+++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 101 insertions(+)

diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index

8cb7a069c8..307fbc3abe 100644

--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c

@@ -98,6 +98,8 @@ static const struct rte_flow_desc_data

rte_flow_desc_item[] = {
  	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
  	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
  	MK_FLOW_ITEM(GENEVE_OPT, sizeof(struct
rte_flow_item_geneve_opt)),
+	MK_FLOW_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
+	MK_FLOW_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
  	MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
  	MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)),  }; diff --git
a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
70f455d47d..93205b7d1e 100644

--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h

@@ -554,6 +554,20 @@ enum rte_flow_item_type {
  	 */
  	RTE_FLOW_ITEM_TYPE_GENEVE_OPT,

+	/**
+	 * Matches L2TPV2 Header.
+	 *
+	 * See struct rte_flow_item_l2tpv2.
+	 */
+	RTE_FLOW_ITEM_TYPE_L2TPV2,
+
+	/**
+	 * Matches PPP Header.
+	 *
+	 * See struct rte_flow_item_ppp.
+	 */
+	RTE_FLOW_ITEM_TYPE_PPP,
+

Why did you push the new items here and not in the end?

quoted

  	/**
  	 * [META]
  	 *

@@ -1799,6 +1813,91 @@ static const struct rte_flow_item_conntrack

rte_flow_item_conntrack_mask = {  };  #endif

+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ * RTE_FLOW_ITEM_TYPE_L2TPV2
+ *
+ * Matches L2TPv2 Header
+ */
+RTE_STD_C11
+struct rte_flow_item_l2tpv2 {
+	rte_be16_t flags_version;   /**< flag(12)  version(2). version must be
2 */
+	union{
+		struct{
+			rte_be16_t length;  /**< length(16) */
+			rte_be16_t tunnel_id;   /**< tunnel id(16) */
+			rte_be16_t session_id;  /**< session id(16) */
+			rte_be32_t ns_nr;   /**< Ns(16) + Nr(16) */

Why not split those fields?

Hi Ori,

As far as I remember the decision was to define protocol headers separately
and use them as first element in the flow_item struct, this enables casting
the flow_item to protocol. And Andrew did some cleanups in the past related
to this.
Can't we use the same logic for this case?