DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH] ethdev: add more protocol support in flow API
@ 2019-08-14  3:00 Wang Ying A
  2019-08-14  3:24 ` [dpdk-dev] [PATCH v2] " Wang Ying A
  0 siblings, 1 reply; 4+ messages in thread
From: Wang Ying A @ 2019-08-14  3:00 UTC (permalink / raw)
  To: qi.z.zhang; +Cc: xiaolong.ye, qiming.yang, dev, ying.a.wang

Add new protocol header match support as below

RTE_FLOW_ITEM_TYPE_GTP_PSC
	- matches a GTP PDU extension header (type is 0x85:
	PDU Session Container)
RTE_FLOW_ITEM_TYPE_PPPOES
	- matches a PPPoE Session header.
RTE_FLOW_ITEM_TYPE_PPPOED
	- matches a PPPoE Discovery stage header.

Change-Id: I9da6f2c32aca5611ab5a7bb2699f2ad6d3071c59
Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
---
 app/test-pmd/cmdline_flow.c                 | 80 +++++++++++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst          | 25 +++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
 lib/librte_ethdev/rte_flow.c                |  3 ++
 lib/librte_ethdev/rte_flow.h                | 71 +++++++++++++++++++++++++
 5 files changed, 189 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 495871394..79bce43e7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -167,6 +167,13 @@ enum index {
 	ITEM_GTP_TEID,
 	ITEM_GTPC,
 	ITEM_GTPU,
+	ITEM_GTP_PSC,
+	ITEM_GTP_PSC_QFI,
+	ITEM_GTP_PSC_PDU_T,
+	ITEM_PPPOES,
+	ITEM_PPPOED,
+	ITEM_PPPOE_SEID,
+	ITEM_PPPOE_PROTO,
 	ITEM_GENEVE,
 	ITEM_GENEVE_VNI,
 	ITEM_GENEVE_PROTO,
@@ -651,6 +658,9 @@ static const enum index next_item[] = {
 	ITEM_GTP,
 	ITEM_GTPC,
 	ITEM_GTPU,
+	ITEM_GTP_PSC,
+	ITEM_PPPOES,
+	ITEM_PPPOED,
 	ITEM_GENEVE,
 	ITEM_VXLAN_GPE,
 	ITEM_ARP_ETH_IPV4,
@@ -831,6 +841,20 @@ static const enum index item_gtp[] = {
 	ZERO,
 };
 
+static const enum index item_gtp_psc[] = {
+	ITEM_GTP_PSC_QFI,
+	ITEM_GTP_PSC_PDU_T,
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_pppoe[] = {
+	ITEM_PPPOE_SEID,
+	ITEM_PPPOE_PROTO,
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index item_geneve[] = {
 	ITEM_GENEVE_VNI,
 	ITEM_GENEVE_PROTO,
@@ -2108,6 +2132,56 @@ static const struct token token_list[] = {
 		.next = NEXT(item_gtp),
 		.call = parse_vc,
 	},
+	[ITEM_GTP_PSC] = {
+		.name = "gtp_psc",
+		.help = "match GTP extension header (type is 0x85)",
+		.priv = PRIV_ITEM(GTP_PSC,
+				sizeof(struct rte_flow_item_gtp_psc)),
+		.next = NEXT(item_gtp_psc),
+		.call = parse_vc,
+	},
+	[ITEM_GTP_PSC_QFI] = {
+		.name = "qfi",
+		.help = "QoS flow identifier",
+		.next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gtp_psc,
+					qfi)),
+	},
+	[ITEM_GTP_PSC_PDU_T] = {
+		.name = "pdu_t",
+		.help = "PDU type",
+		.next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gtp_psc,
+					pdu_type)),
+	},
+	[ITEM_PPPOES] = {
+		.name = "pppoes",
+		.help = "match PPPoE Session header",
+		.priv = PRIV_ITEM(PPPOES, sizeof(struct rte_flow_item_pppoe)),
+		.next = NEXT(item_pppoe),
+		.call = parse_vc,
+	},
+	[ITEM_PPPOED] = {
+		.name = "pppoed",
+		.help = "match PPPoE Discovery stage header",
+		.priv = PRIV_ITEM(PPPOED, sizeof(struct rte_flow_item_pppoe)),
+		.next = NEXT(item_pppoe),
+		.call = parse_vc,
+	},
+	[ITEM_PPPOE_SEID] = {
+		.name = "seid",
+		.help = "Session identifier",
+		.next = NEXT(item_pppoe, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pppoe,
+					session_id)),
+	},
+	[ITEM_PPPOE_PROTO] = {
+		.name = "proto_id",
+		.help = "PPPOE protocol identifier",
+		.next = NEXT(item_pppoe, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pppoe,
+					proto_id)),
+	},
 	[ITEM_GENEVE] = {
 		.name = "geneve",
 		.help = "match GENEVE header",
@@ -5753,6 +5827,12 @@ flow_item_default_mask(const struct rte_flow_item *item)
 	case RTE_FLOW_ITEM_TYPE_GTP:
 		mask = &rte_flow_item_gtp_mask;
 		break;
+	case RTE_FLOW_ITEM_TYPE_GTP_PSC:
+		mask = &rte_flow_item_gtp_psc_mask;
+		break;
+	case RTE_FLOW_ITEM_TYPE_PPPOES:
+		mask = &rte_flow_item_pppoe_mask;
+		break;
 	case RTE_FLOW_ITEM_TYPE_ESP:
 		mask = &rte_flow_item_esp_mask;
 		break;
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 821b524b3..d09c42071 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1055,6 +1055,31 @@ flow rules.
 - ``teid``: tunnel endpoint identifier.
 - Default ``mask`` matches teid only.
 
+Item: ``GTP_PSC``
+^^^^^^^^^^^^^^^^^^^^^^^
+
+Matches a GTP Extension header (type is 0x85).
+
+- ``pdu_type``: PDU type.
+- ``qfi``: QoS flow identifier.
+- Default ``mask`` matches QFI only.
+
+Item: ``PPPOES``, ``PPPOED``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Matches a PPPOE header.
+
+Note: PPPOES and PPPOED use the same structure. PPPOES and PPPOED item
+are defined for a user-friendly API when creating PPPOE-Session and
+PPPOE-Discovery flow rules.
+
+- ``v_t_flags``: version (4b), type (4b).
+- ``code``: Message type.
+- ``session_id``: Session identifier.
+- ``length``: Payload length.
+- ``proto_id``: PPP Protocol identifier.
+- Default ``mask`` matches session_id,proto_id.
+
 Item: ``ESP``
 ^^^^^^^^^^^^^
 
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 313e0707e..0da36d5f1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3904,6 +3904,16 @@ This section lists supported pattern items and their attributes, if any.
 
   - ``teid {unsigned}``: tunnel endpoint identifier.
 
+- ``gtp_psc``: match GTPv1 entension header (type is 0x85).
+
+  - ``pdu_type {unsigned}``: PDU type (0 or 1).
+  - ``qfi {unsigned}``: QoS flow identifier.
+
+- ``pppoes``, ``pppoed``: match PPPOE header.
+
+  - ``session_id {unsigned}``: Session identifier.
+  - ``proto_id {unsigned}``: PPP Protocol identifier.
+
 - ``geneve``: match GENEVE header.
 
   - ``vni {unsigned}``: virtual network identifier.
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 18fcb018e..246390da3 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -59,6 +59,9 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 	MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
 	MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
 	MK_FLOW_ITEM(GTPU, sizeof(struct rte_flow_item_gtp)),
+	MK_FLOW_ITEM(GTP_PSC, sizeof(struct rte_flow_item_gtp_psc)),
+	MK_FLOW_ITEM(PPPOES, sizeof(struct rte_flow_item_pppoe)),
+	MK_FLOW_ITEM(PPPOED, sizeof(struct rte_flow_item_pppoe)),
 	MK_FLOW_ITEM(ESP, sizeof(struct rte_flow_item_esp)),
 	MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)),
 	MK_FLOW_ITEM(VXLAN_GPE, sizeof(struct rte_flow_item_vxlan_gpe)),
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b66bf1495..ad5e46190 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -328,6 +328,34 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_GTPU,
 
+	/**
+	 * Matches a GTP PDU extension header (type is 0x85:
+	 * PDU Session Container).
+	 *
+	 * Configure flow for GTP packets with extension header type 0x85.
+	 *
+	 * See struct rte_flow_item_gtp_psc.
+	 */
+	RTE_FLOW_ITEM_TYPE_GTP_PSC,
+
+	/**
+	 * Matches a PPPOE header.
+	 *
+	 * Configure flow for PPPoE Session packets.
+	 *
+	 * See struct rte_flow_item_pppoe.
+	 */
+	RTE_FLOW_ITEM_TYPE_PPPOES,
+
+	/**
+	 * Matches a PPPOE header.
+	 *
+	 * Configure flow for PPPoE Discovery stage packets.
+	 *
+	 * See struct rte_flow_item_pppoe.
+	 */
+	RTE_FLOW_ITEM_TYPE_PPPOED,
+
 	/**
 	 * Matches a ESP header.
 	 *
@@ -922,6 +950,49 @@ static const struct rte_flow_item_gtp rte_flow_item_gtp_mask = {
 };
 #endif
 
+/**
+ * RTE_FLOW_ITEM_TYPE_GTP_PSC.
+ *
+ * Matches a GTP-extension header
+ * (type is 0x85: PDU Session Container).
+ */
+struct rte_flow_item_gtp_psc {
+	uint8_t pdu_type; /**< PDU type (0 or 1). */
+	uint8_t qfi; /**< QoS flow identifier. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
+#ifndef __cplusplus
+static const struct rte_flow_item_gtp_psc
+rte_flow_item_gtp_psc_mask = {
+	.qfi = 0x3f,
+};
+#endif
+
+/**
+ * RTE_FLOW_ITEM_TYPE_PPPOE.
+ *
+ * Matches a PPPOE header.
+ */
+struct rte_flow_item_pppoe {
+	/**
+	 * Version (4b), type (4b).
+	 */
+	uint8_t v_t_flags;
+	uint8_t code; /**< Message type. */
+	rte_be16_t session_id; /**< Session identifier. */
+	rte_be16_t length; /**< Payload length. */
+	rte_be16_t proto_id; /**< PPP Protocol identifier. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_PPPOE. */
+#ifndef __cplusplus
+static const struct rte_flow_item_pppoe rte_flow_item_pppoe_mask = {
+	.session_id = RTE_BE16(0xffff),
+	.proto_id = RTE_BE16(0xffff),
+};
+#endif
+
 /**
  * RTE_FLOW_ITEM_TYPE_ESP
  *
-- 
2.15.1


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

* [dpdk-dev] [PATCH v2] ethdev: add more protocol support in flow API
  2019-08-14  3:00 [dpdk-dev] [PATCH] ethdev: add more protocol support in flow API Wang Ying A
@ 2019-08-14  3:24 ` " Wang Ying A
  2019-08-14  9:08   ` Adrien Mazarguil
  0 siblings, 1 reply; 4+ messages in thread
From: Wang Ying A @ 2019-08-14  3:24 UTC (permalink / raw)
  To: qi.z.zhang; +Cc: xiaolong.ye, qiming.yang, dev, ying.a.wang

Add new protocol header match support as below

RTE_FLOW_ITEM_TYPE_GTP_PSC
	- matches a GTP PDU extension header (type is 0x85:
	PDU Session Container)
RTE_FLOW_ITEM_TYPE_PPPOES
	- matches a PPPoE Session header.
RTE_FLOW_ITEM_TYPE_PPPOED
	- matches a PPPoE Discovery stage header.

Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
---
---
v2: Remove Gerrit Change-Id's.
---
 app/test-pmd/cmdline_flow.c                 | 80 +++++++++++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst          | 25 +++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
 lib/librte_ethdev/rte_flow.c                |  3 ++
 lib/librte_ethdev/rte_flow.h                | 71 +++++++++++++++++++++++++
 5 files changed, 189 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 495871394..79bce43e7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -167,6 +167,13 @@ enum index {
 	ITEM_GTP_TEID,
 	ITEM_GTPC,
 	ITEM_GTPU,
+	ITEM_GTP_PSC,
+	ITEM_GTP_PSC_QFI,
+	ITEM_GTP_PSC_PDU_T,
+	ITEM_PPPOES,
+	ITEM_PPPOED,
+	ITEM_PPPOE_SEID,
+	ITEM_PPPOE_PROTO,
 	ITEM_GENEVE,
 	ITEM_GENEVE_VNI,
 	ITEM_GENEVE_PROTO,
@@ -651,6 +658,9 @@ static const enum index next_item[] = {
 	ITEM_GTP,
 	ITEM_GTPC,
 	ITEM_GTPU,
+	ITEM_GTP_PSC,
+	ITEM_PPPOES,
+	ITEM_PPPOED,
 	ITEM_GENEVE,
 	ITEM_VXLAN_GPE,
 	ITEM_ARP_ETH_IPV4,
@@ -831,6 +841,20 @@ static const enum index item_gtp[] = {
 	ZERO,
 };
 
+static const enum index item_gtp_psc[] = {
+	ITEM_GTP_PSC_QFI,
+	ITEM_GTP_PSC_PDU_T,
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_pppoe[] = {
+	ITEM_PPPOE_SEID,
+	ITEM_PPPOE_PROTO,
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index item_geneve[] = {
 	ITEM_GENEVE_VNI,
 	ITEM_GENEVE_PROTO,
@@ -2108,6 +2132,56 @@ static const struct token token_list[] = {
 		.next = NEXT(item_gtp),
 		.call = parse_vc,
 	},
+	[ITEM_GTP_PSC] = {
+		.name = "gtp_psc",
+		.help = "match GTP extension header (type is 0x85)",
+		.priv = PRIV_ITEM(GTP_PSC,
+				sizeof(struct rte_flow_item_gtp_psc)),
+		.next = NEXT(item_gtp_psc),
+		.call = parse_vc,
+	},
+	[ITEM_GTP_PSC_QFI] = {
+		.name = "qfi",
+		.help = "QoS flow identifier",
+		.next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gtp_psc,
+					qfi)),
+	},
+	[ITEM_GTP_PSC_PDU_T] = {
+		.name = "pdu_t",
+		.help = "PDU type",
+		.next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gtp_psc,
+					pdu_type)),
+	},
+	[ITEM_PPPOES] = {
+		.name = "pppoes",
+		.help = "match PPPoE Session header",
+		.priv = PRIV_ITEM(PPPOES, sizeof(struct rte_flow_item_pppoe)),
+		.next = NEXT(item_pppoe),
+		.call = parse_vc,
+	},
+	[ITEM_PPPOED] = {
+		.name = "pppoed",
+		.help = "match PPPoE Discovery stage header",
+		.priv = PRIV_ITEM(PPPOED, sizeof(struct rte_flow_item_pppoe)),
+		.next = NEXT(item_pppoe),
+		.call = parse_vc,
+	},
+	[ITEM_PPPOE_SEID] = {
+		.name = "seid",
+		.help = "Session identifier",
+		.next = NEXT(item_pppoe, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pppoe,
+					session_id)),
+	},
+	[ITEM_PPPOE_PROTO] = {
+		.name = "proto_id",
+		.help = "PPPOE protocol identifier",
+		.next = NEXT(item_pppoe, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pppoe,
+					proto_id)),
+	},
 	[ITEM_GENEVE] = {
 		.name = "geneve",
 		.help = "match GENEVE header",
@@ -5753,6 +5827,12 @@ flow_item_default_mask(const struct rte_flow_item *item)
 	case RTE_FLOW_ITEM_TYPE_GTP:
 		mask = &rte_flow_item_gtp_mask;
 		break;
+	case RTE_FLOW_ITEM_TYPE_GTP_PSC:
+		mask = &rte_flow_item_gtp_psc_mask;
+		break;
+	case RTE_FLOW_ITEM_TYPE_PPPOES:
+		mask = &rte_flow_item_pppoe_mask;
+		break;
 	case RTE_FLOW_ITEM_TYPE_ESP:
 		mask = &rte_flow_item_esp_mask;
 		break;
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 821b524b3..d09c42071 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1055,6 +1055,31 @@ flow rules.
 - ``teid``: tunnel endpoint identifier.
 - Default ``mask`` matches teid only.
 
+Item: ``GTP_PSC``
+^^^^^^^^^^^^^^^^^^^^^^^
+
+Matches a GTP Extension header (type is 0x85).
+
+- ``pdu_type``: PDU type.
+- ``qfi``: QoS flow identifier.
+- Default ``mask`` matches QFI only.
+
+Item: ``PPPOES``, ``PPPOED``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Matches a PPPOE header.
+
+Note: PPPOES and PPPOED use the same structure. PPPOES and PPPOED item
+are defined for a user-friendly API when creating PPPOE-Session and
+PPPOE-Discovery flow rules.
+
+- ``v_t_flags``: version (4b), type (4b).
+- ``code``: Message type.
+- ``session_id``: Session identifier.
+- ``length``: Payload length.
+- ``proto_id``: PPP Protocol identifier.
+- Default ``mask`` matches session_id,proto_id.
+
 Item: ``ESP``
 ^^^^^^^^^^^^^
 
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 313e0707e..0da36d5f1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3904,6 +3904,16 @@ This section lists supported pattern items and their attributes, if any.
 
   - ``teid {unsigned}``: tunnel endpoint identifier.
 
+- ``gtp_psc``: match GTPv1 entension header (type is 0x85).
+
+  - ``pdu_type {unsigned}``: PDU type (0 or 1).
+  - ``qfi {unsigned}``: QoS flow identifier.
+
+- ``pppoes``, ``pppoed``: match PPPOE header.
+
+  - ``session_id {unsigned}``: Session identifier.
+  - ``proto_id {unsigned}``: PPP Protocol identifier.
+
 - ``geneve``: match GENEVE header.
 
   - ``vni {unsigned}``: virtual network identifier.
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 18fcb018e..246390da3 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -59,6 +59,9 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 	MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
 	MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
 	MK_FLOW_ITEM(GTPU, sizeof(struct rte_flow_item_gtp)),
+	MK_FLOW_ITEM(GTP_PSC, sizeof(struct rte_flow_item_gtp_psc)),
+	MK_FLOW_ITEM(PPPOES, sizeof(struct rte_flow_item_pppoe)),
+	MK_FLOW_ITEM(PPPOED, sizeof(struct rte_flow_item_pppoe)),
 	MK_FLOW_ITEM(ESP, sizeof(struct rte_flow_item_esp)),
 	MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)),
 	MK_FLOW_ITEM(VXLAN_GPE, sizeof(struct rte_flow_item_vxlan_gpe)),
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b66bf1495..ad5e46190 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -328,6 +328,34 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_GTPU,
 
+	/**
+	 * Matches a GTP PDU extension header (type is 0x85:
+	 * PDU Session Container).
+	 *
+	 * Configure flow for GTP packets with extension header type 0x85.
+	 *
+	 * See struct rte_flow_item_gtp_psc.
+	 */
+	RTE_FLOW_ITEM_TYPE_GTP_PSC,
+
+	/**
+	 * Matches a PPPOE header.
+	 *
+	 * Configure flow for PPPoE Session packets.
+	 *
+	 * See struct rte_flow_item_pppoe.
+	 */
+	RTE_FLOW_ITEM_TYPE_PPPOES,
+
+	/**
+	 * Matches a PPPOE header.
+	 *
+	 * Configure flow for PPPoE Discovery stage packets.
+	 *
+	 * See struct rte_flow_item_pppoe.
+	 */
+	RTE_FLOW_ITEM_TYPE_PPPOED,
+
 	/**
 	 * Matches a ESP header.
 	 *
@@ -922,6 +950,49 @@ static const struct rte_flow_item_gtp rte_flow_item_gtp_mask = {
 };
 #endif
 
+/**
+ * RTE_FLOW_ITEM_TYPE_GTP_PSC.
+ *
+ * Matches a GTP-extension header
+ * (type is 0x85: PDU Session Container).
+ */
+struct rte_flow_item_gtp_psc {
+	uint8_t pdu_type; /**< PDU type (0 or 1). */
+	uint8_t qfi; /**< QoS flow identifier. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
+#ifndef __cplusplus
+static const struct rte_flow_item_gtp_psc
+rte_flow_item_gtp_psc_mask = {
+	.qfi = 0x3f,
+};
+#endif
+
+/**
+ * RTE_FLOW_ITEM_TYPE_PPPOE.
+ *
+ * Matches a PPPOE header.
+ */
+struct rte_flow_item_pppoe {
+	/**
+	 * Version (4b), type (4b).
+	 */
+	uint8_t v_t_flags;
+	uint8_t code; /**< Message type. */
+	rte_be16_t session_id; /**< Session identifier. */
+	rte_be16_t length; /**< Payload length. */
+	rte_be16_t proto_id; /**< PPP Protocol identifier. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_PPPOE. */
+#ifndef __cplusplus
+static const struct rte_flow_item_pppoe rte_flow_item_pppoe_mask = {
+	.session_id = RTE_BE16(0xffff),
+	.proto_id = RTE_BE16(0xffff),
+};
+#endif
+
 /**
  * RTE_FLOW_ITEM_TYPE_ESP
  *
-- 
2.15.1


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

* Re: [dpdk-dev] [PATCH v2] ethdev: add more protocol support in flow API
  2019-08-14  3:24 ` [dpdk-dev] [PATCH v2] " Wang Ying A
