DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] rte_flow: add eCPRI key fields to flow API
@ 2020-06-28 16:20 Bing Zhao
  2020-07-02  6:46 ` [dpdk-dev] [PATCH v2] " Bing Zhao
  0 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-06-28 16:20 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev

Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.

eCPRI is a packet based protocol used in the fronthaul interface of
5G networks. Header format definition could be found in the
specification via the link below:
https://www.gigalight.com/downloads/standards/ecpri-specification.pdf

eCPRI message can be over Ethernet layer (.1Q supported also) or over
UDP layer. Message header formats are the same in these two variants.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
 doc/guides/prog_guide/rte_flow.rst |   8 ++
 lib/librte_ethdev/rte_flow.c       |   1 +
 lib/librte_ethdev/rte_flow.h       |  31 ++++++
 lib/librte_net/Makefile            |   1 +
 lib/librte_net/meson.build         |   3 +-
 lib/librte_net/rte_ecpri.h         | 160 +++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h         |   1 +
 7 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index d5dd18ce99..669d519233 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1362,6 +1362,14 @@ Matches a PFCP Header.
 - ``seid``: session endpoint identifier.
 - Default ``mask`` matches s_field and seid.
 
+Item: ``ECPRI``
+^^^^^^^^^^^^^
+
+Matches a eCPRI header.
+
+- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
+- Default ``mask`` matches message type of common header only.
+
 Actions
 ~~~~~~~
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5f73..f8fdd68fe9 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -95,6 +95,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
 	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
 	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
+	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199192..2683466f7a 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -28,6 +28,7 @@
 #include <rte_byteorder.h>
 #include <rte_esp.h>
 #include <rte_higig.h>
+#include <rte_ecpri.h>
 #include <rte_mbuf.h>
 #include <rte_mbuf_dyn.h>
 
@@ -527,6 +528,13 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_PFCP,
 
+	/**
+	 * Matches eCPRI Header.
+	 * See struct rte_flow_item_ecpri.
+	 *
+	 */
+	RTE_FLOW_ITEM_TYPE_ECPRI,
+
 };
 
 /**
@@ -1546,6 +1554,29 @@ static const struct rte_flow_item_pfcp rte_flow_item_pfcp_mask = {
 };
 #endif
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_ECPRI
+ *
+ * Match eCPRI Header
+ */
+struct rte_flow_item_ecpri {
+	struct rte_ecpri_msg_hdr hdr;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
+#ifndef __cplusplus
+static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
+	.hdr = {
+		.common = {
+			.type = 0xFF,
+		},
+	},
+};
+#endif
+
 /**
  * Matching pattern item definition.
  *
diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index aa1d6fed5a..9830e771b3 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index f799349b3e..24ed8253b4 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -15,7 +15,8 @@ headers = files('rte_ip.h',
 	'rte_net.h',
 	'rte_net_crc.h',
 	'rte_mpls.h',
-	'rte_higig.h')
+	'rte_higig.h',
+	'rte_ecpri.h')
 
 sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
new file mode 100644
index 0000000000..c1fd000775
--- /dev/null
+++ b/lib/librte_net/rte_ecpri.h
@@ -0,0 +1,160 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef _RTE_ECPRI_H_
+#define _RTE_ECPRI_H_
+
+#include <stdint.h>
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
+ * Other values are reserved for future
+ */
+#define RTE_ECPRI_REV_UPTO_20		1
+
+/**
+ * eCPRI message types in specifications
+ * IWF* types will only be supported from rev.2
+ */
+#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
+#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
+#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
+#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
+#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
+#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
+#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
+#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
+#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
+#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
+#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
+#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
+
+/**
+ * eCPRI Common Header
+ */
+RTE_STD_C11
+struct rte_ecpri_common_hdr {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t size:16;		/**< Payload Size */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t revision:4;		/**< Protocol Revision */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t revision:4;		/**< Protocol Revision */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t size:16;		/**< Payload Size */
+#endif
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #0: IQ Data
+ */
+struct rte_ecpri_msg_iq_data {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #1: Bit Sequence
+ */
+struct rte_ecpri_msg_bit_seq {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #2: Real-Time Control Data
+ */
+struct rte_ecpri_msg_rtc_ctrl {
+	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #3: Generic Data Transfer
+ */
+struct rte_ecpri_msg_gen_data {
+	rte_be32_t pc_id;		/**< Physical channel ID */
+	rte_be32_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #4: Remote Memory Access
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_rm_access {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t ele_id:16;		/**< Element ID */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t ele_id:16;		/**< Element ID */
+#endif
+	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
+	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
+	rte_be16_t length;		/**< number of bytes */
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #5: One-Way Delay Measurement
+ */
+struct rte_ecpri_msg_delay_measure {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #6: Remote Reset
+ */
+struct rte_ecpri_msg_remote_reset {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #7: Event Indication
+ */
+struct rte_ecpri_msg_event_ind {
+	uint8_t evt_id;			/**< Event ID */
+	uint8_t evt_type;		/**< Event Type */
+	uint8_t seq;			/**< Sequence Number */
+	uint8_t number;			/**< Number of Faults/Notif */
+};
+
+/**
+ * eCPRI Message Header Format: Common Header + Message Types
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_hdr {
+	struct rte_ecpri_common_hdr common;
+	union {
+		struct rte_ecpri_msg_iq_data type0;
+		struct rte_ecpri_msg_bit_seq type1;
+		struct rte_ecpri_msg_rtc_ctrl type2;
+		struct rte_ecpri_msg_bit_seq type3;
+		struct rte_ecpri_msg_rm_access type4;
+		struct rte_ecpri_msg_delay_measure type5;
+		struct rte_ecpri_msg_remote_reset type6;
+		struct rte_ecpri_msg_event_ind type7;
+		uint32_t dummy[3];
+	};
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ECPRI_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 0ae4e75b6c..184a3f9b43 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -304,6 +304,7 @@ struct rte_vlan_hdr {
 #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
 #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
 #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
+#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
 
 /**
  * Extract VLAN tag information into mbuf
-- 
2.19.0.windows.1


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

* [dpdk-dev] [PATCH v2] rte_flow: add eCPRI key fields to flow API
  2020-06-28 16:20 [dpdk-dev] [PATCH] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-02  6:46 ` Bing Zhao
  2020-07-02  8:06   ` Ori Kam
  2020-07-02 12:53   ` [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  0 siblings, 2 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-02  6:46 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev

Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.

eCPRI is a packet based protocol used in the fronthaul interface of
5G networks. Header format definition could be found in the
specification via the link below:
https://www.gigalight.com/downloads/standards/ecpri-specification.pdf

eCPRI message can be over Ethernet layer (.1Q supported also) or over
UDP layer. Message header formats are the same in these two variants.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
v2: Add dw0 for the eCPRI common header to switch the endianess, and
    use fixed u32 value with big-endian for rte_flow_item_ecpri_mask.
    It is due to the fact that global variable only support constant
    expression in C when building.
---
 doc/guides/prog_guide/rte_flow.rst |   8 ++
 lib/librte_ethdev/rte_flow.c       |   1 +
 lib/librte_ethdev/rte_flow.h       |  29 +++++++
 lib/librte_net/Makefile            |   1 +
 lib/librte_net/meson.build         |   3 +-
 lib/librte_net/rte_ecpri.h         | 163 +++++++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h         |   1 +
 7 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index d5dd18c..669d519 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1362,6 +1362,14 @@ Matches a PFCP Header.
 - ``seid``: session endpoint identifier.
 - Default ``mask`` matches s_field and seid.
 
+Item: ``ECPRI``
+^^^^^^^^^^^^^
+
+Matches a eCPRI header.
+
+- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
+- Default ``mask`` matches message type of common header only.
+
 Actions
 ~~~~~~~
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5..f8fdd68 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -95,6 +95,7 @@ struct rte_flow_desc_data {
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
 	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
 	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
+	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..216ece7 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -28,6 +28,7 @@
 #include <rte_byteorder.h>
 #include <rte_esp.h>
 #include <rte_higig.h>
+#include <rte_ecpri.h>
 #include <rte_mbuf.h>
 #include <rte_mbuf_dyn.h>
 
@@ -527,6 +528,13 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_PFCP,
 
+	/**
+	 * Matches eCPRI Header.
+	 * See struct rte_flow_item_ecpri.
+	 *
+	 */
+	RTE_FLOW_ITEM_TYPE_ECPRI,
+
 };
 
 /**
@@ -1547,6 +1555,27 @@ struct rte_flow_item_pfcp {
 #endif
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_ECPRI
+ *
+ * Match eCPRI Header
+ */
+struct rte_flow_item_ecpri {
+	struct rte_ecpri_msg_hdr hdr;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
+#ifndef __cplusplus
+static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
+	.hdr = {
+		.dw0 = RTE_BE32(0x00ff0000),
+	},
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index aa1d6fe..9830e77 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index f799349..24ed825 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -15,7 +15,8 @@ headers = files('rte_ip.h',
 	'rte_net.h',
 	'rte_net_crc.h',
 	'rte_mpls.h',
-	'rte_higig.h')
+	'rte_higig.h',
+	'rte_ecpri.h')
 
 sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
new file mode 100644
index 0000000..31974b2
--- /dev/null
+++ b/lib/librte_net/rte_ecpri.h
@@ -0,0 +1,163 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef _RTE_ECPRI_H_
+#define _RTE_ECPRI_H_
+
+#include <stdint.h>
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
+ * Other values are reserved for future
+ */
+#define RTE_ECPRI_REV_UPTO_20		1
+
+/**
+ * eCPRI message types in specifications
+ * IWF* types will only be supported from rev.2
+ */
+#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
+#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
+#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
+#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
+#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
+#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
+#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
+#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
+#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
+#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
+#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
+#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
+
+/**
+ * eCPRI Common Header
+ */
+RTE_STD_C11
+struct rte_ecpri_common_hdr {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t size:16;		/**< Payload Size */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t revision:4;		/**< Protocol Revision */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t revision:4;		/**< Protocol Revision */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t size:16;		/**< Payload Size */
+#endif
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #0: IQ Data
+ */
+struct rte_ecpri_msg_iq_data {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #1: Bit Sequence
+ */
+struct rte_ecpri_msg_bit_seq {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #2: Real-Time Control Data
+ */
+struct rte_ecpri_msg_rtc_ctrl {
+	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #3: Generic Data Transfer
+ */
+struct rte_ecpri_msg_gen_data {
+	rte_be32_t pc_id;		/**< Physical channel ID */
+	rte_be32_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #4: Remote Memory Access
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_rm_access {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t ele_id:16;		/**< Element ID */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t ele_id:16;		/**< Element ID */
+#endif
+	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
+	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
+	rte_be16_t length;		/**< number of bytes */
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #5: One-Way Delay Measurement
+ */
+struct rte_ecpri_msg_delay_measure {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #6: Remote Reset
+ */
+struct rte_ecpri_msg_remote_reset {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #7: Event Indication
+ */
+struct rte_ecpri_msg_event_ind {
+	uint8_t evt_id;			/**< Event ID */
+	uint8_t evt_type;		/**< Event Type */
+	uint8_t seq;			/**< Sequence Number */
+	uint8_t number;			/**< Number of Faults/Notif */
+};
+
+/**
+ * eCPRI Message Header Format: Common Header + Message Types
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_hdr {
+	union {
+		struct rte_ecpri_common_hdr common;
+		uint32_t dw0;
+	};
+	union {
+		struct rte_ecpri_msg_iq_data type0;
+		struct rte_ecpri_msg_bit_seq type1;
+		struct rte_ecpri_msg_rtc_ctrl type2;
+		struct rte_ecpri_msg_bit_seq type3;
+		struct rte_ecpri_msg_rm_access type4;
+		struct rte_ecpri_msg_delay_measure type5;
+		struct rte_ecpri_msg_remote_reset type6;
+		struct rte_ecpri_msg_event_ind type7;
+		uint32_t dummy[3];
+	};
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ECPRI_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 0ae4e75..184a3f9 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -304,6 +304,7 @@ struct rte_vlan_hdr {
 #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
 #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
 #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
+#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
 
 /**
  * Extract VLAN tag information into mbuf
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v2] rte_flow: add eCPRI key fields to flow API
  2020-07-02  6:46 ` [dpdk-dev] [PATCH v2] " Bing Zhao
@ 2020-07-02  8:06   ` Ori Kam
  2020-07-02 12:53   ` [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  1 sibling, 0 replies; 28+ messages in thread
From: Ori Kam @ 2020-07-02  8:06 UTC (permalink / raw)
  To: Bing Zhao, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, olivier.matz
  Cc: dev

Hi Bing,

Can you add also testpmd patch?

Thanks,
Ori

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Thursday, July 2, 2020 9:46 AM
> To: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com; arybchenko@solarflare.com; olivier.matz@6wind.com
> Cc: dev@dpdk.org
> Subject: [PATCH v2] rte_flow: add eCPRI key fields to flow API
> 
> Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.
> 
> eCPRI is a packet based protocol used in the fronthaul interface of
> 5G networks. Header format definition could be found in the
> specification via the link below:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.giga
> light.com%2Fdownloads%2Fstandards%2Fecpri-
> specification.pdf&amp;data=02%7C01%7Corika%40mellanox.com%7C368410e
> 6bc774bf316ff08d81e539e2e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%
> 7C1%7C637292691769606941&amp;sdata=cGZ0vkX%2FD%2FdsPNELatW6LOOs
> r0Cm%2Faf%2F8zV35dkof9M%3D&amp;reserved=0
> 
> eCPRI message can be over Ethernet layer (.1Q supported also) or over
> UDP layer. Message header formats are the same in these two variants.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> ---
> v2: Add dw0 for the eCPRI common header to switch the endianess, and
>     use fixed u32 value with big-endian for rte_flow_item_ecpri_mask.
>     It is due to the fact that global variable only support constant
>     expression in C when building.
> ---
>  doc/guides/prog_guide/rte_flow.rst |   8 ++
>  lib/librte_ethdev/rte_flow.c       |   1 +
>  lib/librte_ethdev/rte_flow.h       |  29 +++++++
>  lib/librte_net/Makefile            |   1 +
>  lib/librte_net/meson.build         |   3 +-
>  lib/librte_net/rte_ecpri.h         | 163
> +++++++++++++++++++++++++++++++++++++
>  lib/librte_net/rte_ether.h         |   1 +
>  7 files changed, 205 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_net/rte_ecpri.h
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index d5dd18c..669d519 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1362,6 +1362,14 @@ Matches a PFCP Header.
>  - ``seid``: session endpoint identifier.
>  - Default ``mask`` matches s_field and seid.
> 
> +Item: ``ECPRI``
> +^^^^^^^^^^^^^
> +
> +Matches a eCPRI header.
> +
> +- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
> +- Default ``mask`` matches message type of common header only.
> +
>  Actions
>  ~~~~~~~
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 1685be5..f8fdd68 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -95,6 +95,7 @@ struct rte_flow_desc_data {
>  	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
>  	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
>  	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
> +	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
>  };
> 
>  /** Generate flow_action[] entry. */
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b0e4199..216ece7 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -28,6 +28,7 @@
>  #include <rte_byteorder.h>
>  #include <rte_esp.h>
>  #include <rte_higig.h>
> +#include <rte_ecpri.h>
>  #include <rte_mbuf.h>
>  #include <rte_mbuf_dyn.h>
> 
> @@ -527,6 +528,13 @@ enum rte_flow_item_type {
>  	 */
>  	RTE_FLOW_ITEM_TYPE_PFCP,
> 
> +	/**
> +	 * Matches eCPRI Header.
> +	 * See struct rte_flow_item_ecpri.
> +	 *
> +	 */
> +	RTE_FLOW_ITEM_TYPE_ECPRI,
> +
>  };
> 
>  /**
> @@ -1547,6 +1555,27 @@ struct rte_flow_item_pfcp {
>  #endif
> 
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ITEM_TYPE_ECPRI
> + *
> + * Match eCPRI Header
> + */
> +struct rte_flow_item_ecpri {
> +	struct rte_ecpri_msg_hdr hdr;
> +};
> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
> +#ifndef __cplusplus
> +static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
> +	.hdr = {
> +		.dw0 = RTE_BE32(0x00ff0000),

I think this should be all zero as default mask.

> +	},
> +};
> +#endif
> +
> +/**
>   * Matching pattern item definition.
>   *
>   * A pattern is formed by stacking items starting from the lowest protocol
> diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
> index aa1d6fe..9830e77 100644
> --- a/lib/librte_net/Makefile
> +++ b/lib/librte_net/Makefile
> @@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include +=
> rte_sctp.h rte_icmp.h rte_arp.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h
> rte_net.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h
> rte_higig.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
> 
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
> index f799349..24ed825 100644
> --- a/lib/librte_net/meson.build
> +++ b/lib/librte_net/meson.build
> @@ -15,7 +15,8 @@ headers = files('rte_ip.h',
>  	'rte_net.h',
>  	'rte_net_crc.h',
>  	'rte_mpls.h',
> -	'rte_higig.h')
> +	'rte_higig.h',
> +	'rte_ecpri.h')
> 
>  sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
>  deps += ['mbuf']
> diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
> new file mode 100644
> index 0000000..31974b2
> --- /dev/null
> +++ b/lib/librte_net/rte_ecpri.h
> @@ -0,0 +1,163 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2020 Mellanox Technologies, Ltd
> + */
> +
> +#ifndef _RTE_ECPRI_H_
> +#define _RTE_ECPRI_H_
> +
> +#include <stdint.h>
> +#include <rte_byteorder.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
> + * Other values are reserved for future
> + */
> +#define RTE_ECPRI_REV_UPTO_20		1
> +
> +/**
> + * eCPRI message types in specifications
> + * IWF* types will only be supported from rev.2
> + */
> +#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
> +#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
> +#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
> +#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
> +#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
> +#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
> +#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
> +#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
> +#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
> +#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
> +#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
> +#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
> +
> +/**
> + * eCPRI Common Header
> + */
> +RTE_STD_C11
> +struct rte_ecpri_common_hdr {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	uint32_t size:16;		/**< Payload Size */
> +	uint32_t type:8;		/**< Message Type */
> +	uint32_t c:1;			/**< Concatenation Indicator */
> +	uint32_t res:3;			/**< Reserved */
> +	uint32_t revision:4;		/**< Protocol Revision */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	uint32_t revision:4;		/**< Protocol Revision */
> +	uint32_t res:3;			/**< Reserved */
> +	uint32_t c:1;			/**< Concatenation Indicator */
> +	uint32_t type:8;		/**< Message Type */
> +	uint32_t size:16;		/**< Payload Size */
> +#endif
> +} __rte_packed;
> +
> +/**
> + * eCPRI Message Header of Type #0: IQ Data
> + */
> +struct rte_ecpri_msg_iq_data {
> +	rte_be16_t pc_id;		/**< Physical channel ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #1: Bit Sequence
> + */
> +struct rte_ecpri_msg_bit_seq {
> +	rte_be16_t pc_id;		/**< Physical channel ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #2: Real-Time Control Data
> + */
> +struct rte_ecpri_msg_rtc_ctrl {
> +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #3: Generic Data Transfer
> + */
> +struct rte_ecpri_msg_gen_data {
> +	rte_be32_t pc_id;		/**< Physical channel ID */
> +	rte_be32_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #4: Remote Memory Access
> + */
> +RTE_STD_C11
> +struct rte_ecpri_msg_rm_access {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	uint32_t ele_id:16;		/**< Element ID */
> +	uint32_t rr:4;			/**< Req/Resp */
> +	uint32_t rw:4;			/**< Read/Write */
> +	uint32_t rma_id:8;		/**< Remote Memory Access ID */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	uint32_t rma_id:8;		/**< Remote Memory Access ID */
> +	uint32_t rw:4;			/**< Read/Write */
> +	uint32_t rr:4;			/**< Req/Resp */
> +	uint32_t ele_id:16;		/**< Element ID */
> +#endif
> +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
> +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> +	rte_be16_t length;		/**< number of bytes */
> +} __rte_packed;
> +
> +/**
> + * eCPRI Message Header of Type #5: One-Way Delay Measurement
> + */
> +struct rte_ecpri_msg_delay_measure {
> +	uint8_t msr_id;			/**< Measurement ID */
> +	uint8_t act_type;		/**< Action Type */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #6: Remote Reset
> + */
> +struct rte_ecpri_msg_remote_reset {
> +	uint8_t msr_id;			/**< Measurement ID */
> +	uint8_t act_type;		/**< Action Type */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #7: Event Indication
> + */
> +struct rte_ecpri_msg_event_ind {
> +	uint8_t evt_id;			/**< Event ID */
> +	uint8_t evt_type;		/**< Event Type */
> +	uint8_t seq;			/**< Sequence Number */
> +	uint8_t number;			/**< Number of Faults/Notif
> */
> +};
> +
> +/**
> + * eCPRI Message Header Format: Common Header + Message Types
> + */
> +RTE_STD_C11
> +struct rte_ecpri_msg_hdr {
> +	union {
> +		struct rte_ecpri_common_hdr common;
> +		uint32_t dw0;
> +	};
> +	union {
> +		struct rte_ecpri_msg_iq_data type0;
> +		struct rte_ecpri_msg_bit_seq type1;
> +		struct rte_ecpri_msg_rtc_ctrl type2;
> +		struct rte_ecpri_msg_bit_seq type3;
> +		struct rte_ecpri_msg_rm_access type4;
> +		struct rte_ecpri_msg_delay_measure type5;
> +		struct rte_ecpri_msg_remote_reset type6;
> +		struct rte_ecpri_msg_event_ind type7;
> +		uint32_t dummy[3];
> +	};
> +};
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_ECPRI_H_ */
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index 0ae4e75..184a3f9 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -304,6 +304,7 @@ struct rte_vlan_hdr {
>  #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
>  #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
>  #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
> +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q
> supported). */
> 
>  /**
>   * Extract VLAN tag information into mbuf
> --
> 1.8.3.1


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

* [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow
  2020-07-02  6:46 ` [dpdk-dev] [PATCH v2] " Bing Zhao
  2020-07-02  8:06   ` Ori Kam
@ 2020-07-02 12:53   ` Bing Zhao
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
                       ` (2 more replies)
  1 sibling, 3 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-02 12:53 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

This patch set contains two commits.
1. header definition of the ethdev API
2. testpmd support for the eCPRI flow item

Bing Zhao (2):
  rte_flow: add eCPRI key fields to flow API
  app/testpmd: add eCPRI in flow creation patterns

 app/test-pmd/cmdline_flow.c        | 143 ++++++++++++++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst |   8 ++
 lib/librte_ethdev/rte_flow.c       |   1 +
 lib/librte_ethdev/rte_flow.h       |  31 +++++++
 lib/librte_net/Makefile            |   1 +
 lib/librte_net/meson.build         |   3 +-
 lib/librte_net/rte_ecpri.h         | 163 +++++++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h         |   1 +
 8 files changed, 350 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

-- 
2.5.5


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

* [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-02 12:53   ` [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
@ 2020-07-02 12:53     ` Bing Zhao
  2020-07-05 11:34       ` Ori Kam
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
  2020-07-07 15:36     ` [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-02 12:53 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.

eCPRI is a packet based protocol used in the fronthaul interface of
5G networks. Header format definition could be found in the
specification via the link below:
https://www.gigalight.com/downloads/standards/ecpri-specification.pdf

eCPRI message can be over Ethernet layer (.1Q supported also) or over
UDP layer. Message header formats are the same in these two variants.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
 doc/guides/prog_guide/rte_flow.rst |   8 ++
 lib/librte_ethdev/rte_flow.c       |   1 +
 lib/librte_ethdev/rte_flow.h       |  31 +++++++
 lib/librte_net/Makefile            |   1 +
 lib/librte_net/meson.build         |   3 +-
 lib/librte_net/rte_ecpri.h         | 163 +++++++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h         |   1 +
 7 files changed, 207 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index d5dd18c..669d519 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1362,6 +1362,14 @@ Matches a PFCP Header.
 - ``seid``: session endpoint identifier.
 - Default ``mask`` matches s_field and seid.
 
+Item: ``ECPRI``
+^^^^^^^^^^^^^
+
+Matches a eCPRI header.
+
+- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
+- Default ``mask`` matches message type of common header only.
+
 Actions
 ~~~~~~~
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5..f8fdd68 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -95,6 +95,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
 	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
 	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
+	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..8a90226 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -28,6 +28,7 @@
 #include <rte_byteorder.h>
 #include <rte_esp.h>
 #include <rte_higig.h>
+#include <rte_ecpri.h>
 #include <rte_mbuf.h>
 #include <rte_mbuf_dyn.h>
 
@@ -527,6 +528,15 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_PFCP,
 
+	/**
+	 * Matches eCPRI Header.
+	 *
+	 * Configure flow for eCPRI over ETH or UDP packets.
+	 *
+	 * See struct rte_flow_item_ecpri.
+	 */
+	RTE_FLOW_ITEM_TYPE_ECPRI,
+
 };
 
 /**
@@ -1547,6 +1557,27 @@ static const struct rte_flow_item_pfcp rte_flow_item_pfcp_mask = {
 #endif
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_ECPRI
+ *
+ * Match eCPRI Header
+ */
+struct rte_flow_item_ecpri {
+	struct rte_ecpri_msg_hdr hdr;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
+#ifndef __cplusplus
+static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
+	.hdr = {
+		.dw0 = 0x0,
+	},
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index aa1d6fe..9830e77 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index f799349..24ed825 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -15,7 +15,8 @@ headers = files('rte_ip.h',
 	'rte_net.h',
 	'rte_net_crc.h',
 	'rte_mpls.h',
-	'rte_higig.h')
+	'rte_higig.h',
+	'rte_ecpri.h')
 
 sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
new file mode 100644
index 0000000..31974b2
--- /dev/null
+++ b/lib/librte_net/rte_ecpri.h
@@ -0,0 +1,163 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef _RTE_ECPRI_H_
+#define _RTE_ECPRI_H_
+
+#include <stdint.h>
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
+ * Other values are reserved for future
+ */
+#define RTE_ECPRI_REV_UPTO_20		1
+
+/**
+ * eCPRI message types in specifications
+ * IWF* types will only be supported from rev.2
+ */
+#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
+#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
+#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
+#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
+#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
+#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
+#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
+#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
+#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
+#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
+#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
+#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
+
+/**
+ * eCPRI Common Header
+ */
+RTE_STD_C11
+struct rte_ecpri_common_hdr {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t size:16;		/**< Payload Size */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t revision:4;		/**< Protocol Revision */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t revision:4;		/**< Protocol Revision */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t size:16;		/**< Payload Size */
+#endif
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #0: IQ Data
+ */
+struct rte_ecpri_msg_iq_data {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #1: Bit Sequence
+ */
+struct rte_ecpri_msg_bit_seq {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #2: Real-Time Control Data
+ */
+struct rte_ecpri_msg_rtc_ctrl {
+	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #3: Generic Data Transfer
+ */
+struct rte_ecpri_msg_gen_data {
+	rte_be32_t pc_id;		/**< Physical channel ID */
+	rte_be32_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #4: Remote Memory Access
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_rm_access {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t ele_id:16;		/**< Element ID */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t ele_id:16;		/**< Element ID */
+#endif
+	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
+	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
+	rte_be16_t length;		/**< number of bytes */
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #5: One-Way Delay Measurement
+ */
+struct rte_ecpri_msg_delay_measure {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #6: Remote Reset
+ */
+struct rte_ecpri_msg_remote_reset {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #7: Event Indication
+ */
+struct rte_ecpri_msg_event_ind {
+	uint8_t evt_id;			/**< Event ID */
+	uint8_t evt_type;		/**< Event Type */
+	uint8_t seq;			/**< Sequence Number */
+	uint8_t number;			/**< Number of Faults/Notif */
+};
+
+/**
+ * eCPRI Message Header Format: Common Header + Message Types
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_hdr {
+	union {
+		struct rte_ecpri_common_hdr common;
+		uint32_t dw0;
+	};
+	union {
+		struct rte_ecpri_msg_iq_data type0;
+		struct rte_ecpri_msg_bit_seq type1;
+		struct rte_ecpri_msg_rtc_ctrl type2;
+		struct rte_ecpri_msg_bit_seq type3;
+		struct rte_ecpri_msg_rm_access type4;
+		struct rte_ecpri_msg_delay_measure type5;
+		struct rte_ecpri_msg_remote_reset type6;
+		struct rte_ecpri_msg_event_ind type7;
+		uint32_t dummy[3];
+	};
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ECPRI_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 0ae4e75..184a3f9 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -304,6 +304,7 @@ struct rte_vlan_hdr {
 #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
 #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
 #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
+#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
 
 /**
  * Extract VLAN tag information into mbuf
-- 
2.5.5


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

* [dpdk-dev] [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns
  2020-07-02 12:53   ` [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-02 12:53     ` Bing Zhao
  2020-07-05 11:36       ` Ori Kam
  2020-07-07 15:36     ` [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-02 12:53 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

In order to verify offloading of eCPRI protocol via flow rules, the
command line of flow creation should support the parsing of the eCPRI
pattern.

Based on the specification, one eCPRI message will have the common
header and payload. Payload format is various based on the type field
of the common header. Fixed strings will be used instead of integer
to make the CLI easy for auto-completion.

The testpmd command line examples of flow to match eCPRI item are
listed below:
  1. flow create 0 ... pattern eth / ecpri / end actions ...
    This is to match all eCPRI messages.
  2. flow create 0 ... pattern eth / ecpri common type rtc_ctrl / end actions ...
    This is to match all eCPRI messages with the type #2 - "Real-Time
    Control Data".
  3. flow create 0 ... pattern eth / ecpri common type iq_data pc_id is [U16Int] / end actions ...
    This is to match eCPRI messages with the type #0 - "IQ Data", and
    the physical channel ID 'pc_id' of the messages is a specific
    value. Since the sequence ID is changeable, there is no need to
    match that field in the flow.
Currently, only type #0, #2 and #5 will be supported.

Since eCPRI could be over Ethernet layer (or after .1Q) and UDP
layer, it is the PMD driver's responsibility to check whether eCPRI
is supported and which protocol stack is supported. Network byte
order should be used for eCPRI header, the same as other headers.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 143 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 4e2006c..801581e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -230,6 +230,15 @@ enum index {
 	ITEM_PFCP,
 	ITEM_PFCP_S_FIELD,
 	ITEM_PFCP_SEID,
+	ITEM_ECPRI,
+	ITEM_ECPRI_COMMON,
+	ITEM_ECPRI_COMMON_TYPE,
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ITEM_ECPRI_MSG_IQ_DATA_PCID,
+	ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+	ITEM_ECPRI_MSG_DLY_MSR_MSRID,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -791,6 +800,7 @@ static const enum index next_item[] = {
 	ITEM_ESP,
 	ITEM_AH,
 	ITEM_PFCP,
+	ITEM_ECPRI,
 	END_SET,
 	ZERO,
 };
@@ -1101,6 +1111,24 @@ static const enum index item_l2tpv3oip[] = {
 	ZERO,
 };
 
+static const enum index item_ecpri[] = {
+	ITEM_ECPRI_COMMON,
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_ecpri_common[] = {
+	ITEM_ECPRI_COMMON_TYPE,
+	ZERO,
+};
+
+static const enum index item_ecpri_common_type[] = {
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -1409,6 +1437,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_item_ecpri_type(struct context *, const struct token *,
+				    const char *, unsigned int,
+				    void *, unsigned int);
 static int parse_vc_action_rss(struct context *, const struct token *,
 			       const char *, unsigned int, void *,
 			       unsigned int);
@@ -2802,6 +2833,66 @@ static const struct token token_list[] = {
 		.next = NEXT(item_pfcp, NEXT_ENTRY(UNSIGNED), item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pfcp, seid)),
 	},
+	[ITEM_ECPRI] = {
+		.name = "ecpri",
+		.help = "match eCPRI header",
+		.priv = PRIV_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
+		.next = NEXT(item_ecpri),
+		.call = parse_vc,
+	},
+	[ITEM_ECPRI_COMMON] = {
+		.name = "common",
+		.help = "eCPRI common header",
+		.next = NEXT(item_ecpri_common),
+	},
+	[ITEM_ECPRI_COMMON_TYPE] = {
+		.name = "type",
+		.help = "type of common header",
+		.next = NEXT(item_ecpri_common_type),
+		.args = ARGS(ARG_ENTRY_HTON(struct rte_flow_item_ecpri)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_IQ_DATA] = {
+		.name = "iq_data",
+		.help = "Type #0: IQ Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_IQ_DATA_PCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_IQ_DATA_PCID] = {
+		.name = "pc_id",
+		.help = "Physical Channel ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type0.pc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_RTC_CTRL] = {
+		.name = "rtc_ctrl",
+		.help = "Type #2: Real-Time Control Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_RTC_CTRL_RTCID] = {
+		.name = "rtc_id",
+		.help = "Real-Time Control Data ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type2.rtc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_DLY_MSR] = {
+		.name = "delay_measure",
+		.help = "Type #5: One-Way Delay Measurement",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_DLY_MSR_MSRID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_DLY_MSR_MSRID] = {
+		.name = "msr_id",
+		.help = "Measurement ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type5.msr_id)),
+	},
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
@@ -4124,6 +4215,58 @@ parse_vc_conf(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse eCPRI common header type field. */
+static int
+parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
+			 const char *str, unsigned int len,
+			 void *buf, unsigned int size)
+{
+	struct rte_flow_item_ecpri *ecpri;
+	struct rte_flow_item_ecpri *ecpri_mask;
+	struct rte_flow_item *item;
+	uint32_t data_size;
+	uint8_t msg_type;
+	struct buffer *out = buf;
+	const struct arg *arg;
+
+	(void)size;
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	switch (ctx->curr) {
+	case ITEM_ECPRI_COMMON_TYPE_IQ_DATA:
+		msg_type = RTE_ECPRI_MSG_TYPE_IQ_DATA;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_RTC_CTRL:
+		msg_type = RTE_ECPRI_MSG_TYPE_RTC_CTRL;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_DLY_MSR:
+		msg_type = RTE_ECPRI_MSG_TYPE_DLY_MSR;
+		break;
+	default:
+		return -1;
+	}
+	if (!ctx->object)
+		return len;
+	arg = pop_args(ctx);
+	if (!arg)
+		return -1;
+	ecpri = (struct rte_flow_item_ecpri *)out->args.vc.data;
+	ecpri->hdr.common.type = msg_type;
+	data_size = ctx->objdata / 3; /* spec, last, mask */
+	ecpri_mask = (struct rte_flow_item_ecpri *)(out->args.vc.data +
+						    (data_size * 2));
+	ecpri_mask->hdr.common.type = 0xFF;
+	if (arg->hton) {
+		ecpri->hdr.dw0 = rte_cpu_to_be_32(ecpri->hdr.dw0);
+		ecpri_mask->hdr.dw0 = rte_cpu_to_be_32(ecpri_mask->hdr.dw0);
+	}
+	item = &out->args.vc.pattern[out->args.vc.pattern_n - 1];
+	item->spec = ecpri;
+	item->mask = ecpri_mask;
+	return len;
+}
+
 /** Parse RSS action. */
 static int
 parse_vc_action_rss(struct context *ctx, const struct token *token,
-- 
2.5.5


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

* Re: [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-05 11:34       ` Ori Kam
  0 siblings, 0 replies; 28+ messages in thread
From: Ori Kam @ 2020-07-05 11:34 UTC (permalink / raw)
  To: Bing Zhao, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

Hi Bing,

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Thursday, July 2, 2020 3:54 PM
> Subject: [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API
> 
> Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.
> 
> eCPRI is a packet based protocol used in the fronthaul interface of
> 5G networks. Header format definition could be found in the
> specification via the link below:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.giga
> light.com%2Fdownloads%2Fstandards%2Fecpri-
> specification.pdf&amp;data=02%7C01%7Corika%40mellanox.com%7C504c3c3c
> 131d4707b38e08d81e871030%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%
> 7C1%7C637292912736066748&amp;sdata=QA%2FjVcj9ImXbuzoeyM3se25eHgL
> RNneA9MM5tHQzAMA%3D&amp;reserved=0
> 
> eCPRI message can be over Ethernet layer (.1Q supported also) or over
> UDP layer. Message header formats are the same in these two variants.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> ---

Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori

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

* Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
@ 2020-07-05 11:36       ` Ori Kam
  0 siblings, 0 replies; 28+ messages in thread
From: Ori Kam @ 2020-07-05 11:36 UTC (permalink / raw)
  To: Bing Zhao, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

Hi Bing,

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Thursday, July 2, 2020 3:54 PM
> To: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com; arybchenko@solarflare.com; olivier.matz@6wind.com
> Cc: dev@dpdk.org; wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com
> Subject: [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns
> 
> In order to verify offloading of eCPRI protocol via flow rules, the
> command line of flow creation should support the parsing of the eCPRI
> pattern.
> 
> Based on the specification, one eCPRI message will have the common
> header and payload. Payload format is various based on the type field
> of the common header. Fixed strings will be used instead of integer
> to make the CLI easy for auto-completion.
> 
> The testpmd command line examples of flow to match eCPRI item are
> listed below:
>   1. flow create 0 ... pattern eth / ecpri / end actions ...
>     This is to match all eCPRI messages.
>   2. flow create 0 ... pattern eth / ecpri common type rtc_ctrl / end actions ...
>     This is to match all eCPRI messages with the type #2 - "Real-Time
>     Control Data".
>   3. flow create 0 ... pattern eth / ecpri common type iq_data pc_id is [U16Int] /
> end actions ...
>     This is to match eCPRI messages with the type #0 - "IQ Data", and
>     the physical channel ID 'pc_id' of the messages is a specific
>     value. Since the sequence ID is changeable, there is no need to
>     match that field in the flow.
> Currently, only type #0, #2 and #5 will be supported.
> 
> Since eCPRI could be over Ethernet layer (or after .1Q) and UDP
> layer, it is the PMD driver's responsibility to check whether eCPRI
> is supported and which protocol stack is supported. Network byte
> order should be used for eCPRI header, the same as other headers.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> ---
>  app/test-pmd/cmdline_flow.c | 143
> ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 143 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 4e2006c..801581e 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -230,6 +230,15 @@ enum index {
>  	ITEM_PFCP,
>  	ITEM_PFCP_S_FIELD,
>  	ITEM_PFCP_SEID,
> +	ITEM_ECPRI,
> +	ITEM_ECPRI_COMMON,
> +	ITEM_ECPRI_COMMON_TYPE,
> +	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
> +	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
> +	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
> +	ITEM_ECPRI_MSG_IQ_DATA_PCID,
> +	ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
> +	ITEM_ECPRI_MSG_DLY_MSR_MSRID,
> 
>  	/* Validate/create actions. */
>  	ACTIONS,
> @@ -791,6 +800,7 @@ static const enum index next_item[] = {
>  	ITEM_ESP,
>  	ITEM_AH,
>  	ITEM_PFCP,
> +	ITEM_ECPRI,
>  	END_SET,
>  	ZERO,
>  };
> @@ -1101,6 +1111,24 @@ static const enum index item_l2tpv3oip[] = {
>  	ZERO,
>  };
> 
> +static const enum index item_ecpri[] = {
> +	ITEM_ECPRI_COMMON,
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
> +static const enum index item_ecpri_common[] = {
> +	ITEM_ECPRI_COMMON_TYPE,
> +	ZERO,
> +};
> +
> +static const enum index item_ecpri_common_type[] = {
> +	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
> +	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
> +	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
> +	ZERO,
> +};
> +
>  static const enum index next_action[] = {
>  	ACTION_END,
>  	ACTION_VOID,
> @@ -1409,6 +1437,9 @@ static int parse_vc_spec(struct context *, const struct
> token *,
>  			 const char *, unsigned int, void *, unsigned int);
>  static int parse_vc_conf(struct context *, const struct token *,
>  			 const char *, unsigned int, void *, unsigned int);
> +static int parse_vc_item_ecpri_type(struct context *, const struct token *,
> +				    const char *, unsigned int,
> +				    void *, unsigned int);
>  static int parse_vc_action_rss(struct context *, const struct token *,
>  			       const char *, unsigned int, void *,
>  			       unsigned int);
> @@ -2802,6 +2833,66 @@ static const struct token token_list[] = {
>  		.next = NEXT(item_pfcp, NEXT_ENTRY(UNSIGNED),
> item_param),
>  		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pfcp,
> seid)),
>  	},
> +	[ITEM_ECPRI] = {
> +		.name = "ecpri",
> +		.help = "match eCPRI header",
> +		.priv = PRIV_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
> +		.next = NEXT(item_ecpri),
> +		.call = parse_vc,
> +	},
> +	[ITEM_ECPRI_COMMON] = {
> +		.name = "common",
> +		.help = "eCPRI common header",
> +		.next = NEXT(item_ecpri_common),
> +	},
> +	[ITEM_ECPRI_COMMON_TYPE] = {
> +		.name = "type",
> +		.help = "type of common header",
> +		.next = NEXT(item_ecpri_common_type),
> +		.args = ARGS(ARG_ENTRY_HTON(struct rte_flow_item_ecpri)),
> +	},
> +	[ITEM_ECPRI_COMMON_TYPE_IQ_DATA] = {
> +		.name = "iq_data",
> +		.help = "Type #0: IQ Data",
> +		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_IQ_DATA_PCID,
> +					ITEM_NEXT)),
> +		.call = parse_vc_item_ecpri_type,
> +	},
> +	[ITEM_ECPRI_MSG_IQ_DATA_PCID] = {
> +		.name = "pc_id",
> +		.help = "Physical Channel ID",
> +		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED),
> item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
> +				hdr.type0.pc_id)),
> +	},
> +	[ITEM_ECPRI_COMMON_TYPE_RTC_CTRL] = {
> +		.name = "rtc_ctrl",
> +		.help = "Type #2: Real-Time Control Data",
> +		.next =
> NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
> +					ITEM_NEXT)),
> +		.call = parse_vc_item_ecpri_type,
> +	},
> +	[ITEM_ECPRI_MSG_RTC_CTRL_RTCID] = {
> +		.name = "rtc_id",
> +		.help = "Real-Time Control Data ID",
> +		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED),
> item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
> +				hdr.type2.rtc_id)),
> +	},
> +	[ITEM_ECPRI_COMMON_TYPE_DLY_MSR] = {
> +		.name = "delay_measure",
> +		.help = "Type #5: One-Way Delay Measurement",
> +		.next =
> NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_DLY_MSR_MSRID,
> +					ITEM_NEXT)),
> +		.call = parse_vc_item_ecpri_type,
> +	},
> +	[ITEM_ECPRI_MSG_DLY_MSR_MSRID] = {
> +		.name = "msr_id",
> +		.help = "Measurement ID",
> +		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED),
> item_param),
> +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
> +				hdr.type5.msr_id)),
> +	},
>  	/* Validate/create actions. */
>  	[ACTIONS] = {
>  		.name = "actions",
> @@ -4124,6 +4215,58 @@ parse_vc_conf(struct context *ctx, const struct
> token *token,
>  	return len;
>  }
> 
> +/** Parse eCPRI common header type field. */
> +static int
> +parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
> +			 const char *str, unsigned int len,
> +			 void *buf, unsigned int size)
> +{
> +	struct rte_flow_item_ecpri *ecpri;
> +	struct rte_flow_item_ecpri *ecpri_mask;
> +	struct rte_flow_item *item;
> +	uint32_t data_size;
> +	uint8_t msg_type;
> +	struct buffer *out = buf;
> +	const struct arg *arg;
> +
> +	(void)size;
> +	/* Token name must match. */
> +	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> +		return -1;
> +	switch (ctx->curr) {
> +	case ITEM_ECPRI_COMMON_TYPE_IQ_DATA:
> +		msg_type = RTE_ECPRI_MSG_TYPE_IQ_DATA;
> +		break;
> +	case ITEM_ECPRI_COMMON_TYPE_RTC_CTRL:
> +		msg_type = RTE_ECPRI_MSG_TYPE_RTC_CTRL;
> +		break;
> +	case ITEM_ECPRI_COMMON_TYPE_DLY_MSR:
> +		msg_type = RTE_ECPRI_MSG_TYPE_DLY_MSR;
> +		break;
> +	default:
> +		return -1;
> +	}
> +	if (!ctx->object)
> +		return len;
> +	arg = pop_args(ctx);
> +	if (!arg)
> +		return -1;
> +	ecpri = (struct rte_flow_item_ecpri *)out->args.vc.data;
> +	ecpri->hdr.common.type = msg_type;
> +	data_size = ctx->objdata / 3; /* spec, last, mask */
> +	ecpri_mask = (struct rte_flow_item_ecpri *)(out->args.vc.data +
> +						    (data_size * 2));
> +	ecpri_mask->hdr.common.type = 0xFF;
> +	if (arg->hton) {
> +		ecpri->hdr.dw0 = rte_cpu_to_be_32(ecpri->hdr.dw0);
> +		ecpri_mask->hdr.dw0 = rte_cpu_to_be_32(ecpri_mask-
> >hdr.dw0);
> +	}
> +	item = &out->args.vc.pattern[out->args.vc.pattern_n - 1];
> +	item->spec = ecpri;
> +	item->mask = ecpri_mask;
> +	return len;
> +}
> +
>  /** Parse RSS action. */
>  static int
>  parse_vc_action_rss(struct context *ctx, const struct token *token,
> --


Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori
> 2.5.5


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

* [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow
  2020-07-02 12:53   ` [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
  2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
@ 2020-07-07 15:36     ` Bing Zhao
  2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
                         ` (2 more replies)
  2 siblings, 3 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-07 15:36 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

This patch set contains two commits.
1. header definition of the ethdev API
2. testpmd support for the eCPRI flow item

---
v2: Add dw0 for the eCPRI common header to switch the endianess, and
    use fixed u32 value with big-endian for rte_flow_item_ecpri_mask.
    It is due to the fact that global variable only support constant
    expression in C when building.
v3: Add commit for testpmd support
v4: update release notes part
---

Bing Zhao (2):
  rte_flow: add eCPRI key fields to flow API
  app/testpmd: add eCPRI in flow creation patterns

 app/test-pmd/cmdline_flow.c            | 143 +++++++++++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst     |   8 ++
 doc/guides/rel_notes/release_20_08.rst |   5 +
 lib/librte_ethdev/rte_flow.c           |   1 +
 lib/librte_ethdev/rte_flow.h           |  31 +++++++
 lib/librte_net/Makefile                |   1 +
 lib/librte_net/meson.build             |   3 +-
 lib/librte_net/rte_ecpri.h             | 163 +++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h             |   1 +
 9 files changed, 355 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-07 15:36     ` [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
@ 2020-07-07 15:36       ` Bing Zhao
  2020-07-08 18:49         ` Akhil Goyal
  2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
  2020-07-10  8:45       ` [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-07 15:36 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.

eCPRI is a packet based protocol used in the fronthaul interface of
5G networks. Header format definition could be found in the
specification via the link below:
https://www.gigalight.com/downloads/standards/ecpri-specification.pdf

eCPRI message can be over Ethernet layer (.1Q supported also) or over
UDP layer. Message header formats are the same in these two variants.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 doc/guides/prog_guide/rte_flow.rst     |   8 ++
 doc/guides/rel_notes/release_20_08.rst |   5 +
 lib/librte_ethdev/rte_flow.c           |   1 +
 lib/librte_ethdev/rte_flow.h           |  31 +++++++
 lib/librte_net/Makefile                |   1 +
 lib/librte_net/meson.build             |   3 +-
 lib/librte_net/rte_ecpri.h             | 163 +++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h             |   1 +
 8 files changed, 212 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index d5dd18c..669d519 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1362,6 +1362,14 @@ Matches a PFCP Header.
 - ``seid``: session endpoint identifier.
 - Default ``mask`` matches s_field and seid.
 
+Item: ``ECPRI``
+^^^^^^^^^^^^^
+
+Matches a eCPRI header.
+
+- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
+- Default ``mask`` matches message type of common header only.
+
 Actions
 ~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst
index 5cbc4ce..2140891 100644
--- a/doc/guides/rel_notes/release_20_08.rst
+++ b/doc/guides/rel_notes/release_20_08.rst
@@ -98,6 +98,11 @@ New Features
   which are used to access packet data in a safe manner. Currently JIT support
   for these instructions is implemented for x86 only.
 
+* **Added eCPRI protocol support in rte_flow.**
+
+  The ``ECPRI`` item have been added to support eCPRI packet offloading for
+  5G network.
+
 * **Added flow performance test application.**
 
   Added new application to test ``rte_flow`` performance, including:
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5..f8fdd68 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -95,6 +95,7 @@ struct rte_flow_desc_data {
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
 	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
 	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
+	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..8a90226 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -28,6 +28,7 @@
 #include <rte_byteorder.h>
 #include <rte_esp.h>
 #include <rte_higig.h>
+#include <rte_ecpri.h>
 #include <rte_mbuf.h>
 #include <rte_mbuf_dyn.h>
 
@@ -527,6 +528,15 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_PFCP,
 
+	/**
+	 * Matches eCPRI Header.
+	 *
+	 * Configure flow for eCPRI over ETH or UDP packets.
+	 *
+	 * See struct rte_flow_item_ecpri.
+	 */
+	RTE_FLOW_ITEM_TYPE_ECPRI,
+
 };
 
 /**
@@ -1547,6 +1557,27 @@ struct rte_flow_item_pfcp {
 #endif
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_ECPRI
+ *
+ * Match eCPRI Header
+ */
+struct rte_flow_item_ecpri {
+	struct rte_ecpri_msg_hdr hdr;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
+#ifndef __cplusplus
+static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
+	.hdr = {
+		.dw0 = 0x0,
+	},
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index aa1d6fe..9830e77 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index f799349..24ed825 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -15,7 +15,8 @@ headers = files('rte_ip.h',
 	'rte_net.h',
 	'rte_net_crc.h',
 	'rte_mpls.h',
-	'rte_higig.h')
+	'rte_higig.h',
+	'rte_ecpri.h')
 
 sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
new file mode 100644
index 0000000..31974b2
--- /dev/null
+++ b/lib/librte_net/rte_ecpri.h
@@ -0,0 +1,163 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef _RTE_ECPRI_H_
+#define _RTE_ECPRI_H_
+
+#include <stdint.h>
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
+ * Other values are reserved for future
+ */
+#define RTE_ECPRI_REV_UPTO_20		1
+
+/**
+ * eCPRI message types in specifications
+ * IWF* types will only be supported from rev.2
+ */
+#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
+#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
+#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
+#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
+#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
+#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
+#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
+#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
+#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
+#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
+#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
+#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
+
+/**
+ * eCPRI Common Header
+ */
+RTE_STD_C11
+struct rte_ecpri_common_hdr {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t size:16;		/**< Payload Size */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t revision:4;		/**< Protocol Revision */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t revision:4;		/**< Protocol Revision */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t size:16;		/**< Payload Size */
+#endif
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #0: IQ Data
+ */
+struct rte_ecpri_msg_iq_data {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #1: Bit Sequence
+ */
+struct rte_ecpri_msg_bit_seq {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #2: Real-Time Control Data
+ */
+struct rte_ecpri_msg_rtc_ctrl {
+	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #3: Generic Data Transfer
+ */
+struct rte_ecpri_msg_gen_data {
+	rte_be32_t pc_id;		/**< Physical channel ID */
+	rte_be32_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #4: Remote Memory Access
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_rm_access {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t ele_id:16;		/**< Element ID */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t ele_id:16;		/**< Element ID */
+#endif
+	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
+	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
+	rte_be16_t length;		/**< number of bytes */
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #5: One-Way Delay Measurement
+ */
+struct rte_ecpri_msg_delay_measure {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #6: Remote Reset
+ */
+struct rte_ecpri_msg_remote_reset {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #7: Event Indication
+ */
+struct rte_ecpri_msg_event_ind {
+	uint8_t evt_id;			/**< Event ID */
+	uint8_t evt_type;		/**< Event Type */
+	uint8_t seq;			/**< Sequence Number */
+	uint8_t number;			/**< Number of Faults/Notif */
+};
+
+/**
+ * eCPRI Message Header Format: Common Header + Message Types
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_hdr {
+	union {
+		struct rte_ecpri_common_hdr common;
+		uint32_t dw0;
+	};
+	union {
+		struct rte_ecpri_msg_iq_data type0;
+		struct rte_ecpri_msg_bit_seq type1;
+		struct rte_ecpri_msg_rtc_ctrl type2;
+		struct rte_ecpri_msg_bit_seq type3;
+		struct rte_ecpri_msg_rm_access type4;
+		struct rte_ecpri_msg_delay_measure type5;
+		struct rte_ecpri_msg_remote_reset type6;
+		struct rte_ecpri_msg_event_ind type7;
+		uint32_t dummy[3];
+	};
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ECPRI_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 0ae4e75..184a3f9 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -304,6 +304,7 @@ struct rte_vlan_hdr {
 #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
 #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
 #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
+#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
 
 /**
  * Extract VLAN tag information into mbuf
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v4 2/2] app/testpmd: add eCPRI in flow creation patterns
  2020-07-07 15:36     ` [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-07 15:36       ` Bing Zhao
  2020-07-10  8:45       ` [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2 siblings, 0 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-07 15:36 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

In order to verify offloading of eCPRI protocol via flow rules, the
command line of flow creation should support the parsing of the eCPRI
pattern.

Based on the specification, one eCPRI message will have the common
header and payload. Payload format is various based on the type field
of the common header. Fixed strings will be used instead of integer
to make the CLI easy for auto-completion.

The testpmd command line examples of flow to match eCPRI item are
listed below:
  1. flow create 0 ... pattern eth / ecpri / end actions ...
    This is to match all eCPRI messages.
  2. flow create 0 ... pattern eth / ecpri common type rtc_ctrl / end actions ...
    This is to match all eCPRI messages with the type #2 - "Real-Time
    Control Data".
  3. flow create 0 ... pattern eth / ecpri common type iq_data pc_id is [U16Int] / end actions ...
    This is to match eCPRI messages with the type #0 - "IQ Data", and
    the physical channel ID 'pc_id' of the messages is a specific
    value. Since the sequence ID is changeable, there is no need to
    match that field in the flow.
Currently, only type #0, #2 and #5 will be supported.

Since eCPRI could be over Ethernet layer (or after .1Q) and UDP
layer, it is the PMD driver's responsibility to check whether eCPRI
is supported and which protocol stack is supported. Network byte
order should be used for eCPRI header, the same as other headers.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 143 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 4e2006c..801581e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -230,6 +230,15 @@ enum index {
 	ITEM_PFCP,
 	ITEM_PFCP_S_FIELD,
 	ITEM_PFCP_SEID,
+	ITEM_ECPRI,
+	ITEM_ECPRI_COMMON,
+	ITEM_ECPRI_COMMON_TYPE,
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ITEM_ECPRI_MSG_IQ_DATA_PCID,
+	ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+	ITEM_ECPRI_MSG_DLY_MSR_MSRID,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -791,6 +800,7 @@ struct parse_action_priv {
 	ITEM_ESP,
 	ITEM_AH,
 	ITEM_PFCP,
+	ITEM_ECPRI,
 	END_SET,
 	ZERO,
 };
@@ -1101,6 +1111,24 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index item_ecpri[] = {
+	ITEM_ECPRI_COMMON,
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_ecpri_common[] = {
+	ITEM_ECPRI_COMMON_TYPE,
+	ZERO,
+};
+
+static const enum index item_ecpri_common_type[] = {
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -1409,6 +1437,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_item_ecpri_type(struct context *, const struct token *,
+				    const char *, unsigned int,
+				    void *, unsigned int);
 static int parse_vc_action_rss(struct context *, const struct token *,
 			       const char *, unsigned int, void *,
 			       unsigned int);
@@ -2802,6 +2833,66 @@ static int comp_set_raw_index(struct context *, const struct token *,
 		.next = NEXT(item_pfcp, NEXT_ENTRY(UNSIGNED), item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pfcp, seid)),
 	},
+	[ITEM_ECPRI] = {
+		.name = "ecpri",
+		.help = "match eCPRI header",
+		.priv = PRIV_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
+		.next = NEXT(item_ecpri),
+		.call = parse_vc,
+	},
+	[ITEM_ECPRI_COMMON] = {
+		.name = "common",
+		.help = "eCPRI common header",
+		.next = NEXT(item_ecpri_common),
+	},
+	[ITEM_ECPRI_COMMON_TYPE] = {
+		.name = "type",
+		.help = "type of common header",
+		.next = NEXT(item_ecpri_common_type),
+		.args = ARGS(ARG_ENTRY_HTON(struct rte_flow_item_ecpri)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_IQ_DATA] = {
+		.name = "iq_data",
+		.help = "Type #0: IQ Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_IQ_DATA_PCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_IQ_DATA_PCID] = {
+		.name = "pc_id",
+		.help = "Physical Channel ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type0.pc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_RTC_CTRL] = {
+		.name = "rtc_ctrl",
+		.help = "Type #2: Real-Time Control Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_RTC_CTRL_RTCID] = {
+		.name = "rtc_id",
+		.help = "Real-Time Control Data ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type2.rtc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_DLY_MSR] = {
+		.name = "delay_measure",
+		.help = "Type #5: One-Way Delay Measurement",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_DLY_MSR_MSRID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_DLY_MSR_MSRID] = {
+		.name = "msr_id",
+		.help = "Measurement ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type5.msr_id)),
+	},
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
@@ -4124,6 +4215,58 @@ static int comp_set_raw_index(struct context *, const struct token *,
 	return len;
 }
 
+/** Parse eCPRI common header type field. */
+static int
+parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
+			 const char *str, unsigned int len,
+			 void *buf, unsigned int size)
+{
+	struct rte_flow_item_ecpri *ecpri;
+	struct rte_flow_item_ecpri *ecpri_mask;
+	struct rte_flow_item *item;
+	uint32_t data_size;
+	uint8_t msg_type;
+	struct buffer *out = buf;
+	const struct arg *arg;
+
+	(void)size;
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	switch (ctx->curr) {
+	case ITEM_ECPRI_COMMON_TYPE_IQ_DATA:
+		msg_type = RTE_ECPRI_MSG_TYPE_IQ_DATA;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_RTC_CTRL:
+		msg_type = RTE_ECPRI_MSG_TYPE_RTC_CTRL;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_DLY_MSR:
+		msg_type = RTE_ECPRI_MSG_TYPE_DLY_MSR;
+		break;
+	default:
+		return -1;
+	}
+	if (!ctx->object)
+		return len;
+	arg = pop_args(ctx);
+	if (!arg)
+		return -1;
+	ecpri = (struct rte_flow_item_ecpri *)out->args.vc.data;
+	ecpri->hdr.common.type = msg_type;
+	data_size = ctx->objdata / 3; /* spec, last, mask */
+	ecpri_mask = (struct rte_flow_item_ecpri *)(out->args.vc.data +
+						    (data_size * 2));
+	ecpri_mask->hdr.common.type = 0xFF;
+	if (arg->hton) {
+		ecpri->hdr.dw0 = rte_cpu_to_be_32(ecpri->hdr.dw0);
+		ecpri_mask->hdr.dw0 = rte_cpu_to_be_32(ecpri_mask->hdr.dw0);
+	}
+	item = &out->args.vc.pattern[out->args.vc.pattern_n - 1];
+	item->spec = ecpri;
+	item->mask = ecpri_mask;
+	return len;
+}
+
 /** Parse RSS action. */
 static int
 parse_vc_action_rss(struct context *ctx, const struct token *token,
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-08 18:49         ` Akhil Goyal
  2020-07-09  3:58           ` Bing Zhao
  0 siblings, 1 reply; 28+ messages in thread
From: Akhil Goyal @ 2020-07-08 18:49 UTC (permalink / raw)
  To: Bing Zhao, orika, john.mcnamara, marko.kovacevic, thomas,
	ferruh.yigit, arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger


> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
> + * Other values are reserved for future
> + */
> +#define RTE_ECPRI_REV_UPTO_20		1
> +
> +/**
> + * eCPRI message types in specifications
> + * IWF* types will only be supported from rev.2
> + */
> +#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
> +#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
> +#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
> +#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
> +#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
> +#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
> +#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
> +#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
> +#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
> +#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
> +#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
> +#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11

Should we have a comment for reserved and vendor specific message types as well?

> +
> +/**
> + * eCPRI Common Header
> + */
> +RTE_STD_C11
> +struct rte_ecpri_common_hdr {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	uint32_t size:16;		/**< Payload Size */
> +	uint32_t type:8;		/**< Message Type */
> +	uint32_t c:1;			/**< Concatenation Indicator */
> +	uint32_t res:3;			/**< Reserved */
> +	uint32_t revision:4;		/**< Protocol Revision */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	uint32_t revision:4;		/**< Protocol Revision */
> +	uint32_t res:3;			/**< Reserved */
> +	uint32_t c:1;			/**< Concatenation Indicator */
> +	uint32_t type:8;		/**< Message Type */
> +	uint32_t size:16;		/**< Payload Size */
> +#endif
> +} __rte_packed;
> +
> +/**
> + * eCPRI Message Header of Type #0: IQ Data
> + */
> +struct rte_ecpri_msg_iq_data {
> +	rte_be16_t pc_id;		/**< Physical channel ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #1: Bit Sequence
> + */
> +struct rte_ecpri_msg_bit_seq {
> +	rte_be16_t pc_id;		/**< Physical channel ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #2: Real-Time Control Data
> + */
> +struct rte_ecpri_msg_rtc_ctrl {
> +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #3: Generic Data Transfer
> + */
> +struct rte_ecpri_msg_gen_data {
> +	rte_be32_t pc_id;		/**< Physical channel ID */
> +	rte_be32_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #4: Remote Memory Access
> + */
> +RTE_STD_C11
> +struct rte_ecpri_msg_rm_access {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	uint32_t ele_id:16;		/**< Element ID */
> +	uint32_t rr:4;			/**< Req/Resp */
> +	uint32_t rw:4;			/**< Read/Write */
> +	uint32_t rma_id:8;		/**< Remote Memory Access ID */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	uint32_t rma_id:8;		/**< Remote Memory Access ID */
> +	uint32_t rw:4;			/**< Read/Write */
> +	uint32_t rr:4;			/**< Req/Resp */
> +	uint32_t ele_id:16;		/**< Element ID */
> +#endif
> +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
> +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> +	rte_be16_t length;		/**< number of bytes */
> +} __rte_packed;
> +
> +/**
> + * eCPRI Message Header of Type #5: One-Way Delay Measurement
> + */
> +struct rte_ecpri_msg_delay_measure {
> +	uint8_t msr_id;			/**< Measurement ID */
> +	uint8_t act_type;		/**< Action Type */

Should we also add timestamp and compensation fields as well here?

> +};
> +
> +/**
> + * eCPRI Message Header of Type #6: Remote Reset
> + */
> +struct rte_ecpri_msg_remote_reset {
> +	uint8_t msr_id;			/**< Measurement ID */
> +	uint8_t act_type;		/**< Action Type */
> +};

I think it is a copy paste error.
It should have uint16_t reset_id and uint8_t reset_code_op

> +
> +/**
> + * eCPRI Message Header of Type #7: Event Indication
> + */
> +struct rte_ecpri_msg_event_ind {
> +	uint8_t evt_id;			/**< Event ID */
> +	uint8_t evt_type;		/**< Event Type */
> +	uint8_t seq;			/**< Sequence Number */
> +	uint8_t number;			/**< Number of Faults/Notif */
> +};
Should we also define enums for evt_type and other fields in this file.

> +
> +/**
> + * eCPRI Message Header Format: Common Header + Message Types
> + */
> +RTE_STD_C11
> +struct rte_ecpri_msg_hdr {
> +	union {
> +		struct rte_ecpri_common_hdr common;
> +		uint32_t dw0;
> +	};
> +	union {
> +		struct rte_ecpri_msg_iq_data type0;
> +		struct rte_ecpri_msg_bit_seq type1;
> +		struct rte_ecpri_msg_rtc_ctrl type2;
> +		struct rte_ecpri_msg_bit_seq type3;
> +		struct rte_ecpri_msg_rm_access type4;
> +		struct rte_ecpri_msg_delay_measure type5;
> +		struct rte_ecpri_msg_remote_reset type6;
> +		struct rte_ecpri_msg_event_ind type7;
> +		uint32_t dummy[3];

Why 3 dummy? IWF messages are 4.

> +	};
> +};
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_ECPRI_H_ */
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index 0ae4e75..184a3f9 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -304,6 +304,7 @@ struct rte_vlan_hdr {
>  #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
>  #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
>  #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
> +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported).
> */
> 
>  /**
>   * Extract VLAN tag information into mbuf
> --
> 1.8.3.1


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

* Re: [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-08 18:49         ` Akhil Goyal
@ 2020-07-09  3:58           ` Bing Zhao
  0 siblings, 0 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-09  3:58 UTC (permalink / raw)
  To: Akhil Goyal, Ori Kam, john.mcnamara, marko.kovacevic,
	Thomas Monjalon, ferruh.yigit, arybchenko, olivier.matz
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, July 9, 2020 2:50 AM
> To: Bing Zhao <bingz@mellanox.com>; Ori Kam
> <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; olivier.matz@6wind.com
> Cc: dev@dpdk.org; wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com
> Subject: RE: [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields
> to flow API
> 
> 
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
> > + * Other values are reserved for future  */
> > +#define RTE_ECPRI_REV_UPTO_20		1
> > +
> > +/**
> > + * eCPRI message types in specifications
> > + * IWF* types will only be supported from rev.2  */
> > +#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
> > +#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
> > +#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
> > +#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
> > +#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
> > +#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
> > +#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
> > +#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
> > +#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
> > +#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
> > +#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
> > +#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
> 
> Should we have a comment for reserved and vendor specific message
> types as well?

Yes, thanks. We can have one line or two lines of comments to describe here.

> 
> > +
> > +/**
> > + * eCPRI Common Header
> > + */
> > +RTE_STD_C11
> > +struct rte_ecpri_common_hdr {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +	uint32_t size:16;		/**< Payload Size */
> > +	uint32_t type:8;		/**< Message Type */
> > +	uint32_t c:1;			/**< Concatenation Indicator
> */
> > +	uint32_t res:3;			/**< Reserved */
> > +	uint32_t revision:4;		/**< Protocol Revision */
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +	uint32_t revision:4;		/**< Protocol Revision */
> > +	uint32_t res:3;			/**< Reserved */
> > +	uint32_t c:1;			/**< Concatenation Indicator
> */
> > +	uint32_t type:8;		/**< Message Type */
> > +	uint32_t size:16;		/**< Payload Size */
> > +#endif
> > +} __rte_packed;
> > +
> > +/**
> > + * eCPRI Message Header of Type #0: IQ Data  */ struct
> > +rte_ecpri_msg_iq_data {
> > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > +	rte_be16_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #1: Bit Sequence  */ struct
> > +rte_ecpri_msg_bit_seq {
> > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > +	rte_be16_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #2: Real-Time Control Data  */
> struct
> > +rte_ecpri_msg_rtc_ctrl {
> > +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID
> */
> > +	rte_be16_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #3: Generic Data Transfer  */
> struct
> > +rte_ecpri_msg_gen_data {
> > +	rte_be32_t pc_id;		/**< Physical channel ID */
> > +	rte_be32_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #4: Remote Memory Access  */
> > +RTE_STD_C11
> > +struct rte_ecpri_msg_rm_access {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +	uint32_t ele_id:16;		/**< Element ID */
> > +	uint32_t rr:4;			/**< Req/Resp */
> > +	uint32_t rw:4;			/**< Read/Write */
> > +	uint32_t rma_id:8;		/**< Remote Memory Access
> ID */
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +	uint32_t rma_id:8;		/**< Remote Memory Access
> ID */
> > +	uint32_t rw:4;			/**< Read/Write */
> > +	uint32_t rr:4;			/**< Req/Resp */
> > +	uint32_t ele_id:16;		/**< Element ID */
> > +#endif
> > +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB)
> */
> > +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB)
> */
> > +	rte_be16_t length;		/**< number of bytes */
> > +} __rte_packed;
> > +
> > +/**
> > + * eCPRI Message Header of Type #5: One-Way Delay
> Measurement  */
> > +struct rte_ecpri_msg_delay_measure {
> > +	uint8_t msr_id;			/**< Measurement ID */
> > +	uint8_t act_type;		/**< Action Type */
> 
> Should we also add timestamp and compensation fields as well here?

OK, we can add it. I am not sure if there is a strong requirement for these fields.
This file is planned to define the headers of an eCPRI message. From the specification,
only the common header is named with "header". And the first bytes in the message
body will be recognized with different formats based on the type, as we know.
Regarding some message types, the first several bytes could be considered as the
"sub header" of the message, then followed by user data bytes. The length of the user
specific data is variable as well as the content.
In this case, we can that the timestamp and compensation are the user specific
data, and in each packet, it will have a different value.

> 
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #6: Remote Reset  */ struct
> > +rte_ecpri_msg_remote_reset {
> > +	uint8_t msr_id;			/**< Measurement ID */
> > +	uint8_t act_type;		/**< Action Type */
> > +};
> 
> I think it is a copy paste error.
> It should have uint16_t reset_id and uint8_t reset_code_op

Nice catch, thanks a lot.

> 
> > +
> > +/**
> > + * eCPRI Message Header of Type #7: Event Indication  */ struct
> > +rte_ecpri_msg_event_ind {
> > +	uint8_t evt_id;			/**< Event ID */
> > +	uint8_t evt_type;		/**< Event Type */
> > +	uint8_t seq;			/**< Sequence Number */
> > +	uint8_t number;			/**< Number of
> Faults/Notif */
> > +};
> Should we also define enums for evt_type and other fields in this file.

For event indication type, we could. We can use #define instead of enum since this
should be fixed value and there is no change in the next releases of the spec.
Also, other fields are not exposed in the "header" now, so to my understanding, no
need to cover them at this stage.

> 
> > +
> > +/**
> > + * eCPRI Message Header Format: Common Header + Message
> Types  */
> > +RTE_STD_C11
> > +struct rte_ecpri_msg_hdr {
> > +	union {
> > +		struct rte_ecpri_common_hdr common;
> > +		uint32_t dw0;
> > +	};
> > +	union {
> > +		struct rte_ecpri_msg_iq_data type0;
> > +		struct rte_ecpri_msg_bit_seq type1;
> > +		struct rte_ecpri_msg_rtc_ctrl type2;
> > +		struct rte_ecpri_msg_bit_seq type3;
> > +		struct rte_ecpri_msg_rm_access type4;
> > +		struct rte_ecpri_msg_delay_measure type5;
> > +		struct rte_ecpri_msg_remote_reset type6;
> > +		struct rte_ecpri_msg_event_ind type7;
> > +		uint32_t dummy[3];
> 
> Why 3 dummy? IWF messages are 4.

This is a union of the message payload body but not the "place holders" for
IWF messages. IWF messages headers are a little bit complex and not defined
in revision 1.x IIRC. This filed as well as the "dw0" is only SW level concept to
simplify the work in the driver and make compiler happy without any cost.
Some critical flags of a compiler may complain when forcing casting the structure
starts from a bit-field to a u32. And when the DW 4 bytes needs to be dumped or
checked, and when doing endianness swap, this would be easier. Maybe this field
should have a better name 😊
3 DWs for the payload sub header part is enough now. If we add the timestamp and
comp part, then we should enlarge this.

> 
> > +	};
> > +};
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* _RTE_ECPRI_H_ */
> > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> > index 0ae4e75..184a3f9 100644
> > --- a/lib/librte_net/rte_ether.h
> > +++ b/lib/librte_net/rte_ether.h
> > @@ -304,6 +304,7 @@ struct rte_vlan_hdr {  #define
> RTE_ETHER_TYPE_LLDP
> > 0x88CC /**< LLDP Protocol. */  #define RTE_ETHER_TYPE_MPLS
> 0x8847 /**<
> > MPLS ethertype. */  #define RTE_ETHER_TYPE_MPLSM 0x8848 /**<
> MPLS
> > multicast ethertype. */
> > +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q
> supported).
> > */
> >
> >  /**
> >   * Extract VLAN tag information into mbuf
> > --
> > 1.8.3.1


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

* [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow
  2020-07-07 15:36     ` [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
  2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
@ 2020-07-10  8:45       ` Bing Zhao
  2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
                           ` (2 more replies)
  2 siblings, 3 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-10  8:45 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz, akhil.goyal
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

This patch set contains two commits.
1. header definition of the ethdev API
2. testpmd support for the eCPRI flow item

---
v2: Add dw0 for the eCPRI common header to switch the endianess, and
    use fixed u32 value with big-endian for rte_flow_item_ecpri_mask.
    It is due to the fact that global variable only support constant
    expression in C when building.
v3: Add commit for testpmd support.
v4: update release notes part.
v5: fix type#6 define, add event indication macros, and comments for
    revisions.
---

Bing Zhao (2):
  rte_flow: add eCPRI key fields to flow API
  app/testpmd: add eCPRI in flow creation patterns

 app/test-pmd/cmdline_flow.c            | 143 ++++++++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst     |   8 ++
 doc/guides/rel_notes/release_20_08.rst |   5 +
 lib/librte_ethdev/rte_flow.c           |   1 +
 lib/librte_ethdev/rte_flow.h           |  31 ++++++
 lib/librte_net/Makefile                |   1 +
 lib/librte_net/meson.build             |   3 +-
 lib/librte_net/rte_ecpri.h             | 182 +++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h             |   1 +
 9 files changed, 374 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-10  8:45       ` [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
@ 2020-07-10  8:45         ` Bing Zhao
  2020-07-10 14:31           ` Olivier Matz
  2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
  2020-07-12 13:35         ` [dpdk-dev] [PATCH v6 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-10  8:45 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz, akhil.goyal
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.

eCPRI is a packet based protocol used in the fronthaul interface of
5G networks. Header format definition could be found in the
specification via the link below:
https://www.gigalight.com/downloads/standards/ecpri-specification.pdf

eCPRI message can be over Ethernet layer (.1Q supported also) or over
UDP layer. Message header formats are the same in these two variants.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 doc/guides/prog_guide/rte_flow.rst     |   8 ++
 doc/guides/rel_notes/release_20_08.rst |   5 +
 lib/librte_ethdev/rte_flow.c           |   1 +
 lib/librte_ethdev/rte_flow.h           |  31 ++++++
 lib/librte_net/Makefile                |   1 +
 lib/librte_net/meson.build             |   3 +-
 lib/librte_net/rte_ecpri.h             | 182 +++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h             |   1 +
 8 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index d5dd18c..669d519 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1362,6 +1362,14 @@ Matches a PFCP Header.
 - ``seid``: session endpoint identifier.
 - Default ``mask`` matches s_field and seid.
 
+Item: ``ECPRI``
+^^^^^^^^^^^^^
+
+Matches a eCPRI header.
+
+- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
+- Default ``mask`` matches message type of common header only.
+
 Actions
 ~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst
index 988474c..19feb68 100644
--- a/doc/guides/rel_notes/release_20_08.rst
+++ b/doc/guides/rel_notes/release_20_08.rst
@@ -184,6 +184,11 @@ New Features
   which are used to access packet data in a safe manner. Currently JIT support
   for these instructions is implemented for x86 only.
 
+* **Added eCPRI protocol support in rte_flow.**
+
+  The ``ECPRI`` item have been added to support eCPRI packet offloading for
+  5G network.
+
 * **Added flow performance test application.**
 
   Added new application to test ``rte_flow`` performance, including:
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5..f8fdd68 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -95,6 +95,7 @@ struct rte_flow_desc_data {
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
 	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
 	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
+	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..8a90226 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -28,6 +28,7 @@
 #include <rte_byteorder.h>
 #include <rte_esp.h>
 #include <rte_higig.h>
+#include <rte_ecpri.h>
 #include <rte_mbuf.h>
 #include <rte_mbuf_dyn.h>
 
@@ -527,6 +528,15 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_PFCP,
 
+	/**
+	 * Matches eCPRI Header.
+	 *
+	 * Configure flow for eCPRI over ETH or UDP packets.
+	 *
+	 * See struct rte_flow_item_ecpri.
+	 */
+	RTE_FLOW_ITEM_TYPE_ECPRI,
+
 };
 
 /**
@@ -1547,6 +1557,27 @@ struct rte_flow_item_pfcp {
 #endif
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_ECPRI
+ *
+ * Match eCPRI Header
+ */
+struct rte_flow_item_ecpri {
+	struct rte_ecpri_msg_hdr hdr;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
+#ifndef __cplusplus
+static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
+	.hdr = {
+		.dw0 = 0x0,
+	},
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index aa1d6fe..9830e77 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index f799349..24ed825 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -15,7 +15,8 @@ headers = files('rte_ip.h',
 	'rte_net.h',
 	'rte_net_crc.h',
 	'rte_mpls.h',
-	'rte_higig.h')
+	'rte_higig.h',
+	'rte_ecpri.h')
 
 sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
new file mode 100644
index 0000000..60fb4f7
--- /dev/null
+++ b/lib/librte_net/rte_ecpri.h
@@ -0,0 +1,182 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef _RTE_ECPRI_H_
+#define _RTE_ECPRI_H_
+
+#include <stdint.h>
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
+ * Other values are reserved for future
+ */
+#define RTE_ECPRI_REV_UP_TO_20		1
+
+/**
+ * eCPRI message types in specifications
+ * IWF* types will only be supported from rev.2
+ * 12-63: Reserved for future revision
+ * 64-255: Vendor Specific
+ */
+#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
+#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
+#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
+#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
+#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
+#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
+#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
+#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
+#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
+#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
+#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
+#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
+
+/**
+ * Event Type of Message Type #7: Event Indication
+ * 0x00: Fault(s) Indication
+ * 0x01: Fault(s) Indication Acknowledge
+ * 0x02: Notification(s) Indication
+ * 0x03: Synchronization Request
+ * 0x04: Synchronization Acknowledge
+ * 0x05: Synchronization End Indication
+ * 0x06...0xFF: Reserved
+ */
+#define RTE_ECPRI_EVT_IND_FAULT_IND	0
+#define RTE_ECPRI_EVT_IND_FAULT_ACK	1
+#define RTE_ECPRI_EVT_IND_NTFY_IND	2
+#define RTE_ECPRI_EVT_IND_SYNC_REQ	3
+#define RTE_ECPRI_EVT_IND_SYNC_ACK	4
+#define RTE_ECPRI_EVT_IND_SYNC_END	5
+
+/**
+ * eCPRI Common Header
+ */
+RTE_STD_C11
+struct rte_ecpri_common_hdr {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t size:16;		/**< Payload Size */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t revision:4;		/**< Protocol Revision */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t revision:4;		/**< Protocol Revision */
+	uint32_t res:3;			/**< Reserved */
+	uint32_t c:1;			/**< Concatenation Indicator */
+	uint32_t type:8;		/**< Message Type */
+	uint32_t size:16;		/**< Payload Size */
+#endif
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #0: IQ Data
+ */
+struct rte_ecpri_msg_iq_data {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #1: Bit Sequence
+ */
+struct rte_ecpri_msg_bit_seq {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #2: Real-Time Control Data
+ */
+struct rte_ecpri_msg_rtc_ctrl {
+	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #3: Generic Data Transfer
+ */
+struct rte_ecpri_msg_gen_data {
+	rte_be32_t pc_id;		/**< Physical channel ID */
+	rte_be32_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #4: Remote Memory Access
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_rm_access {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t ele_id:16;		/**< Element ID */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t ele_id:16;		/**< Element ID */
+#endif
+	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
+	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
+	rte_be16_t length;		/**< number of bytes */
+} __rte_packed;
+
+/**
+ * eCPRI Message Header of Type #5: One-Way Delay Measurement
+ */
+struct rte_ecpri_msg_delay_measure {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #6: Remote Reset
+ */
+struct rte_ecpri_msg_remote_reset {
+	rte_be16_t rst_id;		/**< Reset ID */
+	uint8_t rst_op;			/**< Reset Code Op */
+};
+
+/**
+ * eCPRI Message Header of Type #7: Event Indication
+ */
+struct rte_ecpri_msg_event_ind {
+	uint8_t evt_id;			/**< Event ID */
+	uint8_t evt_type;		/**< Event Type */
+	uint8_t seq;			/**< Sequence Number */
+	uint8_t number;			/**< Number of Faults/Notif */
+};
+
+/**
+ * eCPRI Message Header Format: Common Header + Message Types
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_hdr {
+	union {
+		struct rte_ecpri_common_hdr common;
+		uint32_t dw0;
+	};
+	union {
+		struct rte_ecpri_msg_iq_data type0;
+		struct rte_ecpri_msg_bit_seq type1;
+		struct rte_ecpri_msg_rtc_ctrl type2;
+		struct rte_ecpri_msg_bit_seq type3;
+		struct rte_ecpri_msg_rm_access type4;
+		struct rte_ecpri_msg_delay_measure type5;
+		struct rte_ecpri_msg_remote_reset type6;
+		struct rte_ecpri_msg_event_ind type7;
+		uint32_t dummy[3];
+	};
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ECPRI_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 0ae4e75..184a3f9 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -304,6 +304,7 @@ struct rte_vlan_hdr {
 #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
 #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
 #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
+#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
 
 /**
  * Extract VLAN tag information into mbuf
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v5 2/2] app/testpmd: add eCPRI in flow creation patterns
  2020-07-10  8:45       ` [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-10  8:45         ` Bing Zhao
  2020-07-12 13:35         ` [dpdk-dev] [PATCH v6 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2 siblings, 0 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-10  8:45 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz, akhil.goyal
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

In order to verify offloading of eCPRI protocol via flow rules, the
command line of flow creation should support the parsing of the eCPRI
pattern.

Based on the specification, one eCPRI message will have the common
header and payload. Payload format is various based on the type field
of the common header. Fixed strings will be used instead of integer
to make the CLI easy for auto-completion.

The testpmd command line examples of flow to match eCPRI item are
listed below:
  1. flow create 0 ... pattern eth / ecpri / end actions ...
    This is to match all eCPRI messages.
  2. flow create 0 ... pattern eth / ecpri common type rtc_ctrl / end actions ...
    This is to match all eCPRI messages with the type #2 - "Real-Time
    Control Data".
  3. flow create 0 ... pattern eth / ecpri common type iq_data pc_id is [U16Int] / end actions ...
    This is to match eCPRI messages with the type #0 - "IQ Data", and
    the physical channel ID 'pc_id' of the messages is a specific
    value. Since the sequence ID is changeable, there is no need to
    match that field in the flow.
Currently, only type #0, #2 and #5 will be supported.

Since eCPRI could be over Ethernet layer (or after .1Q) and UDP
layer, it is the PMD driver's responsibility to check whether eCPRI
is supported and which protocol stack is supported. Network byte
order should be used for eCPRI header, the same as other headers.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 143 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 4e2006c..801581e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -230,6 +230,15 @@ enum index {
 	ITEM_PFCP,
 	ITEM_PFCP_S_FIELD,
 	ITEM_PFCP_SEID,
+	ITEM_ECPRI,
+	ITEM_ECPRI_COMMON,
+	ITEM_ECPRI_COMMON_TYPE,
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ITEM_ECPRI_MSG_IQ_DATA_PCID,
+	ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+	ITEM_ECPRI_MSG_DLY_MSR_MSRID,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -791,6 +800,7 @@ struct parse_action_priv {
 	ITEM_ESP,
 	ITEM_AH,
 	ITEM_PFCP,
+	ITEM_ECPRI,
 	END_SET,
 	ZERO,
 };
@@ -1101,6 +1111,24 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index item_ecpri[] = {
+	ITEM_ECPRI_COMMON,
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_ecpri_common[] = {
+	ITEM_ECPRI_COMMON_TYPE,
+	ZERO,
+};
+
+static const enum index item_ecpri_common_type[] = {
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -1409,6 +1437,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_item_ecpri_type(struct context *, const struct token *,
+				    const char *, unsigned int,
+				    void *, unsigned int);
 static int parse_vc_action_rss(struct context *, const struct token *,
 			       const char *, unsigned int, void *,
 			       unsigned int);
@@ -2802,6 +2833,66 @@ static int comp_set_raw_index(struct context *, const struct token *,
 		.next = NEXT(item_pfcp, NEXT_ENTRY(UNSIGNED), item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pfcp, seid)),
 	},
+	[ITEM_ECPRI] = {
+		.name = "ecpri",
+		.help = "match eCPRI header",
+		.priv = PRIV_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
+		.next = NEXT(item_ecpri),
+		.call = parse_vc,
+	},
+	[ITEM_ECPRI_COMMON] = {
+		.name = "common",
+		.help = "eCPRI common header",
+		.next = NEXT(item_ecpri_common),
+	},
+	[ITEM_ECPRI_COMMON_TYPE] = {
+		.name = "type",
+		.help = "type of common header",
+		.next = NEXT(item_ecpri_common_type),
+		.args = ARGS(ARG_ENTRY_HTON(struct rte_flow_item_ecpri)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_IQ_DATA] = {
+		.name = "iq_data",
+		.help = "Type #0: IQ Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_IQ_DATA_PCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_IQ_DATA_PCID] = {
+		.name = "pc_id",
+		.help = "Physical Channel ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type0.pc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_RTC_CTRL] = {
+		.name = "rtc_ctrl",
+		.help = "Type #2: Real-Time Control Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_RTC_CTRL_RTCID] = {
+		.name = "rtc_id",
+		.help = "Real-Time Control Data ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type2.rtc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_DLY_MSR] = {
+		.name = "delay_measure",
+		.help = "Type #5: One-Way Delay Measurement",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_DLY_MSR_MSRID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_DLY_MSR_MSRID] = {
+		.name = "msr_id",
+		.help = "Measurement ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type5.msr_id)),
+	},
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
@@ -4124,6 +4215,58 @@ static int comp_set_raw_index(struct context *, const struct token *,
 	return len;
 }
 
+/** Parse eCPRI common header type field. */
+static int
+parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
+			 const char *str, unsigned int len,
+			 void *buf, unsigned int size)
+{
+	struct rte_flow_item_ecpri *ecpri;
+	struct rte_flow_item_ecpri *ecpri_mask;
+	struct rte_flow_item *item;
+	uint32_t data_size;
+	uint8_t msg_type;
+	struct buffer *out = buf;
+	const struct arg *arg;
+
+	(void)size;
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	switch (ctx->curr) {
+	case ITEM_ECPRI_COMMON_TYPE_IQ_DATA:
+		msg_type = RTE_ECPRI_MSG_TYPE_IQ_DATA;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_RTC_CTRL:
+		msg_type = RTE_ECPRI_MSG_TYPE_RTC_CTRL;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_DLY_MSR:
+		msg_type = RTE_ECPRI_MSG_TYPE_DLY_MSR;
+		break;
+	default:
+		return -1;
+	}
+	if (!ctx->object)
+		return len;
+	arg = pop_args(ctx);
+	if (!arg)
+		return -1;
+	ecpri = (struct rte_flow_item_ecpri *)out->args.vc.data;
+	ecpri->hdr.common.type = msg_type;
+	data_size = ctx->objdata / 3; /* spec, last, mask */
+	ecpri_mask = (struct rte_flow_item_ecpri *)(out->args.vc.data +
+						    (data_size * 2));
+	ecpri_mask->hdr.common.type = 0xFF;
+	if (arg->hton) {
+		ecpri->hdr.dw0 = rte_cpu_to_be_32(ecpri->hdr.dw0);
+		ecpri_mask->hdr.dw0 = rte_cpu_to_be_32(ecpri_mask->hdr.dw0);
+	}
+	item = &out->args.vc.pattern[out->args.vc.pattern_n - 1];
+	item->spec = ecpri;
+	item->mask = ecpri_mask;
+	return len;
+}
+
 /** Parse RSS action. */
 static int
 parse_vc_action_rss(struct context *ctx, const struct token *token,
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-10 14:31           ` Olivier Matz
  2020-07-11  4:25             ` Bing Zhao
  0 siblings, 1 reply; 28+ messages in thread
From: Olivier Matz @ 2020-07-10 14:31 UTC (permalink / raw)
  To: Bing Zhao
  Cc: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, akhil.goyal, dev, wenzhuo.lu, beilei.xing,
	bernard.iremonger

Hi Bing,

On Fri, Jul 10, 2020 at 04:45:22PM +0800, Bing Zhao wrote:
> Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.
> 
> eCPRI is a packet based protocol used in the fronthaul interface of
> 5G networks. Header format definition could be found in the
> specification via the link below:
> https://www.gigalight.com/downloads/standards/ecpri-specification.pdf
> 
> eCPRI message can be over Ethernet layer (.1Q supported also) or over
> UDP layer. Message header formats are the same in these two variants.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst     |   8 ++
>  doc/guides/rel_notes/release_20_08.rst |   5 +
>  lib/librte_ethdev/rte_flow.c           |   1 +
>  lib/librte_ethdev/rte_flow.h           |  31 ++++++
>  lib/librte_net/Makefile                |   1 +
>  lib/librte_net/meson.build             |   3 +-
>  lib/librte_net/rte_ecpri.h             | 182 +++++++++++++++++++++++++++++++++
>  lib/librte_net/rte_ether.h             |   1 +
>  8 files changed, 231 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_net/rte_ecpri.h
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index d5dd18c..669d519 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1362,6 +1362,14 @@ Matches a PFCP Header.
>  - ``seid``: session endpoint identifier.
>  - Default ``mask`` matches s_field and seid.
>  
> +Item: ``ECPRI``
> +^^^^^^^^^^^^^
> +
> +Matches a eCPRI header.
> +
> +- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
> +- Default ``mask`` matches message type of common header only.
> +
>  Actions
>  ~~~~~~~
>  
> diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst
> index 988474c..19feb68 100644
> --- a/doc/guides/rel_notes/release_20_08.rst
> +++ b/doc/guides/rel_notes/release_20_08.rst
> @@ -184,6 +184,11 @@ New Features
>    which are used to access packet data in a safe manner. Currently JIT support
>    for these instructions is implemented for x86 only.
>  
> +* **Added eCPRI protocol support in rte_flow.**
> +
> +  The ``ECPRI`` item have been added to support eCPRI packet offloading for
> +  5G network.
> +
>  * **Added flow performance test application.**
>  
>    Added new application to test ``rte_flow`` performance, including:
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 1685be5..f8fdd68 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -95,6 +95,7 @@ struct rte_flow_desc_data {
>  	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
>  	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
>  	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
> +	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
>  };
>  
>  /** Generate flow_action[] entry. */
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b0e4199..8a90226 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -28,6 +28,7 @@
>  #include <rte_byteorder.h>
>  #include <rte_esp.h>
>  #include <rte_higig.h>
> +#include <rte_ecpri.h>
>  #include <rte_mbuf.h>
>  #include <rte_mbuf_dyn.h>
>  
> @@ -527,6 +528,15 @@ enum rte_flow_item_type {
>  	 */
>  	RTE_FLOW_ITEM_TYPE_PFCP,
>  
> +	/**
> +	 * Matches eCPRI Header.
> +	 *
> +	 * Configure flow for eCPRI over ETH or UDP packets.
> +	 *
> +	 * See struct rte_flow_item_ecpri.
> +	 */
> +	RTE_FLOW_ITEM_TYPE_ECPRI,
> +
>  };
>  
>  /**
> @@ -1547,6 +1557,27 @@ struct rte_flow_item_pfcp {
>  #endif
>  
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ITEM_TYPE_ECPRI
> + *
> + * Match eCPRI Header
> + */
> +struct rte_flow_item_ecpri {
> +	struct rte_ecpri_msg_hdr hdr;
> +};
> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
> +#ifndef __cplusplus
> +static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
> +	.hdr = {
> +		.dw0 = 0x0,
> +	},
> +};
> +#endif
> +
> +/**
>   * Matching pattern item definition.
>   *
>   * A pattern is formed by stacking items starting from the lowest protocol
> diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
> index aa1d6fe..9830e77 100644
> --- a/lib/librte_net/Makefile
> +++ b/lib/librte_net/Makefile
> @@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
>  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
>  
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
> index f799349..24ed825 100644
> --- a/lib/librte_net/meson.build
> +++ b/lib/librte_net/meson.build
> @@ -15,7 +15,8 @@ headers = files('rte_ip.h',
>  	'rte_net.h',
>  	'rte_net_crc.h',
>  	'rte_mpls.h',
> -	'rte_higig.h')
> +	'rte_higig.h',
> +	'rte_ecpri.h')
>  
>  sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
>  deps += ['mbuf']
> diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
> new file mode 100644
> index 0000000..60fb4f7
> --- /dev/null
> +++ b/lib/librte_net/rte_ecpri.h
> @@ -0,0 +1,182 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2020 Mellanox Technologies, Ltd
> + */
> +
> +#ifndef _RTE_ECPRI_H_
> +#define _RTE_ECPRI_H_
> +
> +#include <stdint.h>
> +#include <rte_byteorder.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
> + * Other values are reserved for future
> + */
> +#define RTE_ECPRI_REV_UP_TO_20		1
> +
> +/**
> + * eCPRI message types in specifications
> + * IWF* types will only be supported from rev.2
> + * 12-63: Reserved for future revision
> + * 64-255: Vendor Specific
> + */
> +#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0

Here the doxygen comment applies to RTE_ECPRI_MSG_TYPE_IQ_DATA.

I think it should either be a standard comment (no doxygen), or
something more complex should be done, like grouping:
https://stackoverflow.com/questions/30803156/group-level-documentation-for-preprocessor-defines-in-doxygen
(I didn't try)


> +#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
> +#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
> +#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
> +#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
> +#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
> +#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
> +#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
> +#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
> +#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
> +#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
> +#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
> +
> +/**
> + * Event Type of Message Type #7: Event Indication
> + * 0x00: Fault(s) Indication
> + * 0x01: Fault(s) Indication Acknowledge
> + * 0x02: Notification(s) Indication
> + * 0x03: Synchronization Request
> + * 0x04: Synchronization Acknowledge
> + * 0x05: Synchronization End Indication
> + * 0x06...0xFF: Reserved
> + */
> +#define RTE_ECPRI_EVT_IND_FAULT_IND	0
> +#define RTE_ECPRI_EVT_IND_FAULT_ACK	1
> +#define RTE_ECPRI_EVT_IND_NTFY_IND	2
> +#define RTE_ECPRI_EVT_IND_SYNC_REQ	3
> +#define RTE_ECPRI_EVT_IND_SYNC_ACK	4
> +#define RTE_ECPRI_EVT_IND_SYNC_END	5
> +
> +/**
> + * eCPRI Common Header
> + */
> +RTE_STD_C11
> +struct rte_ecpri_common_hdr {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	uint32_t size:16;		/**< Payload Size */
> +	uint32_t type:8;		/**< Message Type */
> +	uint32_t c:1;			/**< Concatenation Indicator */
> +	uint32_t res:3;			/**< Reserved */
> +	uint32_t revision:4;		/**< Protocol Revision */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	uint32_t revision:4;		/**< Protocol Revision */
> +	uint32_t res:3;			/**< Reserved */
> +	uint32_t c:1;			/**< Concatenation Indicator */
> +	uint32_t type:8;		/**< Message Type */
> +	uint32_t size:16;		/**< Payload Size */
> +#endif
> +} __rte_packed;

Does it really need to be packed? Why next types do not need it?
It looks only those which have bitfields are.


I wonder if the 'dw0' could be in this definition instead of in
struct rte_ecpri_msg_hdr?

Something like this:

struct rte_ecpri_common_hdr {
	union {
		uint32_t u32;
		struct {
			...
		};
	};
};

I see 2 advantages:

- someone that only uses the common_hdr struct can use the .u32
  in its software
- when using it in messages, it looks clearer to me:
    msg.common_hdr.u32 = value;
  instead of:
    msg.dw0 = value;

What do you think?

> +
> +/**
> + * eCPRI Message Header of Type #0: IQ Data
> + */
> +struct rte_ecpri_msg_iq_data {
> +	rte_be16_t pc_id;		/**< Physical channel ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #1: Bit Sequence
> + */
> +struct rte_ecpri_msg_bit_seq {
> +	rte_be16_t pc_id;		/**< Physical channel ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #2: Real-Time Control Data
> + */
> +struct rte_ecpri_msg_rtc_ctrl {
> +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
> +	rte_be16_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #3: Generic Data Transfer
> + */
> +struct rte_ecpri_msg_gen_data {
> +	rte_be32_t pc_id;		/**< Physical channel ID */
> +	rte_be32_t seq_id;		/**< Sequence ID */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #4: Remote Memory Access
> + */
> +RTE_STD_C11
> +struct rte_ecpri_msg_rm_access {
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	uint32_t ele_id:16;		/**< Element ID */
> +	uint32_t rr:4;			/**< Req/Resp */
> +	uint32_t rw:4;			/**< Read/Write */
> +	uint32_t rma_id:8;		/**< Remote Memory Access ID */
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	uint32_t rma_id:8;		/**< Remote Memory Access ID */
> +	uint32_t rw:4;			/**< Read/Write */
> +	uint32_t rr:4;			/**< Req/Resp */
> +	uint32_t ele_id:16;		/**< Element ID */
> +#endif
> +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB) */
> +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> +	rte_be16_t length;		/**< number of bytes */
> +} __rte_packed;
> +
> +/**
> + * eCPRI Message Header of Type #5: One-Way Delay Measurement
> + */
> +struct rte_ecpri_msg_delay_measure {
> +	uint8_t msr_id;			/**< Measurement ID */
> +	uint8_t act_type;		/**< Action Type */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #6: Remote Reset
> + */
> +struct rte_ecpri_msg_remote_reset {
> +	rte_be16_t rst_id;		/**< Reset ID */
> +	uint8_t rst_op;			/**< Reset Code Op */
> +};
> +
> +/**
> + * eCPRI Message Header of Type #7: Event Indication
> + */
> +struct rte_ecpri_msg_event_ind {
> +	uint8_t evt_id;			/**< Event ID */
> +	uint8_t evt_type;		/**< Event Type */
> +	uint8_t seq;			/**< Sequence Number */
> +	uint8_t number;			/**< Number of Faults/Notif */
> +};
> +
> +/**
> + * eCPRI Message Header Format: Common Header + Message Types
> + */
> +RTE_STD_C11
> +struct rte_ecpri_msg_hdr {
> +	union {
> +		struct rte_ecpri_common_hdr common;
> +		uint32_t dw0;
> +	};
> +	union {
> +		struct rte_ecpri_msg_iq_data type0;
> +		struct rte_ecpri_msg_bit_seq type1;
> +		struct rte_ecpri_msg_rtc_ctrl type2;
> +		struct rte_ecpri_msg_bit_seq type3;
> +		struct rte_ecpri_msg_rm_access type4;
> +		struct rte_ecpri_msg_delay_measure type5;
> +		struct rte_ecpri_msg_remote_reset type6;
> +		struct rte_ecpri_msg_event_ind type7;
> +		uint32_t dummy[3];
> +	};
> +};

What is the point in having this struct?

From a software point of view, I think it is a bit risky, because
its size is the size of the largest message. This is probably what
you want in your case, but when a software will rx or tx such
packet, I think they shouldn't use this one. My understanding is
that you only need this structure for the mask in rte_flow.

Also, I'm not sure to understand the purpose of dummy[3], even after reading
your answer to Akhil's question.


> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_ECPRI_H_ */
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index 0ae4e75..184a3f9 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -304,6 +304,7 @@ struct rte_vlan_hdr {
>  #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
>  #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
>  #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
> +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
>  
>  /**
>   * Extract VLAN tag information into mbuf
> -- 
> 1.8.3.1
> 


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

* Re: [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-10 14:31           ` Olivier Matz
@ 2020-07-11  4:25             ` Bing Zhao
  2020-07-12 13:17               ` Olivier Matz
  0 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-11  4:25 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Ori Kam, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, akhil.goyal, dev, wenzhuo.lu,
	beilei.xing, bernard.iremonger

Hi Olivier,
Many thanks for your comments.

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Friday, July 10, 2020 10:31 PM
> To: Bing Zhao <bingz@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; akhil.goyal@nxp.com; dev@dpdk.org;
> wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com
> Subject: Re: [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
> 
> Hi Bing,
> 
> On Fri, Jul 10, 2020 at 04:45:22PM +0800, Bing Zhao wrote:
> > Add a new item "rte_flow_item_ecpri" in order to match eCRPI
> header.
> >
> > eCPRI is a packet based protocol used in the fronthaul interface of 5G
> > networks. Header format definition could be found in the
> specification
> > via the link below:
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> www.
> > gigalight.com%2Fdownloads%2Fstandards%2Fecpri-
> specification.pdf&amp;da
> >
> ta=02%7C01%7Cbingz%40mellanox.com%7C99fe8c970b6648d82f7908
> d824dded33%7
> >
> Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C6372998828794
> 68155&amp;sda
> >
> ta=hv1rAa9I2%2BPpZkoLKzHdEpjrLyvY%2F4IiaEvW3XHnhW4%3D&amp
> ;reserved=0
> >
> > eCPRI message can be over Ethernet layer (.1Q supported also) or
> over
> > UDP layer. Message header formats are the same in these two
> variants.
> >
> > Signed-off-by: Bing Zhao <bingz@mellanox.com>
> > Acked-by: Ori Kam <orika@mellanox.com>
> > ---
> >  doc/guides/prog_guide/rte_flow.rst     |   8 ++
> >  doc/guides/rel_notes/release_20_08.rst |   5 +
> >  lib/librte_ethdev/rte_flow.c           |   1 +
> >  lib/librte_ethdev/rte_flow.h           |  31 ++++++
> >  lib/librte_net/Makefile                |   1 +
> >  lib/librte_net/meson.build             |   3 +-
> >  lib/librte_net/rte_ecpri.h             | 182
> +++++++++++++++++++++++++++++++++
> >  lib/librte_net/rte_ether.h             |   1 +
> >  8 files changed, 231 insertions(+), 1 deletion(-)  create mode 100644
> > lib/librte_net/rte_ecpri.h
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index d5dd18c..669d519 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -1362,6 +1362,14 @@ Matches a PFCP Header.
> >  - ``seid``: session endpoint identifier.
> >  - Default ``mask`` matches s_field and seid.
> >
> > +Item: ``ECPRI``
> > +^^^^^^^^^^^^^
> > +
> > +Matches a eCPRI header.
> > +
> > +- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
> > +- Default ``mask`` matches message type of common header only.
> > +
> >  Actions
> >  ~~~~~~~
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> > b/doc/guides/rel_notes/release_20_08.rst
> > index 988474c..19feb68 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -184,6 +184,11 @@ New Features
> >    which are used to access packet data in a safe manner. Currently
> JIT support
> >    for these instructions is implemented for x86 only.
> >
> > +* **Added eCPRI protocol support in rte_flow.**
> > +
> > +  The ``ECPRI`` item have been added to support eCPRI packet
> > + offloading for  5G network.
> > +
> >  * **Added flow performance test application.**
> >
> >    Added new application to test ``rte_flow`` performance, including:
> > diff --git a/lib/librte_ethdev/rte_flow.c
> > b/lib/librte_ethdev/rte_flow.c index 1685be5..f8fdd68 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -95,6 +95,7 @@ struct rte_flow_desc_data {
> >  	MK_FLOW_ITEM(HIGIG2, sizeof(struct
> rte_flow_item_higig2_hdr)),
> >  	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct
> rte_flow_item_l2tpv3oip)),
> >  	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
> > +	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
> >  };
> >
> >  /** Generate flow_action[] entry. */
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index b0e4199..8a90226 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -28,6 +28,7 @@
> >  #include <rte_byteorder.h>
> >  #include <rte_esp.h>
> >  #include <rte_higig.h>
> > +#include <rte_ecpri.h>
> >  #include <rte_mbuf.h>
> >  #include <rte_mbuf_dyn.h>
> >
> > @@ -527,6 +528,15 @@ enum rte_flow_item_type {
> >  	 */
> >  	RTE_FLOW_ITEM_TYPE_PFCP,
> >
> > +	/**
> > +	 * Matches eCPRI Header.
> > +	 *
> > +	 * Configure flow for eCPRI over ETH or UDP packets.
> > +	 *
> > +	 * See struct rte_flow_item_ecpri.
> > +	 */
> > +	RTE_FLOW_ITEM_TYPE_ECPRI,
> > +
> >  };
> >
> >  /**
> > @@ -1547,6 +1557,27 @@ struct rte_flow_item_pfcp {  #endif
> >
> >  /**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior
> notice
> > + *
> > + * RTE_FLOW_ITEM_TYPE_ECPRI
> > + *
> > + * Match eCPRI Header
> > + */
> > +struct rte_flow_item_ecpri {
> > +	struct rte_ecpri_msg_hdr hdr;
> > +};
> > +
> > +/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */ #ifndef
> __cplusplus
> > +static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask =
> {
> > +	.hdr = {
> > +		.dw0 = 0x0,
> > +	},
> > +};
> > +#endif
> > +
> > +/**
> >   * Matching pattern item definition.
> >   *
> >   * A pattern is formed by stacking items starting from the lowest
> > protocol diff --git a/lib/librte_net/Makefile
> > b/lib/librte_net/Makefile index aa1d6fe..9830e77 100644
> > --- a/lib/librte_net/Makefile
> > +++ b/lib/librte_net/Makefile
> > @@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include
> +=
> > rte_sctp.h rte_icmp.h rte_arp.h
> > SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h
> rte_gre.h
> > rte_net.h  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include +=
> rte_net_crc.h
> > rte_mpls.h rte_higig.h  SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include
> +=
> > rte_gtp.h rte_vxlan.h
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
> >
> >  include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
> > index f799349..24ed825 100644
> > --- a/lib/librte_net/meson.build
> > +++ b/lib/librte_net/meson.build
> > @@ -15,7 +15,8 @@ headers = files('rte_ip.h',
> >  	'rte_net.h',
> >  	'rte_net_crc.h',
> >  	'rte_mpls.h',
> > -	'rte_higig.h')
> > +	'rte_higig.h',
> > +	'rte_ecpri.h')
> >
> >  sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c',
> > 'rte_net_crc.c')  deps += ['mbuf'] diff --git
> > a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h new file
> > mode 100644 index 0000000..60fb4f7
> > --- /dev/null
> > +++ b/lib/librte_net/rte_ecpri.h
> > @@ -0,0 +1,182 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2020 Mellanox Technologies, Ltd  */
> > +
> > +#ifndef _RTE_ECPRI_H_
> > +#define _RTE_ECPRI_H_
> > +
> > +#include <stdint.h>
> > +#include <rte_byteorder.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
> > + * Other values are reserved for future  */
> > +#define RTE_ECPRI_REV_UP_TO_20		1
> > +
> > +/**
> > + * eCPRI message types in specifications
> > + * IWF* types will only be supported from rev.2
> > + * 12-63: Reserved for future revision
> > + * 64-255: Vendor Specific
> > + */
> > +#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
> 
> Here the doxygen comment applies to
> RTE_ECPRI_MSG_TYPE_IQ_DATA.
> 
> I think it should either be a standard comment (no doxygen), or
> something more complex should be done, like grouping:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> stackoverflow.com%2Fquestions%2F30803156%2Fgroup-level-
> documentation-for-preprocessor-defines-in-
> doxygen&amp;data=02%7C01%7Cbingz%40mellanox.com%7C99fe8c9
> 70b6648d82f7908d824dded33%7Ca652971c7d2e4d9ba6a4d149256f46
> 1b%7C0%7C1%7C637299882879478151&amp;sdata=0BGV9rUpTYPpKA
> Thq9BgixZCtnlMVmILVty0f37xJ7I%3D&amp;reserved=0
> (I didn't try)
> 

Thanks, I will have a try to fix this comments.

> 
> > +#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
> > +#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
> > +#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
> > +#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
> > +#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
> > +#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
> > +#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
> > +#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
> > +#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
> > +#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
> > +#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
> > +
> > +/**
> > + * Event Type of Message Type #7: Event Indication
> > + * 0x00: Fault(s) Indication
> > + * 0x01: Fault(s) Indication Acknowledge
> > + * 0x02: Notification(s) Indication
> > + * 0x03: Synchronization Request
> > + * 0x04: Synchronization Acknowledge
> > + * 0x05: Synchronization End Indication
> > + * 0x06...0xFF: Reserved
> > + */
> > +#define RTE_ECPRI_EVT_IND_FAULT_IND	0
> > +#define RTE_ECPRI_EVT_IND_FAULT_ACK	1
> > +#define RTE_ECPRI_EVT_IND_NTFY_IND	2
> > +#define RTE_ECPRI_EVT_IND_SYNC_REQ	3
> > +#define RTE_ECPRI_EVT_IND_SYNC_ACK	4
> > +#define RTE_ECPRI_EVT_IND_SYNC_END	5
> > +
> > +/**
> > + * eCPRI Common Header
> > + */
> > +RTE_STD_C11
> > +struct rte_ecpri_common_hdr {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +	uint32_t size:16;		/**< Payload Size */
> > +	uint32_t type:8;		/**< Message Type */
> > +	uint32_t c:1;			/**< Concatenation Indicator
> */
> > +	uint32_t res:3;			/**< Reserved */
> > +	uint32_t revision:4;		/**< Protocol Revision */
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +	uint32_t revision:4;		/**< Protocol Revision */
> > +	uint32_t res:3;			/**< Reserved */
> > +	uint32_t c:1;			/**< Concatenation Indicator
> */
> > +	uint32_t type:8;		/**< Message Type */
> > +	uint32_t size:16;		/**< Payload Size */
> > +#endif
> > +} __rte_packed;
> 
> Does it really need to be packed? Why next types do not need it?
> It looks only those which have bitfields are.
> 

Nice catch, thanks. For the common header, there is no need to use
the packed attribute, because it is a u32 and the bitfields will be
aligned.
I checked all the definitions again. Only " Type #4: Remote Memory Access"
needs to use the packed attribute.
For other sub-types, "sub-header" part of the message payload will get
aligned by nature. For example, u16 after u16, u8 after u16, these should
be OK.
But in type #4, the address is 48bits wide, with 16bits MSB and 32bits LSB (no
detailed description in the specification, correct me if anything wrong.) Usually,
the 48bits address will be devided as this in a system. And there is no 48-bits
type at all. So we need to define two parts for it: 32b LSB follows 16b MSB.
u32 after u16 should be with packed attribute. Thanks

> 
> I wonder if the 'dw0' could be in this definition instead of in struct
> rte_ecpri_msg_hdr?
> 
> Something like this:
> 
> struct rte_ecpri_common_hdr {
> 	union {
> 		uint32_t u32;
> 		struct {
> 			...
> 		};
> 	};
> };
> 
> I see 2 advantages:
> 
> - someone that only uses the common_hdr struct can use the .u32
>   in its software
> - when using it in messages, it looks clearer to me:
>     msg.common_hdr.u32 = value;
>   instead of:
>     msg.dw0 = value;
> 
> What do you think?

Thanks for the suggestion, this is much better, I will change it.
Indeed, in my original version, no DW(u32) is defined for the header.
After that, I noticed that it would not be easy for the static casting to a u32
from bitfield(the compiler will complain), and it would not be clear to
swap the endian if the user wants to use this header. I added this DW(u32)
to simplify the usage of this header. But yes, if I do not add it here, it would
be not easy or clear for users who just use this header structure.
I will change it. Is it OK if I use the name "dw0"?

> 
> > +
> > +/**
> > + * eCPRI Message Header of Type #0: IQ Data  */ struct
> > +rte_ecpri_msg_iq_data {
> > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > +	rte_be16_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #1: Bit Sequence  */ struct
> > +rte_ecpri_msg_bit_seq {
> > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > +	rte_be16_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #2: Real-Time Control Data  */
> struct
> > +rte_ecpri_msg_rtc_ctrl {
> > +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID
> */
> > +	rte_be16_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #3: Generic Data Transfer  */
> struct
> > +rte_ecpri_msg_gen_data {
> > +	rte_be32_t pc_id;		/**< Physical channel ID */
> > +	rte_be32_t seq_id;		/**< Sequence ID */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #4: Remote Memory Access  */
> > +RTE_STD_C11
> > +struct rte_ecpri_msg_rm_access {
> > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > +	uint32_t ele_id:16;		/**< Element ID */
> > +	uint32_t rr:4;			/**< Req/Resp */
> > +	uint32_t rw:4;			/**< Read/Write */
> > +	uint32_t rma_id:8;		/**< Remote Memory Access
> ID */
> > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > +	uint32_t rma_id:8;		/**< Remote Memory Access
> ID */
> > +	uint32_t rw:4;			/**< Read/Write */
> > +	uint32_t rr:4;			/**< Req/Resp */
> > +	uint32_t ele_id:16;		/**< Element ID */
> > +#endif
> > +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB)
> */
> > +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> > +	rte_be16_t length;		/**< number of bytes */
> > +} __rte_packed;
> > +
> > +/**
> > + * eCPRI Message Header of Type #5: One-Way Delay Measurement
> */
> > +struct rte_ecpri_msg_delay_measure {
> > +	uint8_t msr_id;			/**< Measurement ID */
> > +	uint8_t act_type;		/**< Action Type */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #6: Remote Reset  */ struct
> > +rte_ecpri_msg_remote_reset {
> > +	rte_be16_t rst_id;		/**< Reset ID */
> > +	uint8_t rst_op;			/**< Reset Code Op */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header of Type #7: Event Indication  */ struct
> > +rte_ecpri_msg_event_ind {
> > +	uint8_t evt_id;			/**< Event ID */
> > +	uint8_t evt_type;		/**< Event Type */
> > +	uint8_t seq;			/**< Sequence Number */
> > +	uint8_t number;			/**< Number of
> Faults/Notif */
> > +};
> > +
> > +/**
> > + * eCPRI Message Header Format: Common Header + Message
> Types  */
> > +RTE_STD_C11
> > +struct rte_ecpri_msg_hdr {
> > +	union {
> > +		struct rte_ecpri_common_hdr common;
> > +		uint32_t dw0;
> > +	};
> > +	union {
> > +		struct rte_ecpri_msg_iq_data type0;
> > +		struct rte_ecpri_msg_bit_seq type1;
> > +		struct rte_ecpri_msg_rtc_ctrl type2;
> > +		struct rte_ecpri_msg_bit_seq type3;
> > +		struct rte_ecpri_msg_rm_access type4;
> > +		struct rte_ecpri_msg_delay_measure type5;
> > +		struct rte_ecpri_msg_remote_reset type6;
> > +		struct rte_ecpri_msg_event_ind type7;
> > +		uint32_t dummy[3];
> > +	};
> > +};
> 
> What is the point in having this struct?
> 
> From a software point of view, I think it is a bit risky, because its size is
> the size of the largest message. This is probably what you want in your
> case, but when a software will rx or tx such packet, I think they
> shouldn't use this one. My understanding is that you only need this
> structure for the mask in rte_flow.
> 
> Also, I'm not sure to understand the purpose of dummy[3], even after
> reading your answer to Akhil's question.
> 

Basically YES and no. To my understanding, the eCPRI message format is something
like the ICMP packet format. The message (packet) itself will be parsed into
different formats based on the type of the common header. In the message
payload part, there is no distinct definition of the "sub-header". We can divide
them into the sub-header and data parts based on the specification.
E.g. physical channel ID / real-time control ID / Event ID + type are the parts
that datapath forwarding will only care about. The following timestamp or user data
parts are the parts that the higher layer in the application will use.
1. If an application wants to create some offload flow, or even handle it in the SW, the
common header + first several bytes in the payload should be enough. BUT YES, it is
not good or safe to use it in the higher layer of the application.
2. A higher layer of the application should have its own definition of the whole payload
of a specific sub-type, including the parsing of the user data part after the "sub-header".
It is better for them just skip the first 4 bytes of the eCPRI message or a known offset.
We do not need to cover the upper layers. 
I think some comments could be added here, is it OK?
3. Regarding this structure, I add it because I do not want to introduce a lot of new items
in the rte_flow: new items with structures, new enum types. I prefer one single structure
will cover most of the cases (subtypes). What do you think?
4. About the *dummy* u32, I calculated all the "subheaders" and choose the maximal value
of the length. Two purposes (same as the u32 in the common header):
  a. easy to swap the endianness, but not quite useful. Because some parts are u16 and u8,
    and should not be swapped in a u32. (some physical channel ID and address LSB have 32bits width)
    But if some HW parsed the header u32 by u32, then it would be helpful.
  b. easy for checking in flow API, if the user wants to insert a flow. Some checking should
      be done to confirm if it is wildcard flow (all eCPRI messages or eCPRI message in some specific type),
      or some precise flow (to match the pc id or rtc id, for example). With these fields, 3 DW
      of the masks only need to be check before continuing. Or else, the code needs to check the type
      and a lot of switch-case conditions and go through all different members of each header.

> 
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* _RTE_ECPRI_H_ */
> > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> > index 0ae4e75..184a3f9 100644
> > --- a/lib/librte_net/rte_ether.h
> > +++ b/lib/librte_net/rte_ether.h
> > @@ -304,6 +304,7 @@ struct rte_vlan_hdr {  #define
> RTE_ETHER_TYPE_LLDP
> > 0x88CC /**< LLDP Protocol. */  #define RTE_ETHER_TYPE_MPLS
> 0x8847 /**<
> > MPLS ethertype. */  #define RTE_ETHER_TYPE_MPLSM 0x8848 /**<
> MPLS
> > multicast ethertype. */
> > +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q
> > +supported). */
> >
> >  /**
> >   * Extract VLAN tag information into mbuf
> > --
> > 1.8.3.1
> >


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

* Re: [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-11  4:25             ` Bing Zhao
@ 2020-07-12 13:17               ` Olivier Matz
  2020-07-12 14:28                 ` Bing Zhao
  0 siblings, 1 reply; 28+ messages in thread
From: Olivier Matz @ 2020-07-12 13:17 UTC (permalink / raw)
  To: Bing Zhao
  Cc: Ori Kam, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, akhil.goyal, dev, wenzhuo.lu,
	beilei.xing, bernard.iremonger

Hi Bing,

On Sat, Jul 11, 2020 at 04:25:49AM +0000, Bing Zhao wrote:
> Hi Olivier,
> Many thanks for your comments.

[...]

> > > +/**
> > > + * eCPRI Common Header
> > > + */
> > > +RTE_STD_C11
> > > +struct rte_ecpri_common_hdr {
> > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > +	uint32_t size:16;		/**< Payload Size */
> > > +	uint32_t type:8;		/**< Message Type */
> > > +	uint32_t c:1;			/**< Concatenation Indicator
> > */
> > > +	uint32_t res:3;			/**< Reserved */
> > > +	uint32_t revision:4;		/**< Protocol Revision */
> > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > +	uint32_t revision:4;		/**< Protocol Revision */
> > > +	uint32_t res:3;			/**< Reserved */
> > > +	uint32_t c:1;			/**< Concatenation Indicator
> > */
> > > +	uint32_t type:8;		/**< Message Type */
> > > +	uint32_t size:16;		/**< Payload Size */
> > > +#endif
> > > +} __rte_packed;
> > 
> > Does it really need to be packed? Why next types do not need it?
> > It looks only those which have bitfields are.
> > 
> 
> Nice catch, thanks. For the common header, there is no need to use
> the packed attribute, because it is a u32 and the bitfields will be
> aligned.
> I checked all the definitions again. Only " Type #4: Remote Memory Access"
> needs to use the packed attribute.
> For other sub-types, "sub-header" part of the message payload will get
> aligned by nature. For example, u16 after u16, u8 after u16, these should
> be OK.
> But in type #4, the address is 48bits wide, with 16bits MSB and 32bits LSB (no
> detailed description in the specification, correct me if anything wrong.) Usually,
> the 48bits address will be devided as this in a system. And there is no 48-bits
> type at all. So we need to define two parts for it: 32b LSB follows 16b MSB.
> u32 after u16 should be with packed attribute. Thanks

What about using a bitfield into a uint64_t ? I mean:

	struct rte_ecpri_msg_rm_access {
if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
		...
		uint64_t length:16;		/**< number of bytes */
		uint64_t addr:48;		/**< address */
#else
		...
		uint64_t addr:48;		/**< address */
		uint64_t length:16;		/**< number of bytes */
#endif
	};


> 
> > 
> > I wonder if the 'dw0' could be in this definition instead of in struct
> > rte_ecpri_msg_hdr?
> > 
> > Something like this:
> > 
> > struct rte_ecpri_common_hdr {
> > 	union {
> > 		uint32_t u32;
> > 		struct {
> > 			...
> > 		};
> > 	};
> > };
> > 
> > I see 2 advantages:
> > 
> > - someone that only uses the common_hdr struct can use the .u32
> >   in its software
> > - when using it in messages, it looks clearer to me:
> >     msg.common_hdr.u32 = value;
> >   instead of:
> >     msg.dw0 = value;
> > 
> > What do you think?
> 
> Thanks for the suggestion, this is much better, I will change it.
> Indeed, in my original version, no DW(u32) is defined for the header.
> After that, I noticed that it would not be easy for the static casting to a u32
> from bitfield(the compiler will complain), and it would not be clear to
> swap the endian if the user wants to use this header. I added this DW(u32)
> to simplify the usage of this header. But yes, if I do not add it here, it would
> be not easy or clear for users who just use this header structure.
> I will change it. Is it OK if I use the name "dw0"?

In my opinion, u32 is more usual than dw0.

> 
> > 
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #0: IQ Data  */ struct
> > > +rte_ecpri_msg_iq_data {
> > > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #1: Bit Sequence  */ struct
> > > +rte_ecpri_msg_bit_seq {
> > > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #2: Real-Time Control Data  */
> > struct
> > > +rte_ecpri_msg_rtc_ctrl {
> > > +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID
> > */
> > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #3: Generic Data Transfer  */
> > struct
> > > +rte_ecpri_msg_gen_data {
> > > +	rte_be32_t pc_id;		/**< Physical channel ID */
> > > +	rte_be32_t seq_id;		/**< Sequence ID */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #4: Remote Memory Access  */
> > > +RTE_STD_C11
> > > +struct rte_ecpri_msg_rm_access {
> > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > +	uint32_t ele_id:16;		/**< Element ID */
> > > +	uint32_t rr:4;			/**< Req/Resp */
> > > +	uint32_t rw:4;			/**< Read/Write */
> > > +	uint32_t rma_id:8;		/**< Remote Memory Access
> > ID */
> > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > +	uint32_t rma_id:8;		/**< Remote Memory Access
> > ID */
> > > +	uint32_t rw:4;			/**< Read/Write */
> > > +	uint32_t rr:4;			/**< Req/Resp */
> > > +	uint32_t ele_id:16;		/**< Element ID */
> > > +#endif
> > > +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB)
> > */
> > > +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> > > +	rte_be16_t length;		/**< number of bytes */
> > > +} __rte_packed;
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #5: One-Way Delay Measurement
> > */
> > > +struct rte_ecpri_msg_delay_measure {
> > > +	uint8_t msr_id;			/**< Measurement ID */
> > > +	uint8_t act_type;		/**< Action Type */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #6: Remote Reset  */ struct
> > > +rte_ecpri_msg_remote_reset {
> > > +	rte_be16_t rst_id;		/**< Reset ID */
> > > +	uint8_t rst_op;			/**< Reset Code Op */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header of Type #7: Event Indication  */ struct
> > > +rte_ecpri_msg_event_ind {
> > > +	uint8_t evt_id;			/**< Event ID */
> > > +	uint8_t evt_type;		/**< Event Type */
> > > +	uint8_t seq;			/**< Sequence Number */
> > > +	uint8_t number;			/**< Number of
> > Faults/Notif */
> > > +};
> > > +
> > > +/**
> > > + * eCPRI Message Header Format: Common Header + Message
> > Types  */
> > > +RTE_STD_C11
> > > +struct rte_ecpri_msg_hdr {
> > > +	union {
> > > +		struct rte_ecpri_common_hdr common;
> > > +		uint32_t dw0;
> > > +	};
> > > +	union {
> > > +		struct rte_ecpri_msg_iq_data type0;
> > > +		struct rte_ecpri_msg_bit_seq type1;
> > > +		struct rte_ecpri_msg_rtc_ctrl type2;
> > > +		struct rte_ecpri_msg_bit_seq type3;
> > > +		struct rte_ecpri_msg_rm_access type4;
> > > +		struct rte_ecpri_msg_delay_measure type5;
> > > +		struct rte_ecpri_msg_remote_reset type6;
> > > +		struct rte_ecpri_msg_event_ind type7;
> > > +		uint32_t dummy[3];
> > > +	};
> > > +};
> > 
> > What is the point in having this struct?
> > 
> > From a software point of view, I think it is a bit risky, because its size is
> > the size of the largest message. This is probably what you want in your
> > case, but when a software will rx or tx such packet, I think they
> > shouldn't use this one. My understanding is that you only need this
> > structure for the mask in rte_flow.
> > 
> > Also, I'm not sure to understand the purpose of dummy[3], even after
> > reading your answer to Akhil's question.
> > 
> 
> Basically YES and no. To my understanding, the eCPRI message format is something
> like the ICMP packet format. The message (packet) itself will be parsed into
> different formats based on the type of the common header. In the message
> payload part, there is no distinct definition of the "sub-header". We can divide
> them into the sub-header and data parts based on the specification.
> E.g. physical channel ID / real-time control ID / Event ID + type are the parts
> that datapath forwarding will only care about. The following timestamp or user data
> parts are the parts that the higher layer in the application will use.
> 1. If an application wants to create some offload flow, or even handle it in the SW, the
> common header + first several bytes in the payload should be enough. BUT YES, it is
> not good or safe to use it in the higher layer of the application.
> 2. A higher layer of the application should have its own definition of the whole payload
> of a specific sub-type, including the parsing of the user data part after the "sub-header".
> It is better for them just skip the first 4 bytes of the eCPRI message or a known offset.
> We do not need to cover the upper layers. 

Let me explain what is my vision of how an application would use the
structures (these are completly dummy examples, as I don't know ecpri
protocol at all).

Rx:

	int ecpri_input(struct rte_mbuf *m)
	{
		struct rte_ecpri_common_hdr hdr_copy, *hdr;
		struct rte_ecpri_msg_event_ind event_copy, *event;
		struct app_specific app_copy, *app;

		hdr = rte_pktmbuf_read(m, 0, sizeof(*hdr), &hdr_copy);
		if (unlikely(hdr == NULL))
			return -1;
		switch (hdr->type) {
		...
		case RTE_ECPRI_EVT_IND_NTFY_IND:
			event = rte_pktmbuf_read(m, sizeof(*hdr), sizeof(*event),
				&event_copy);
			if (unlikely(event == NULL))
				return -1;
			...
			app = rte_pktmbuf_read(m, sizeof(*app),
				sizeof(*hdr) + sizeof(*event),
				&app_copy);
			...

Tx:

	int ecpri_output(void)
	{
		struct rte_ecpri_common_hdr *hdr;
		struct rte_ecpri_msg_event_ind *event;
		struct app_specific *app;

		m = rte_pktmbuf_alloc(mp);
		if (unlikely(m == NULL))
			return -1;

		app = rte_pktmbuf_append(m, sizeof(*data));
		if (app == NULL)
			...
		app->... = ...;
		...
		event = rte_pktmbuf_prepend(m, sizeof(*event));
		if (event == NULL)
			...
		event->... = ...;
		...
		hdr = rte_pktmbuf_prepend(m, sizeof(*hdr));
		if (hdr == NULL)
			...
		hdr->... = ...;

		return packet_send(m);
	}

In these 2 examples, we never need the unioned structure (struct
rte_ecpri_msg_hdr).

Using it does not look possible to me, because its size is corresponds
to the largest message, not to the one we are parsing/building.

> I think some comments could be added here, is it OK?
> 3. Regarding this structure, I add it because I do not want to introduce a lot of new items
> in the rte_flow: new items with structures, new enum types. I prefer one single structure
> will cover most of the cases (subtypes). What do you think?
> 4. About the *dummy* u32, I calculated all the "subheaders" and choose the maximal value
> of the length. Two purposes (same as the u32 in the common header):
>   a. easy to swap the endianness, but not quite useful. Because some parts are u16 and u8,
>     and should not be swapped in a u32. (some physical channel ID and address LSB have 32bits width)
>     But if some HW parsed the header u32 by u32, then it would be helpful.
>   b. easy for checking in flow API, if the user wants to insert a flow. Some checking should
>       be done to confirm if it is wildcard flow (all eCPRI messages or eCPRI message in some specific type),
>       or some precise flow (to match the pc id or rtc id, for example). With these fields, 3 DW
>       of the masks only need to be check before continuing. Or else, the code needs to check the type
>       and a lot of switch-case conditions and go through all different members of each header.

Thanks for the clarification.

I'll tend to say that if the rte_ecpri_msg_hdr structure is only
useful for rte_flow, it should be defined inside rte_flow.

However, I have some fears about the dummy[3]. You said it could be
enlarged later: I think it is dangerous to change the size of a
structure that may be used to parse data (and this would be an ABI
change). Also, it seems dummy[3] cannot be used easily to swap
endianness, so is it really useful?


Thanks,
Olivier


> > > +
> > > +#ifdef __cplusplus
> > > +}
> > > +#endif
> > > +
> > > +#endif /* _RTE_ECPRI_H_ */
> > > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> > > index 0ae4e75..184a3f9 100644
> > > --- a/lib/librte_net/rte_ether.h
> > > +++ b/lib/librte_net/rte_ether.h
> > > @@ -304,6 +304,7 @@ struct rte_vlan_hdr {  #define
> > RTE_ETHER_TYPE_LLDP
> > > 0x88CC /**< LLDP Protocol. */  #define RTE_ETHER_TYPE_MPLS
> > 0x8847 /**<
> > > MPLS ethertype. */  #define RTE_ETHER_TYPE_MPLSM 0x8848 /**<
> > MPLS
> > > multicast ethertype. */
> > > +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q
> > > +supported). */
> > >
> > >  /**
> > >   * Extract VLAN tag information into mbuf
> > > --
> > > 1.8.3.1
> > >
> 

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

* [dpdk-dev] [PATCH v6 0/2] rte_flow: introduce eCPRI item for rte_flow
  2020-07-10  8:45       ` [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
  2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
@ 2020-07-12 13:35         ` Bing Zhao
  2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
  2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
  2 siblings, 2 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-12 13:35 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz, akhil.goyal
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

This patch set contains two commits.
1. header definition of the ethdev API
2. testpmd support for the eCPRI flow item

---
v2: Add dw0 for the eCPRI common header to switch the endianess, and
    use fixed u32 value with big-endian for rte_flow_item_ecpri_mask.
    It is due to the fact that global variable only support constant
    expression in C when building.
v3: Add commit for testpmd support.
v4: update release notes part.
v5: fix type#6 define, add event indication macros, and comments for
    revisions.
v6: 1. change the doxygen format into standard format
    2. change the members definition of 'struct rte_ecpri_common_hdr'
    3. fix the missing '^' in prog guide
    4. change the address define in type #4, removed '__packed__'
    5. change the name of 'rte_ecpri_msg_hdr'
       to 'rte_ecpri_combined_msg_hdr'
---

Bing Zhao (2):
  rte_flow: add eCPRI key fields to flow API
  app/testpmd: add eCPRI in flow creation patterns

 app/test-pmd/cmdline_flow.c            | 144 ++++++++++++++++++++++++++
 doc/guides/prog_guide/rte_flow.rst     |   8 ++
 doc/guides/rel_notes/release_20_08.rst |   5 +
 lib/librte_ethdev/rte_flow.c           |   1 +
 lib/librte_ethdev/rte_flow.h           |  33 ++++++
 lib/librte_net/Makefile                |   1 +
 lib/librte_net/meson.build             |   3 +-
 lib/librte_net/rte_ecpri.h             | 183 +++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h             |   1 +
 9 files changed, 378 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-12 13:35         ` [dpdk-dev] [PATCH v6 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
@ 2020-07-12 13:35           ` Bing Zhao
  2020-07-12 14:45             ` Olivier Matz
  2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
  1 sibling, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-12 13:35 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz, akhil.goyal
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.

eCPRI is a packet based protocol used in the fronthaul interface of
5G networks. Header format definition could be found in the
specification via the link below:
https://www.gigalight.com/downloads/standards/ecpri-specification.pdf

eCPRI message can be over Ethernet layer (.1Q supported also) or over
UDP layer. Message header formats are the same in these two variants.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
v2: Add dw0 for the eCPRI common header to switch the endianess, and
    use fixed u32 value with big-endian for rte_flow_item_ecpri_mask.
    It is due to the fact that global variable only support constant
    expression in C when building.
v4: update release notes part.
v5: fix type#6 define, add event indication macros, and comments for
    revisions.
v6: 1. change the doxygen format into standard format
    2. change the members definition of 'struct rte_ecpri_common_hdr'
    3. fix the missing '^' in prog guide
    4. change the address define in type #4, removed '__packed__'
    5. change the name of 'rte_ecpri_msg_hdr'
       to 'rte_ecpri_combined_msg_hdr'
---
 doc/guides/prog_guide/rte_flow.rst     |   8 ++
 doc/guides/rel_notes/release_20_08.rst |   5 +
 lib/librte_ethdev/rte_flow.c           |   1 +
 lib/librte_ethdev/rte_flow.h           |  33 ++++++
 lib/librte_net/Makefile                |   1 +
 lib/librte_net/meson.build             |   3 +-
 lib/librte_net/rte_ecpri.h             | 183 +++++++++++++++++++++++++++++++++
 lib/librte_net/rte_ether.h             |   1 +
 8 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_net/rte_ecpri.h

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index d5dd18c..3e5cd1e 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1362,6 +1362,14 @@ Matches a PFCP Header.
 - ``seid``: session endpoint identifier.
 - Default ``mask`` matches s_field and seid.
 
+Item: ``ECPRI``
+^^^^^^^^^^^^^^^
+
+Matches a eCPRI header.
+
+- ``hdr``: eCPRI header definition (``rte_ecpri.h``).
+- Default ``mask`` matches nothing, for all eCPRI messages.
+
 Actions
 ~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst
index 3e07ee6..67888aa 100644
--- a/doc/guides/rel_notes/release_20_08.rst
+++ b/doc/guides/rel_notes/release_20_08.rst
@@ -205,6 +205,11 @@ New Features
   the  ``5tswap`` swaps source and destination in layers 2,3,4
   for ipv4 and ipv6 in L3 and UDP and TCP in L4.
 
+* **Added eCPRI protocol support in rte_flow.**
+
+  The ``ECPRI`` item have been added to support eCPRI packet offloading for
+  5G network.
+
 * **Added flow performance test application.**
 
   Added new application to test ``rte_flow`` performance, including:
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5..f8fdd68 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -95,6 +95,7 @@ struct rte_flow_desc_data {
 	MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)),
 	MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct rte_flow_item_l2tpv3oip)),
 	MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
+	MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..da8bfa5 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -28,6 +28,7 @@
 #include <rte_byteorder.h>
 #include <rte_esp.h>
 #include <rte_higig.h>
+#include <rte_ecpri.h>
 #include <rte_mbuf.h>
 #include <rte_mbuf_dyn.h>
 
@@ -527,6 +528,15 @@ enum rte_flow_item_type {
 	 */
 	RTE_FLOW_ITEM_TYPE_PFCP,
 
+	/**
+	 * Matches eCPRI Header.
+	 *
+	 * Configure flow for eCPRI over ETH or UDP packets.
+	 *
+	 * See struct rte_flow_item_ecpri.
+	 */
+	RTE_FLOW_ITEM_TYPE_ECPRI,
+
 };
 
 /**
@@ -1547,6 +1557,29 @@ struct rte_flow_item_pfcp {
 #endif
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_ECPRI
+ *
+ * Match eCPRI Header
+ */
+struct rte_flow_item_ecpri {
+	struct rte_ecpri_combined_msg_hdr hdr;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */
+#ifndef __cplusplus
+static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask = {
+	.hdr = {
+		.common = {
+			.u32 = 0x0,
+		},
+	},
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index aa1d6fe..9830e77 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h rte_higig.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_gtp.h rte_vxlan.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ecpri.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index f799349..24ed825 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -15,7 +15,8 @@ headers = files('rte_ip.h',
 	'rte_net.h',
 	'rte_net_crc.h',
 	'rte_mpls.h',
-	'rte_higig.h')
+	'rte_higig.h',
+	'rte_ecpri.h')
 
 sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h
new file mode 100644
index 0000000..d1c4838
--- /dev/null
+++ b/lib/librte_net/rte_ecpri.h
@@ -0,0 +1,183 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef _RTE_ECPRI_H_
+#define _RTE_ECPRI_H_
+
+#include <stdint.h>
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+ * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b
+ * Other values are reserved for future
+ */
+#define RTE_ECPRI_REV_UP_TO_20		1
+
+/*
+ * eCPRI message types in specifications
+ * IWF* types will only be supported from rev.2
+ * 12-63: Reserved for future revision
+ * 64-255: Vendor Specific
+ */
+#define RTE_ECPRI_MSG_TYPE_IQ_DATA	0
+#define RTE_ECPRI_MSG_TYPE_BIT_SEQ	1
+#define RTE_ECPRI_MSG_TYPE_RTC_CTRL	2
+#define RTE_ECPRI_MSG_TYPE_GEN_DATA	3
+#define RTE_ECPRI_MSG_TYPE_RM_ACC	4
+#define RTE_ECPRI_MSG_TYPE_DLY_MSR	5
+#define RTE_ECPRI_MSG_TYPE_RMT_RST	6
+#define RTE_ECPRI_MSG_TYPE_EVT_IND	7
+#define RTE_ECPRI_MSG_TYPE_IWF_UP	8
+#define RTE_ECPRI_MSG_TYPE_IWF_OPT	9
+#define RTE_ECPRI_MSG_TYPE_IWF_MAP	10
+#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL	11
+
+/*
+ * Event Type of Message Type #7: Event Indication
+ * 0x00: Fault(s) Indication
+ * 0x01: Fault(s) Indication Acknowledge
+ * 0x02: Notification(s) Indication
+ * 0x03: Synchronization Request
+ * 0x04: Synchronization Acknowledge
+ * 0x05: Synchronization End Indication
+ * 0x06...0xFF: Reserved
+ */
+#define RTE_ECPRI_EVT_IND_FAULT_IND	0x00
+#define RTE_ECPRI_EVT_IND_FAULT_ACK	0x01
+#define RTE_ECPRI_EVT_IND_NTFY_IND	0x02
+#define RTE_ECPRI_EVT_IND_SYNC_REQ	0x03
+#define RTE_ECPRI_EVT_IND_SYNC_ACK	0x04
+#define RTE_ECPRI_EVT_IND_SYNC_END	0x05
+
+/**
+ * eCPRI Common Header
+ */
+RTE_STD_C11
+struct rte_ecpri_common_hdr {
+	union {
+		rte_be32_t u32;			/**< 4B common header in BE */
+		struct {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+			uint32_t size:16;	/**< Payload Size */
+			uint32_t type:8;	/**< Message Type */
+			uint32_t c:1;		/**< Concatenation Indicator */
+			uint32_t res:3;		/**< Reserved */
+			uint32_t revision:4;	/**< Protocol Revision */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+			uint32_t revision:4;	/**< Protocol Revision */
+			uint32_t res:3;		/**< Reserved */
+			uint32_t c:1;		/**< Concatenation Indicator */
+			uint32_t type:8;	/**< Message Type */
+			uint32_t size:16;	/**< Payload Size */
+#endif
+		};
+	};
+};
+
+/**
+ * eCPRI Message Header of Type #0: IQ Data
+ */
+struct rte_ecpri_msg_iq_data {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #1: Bit Sequence
+ */
+struct rte_ecpri_msg_bit_seq {
+	rte_be16_t pc_id;		/**< Physical channel ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #2: Real-Time Control Data
+ */
+struct rte_ecpri_msg_rtc_ctrl {
+	rte_be16_t rtc_id;		/**< Real-Time Control Data ID */
+	rte_be16_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #3: Generic Data Transfer
+ */
+struct rte_ecpri_msg_gen_data {
+	rte_be32_t pc_id;		/**< Physical channel ID */
+	rte_be32_t seq_id;		/**< Sequence ID */
+};
+
+/**
+ * eCPRI Message Header of Type #4: Remote Memory Access
+ */
+RTE_STD_C11
+struct rte_ecpri_msg_rm_access {
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	uint32_t ele_id:16;		/**< Element ID */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	uint32_t rma_id:8;		/**< Remote Memory Access ID */
+	uint32_t rw:4;			/**< Read/Write */
+	uint32_t rr:4;			/**< Req/Resp */
+	uint32_t ele_id:16;		/**< Element ID */
+#endif
+	uint8_t addr[6];		/**< 48-bits address */
+	rte_be16_t length;		/**< number of bytes */
+};
+
+/**
+ * eCPRI Message Header of Type #5: One-Way Delay Measurement
+ */
+struct rte_ecpri_msg_delay_measure {
+	uint8_t msr_id;			/**< Measurement ID */
+	uint8_t act_type;		/**< Action Type */
+};
+
+/**
+ * eCPRI Message Header of Type #6: Remote Reset
+ */
+struct rte_ecpri_msg_remote_reset {
+	rte_be16_t rst_id;		/**< Reset ID */
+	uint8_t rst_op;			/**< Reset Code Op */
+};
+
+/**
+ * eCPRI Message Header of Type #7: Event Indication
+ */
+struct rte_ecpri_msg_event_ind {
+	uint8_t evt_id;			/**< Event ID */
+	uint8_t evt_type;		/**< Event Type */
+	uint8_t seq;			/**< Sequence Number */
+	uint8_t number;			/**< Number of Faults/Notif */
+};
+
+/**
+ * eCPRI Combined Message Header Format: Common Header + Message Types
+ */
+RTE_STD_C11
+struct rte_ecpri_combined_msg_hdr {
+	struct rte_ecpri_common_hdr common;
+	union {
+		struct rte_ecpri_msg_iq_data type0;
+		struct rte_ecpri_msg_bit_seq type1;
+		struct rte_ecpri_msg_rtc_ctrl type2;
+		struct rte_ecpri_msg_bit_seq type3;
+		struct rte_ecpri_msg_rm_access type4;
+		struct rte_ecpri_msg_delay_measure type5;
+		struct rte_ecpri_msg_remote_reset type6;
+		struct rte_ecpri_msg_event_ind type7;
+		rte_be32_t dummy[3];
+	};
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ECPRI_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index e942556..a358e88 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -307,6 +307,7 @@ struct rte_vlan_hdr {
 #define RTE_ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
 #define RTE_ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
 #define RTE_ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
+#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype (.1Q supported). */
 
 /**
  * Extract VLAN tag information into mbuf
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v6 2/2] app/testpmd: add eCPRI in flow creation patterns
  2020-07-12 13:35         ` [dpdk-dev] [PATCH v6 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
  2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-12 13:35           ` Bing Zhao
  1 sibling, 0 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-12 13:35 UTC (permalink / raw)
  To: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, olivier.matz, akhil.goyal
  Cc: dev, wenzhuo.lu, beilei.xing, bernard.iremonger

In order to verify offloading of eCPRI protocol via flow rules, the
command line of flow creation should support the parsing of the eCPRI
pattern.

Based on the specification, one eCPRI message will have the common
header and payload. Payload format is various based on the type field
of the common header. Fixed strings will be used instead of integer
to make the CLI easy for auto-completion.

The testpmd command line examples of flow to match eCPRI item are
listed below:
  1. flow create 0 ... pattern eth / ecpri / end actions ...
    This is to match all eCPRI messages.
  2. flow create 0 ... pattern eth / ecpri common type rtc_ctrl / end actions ...
    This is to match all eCPRI messages with the type #2 - "Real-Time
    Control Data".
  3. flow create 0 ... pattern eth / ecpri common type iq_data pc_id is [U16Int] / end actions ...
    This is to match eCPRI messages with the type #0 - "IQ Data", and
    the physical channel ID 'pc_id' of the messages is a specific
    value. Since the sequence ID is changeable, there is no need to
    match that field in the flow.
Currently, only type #0, #2 and #5 will be supported.

Since eCPRI could be over Ethernet layer (or after .1Q) and UDP
layer, it is the PMD driver's responsibility to check whether eCPRI
is supported and which protocol stack is supported. Network byte
order should be used for eCPRI header, the same as other headers.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
v3: Add commit for testpmd support.
v6: change the members definition of 'struct rte_ecpri_common_hdr'
---
 app/test-pmd/cmdline_flow.c | 144 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 4e2006c..6263d30 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -230,6 +230,15 @@ enum index {
 	ITEM_PFCP,
 	ITEM_PFCP_S_FIELD,
 	ITEM_PFCP_SEID,
+	ITEM_ECPRI,
+	ITEM_ECPRI_COMMON,
+	ITEM_ECPRI_COMMON_TYPE,
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ITEM_ECPRI_MSG_IQ_DATA_PCID,
+	ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+	ITEM_ECPRI_MSG_DLY_MSR_MSRID,
 
 	/* Validate/create actions. */
 	ACTIONS,
@@ -791,6 +800,7 @@ struct parse_action_priv {
 	ITEM_ESP,
 	ITEM_AH,
 	ITEM_PFCP,
+	ITEM_ECPRI,
 	END_SET,
 	ZERO,
 };
@@ -1101,6 +1111,24 @@ struct parse_action_priv {
 	ZERO,
 };
 
+static const enum index item_ecpri[] = {
+	ITEM_ECPRI_COMMON,
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_ecpri_common[] = {
+	ITEM_ECPRI_COMMON_TYPE,
+	ZERO,
+};
+
+static const enum index item_ecpri_common_type[] = {
+	ITEM_ECPRI_COMMON_TYPE_IQ_DATA,
+	ITEM_ECPRI_COMMON_TYPE_RTC_CTRL,
+	ITEM_ECPRI_COMMON_TYPE_DLY_MSR,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -1409,6 +1437,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_item_ecpri_type(struct context *, const struct token *,
+				    const char *, unsigned int,
+				    void *, unsigned int);
 static int parse_vc_action_rss(struct context *, const struct token *,
 			       const char *, unsigned int, void *,
 			       unsigned int);
@@ -2802,6 +2833,66 @@ static int comp_set_raw_index(struct context *, const struct token *,
 		.next = NEXT(item_pfcp, NEXT_ENTRY(UNSIGNED), item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_pfcp, seid)),
 	},
+	[ITEM_ECPRI] = {
+		.name = "ecpri",
+		.help = "match eCPRI header",
+		.priv = PRIV_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
+		.next = NEXT(item_ecpri),
+		.call = parse_vc,
+	},
+	[ITEM_ECPRI_COMMON] = {
+		.name = "common",
+		.help = "eCPRI common header",
+		.next = NEXT(item_ecpri_common),
+	},
+	[ITEM_ECPRI_COMMON_TYPE] = {
+		.name = "type",
+		.help = "type of common header",
+		.next = NEXT(item_ecpri_common_type),
+		.args = ARGS(ARG_ENTRY_HTON(struct rte_flow_item_ecpri)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_IQ_DATA] = {
+		.name = "iq_data",
+		.help = "Type #0: IQ Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_IQ_DATA_PCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_IQ_DATA_PCID] = {
+		.name = "pc_id",
+		.help = "Physical Channel ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type0.pc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_RTC_CTRL] = {
+		.name = "rtc_ctrl",
+		.help = "Type #2: Real-Time Control Data",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_RTC_CTRL_RTCID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_RTC_CTRL_RTCID] = {
+		.name = "rtc_id",
+		.help = "Real-Time Control Data ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type2.rtc_id)),
+	},
+	[ITEM_ECPRI_COMMON_TYPE_DLY_MSR] = {
+		.name = "delay_measure",
+		.help = "Type #5: One-Way Delay Measurement",
+		.next = NEXT(NEXT_ENTRY(ITEM_ECPRI_MSG_DLY_MSR_MSRID,
+					ITEM_NEXT)),
+		.call = parse_vc_item_ecpri_type,
+	},
+	[ITEM_ECPRI_MSG_DLY_MSR_MSRID] = {
+		.name = "msr_id",
+		.help = "Measurement ID",
+		.next = NEXT(item_ecpri, NEXT_ENTRY(UNSIGNED), item_param),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ecpri,
+				hdr.type5.msr_id)),
+	},
 	/* Validate/create actions. */
 	[ACTIONS] = {
 		.name = "actions",
@@ -4124,6 +4215,59 @@ static int comp_set_raw_index(struct context *, const struct token *,
 	return len;
 }
 
+/** Parse eCPRI common header type field. */
+static int
+parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
+			 const char *str, unsigned int len,
+			 void *buf, unsigned int size)
+{
+	struct rte_flow_item_ecpri *ecpri;
+	struct rte_flow_item_ecpri *ecpri_mask;
+	struct rte_flow_item *item;
+	uint32_t data_size;
+	uint8_t msg_type;
+	struct buffer *out = buf;
+	const struct arg *arg;
+
+	(void)size;
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	switch (ctx->curr) {
+	case ITEM_ECPRI_COMMON_TYPE_IQ_DATA:
+		msg_type = RTE_ECPRI_MSG_TYPE_IQ_DATA;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_RTC_CTRL:
+		msg_type = RTE_ECPRI_MSG_TYPE_RTC_CTRL;
+		break;
+	case ITEM_ECPRI_COMMON_TYPE_DLY_MSR:
+		msg_type = RTE_ECPRI_MSG_TYPE_DLY_MSR;
+		break;
+	default:
+		return -1;
+	}
+	if (!ctx->object)
+		return len;
+	arg = pop_args(ctx);
+	if (!arg)
+		return -1;
+	ecpri = (struct rte_flow_item_ecpri *)out->args.vc.data;
+	ecpri->hdr.common.type = msg_type;
+	data_size = ctx->objdata / 3; /* spec, last, mask */
+	ecpri_mask = (struct rte_flow_item_ecpri *)(out->args.vc.data +
+						    (data_size * 2));
+	ecpri_mask->hdr.common.type = 0xFF;
+	if (arg->hton) {
+		ecpri->hdr.common.u32 = rte_cpu_to_be_32(ecpri->hdr.common.u32);
+		ecpri_mask->hdr.common.u32 =
+				rte_cpu_to_be_32(ecpri_mask->hdr.common.u32);
+	}
+	item = &out->args.vc.pattern[out->args.vc.pattern_n - 1];
+	item->spec = ecpri;
+	item->mask = ecpri_mask;
+	return len;
+}
+
 /** Parse RSS action. */
 static int
 parse_vc_action_rss(struct context *ctx, const struct token *token,
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-12 13:17               ` Olivier Matz
@ 2020-07-12 14:28                 ` Bing Zhao
  2020-07-12 14:43                   ` Olivier Matz
  0 siblings, 1 reply; 28+ messages in thread
From: Bing Zhao @ 2020-07-12 14:28 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Ori Kam, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, akhil.goyal, dev, wenzhuo.lu,
	beilei.xing, bernard.iremonger

Hi Olivier,
Thanks

BR. Bing

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Sunday, July 12, 2020 9:18 PM
> To: Bing Zhao <bingz@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; akhil.goyal@nxp.com; dev@dpdk.org;
> wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com
> Subject: Re: [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
> 
> Hi Bing,
> 
> On Sat, Jul 11, 2020 at 04:25:49AM +0000, Bing Zhao wrote:
> > Hi Olivier,
> > Many thanks for your comments.
> 
> [...]
> 
> > > > +/**
> > > > + * eCPRI Common Header
> > > > + */
> > > > +RTE_STD_C11
> > > > +struct rte_ecpri_common_hdr {
> > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > > +	uint32_t size:16;		/**< Payload Size */
> > > > +	uint32_t type:8;		/**< Message Type */
> > > > +	uint32_t c:1;			/**< Concatenation Indicator
> > > */
> > > > +	uint32_t res:3;			/**< Reserved */
> > > > +	uint32_t revision:4;		/**< Protocol Revision */
> > > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > > +	uint32_t revision:4;		/**< Protocol Revision */
> > > > +	uint32_t res:3;			/**< Reserved */
> > > > +	uint32_t c:1;			/**< Concatenation Indicator
> > > */
> > > > +	uint32_t type:8;		/**< Message Type */
> > > > +	uint32_t size:16;		/**< Payload Size */
> > > > +#endif
> > > > +} __rte_packed;
> > >
> > > Does it really need to be packed? Why next types do not need it?
> > > It looks only those which have bitfields are.
> > >
> >
> > Nice catch, thanks. For the common header, there is no need to use
> the
> > packed attribute, because it is a u32 and the bitfields will be
> > aligned.
> > I checked all the definitions again. Only " Type #4: Remote Memory
> Access"
> > needs to use the packed attribute.
> > For other sub-types, "sub-header" part of the message payload will
> get
> > aligned by nature. For example, u16 after u16, u8 after u16, these
> > should be OK.
> > But in type #4, the address is 48bits wide, with 16bits MSB and 32bits
> > LSB (no detailed description in the specification, correct me if
> > anything wrong.) Usually, the 48bits address will be devided as this
> > in a system. And there is no 48-bits type at all. So we need to define
> two parts for it: 32b LSB follows 16b MSB.
> > u32 after u16 should be with packed attribute. Thanks
> 
> What about using a bitfield into a uint64_t ? I mean:
> 
> 	struct rte_ecpri_msg_rm_access {
> if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> 		...
> 		uint64_t length:16;		/**< number of bytes
> */
> 		uint64_t addr:48;		/**< address */
> #else
> 		...
> 		uint64_t addr:48;		/**< address */
> 		uint64_t length:16;		/**< number of bytes
> */
> #endif
> 	};
> 

Thanks for your suggestion.
https://stackoverflow.com/questions/10906238/warning-when-using-bitfield-with-unsigned-char
AFAIK (from this page), bitfields support only support bool and int. uint64_t is some type of "long"
and most of the compilers should support it. But I am not sure if it is a standard implementation.

> 
> >
> > >
> > > I wonder if the 'dw0' could be in this definition instead of in
> > > struct rte_ecpri_msg_hdr?
> > >
> > > Something like this:
> > >
> > > struct rte_ecpri_common_hdr {
> > > 	union {
> > > 		uint32_t u32;
> > > 		struct {
> > > 			...
> > > 		};
> > > 	};
> > > };
> > >
> > > I see 2 advantages:
> > >
> > > - someone that only uses the common_hdr struct can use the .u32
> > >   in its software
> > > - when using it in messages, it looks clearer to me:
> > >     msg.common_hdr.u32 = value;
> > >   instead of:
> > >     msg.dw0 = value;
> > >
> > > What do you think?
> >
> > Thanks for the suggestion, this is much better, I will change it.
> > Indeed, in my original version, no DW(u32) is defined for the header.
> > After that, I noticed that it would not be easy for the static casting
> > to a u32 from bitfield(the compiler will complain), and it would not
> > be clear to swap the endian if the user wants to use this header. I
> > added this DW(u32) to simplify the usage of this header. But yes, if I
> > do not add it here, it would be not easy or clear for users who just
> use this header structure.
> > I will change it. Is it OK if I use the name "dw0"?
> 
> In my opinion, u32 is more usual than dw0.
> 

I sent patch set v6 with this change, thanks.

> >
> > >
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #0: IQ Data  */ struct
> > > > +rte_ecpri_msg_iq_data {
> > > > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #1: Bit Sequence  */ struct
> > > > +rte_ecpri_msg_bit_seq {
> > > > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #2: Real-Time Control Data  */
> > > struct
> > > > +rte_ecpri_msg_rtc_ctrl {
> > > > +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID
> > > */
> > > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #3: Generic Data Transfer  */
> > > struct
> > > > +rte_ecpri_msg_gen_data {
> > > > +	rte_be32_t pc_id;		/**< Physical channel ID */
> > > > +	rte_be32_t seq_id;		/**< Sequence ID */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #4: Remote Memory Access
> */
> > > > +RTE_STD_C11
> > > > +struct rte_ecpri_msg_rm_access {
> > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > > +	uint32_t ele_id:16;		/**< Element ID */
> > > > +	uint32_t rr:4;			/**< Req/Resp */
> > > > +	uint32_t rw:4;			/**< Read/Write */
> > > > +	uint32_t rma_id:8;		/**< Remote Memory Access
> > > ID */
> > > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > > +	uint32_t rma_id:8;		/**< Remote Memory Access
> > > ID */
> > > > +	uint32_t rw:4;			/**< Read/Write */
> > > > +	uint32_t rr:4;			/**< Req/Resp */
> > > > +	uint32_t ele_id:16;		/**< Element ID */
> > > > +#endif
> > > > +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB)
> > > */
> > > > +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> > > > +	rte_be16_t length;		/**< number of bytes */
> > > > +} __rte_packed;
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #5: One-Way Delay
> Measurement
> > > */
> > > > +struct rte_ecpri_msg_delay_measure {
> > > > +	uint8_t msr_id;			/**< Measurement ID */
> > > > +	uint8_t act_type;		/**< Action Type */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #6: Remote Reset  */ struct
> > > > +rte_ecpri_msg_remote_reset {
> > > > +	rte_be16_t rst_id;		/**< Reset ID */
> > > > +	uint8_t rst_op;			/**< Reset Code Op */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header of Type #7: Event Indication  */ struct
> > > > +rte_ecpri_msg_event_ind {
> > > > +	uint8_t evt_id;			/**< Event ID */
> > > > +	uint8_t evt_type;		/**< Event Type */
> > > > +	uint8_t seq;			/**< Sequence Number */
> > > > +	uint8_t number;			/**< Number of
> > > Faults/Notif */
> > > > +};
> > > > +
> > > > +/**
> > > > + * eCPRI Message Header Format: Common Header + Message
> > > Types  */
> > > > +RTE_STD_C11
> > > > +struct rte_ecpri_msg_hdr {
> > > > +	union {
> > > > +		struct rte_ecpri_common_hdr common;
> > > > +		uint32_t dw0;
> > > > +	};
> > > > +	union {
> > > > +		struct rte_ecpri_msg_iq_data type0;
> > > > +		struct rte_ecpri_msg_bit_seq type1;
> > > > +		struct rte_ecpri_msg_rtc_ctrl type2;
> > > > +		struct rte_ecpri_msg_bit_seq type3;
> > > > +		struct rte_ecpri_msg_rm_access type4;
> > > > +		struct rte_ecpri_msg_delay_measure type5;
> > > > +		struct rte_ecpri_msg_remote_reset type6;
> > > > +		struct rte_ecpri_msg_event_ind type7;
> > > > +		uint32_t dummy[3];
> > > > +	};
> > > > +};
> > >
> > > What is the point in having this struct?
> > >
> > > From a software point of view, I think it is a bit risky, because
> > > its size is the size of the largest message. This is probably what
> > > you want in your case, but when a software will rx or tx such
> > > packet, I think they shouldn't use this one. My understanding is
> > > that you only need this structure for the mask in rte_flow.
> > >
> > > Also, I'm not sure to understand the purpose of dummy[3], even
> after
> > > reading your answer to Akhil's question.
> > >
> >
> > Basically YES and no. To my understanding, the eCPRI message
> format is
> > something like the ICMP packet format. The message (packet) itself
> > will be parsed into different formats based on the type of the
> common
> > header. In the message payload part, there is no distinct definition
> > of the "sub-header". We can divide them into the sub-header and
> data parts based on the specification.
> > E.g. physical channel ID / real-time control ID / Event ID + type are
> > the parts that datapath forwarding will only care about. The
> following
> > timestamp or user data parts are the parts that the higher layer in
> the application will use.
> > 1. If an application wants to create some offload flow, or even
> handle
> > it in the SW, the common header + first several bytes in the payload
> > should be enough. BUT YES, it is not good or safe to use it in the
> higher layer of the application.
> > 2. A higher layer of the application should have its own definition of
> > the whole payload of a specific sub-type, including the parsing of the
> user data part after the "sub-header".
> > It is better for them just skip the first 4 bytes of the eCPRI message or
> a known offset.
> > We do not need to cover the upper layers.
> 
> Let me explain what is my vision of how an application would use the
> structures (these are completly dummy examples, as I don't know
> ecpri protocol at all).
> 
> Rx:
> 
> 	int ecpri_input(struct rte_mbuf *m)
> 	{
> 		struct rte_ecpri_common_hdr hdr_copy, *hdr;
> 		struct rte_ecpri_msg_event_ind event_copy, *event;
> 		struct app_specific app_copy, *app;
> 
> 		hdr = rte_pktmbuf_read(m, 0, sizeof(*hdr),
> &hdr_copy);
> 		if (unlikely(hdr == NULL))
> 			return -1;
> 		switch (hdr->type) {
> 		...
> 		case RTE_ECPRI_EVT_IND_NTFY_IND:
> 			event = rte_pktmbuf_read(m, sizeof(*hdr),
> sizeof(*event),
> 				&event_copy);
> 			if (unlikely(event == NULL))
> 				return -1;
> 			...
> 			app = rte_pktmbuf_read(m, sizeof(*app),
> 				sizeof(*hdr) + sizeof(*event),
> 				&app_copy);
> 			...
> 
> Tx:
> 
> 	int ecpri_output(void)
> 	{
> 		struct rte_ecpri_common_hdr *hdr;
> 		struct rte_ecpri_msg_event_ind *event;
> 		struct app_specific *app;
> 
> 		m = rte_pktmbuf_alloc(mp);
> 		if (unlikely(m == NULL))
> 			return -1;
> 
> 		app = rte_pktmbuf_append(m, sizeof(*data));
> 		if (app == NULL)
> 			...
> 		app->... = ...;
> 		...
> 		event = rte_pktmbuf_prepend(m, sizeof(*event));
> 		if (event == NULL)
> 			...
> 		event->... = ...;
> 		...
> 		hdr = rte_pktmbuf_prepend(m, sizeof(*hdr));
> 		if (hdr == NULL)
> 			...
> 		hdr->... = ...;
> 
> 		return packet_send(m);
> 	}
> 
> In these 2 examples, we never need the unioned structure (struct
> rte_ecpri_msg_hdr).
> 
> Using it does not look possible to me, because its size is corresponds to
> the largest message, not to the one we are parsing/building.
> 

Yes, in the cases, we do not need the unioned structures at all.
Since the common header is always 4 bytes, an application could use the
sub-types structures started from an offset of 4 of eCPRI layer, as in your example.
This is in the datapath. My original purpose is for some "control path", typically
the flow (not sure if any other new lib implementation) API, then the union
could be used there w/o treating the common header and message payload
header in a separate way and then combine them together. In this case, only
the first several bytes will be accessed and checked, there will be no change
of message itself, and then just passing it to datapath for further handling as
in your example.

> > I think some comments could be added here, is it OK?
> > 3. Regarding this structure, I add it because I do not want to
> > introduce a lot of new items in the rte_flow: new items with
> > structures, new enum types. I prefer one single structure will cover
> most of the cases (subtypes). What do you think?
> > 4. About the *dummy* u32, I calculated all the "subheaders" and
> choose
> > the maximal value of the length. Two purposes (same as the u32 in
> the common header):
> >   a. easy to swap the endianness, but not quite useful. Because some
> parts are u16 and u8,
> >     and should not be swapped in a u32. (some physical channel ID
> and address LSB have 32bits width)
> >     But if some HW parsed the header u32 by u32, then it would be
> helpful.
> >   b. easy for checking in flow API, if the user wants to insert a flow.
> Some checking should
> >       be done to confirm if it is wildcard flow (all eCPRI messages or
> eCPRI message in some specific type),
> >       or some precise flow (to match the pc id or rtc id, for example).
> With these fields, 3 DW
> >       of the masks only need to be check before continuing. Or else, the
> code needs to check the type
> >       and a lot of switch-case conditions and go through all different
> members of each header.
> 
> Thanks for the clarification.
> 
> I'll tend to say that if the rte_ecpri_msg_hdr structure is only useful for
> rte_flow, it should be defined inside rte_flow.
> 

Right now, yes it will be used in rte_flow. But I checked the current implementations
of each item, almost all the headers are defined in their own protocol files. So in v6,
I change the name of it, in order not to confuse the users of this API, would it be OK?
Thanks

> However, I have some fears about the dummy[3]. You said it could be
> enlarged later: I think it is dangerous to change the size of a structure
> that may be used to parse data (and this would be an ABI change).
> Also, it seems dummy[3] cannot be used easily to swap endianness, so
> is it really useful?
> 

It might be enlarger but not for now, until a new revision of this specification. For
all the subtypes it has right now, the specification will remain them as same as today.
Only new types will be added then. So after several years, we may consider to change it
in the LTS. Is it OK?
In most cases, the endianness swap could be easy, we will swap it in each DW / u32. Tome
the exception is that some field crosses the u32 boundary, like the address in type#4, we may
treat it separately. And the most useful case is for the mask checking, it could simplify the
checking to at most 3 (u32==0?) without going through each member of different types.

And v6 already sent, I change the code based on your suggestion. Would you please
help to give some comments also?

Appreciate for your help and suggestion.

> 
> Thanks,
> Olivier
> 
> 
> > > > +
> > > > +#ifdef __cplusplus
> > > > +}
> > > > +#endif
> > > > +
> > > > +#endif /* _RTE_ECPRI_H_ */
> > > > diff --git a/lib/librte_net/rte_ether.h
> > > > b/lib/librte_net/rte_ether.h index 0ae4e75..184a3f9 100644
> > > > --- a/lib/librte_net/rte_ether.h
> > > > +++ b/lib/librte_net/rte_ether.h
> > > > @@ -304,6 +304,7 @@ struct rte_vlan_hdr {  #define
> > > RTE_ETHER_TYPE_LLDP
> > > > 0x88CC /**< LLDP Protocol. */  #define RTE_ETHER_TYPE_MPLS
> > > 0x8847 /**<
> > > > MPLS ethertype. */  #define RTE_ETHER_TYPE_MPLSM 0x8848
> /**<
> > > MPLS
> > > > multicast ethertype. */
> > > > +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype
> (.1Q
> > > > +supported). */
> > > >
> > > >  /**
> > > >   * Extract VLAN tag information into mbuf
> > > > --
> > > > 1.8.3.1
> > > >
> >

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

* Re: [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-12 14:28                 ` Bing Zhao
@ 2020-07-12 14:43                   ` Olivier Matz
  0 siblings, 0 replies; 28+ messages in thread
From: Olivier Matz @ 2020-07-12 14:43 UTC (permalink / raw)
  To: Bing Zhao
  Cc: Ori Kam, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, akhil.goyal, dev, wenzhuo.lu,
	beilei.xing, bernard.iremonger

On Sun, Jul 12, 2020 at 02:28:03PM +0000, Bing Zhao wrote:
> Hi Olivier,
> Thanks
> 
> BR. Bing
> 
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Sunday, July 12, 2020 9:18 PM
> > To: Bing Zhao <bingz@mellanox.com>
> > Cc: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> > marko.kovacevic@intel.com; Thomas Monjalon
> > <thomas@monjalon.net>; ferruh.yigit@intel.com;
> > arybchenko@solarflare.com; akhil.goyal@nxp.com; dev@dpdk.org;
> > wenzhuo.lu@intel.com; beilei.xing@intel.com;
> > bernard.iremonger@intel.com
> > Subject: Re: [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API
> > 
> > Hi Bing,
> > 
> > On Sat, Jul 11, 2020 at 04:25:49AM +0000, Bing Zhao wrote:
> > > Hi Olivier,
> > > Many thanks for your comments.
> > 
> > [...]
> > 
> > > > > +/**
> > > > > + * eCPRI Common Header
> > > > > + */
> > > > > +RTE_STD_C11
> > > > > +struct rte_ecpri_common_hdr {
> > > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > > > +	uint32_t size:16;		/**< Payload Size */
> > > > > +	uint32_t type:8;		/**< Message Type */
> > > > > +	uint32_t c:1;			/**< Concatenation Indicator
> > > > */
> > > > > +	uint32_t res:3;			/**< Reserved */
> > > > > +	uint32_t revision:4;		/**< Protocol Revision */
> > > > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > > > +	uint32_t revision:4;		/**< Protocol Revision */
> > > > > +	uint32_t res:3;			/**< Reserved */
> > > > > +	uint32_t c:1;			/**< Concatenation Indicator
> > > > */
> > > > > +	uint32_t type:8;		/**< Message Type */
> > > > > +	uint32_t size:16;		/**< Payload Size */
> > > > > +#endif
> > > > > +} __rte_packed;
> > > >
> > > > Does it really need to be packed? Why next types do not need it?
> > > > It looks only those which have bitfields are.
> > > >
> > >
> > > Nice catch, thanks. For the common header, there is no need to use
> > the
> > > packed attribute, because it is a u32 and the bitfields will be
> > > aligned.
> > > I checked all the definitions again. Only " Type #4: Remote Memory
> > Access"
> > > needs to use the packed attribute.
> > > For other sub-types, "sub-header" part of the message payload will
> > get
> > > aligned by nature. For example, u16 after u16, u8 after u16, these
> > > should be OK.
> > > But in type #4, the address is 48bits wide, with 16bits MSB and 32bits
> > > LSB (no detailed description in the specification, correct me if
> > > anything wrong.) Usually, the 48bits address will be devided as this
> > > in a system. And there is no 48-bits type at all. So we need to define
> > two parts for it: 32b LSB follows 16b MSB.
> > > u32 after u16 should be with packed attribute. Thanks
> > 
> > What about using a bitfield into a uint64_t ? I mean:
> > 
> > 	struct rte_ecpri_msg_rm_access {
> > if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > 		...
> > 		uint64_t length:16;		/**< number of bytes
> > */
> > 		uint64_t addr:48;		/**< address */
> > #else
> > 		...
> > 		uint64_t addr:48;		/**< address */
> > 		uint64_t length:16;		/**< number of bytes
> > */
> > #endif
> > 	};
> > 
> 
> Thanks for your suggestion.
> https://stackoverflow.com/questions/10906238/warning-when-using-bitfield-with-unsigned-char
> AFAIK (from this page), bitfields support only support bool and int. uint64_t is some type of "long"
> and most of the compilers should support it. But I am not sure if it is a standard implementation.

The uint8_t[6], as in your v6, is a good idea.


> > >
> > > >
> > > > I wonder if the 'dw0' could be in this definition instead of in
> > > > struct rte_ecpri_msg_hdr?
> > > >
> > > > Something like this:
> > > >
> > > > struct rte_ecpri_common_hdr {
> > > > 	union {
> > > > 		uint32_t u32;
> > > > 		struct {
> > > > 			...
> > > > 		};
> > > > 	};
> > > > };
> > > >
> > > > I see 2 advantages:
> > > >
> > > > - someone that only uses the common_hdr struct can use the .u32
> > > >   in its software
> > > > - when using it in messages, it looks clearer to me:
> > > >     msg.common_hdr.u32 = value;
> > > >   instead of:
> > > >     msg.dw0 = value;
> > > >
> > > > What do you think?
> > >
> > > Thanks for the suggestion, this is much better, I will change it.
> > > Indeed, in my original version, no DW(u32) is defined for the header.
> > > After that, I noticed that it would not be easy for the static casting
> > > to a u32 from bitfield(the compiler will complain), and it would not
> > > be clear to swap the endian if the user wants to use this header. I
> > > added this DW(u32) to simplify the usage of this header. But yes, if I
> > > do not add it here, it would be not easy or clear for users who just
> > use this header structure.
> > > I will change it. Is it OK if I use the name "dw0"?
> > 
> > In my opinion, u32 is more usual than dw0.
> > 
> 
> I sent patch set v6 with this change, thanks.
> 
> > >
> > > >
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #0: IQ Data  */ struct
> > > > > +rte_ecpri_msg_iq_data {
> > > > > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > > > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #1: Bit Sequence  */ struct
> > > > > +rte_ecpri_msg_bit_seq {
> > > > > +	rte_be16_t pc_id;		/**< Physical channel ID */
> > > > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #2: Real-Time Control Data  */
> > > > struct
> > > > > +rte_ecpri_msg_rtc_ctrl {
> > > > > +	rte_be16_t rtc_id;		/**< Real-Time Control Data ID
> > > > */
> > > > > +	rte_be16_t seq_id;		/**< Sequence ID */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #3: Generic Data Transfer  */
> > > > struct
> > > > > +rte_ecpri_msg_gen_data {
> > > > > +	rte_be32_t pc_id;		/**< Physical channel ID */
> > > > > +	rte_be32_t seq_id;		/**< Sequence ID */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #4: Remote Memory Access
> > */
> > > > > +RTE_STD_C11
> > > > > +struct rte_ecpri_msg_rm_access {
> > > > > +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > > > > +	uint32_t ele_id:16;		/**< Element ID */
> > > > > +	uint32_t rr:4;			/**< Req/Resp */
> > > > > +	uint32_t rw:4;			/**< Read/Write */
> > > > > +	uint32_t rma_id:8;		/**< Remote Memory Access
> > > > ID */
> > > > > +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > > > > +	uint32_t rma_id:8;		/**< Remote Memory Access
> > > > ID */
> > > > > +	uint32_t rw:4;			/**< Read/Write */
> > > > > +	uint32_t rr:4;			/**< Req/Resp */
> > > > > +	uint32_t ele_id:16;		/**< Element ID */
> > > > > +#endif
> > > > > +	rte_be16_t addr_m;		/**< 48-bits address (16 MSB)
> > > > */
> > > > > +	rte_be32_t addr_l;		/**< 48-bits address (32 LSB) */
> > > > > +	rte_be16_t length;		/**< number of bytes */
> > > > > +} __rte_packed;
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #5: One-Way Delay
> > Measurement
> > > > */
> > > > > +struct rte_ecpri_msg_delay_measure {
> > > > > +	uint8_t msr_id;			/**< Measurement ID */
> > > > > +	uint8_t act_type;		/**< Action Type */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #6: Remote Reset  */ struct
> > > > > +rte_ecpri_msg_remote_reset {
> > > > > +	rte_be16_t rst_id;		/**< Reset ID */
> > > > > +	uint8_t rst_op;			/**< Reset Code Op */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header of Type #7: Event Indication  */ struct
> > > > > +rte_ecpri_msg_event_ind {
> > > > > +	uint8_t evt_id;			/**< Event ID */
> > > > > +	uint8_t evt_type;		/**< Event Type */
> > > > > +	uint8_t seq;			/**< Sequence Number */
> > > > > +	uint8_t number;			/**< Number of
> > > > Faults/Notif */
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * eCPRI Message Header Format: Common Header + Message
> > > > Types  */
> > > > > +RTE_STD_C11
> > > > > +struct rte_ecpri_msg_hdr {
> > > > > +	union {
> > > > > +		struct rte_ecpri_common_hdr common;
> > > > > +		uint32_t dw0;
> > > > > +	};
> > > > > +	union {
> > > > > +		struct rte_ecpri_msg_iq_data type0;
> > > > > +		struct rte_ecpri_msg_bit_seq type1;
> > > > > +		struct rte_ecpri_msg_rtc_ctrl type2;
> > > > > +		struct rte_ecpri_msg_bit_seq type3;
> > > > > +		struct rte_ecpri_msg_rm_access type4;
> > > > > +		struct rte_ecpri_msg_delay_measure type5;
> > > > > +		struct rte_ecpri_msg_remote_reset type6;
> > > > > +		struct rte_ecpri_msg_event_ind type7;
> > > > > +		uint32_t dummy[3];
> > > > > +	};
> > > > > +};
> > > >
> > > > What is the point in having this struct?
> > > >
> > > > From a software point of view, I think it is a bit risky, because
> > > > its size is the size of the largest message. This is probably what
> > > > you want in your case, but when a software will rx or tx such
> > > > packet, I think they shouldn't use this one. My understanding is
> > > > that you only need this structure for the mask in rte_flow.
> > > >
> > > > Also, I'm not sure to understand the purpose of dummy[3], even
> > after
> > > > reading your answer to Akhil's question.
> > > >
> > >
> > > Basically YES and no. To my understanding, the eCPRI message
> > format is
> > > something like the ICMP packet format. The message (packet) itself
> > > will be parsed into different formats based on the type of the
> > common
> > > header. In the message payload part, there is no distinct definition
> > > of the "sub-header". We can divide them into the sub-header and
> > data parts based on the specification.
> > > E.g. physical channel ID / real-time control ID / Event ID + type are
> > > the parts that datapath forwarding will only care about. The
> > following
> > > timestamp or user data parts are the parts that the higher layer in
> > the application will use.
> > > 1. If an application wants to create some offload flow, or even
> > handle
> > > it in the SW, the common header + first several bytes in the payload
> > > should be enough. BUT YES, it is not good or safe to use it in the
> > higher layer of the application.
> > > 2. A higher layer of the application should have its own definition of
> > > the whole payload of a specific sub-type, including the parsing of the
> > user data part after the "sub-header".
> > > It is better for them just skip the first 4 bytes of the eCPRI message or
> > a known offset.
> > > We do not need to cover the upper layers.
> > 
> > Let me explain what is my vision of how an application would use the
> > structures (these are completly dummy examples, as I don't know
> > ecpri protocol at all).
> > 
> > Rx:
> > 
> > 	int ecpri_input(struct rte_mbuf *m)
> > 	{
> > 		struct rte_ecpri_common_hdr hdr_copy, *hdr;
> > 		struct rte_ecpri_msg_event_ind event_copy, *event;
> > 		struct app_specific app_copy, *app;
> > 
> > 		hdr = rte_pktmbuf_read(m, 0, sizeof(*hdr),
> > &hdr_copy);
> > 		if (unlikely(hdr == NULL))
> > 			return -1;
> > 		switch (hdr->type) {
> > 		...
> > 		case RTE_ECPRI_EVT_IND_NTFY_IND:
> > 			event = rte_pktmbuf_read(m, sizeof(*hdr),
> > sizeof(*event),
> > 				&event_copy);
> > 			if (unlikely(event == NULL))
> > 				return -1;
> > 			...
> > 			app = rte_pktmbuf_read(m, sizeof(*app),
> > 				sizeof(*hdr) + sizeof(*event),
> > 				&app_copy);
> > 			...
> > 
> > Tx:
> > 
> > 	int ecpri_output(void)
> > 	{
> > 		struct rte_ecpri_common_hdr *hdr;
> > 		struct rte_ecpri_msg_event_ind *event;
> > 		struct app_specific *app;
> > 
> > 		m = rte_pktmbuf_alloc(mp);
> > 		if (unlikely(m == NULL))
> > 			return -1;
> > 
> > 		app = rte_pktmbuf_append(m, sizeof(*data));
> > 		if (app == NULL)
> > 			...
> > 		app->... = ...;
> > 		...
> > 		event = rte_pktmbuf_prepend(m, sizeof(*event));
> > 		if (event == NULL)
> > 			...
> > 		event->... = ...;
> > 		...
> > 		hdr = rte_pktmbuf_prepend(m, sizeof(*hdr));
> > 		if (hdr == NULL)
> > 			...
> > 		hdr->... = ...;
> > 
> > 		return packet_send(m);
> > 	}
> > 
> > In these 2 examples, we never need the unioned structure (struct
> > rte_ecpri_msg_hdr).
> > 
> > Using it does not look possible to me, because its size is corresponds to
> > the largest message, not to the one we are parsing/building.
> > 
> 
> Yes, in the cases, we do not need the unioned structures at all.
> Since the common header is always 4 bytes, an application could use the
> sub-types structures started from an offset of 4 of eCPRI layer, as in your example.
> This is in the datapath. My original purpose is for some "control path", typically
> the flow (not sure if any other new lib implementation) API, then the union
> could be used there w/o treating the common header and message payload
> header in a separate way and then combine them together. In this case, only
> the first several bytes will be accessed and checked, there will be no change
> of message itself, and then just passing it to datapath for further handling as
> in your example.
> 
> > > I think some comments could be added here, is it OK?
> > > 3. Regarding this structure, I add it because I do not want to
> > > introduce a lot of new items in the rte_flow: new items with
> > > structures, new enum types. I prefer one single structure will cover
> > most of the cases (subtypes). What do you think?
> > > 4. About the *dummy* u32, I calculated all the "subheaders" and
> > choose
> > > the maximal value of the length. Two purposes (same as the u32 in
> > the common header):
> > >   a. easy to swap the endianness, but not quite useful. Because some
> > parts are u16 and u8,
> > >     and should not be swapped in a u32. (some physical channel ID
> > and address LSB have 32bits width)
> > >     But if some HW parsed the header u32 by u32, then it would be
> > helpful.
> > >   b. easy for checking in flow API, if the user wants to insert a flow.
> > Some checking should
> > >       be done to confirm if it is wildcard flow (all eCPRI messages or
> > eCPRI message in some specific type),
> > >       or some precise flow (to match the pc id or rtc id, for example).
> > With these fields, 3 DW
> > >       of the masks only need to be check before continuing. Or else, the
> > code needs to check the type
> > >       and a lot of switch-case conditions and go through all different
> > members of each header.
> > 
> > Thanks for the clarification.
> > 
> > I'll tend to say that if the rte_ecpri_msg_hdr structure is only useful for
> > rte_flow, it should be defined inside rte_flow.
> > 
> 
> Right now, yes it will be used in rte_flow. But I checked the current implementations
> of each item, almost all the headers are defined in their own protocol files. So in v6,
> I change the name of it, in order not to confuse the users of this API, would it be OK?
> Thanks

OK

> 
> > However, I have some fears about the dummy[3]. You said it could be
> > enlarged later: I think it is dangerous to change the size of a structure
> > that may be used to parse data (and this would be an ABI change).
> > Also, it seems dummy[3] cannot be used easily to swap endianness, so
> > is it really useful?
> > 
> 
> It might be enlarger but not for now, until a new revision of this specification. For
> all the subtypes it has right now, the specification will remain them as same as today.
> Only new types will be added then. So after several years, we may consider to change it
> in the LTS. Is it OK?

OK, I think in this case it may even be another structure

> In most cases, the endianness swap could be easy, we will swap it in each DW / u32. Tome
> the exception is that some field crosses the u32 boundary, like the address in type#4, we may
> treat it separately. And the most useful case is for the mask checking, it could simplify the
> checking to at most 3 (u32==0?) without going through each member of different types.
> 
> And v6 already sent, I change the code based on your suggestion. Would you please
> help to give some comments also?
> 
> Appreciate for your help and suggestion.
> 
> > 
> > Thanks,
> > Olivier
> > 
> > 
> > > > > +
> > > > > +#ifdef __cplusplus
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > +#endif /* _RTE_ECPRI_H_ */
> > > > > diff --git a/lib/librte_net/rte_ether.h
> > > > > b/lib/librte_net/rte_ether.h index 0ae4e75..184a3f9 100644
> > > > > --- a/lib/librte_net/rte_ether.h
> > > > > +++ b/lib/librte_net/rte_ether.h
> > > > > @@ -304,6 +304,7 @@ struct rte_vlan_hdr {  #define
> > > > RTE_ETHER_TYPE_LLDP
> > > > > 0x88CC /**< LLDP Protocol. */  #define RTE_ETHER_TYPE_MPLS
> > > > 0x8847 /**<
> > > > > MPLS ethertype. */  #define RTE_ETHER_TYPE_MPLSM 0x8848
> > /**<
> > > > MPLS
> > > > > multicast ethertype. */
> > > > > +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype
> > (.1Q
> > > > > +supported). */
> > > > >
> > > > >  /**
> > > > >   * Extract VLAN tag information into mbuf
> > > > > --
> > > > > 1.8.3.1
> > > > >
> > >

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

* Re: [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
@ 2020-07-12 14:45             ` Olivier Matz
  2020-07-12 14:50               ` Bing Zhao
  2020-07-13  0:50               ` Thomas Monjalon
  0 siblings, 2 replies; 28+ messages in thread
From: Olivier Matz @ 2020-07-12 14:45 UTC (permalink / raw)
  To: Bing Zhao
  Cc: orika, john.mcnamara, marko.kovacevic, thomas, ferruh.yigit,
	arybchenko, akhil.goyal, dev, wenzhuo.lu, beilei.xing,
	bernard.iremonger

On Sun, Jul 12, 2020 at 09:35:02PM +0800, Bing Zhao wrote:
> Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.
> 
> eCPRI is a packet based protocol used in the fronthaul interface of
> 5G networks. Header format definition could be found in the
> specification via the link below:
> https://www.gigalight.com/downloads/standards/ecpri-specification.pdf
> 
> eCPRI message can be over Ethernet layer (.1Q supported also) or over
> UDP layer. Message header formats are the same in these two variants.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks!

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

* Re: [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-12 14:45             ` Olivier Matz
@ 2020-07-12 14:50               ` Bing Zhao
  2020-07-13  0:50               ` Thomas Monjalon
  1 sibling, 0 replies; 28+ messages in thread
From: Bing Zhao @ 2020-07-12 14:50 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Ori Kam, john.mcnamara, marko.kovacevic, Thomas Monjalon,
	ferruh.yigit, arybchenko, akhil.goyal, dev, wenzhuo.lu,
	beilei.xing, bernard.iremonger

Many thanks for your big help.

BR. Bing

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Sunday, July 12, 2020 10:46 PM
> To: Bing Zhao <bingz@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; akhil.goyal@nxp.com; dev@dpdk.org;
> wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com
> Subject: Re: [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API
> 
> On Sun, Jul 12, 2020 at 09:35:02PM +0800, Bing Zhao wrote:
> > Add a new item "rte_flow_item_ecpri" in order to match eCRPI
> header.
> >
> > eCPRI is a packet based protocol used in the fronthaul interface of 5G
> > networks. Header format definition could be found in the
> specification
> > via the link below:
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> www.
> > gigalight.com%2Fdownloads%2Fstandards%2Fecpri-
> specification.pdf&amp;da
> >
> ta=02%7C01%7Cbingz%40mellanox.com%7C5ffe8275a9594dfc6d9f08d
> 826724506%7
> >
> Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C6373016195227
> 27143&amp;sda
> >
> ta=%2BdY5DigE0mMLu7dOVl3OqPrVvquDLCKTtWI3M%2BSfK3A%3D&
> amp;reserved=0
> >
> > eCPRI message can be over Ethernet layer (.1Q supported also) or
> over
> > UDP layer. Message header formats are the same in these two
> variants.
> >
> > Signed-off-by: Bing Zhao <bingz@mellanox.com>
> > Acked-by: Ori Kam <orika@mellanox.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Thanks!

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

* Re: [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-12 14:45             ` Olivier Matz
  2020-07-12 14:50               ` Bing Zhao
@ 2020-07-13  0:50               ` Thomas Monjalon
  2020-07-13  8:30                 ` Ferruh Yigit
  1 sibling, 1 reply; 28+ messages in thread
From: Thomas Monjalon @ 2020-07-13  0:50 UTC (permalink / raw)
  To: Bing Zhao
  Cc: dev, orika, john.mcnamara, marko.kovacevic, ferruh.yigit,
	arybchenko, akhil.goyal, dev, wenzhuo.lu, beilei.xing,
	bernard.iremonger, Olivier Matz

12/07/2020 16:45, Olivier Matz:
> On Sun, Jul 12, 2020 at 09:35:02PM +0800, Bing Zhao wrote:
> > Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.
> > 
> > eCPRI is a packet based protocol used in the fronthaul interface of
> > 5G networks. Header format definition could be found in the
> > specification via the link below:
> > https://www.gigalight.com/downloads/standards/ecpri-specification.pdf
> > 
> > eCPRI message can be over Ethernet layer (.1Q supported also) or over
> > UDP layer. Message header formats are the same in these two variants.
> > 
> > Signed-off-by: Bing Zhao <bingz@mellanox.com>
> > Acked-by: Ori Kam <orika@mellanox.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Thanks!

Glad to see this 5G feature is ready to be merged in last minute of 20.08-rc1.

Applied, thanks



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

* Re: [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API
  2020-07-13  0:50               ` Thomas Monjalon
@ 2020-07-13  8:30                 ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2020-07-13  8:30 UTC (permalink / raw)
  To: Thomas Monjalon, Bing Zhao
  Cc: dev, orika, john.mcnamara, marko.kovacevic, arybchenko,
	akhil.goyal, wenzhuo.lu, beilei.xing, bernard.iremonger,
	Olivier Matz

On 7/13/2020 1:50 AM, Thomas Monjalon wrote:
> 12/07/2020 16:45, Olivier Matz:
>> On Sun, Jul 12, 2020 at 09:35:02PM +0800, Bing Zhao wrote:
>>> Add a new item "rte_flow_item_ecpri" in order to match eCRPI header.
>>>
>>> eCPRI is a packet based protocol used in the fronthaul interface of
>>> 5G networks. Header format definition could be found in the
>>> specification via the link below:
>>> https://www.gigalight.com/downloads/standards/ecpri-specification.pdf
>>>
>>> eCPRI message can be over Ethernet layer (.1Q supported also) or over
>>> UDP layer. Message header formats are the same in these two variants.
>>>
>>> Signed-off-by: Bing Zhao <bingz@mellanox.com>
>>> Acked-by: Ori Kam <orika@mellanox.com>
>>
>> Acked-by: Olivier Matz <olivier.matz@6wind.com>
>>
>> Thanks!
> 
> Glad to see this 5G feature is ready to be merged in last minute of 20.08-rc1.
> 
> Applied, thanks
> 

Thanks Thomas for getting it.

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

end of thread, other threads:[~2020-07-13  8:30 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-28 16:20 [dpdk-dev] [PATCH] rte_flow: add eCPRI key fields to flow API Bing Zhao
2020-07-02  6:46 ` [dpdk-dev] [PATCH v2] " Bing Zhao
2020-07-02  8:06   ` Ori Kam
2020-07-02 12:53   ` [dpdk-dev] [PATCH v3 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
2020-07-05 11:34       ` Ori Kam
2020-07-02 12:53     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
2020-07-05 11:36       ` Ori Kam
2020-07-07 15:36     ` [dpdk-dev] [PATCH v4 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
2020-07-08 18:49         ` Akhil Goyal
2020-07-09  3:58           ` Bing Zhao
2020-07-07 15:36       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
2020-07-10  8:45       ` [dpdk-dev] [PATCH v5 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
2020-07-10 14:31           ` Olivier Matz
2020-07-11  4:25             ` Bing Zhao
2020-07-12 13:17               ` Olivier Matz
2020-07-12 14:28                 ` Bing Zhao
2020-07-12 14:43                   ` Olivier Matz
2020-07-10  8:45         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao
2020-07-12 13:35         ` [dpdk-dev] [PATCH v6 0/2] rte_flow: introduce eCPRI item for rte_flow Bing Zhao
2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 1/2] rte_flow: add eCPRI key fields to flow API Bing Zhao
2020-07-12 14:45             ` Olivier Matz
2020-07-12 14:50               ` Bing Zhao
2020-07-13  0:50               ` Thomas Monjalon
2020-07-13  8:30                 ` Ferruh Yigit
2020-07-12 13:35           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add eCPRI in flow creation patterns Bing Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).