@ 2019-08-14  9:08   ` Adrien Mazarguil
  2019-08-19 11:53     ` Zhang, Qi Z
  0 siblings, 1 reply; 4+ messages in thread
From: Adrien Mazarguil @ 2019-08-14  9:08 UTC (permalink / raw)
  To: Wang Ying A; +Cc: qi.z.zhang, xiaolong.ye, qiming.yang, dev

Hi Wang Ying,

On Wed, Aug 14, 2019 at 11:24:30AM +0800, Wang Ying A wrote:
> Add new protocol header match support as below
> 
> RTE_FLOW_ITEM_TYPE_GTP_PSC
> 	- matches a GTP PDU extension header (type is 0x85:
> 	PDU Session Container)
> RTE_FLOW_ITEM_TYPE_PPPOES
> 	- matches a PPPoE Session header.
> RTE_FLOW_ITEM_TYPE_PPPOED
> 	- matches a PPPoE Discovery stage header.
> 
> Signed-off-by: Wang Ying A <ying.a.wang@intel.com>

OK, please split this patch, one for GTP and the other for PPPoE to make
title less vague than "add more protocol support".

For PPPoE, the distinction between session and discovery is not a bad idea
but since discovery packets typically have a different format (tags in place
of protocol), I'm not sure they should share a common structure.

How about a single "PPPOE" item without proto_id to cover both session and
discovery, then later add separate tag items on a needed basis for each
possible/optional tag (e.g. PPPOE_TAG_SERVICE_NAME)? Likewise proto_id would
be provided through a separate optional item if relevant (PPPOE_PROTO_ID).
Such an approach is already used for IPV6 and IPV6_EXT.

Another benefit is that applications could match PPPoE regardless of session
or discovery when they do not care, while PPPOED/PPPOES make that
distinction mandatory.

Also by inserting new entries in the middle of the pattern items list, this
patch breaks ABI. I think it's not on purpose, so please move them to the
end (not grouping them with existing GTP stuff is fine, ABI compat is more
important.) This must be reflected in rte_flow.h, rte_flow.c, testpmd and
documentation.

More nits below.

> ---
> ---
> v2: Remove Gerrit Change-Id's.
> ---
>  app/test-pmd/cmdline_flow.c                 | 80 +++++++++++++++++++++++++++++
>  doc/guides/prog_guide/rte_flow.rst          | 25 +++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
>  lib/librte_ethdev/rte_flow.c                |  3 ++
>  lib/librte_ethdev/rte_flow.h                | 71 +++++++++++++++++++++++++
>  5 files changed, 189 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
[...]
> +	[ITEM_PPPOES] = {
> +		.name = "pppoes",
> +		.help = "match PPPoE Session header",

Session => session

> +		.priv = PRIV_ITEM(PPPOES, sizeof(struct rte_flow_item_pppoe)),
> +		.next = NEXT(item_pppoe),
> +		.call = parse_vc,
> +	},
> +	[ITEM_PPPOED] = {
> +		.name = "pppoed",
> +		.help = "match PPPoE Discovery stage header",

Discovery => discovery

> +		.priv = PRIV_ITEM(PPPOED, sizeof(struct rte_flow_item_pppoe)),
> +		.next = NEXT(item_pppoe),
> +		.call = parse_vc,
> +	},
> +	[ITEM_PPPOE_SEID] = {
> +		.name = "seid",
> +		.help = "Session identifier",

Session => session

[...]
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 821b524b3..d09c42071 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1055,6 +1055,31 @@ flow rules.
>  - ``teid``: tunnel endpoint identifier.
>  - Default ``mask`` matches teid only.
>  
> +Item: ``GTP_PSC``
> +^^^^^^^^^^^^^^^^^^^^^^^

Too many "^^^"'s.

[...]
> +Item: ``PPPOES``, ``PPPOED``
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Matches a PPPOE header.

PPPOE => PPPoE

> +
> +Note: PPPOES and PPPOED use the same structure. PPPOES and PPPOED item

item => items (or better, "pattern items")

> +are defined for a user-friendly API when creating PPPOE-Session and
> +PPPOE-Discovery flow rules.

Super nit: use "PPPoE" when mentioning the protocol itself and "PPPOE" when
mentioning the pattern item.

> +
> +- ``v_t_flags``: version (4b), type (4b).

Why "flags"? I don't see any so you could name it "version_type" (same in
documentation).

> +- ``code``: Message type.

Message => message

> +- ``session_id``: Session identifier.

Session => session

> +- ``length``: Payload length.

Payload => payload

> +- ``proto_id``: PPP Protocol identifier.

Protocol => protocol

> +- Default ``mask`` matches session_id,proto_id.

Missing space between session_id and proto_id.

> +
>  Item: ``ESP``
>  ^^^^^^^^^^^^^
>  
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 313e0707e..0da36d5f1 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3904,6 +3904,16 @@ This section lists supported pattern items and their attributes, if any.
>  
>    - ``teid {unsigned}``: tunnel endpoint identifier.
>  
> +- ``gtp_psc``: match GTPv1 entension header (type is 0x85).
> +
> +  - ``pdu_type {unsigned}``: PDU type (0 or 1).
> +  - ``qfi {unsigned}``: QoS flow identifier.
> +
> +- ``pppoes``, ``pppoed``: match PPPOE header.

PPPOE => PPPoE

[...]
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b66bf1495..ad5e46190 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -328,6 +328,34 @@ enum rte_flow_item_type {
>  	 */
>  	RTE_FLOW_ITEM_TYPE_GTPU,
>  
> +	/**
> +	 * Matches a GTP PDU extension header (type is 0x85:
> +	 * PDU Session Container).

Session Container => session container

> +	 *
> +	 * Configure flow for GTP packets with extension header type 0x85.
> +	 *
> +	 * See struct rte_flow_item_gtp_psc.
> +	 */
> +	RTE_FLOW_ITEM_TYPE_GTP_PSC,
> +
> +	/**
> +	 * Matches a PPPOE header.
> +	 *
> +	 * Configure flow for PPPoE Session packets.

Session => session

> +	 *
> +	 * See struct rte_flow_item_pppoe.
> +	 */
> +	RTE_FLOW_ITEM_TYPE_PPPOES,
> +
> +	/**
> +	 * Matches a PPPOE header.
> +	 *
> +	 * Configure flow for PPPoE Discovery stage packets.

Discovery => discovery

> +	 *
> +	 * See struct rte_flow_item_pppoe.
> +	 */
> +	RTE_FLOW_ITEM_TYPE_PPPOED,
> +
>  	/**
>  	 * Matches a ESP header.
>  	 *
> @@ -922,6 +950,49 @@ static const struct rte_flow_item_gtp rte_flow_item_gtp_mask = {
>  };
>  #endif
>  
> +/**
> + * RTE_FLOW_ITEM_TYPE_GTP_PSC.
> + *
> + * Matches a GTP-extension header
> + * (type is 0x85: PDU Session Container).

Session Container => session container

(crusade against superfluous caps!)

[...]
> +/**
> + * RTE_FLOW_ITEM_TYPE_PPPOE.
> + *
> + * Matches a PPPOE header.
> + */
> +struct rte_flow_item_pppoe {
> +	/**
> +	 * Version (4b), type (4b).
> +	 */
> +	uint8_t v_t_flags;

v_t_flags => version_type

> +	uint8_t code; /**< Message type. */
> +	rte_be16_t session_id; /**< Session identifier. */
> +	rte_be16_t length; /**< Payload length. */
> +	rte_be16_t proto_id; /**< PPP Protocol identifier. */

As discussed, I suggest dropping proto_id to make this a generic match for
PPPoE.

> +};
> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_PPPOE. */
> +#ifndef __cplusplus
> +static const struct rte_flow_item_pppoe rte_flow_item_pppoe_mask = {
> +	.session_id = RTE_BE16(0xffff),
> +	.proto_id = RTE_BE16(0xffff),
> +};

I think the default for PPPoE should be an empty mask that simply says
"match PPPoE" since session_id only becomes known after negotiation and
proto_id shouldn't be part of this object.

-- 
Adrien Mazarguil
6WIND

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

* Re: [dpdk-dev] [PATCH v2] ethdev: add more protocol support in flow API
  2019-08-14  9:08   ` Adrien Mazarguil
@ 2019-08-19 11:53     ` Zhang, Qi Z
  0 siblings, 0 replies; 4+ messages in thread
From: Zhang, Qi Z @ 2019-08-19 11:53 UTC (permalink / raw)
  To: Adrien Mazarguil, Wang, Ying A; +Cc: Ye, Xiaolong, Yang, Qiming, dev



> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Wednesday, August 14, 2019 5:08 PM
> To: Wang, Ying A <ying.a.wang@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>;
> Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: add more protocol support in flow
> API
> 
> Hi Wang Ying,
> 
> On Wed, Aug 14, 2019 at 11:24:30AM +0800, Wang Ying A wrote:
> > Add new protocol header match support as below
> >
> > RTE_FLOW_ITEM_TYPE_GTP_PSC
> > 	- matches a GTP PDU extension header (type is 0x85:
> > 	PDU Session Container)
> > RTE_FLOW_ITEM_TYPE_PPPOES
> > 	- matches a PPPoE Session header.
> > RTE_FLOW_ITEM_TYPE_PPPOED
> > 	- matches a PPPoE Discovery stage header.
> >
> > Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
> 
> OK, please split this patch, one for GTP and the other for PPPoE to make title
> less vague than "add more protocol support".
> 
> For PPPoE, the distinction between session and discovery is not a bad idea but
> since discovery packets typically have a different format (tags in place of
> protocol), I'm not sure they should share a common structure.
> 
> How about a single "PPPOE" item without proto_id to cover both session and
> discovery, then later add separate tag items on a needed basis for each
> possible/optional tag (e.g. PPPOE_TAG_SERVICE_NAME)? Likewise proto_id
> would be provided through a separate optional item if relevant
> (PPPOE_PROTO_ID).

PPPoE Discovery had PPPoE Session has different ethertype 0x8863 , 0x8864, so I think they should belong to separate
match item,( image the case we only need to match a PPPoE Session flow but don't care what kind of protocol payload it have )
but I agree they should only share the common part of the header,
then for PPPoED, it can append optional PPPoE Tag item(s) and for PPPoES , it can append optional PPPoE Protocol item(s)

> Such an approach is already used for IPV6 and IPV6_EXT.
> 
> Another benefit is that applications could match PPPoE regardless of session or
> discovery when they do not care, while PPPOED/PPPOES make that distinction
> mandatory.
> 
> Also by inserting new entries in the middle of the pattern items list, this patch
> breaks ABI. I think it's not on purpose, so please move them to the end (not
> grouping them with existing GTP stuff is fine, ABI compat is more
> important.) This must be reflected in rte_flow.h, rte_flow.c, testpmd and
> documentation.
> 
> More nits below.
> 
> > ---
> > ---
> > v2: Remove Gerrit Change-Id's.
> > ---
> >  app/test-pmd/cmdline_flow.c                 | 80
> +++++++++++++++++++++++++++++
> >  doc/guides/prog_guide/rte_flow.rst          | 25 +++++++++
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
> >  lib/librte_ethdev/rte_flow.c                |  3 ++
> >  lib/librte_ethdev/rte_flow.h                | 71
> +++++++++++++++++++++++++
> >  5 files changed, 189 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> [...]
> > +	[ITEM_PPPOES] = {
> > +		.name = "pppoes",
> > +		.help = "match PPPoE Session header",
> 
> Session => session
> 
> > +		.priv = PRIV_ITEM(PPPOES, sizeof(struct rte_flow_item_pppoe)),
> > +		.next = NEXT(item_pppoe),
> > +		.call = parse_vc,
> > +	},
> > +	[ITEM_PPPOED] = {
> > +		.name = "pppoed",
> > +		.help = "match PPPoE Discovery stage header",
> 
> Discovery => discovery
> 
> > +		.priv = PRIV_ITEM(PPPOED, sizeof(struct rte_flow_item_pppoe)),
> > +		.next = NEXT(item_pppoe),
> > +		.call = parse_vc,
> > +	},
> > +	[ITEM_PPPOE_SEID] = {
> > +		.name = "seid",
> > +		.help = "Session identifier",
> 
> Session => session
> 
> [...]
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 821b524b3..d09c42071 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -1055,6 +1055,31 @@ flow rules.
> >  - ``teid``: tunnel endpoint identifier.
> >  - Default ``mask`` matches teid only.
> >
> > +Item: ``GTP_PSC``
> > +^^^^^^^^^^^^^^^^^^^^^^^
> 
> Too many "^^^"'s.
> 
> [...]
> > +Item: ``PPPOES``, ``PPPOED``
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Matches a PPPOE header.
> 
> PPPOE => PPPoE
> 
> > +
> > +Note: PPPOES and PPPOED use the same structure. PPPOES and PPPOED
> > +item
> 
> item => items (or better, "pattern items")
> 
> > +are defined for a user-friendly API when creating PPPOE-Session and
> > +PPPOE-Discovery flow rules.
> 
> Super nit: use "PPPoE" when mentioning the protocol itself and "PPPOE" when
> mentioning the pattern item.
> 
> > +
> > +- ``v_t_flags``: version (4b), type (4b).
> 
> Why "flags"? I don't see any so you could name it "version_type" (same in
> documentation).
> 
> > +- ``code``: Message type.
> 
> Message => message
> 
> > +- ``session_id``: Session identifier.
> 
> Session => session
> 
> > +- ``length``: Payload length.
> 
> Payload => payload
> 
> > +- ``proto_id``: PPP Protocol identifier.
> 
> Protocol => protocol
> 
> > +- Default ``mask`` matches session_id,proto_id.
> 
> Missing space between session_id and proto_id.
> 
> > +
> >  Item: ``ESP``
> >  ^^^^^^^^^^^^^
> >
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index 313e0707e..0da36d5f1 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -3904,6 +3904,16 @@ This section lists supported pattern items and
> their attributes, if any.
> >
> >    - ``teid {unsigned}``: tunnel endpoint identifier.
> >
> > +- ``gtp_psc``: match GTPv1 entension header (type is 0x85).
> > +
> > +  - ``pdu_type {unsigned}``: PDU type (0 or 1).
> > +  - ``qfi {unsigned}``: QoS flow identifier.
> > +
> > +- ``pppoes``, ``pppoed``: match PPPOE header.
> 
> PPPOE => PPPoE
> 
> [...]
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index b66bf1495..ad5e46190 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -328,6 +328,34 @@ enum rte_flow_item_type {
> >  	 */
> >  	RTE_FLOW_ITEM_TYPE_GTPU,
> >
> > +	/**
> > +	 * Matches a GTP PDU extension header (type is 0x85:
> > +	 * PDU Session Container).
> 
> Session Container => session container
> 
> > +	 *
> > +	 * Configure flow for GTP packets with extension header type 0x85.
> > +	 *
> > +	 * See struct rte_flow_item_gtp_psc.
> > +	 */
> > +	RTE_FLOW_ITEM_TYPE_GTP_PSC,
> > +
> > +	/**
> > +	 * Matches a PPPOE header.
> > +	 *
> > +	 * Configure flow for PPPoE Session packets.
> 
> Session => session
> 
> > +	 *
> > +	 * See struct rte_flow_item_pppoe.
> > +	 */
> > +	RTE_FLOW_ITEM_TYPE_PPPOES,
> > +
> > +	/**
> > +	 * Matches a PPPOE header.
> > +	 *
> > +	 * Configure flow for PPPoE Discovery stage packets.
> 
> Discovery => discovery
> 
> > +	 *
> > +	 * See struct rte_flow_item_pppoe.
> > +	 */
> > +	RTE_FLOW_ITEM_TYPE_PPPOED,
> > +
> >  	/**
> >  	 * Matches a ESP header.
> >  	 *
> > @@ -922,6 +950,49 @@ static const struct rte_flow_item_gtp
> > rte_flow_item_gtp_mask = {  };  #endif
> >
> > +/**
> > + * RTE_FLOW_ITEM_TYPE_GTP_PSC.
> > + *
> > + * Matches a GTP-extension header
> > + * (type is 0x85: PDU Session Container).
> 
> Session Container => session container
> 
> (crusade against superfluous caps!)
> 
> [...]
> > +/**
> > + * RTE_FLOW_ITEM_TYPE_PPPOE.
> > + *
> > + * Matches a PPPOE header.
> > + */
> > +struct rte_flow_item_pppoe {
> > +	/**
> > +	 * Version (4b), type (4b).
> > +	 */
> > +	uint8_t v_t_flags;
> 
> v_t_flags => version_type
> 
> > +	uint8_t code; /**< Message type. */
> > +	rte_be16_t session_id; /**< Session identifier. */
> > +	rte_be16_t length; /**< Payload length. */
> > +	rte_be16_t proto_id; /**< PPP Protocol identifier. */
> 
> As discussed, I suggest dropping proto_id to make this a generic match for
> PPPoE.
> 
> > +};
> > +
> > +/** Default mask for RTE_FLOW_ITEM_TYPE_PPPOE. */ #ifndef __cplusplus
> > +static const struct rte_flow_item_pppoe rte_flow_item_pppoe_mask = {
> > +	.session_id = RTE_BE16(0xffff),
> > +	.proto_id = RTE_BE16(0xffff),
> > +};
> 
> I think the default for PPPoE should be an empty mask that simply says "match
> PPPoE" since session_id only becomes known after negotiation and proto_id
> shouldn't be part of this object.
> 
> --
> Adrien Mazarguil
> 6WIND

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14  3:00 [dpdk-dev] [PATCH] ethdev: add more protocol support in flow API Wang Ying A
2019-08-14  3:24 ` [dpdk-dev] [PATCH v2] " Wang Ying A
2019-08-14  9:08   ` Adrien Mazarguil
2019-08-19 11:53     ` Zhang, Qi Z

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox