DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction
@ 2020-10-25  7:13 Haiyue Wang
  2020-10-26  6:50 ` [dpdk-dev] [PATCH v2] net/ice: refactor the protocol extraction design Haiyue Wang
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Haiyue Wang @ 2020-10-25  7:13 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, jia.guo, qi.z.zhang, zhaoyan.chen, Haiyue Wang,
	Qiming Yang, Ray Kinsella, Neil Horman

Current dynamic mbuf design is that the driver will register the needed
field and flags at the device probing time, this will make iavf PMD use
different names to register the dynamic mbuf field and flags, but both
of them use the exactly same protocol extraction metadata.

This will run out of the limited dynamic mbuf resource, meanwhile, the
application has to handle the dynamic mbuf separately.

For making things simple and consistent, refactor dynamic mbuf in data
extraction handling: the PMD just lookups the same name at the queue
setup time after the application registers it.

In other words, make the dynamic mbuf string name as API, not the data
object which is defined in each PMD.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 drivers/net/ice/ice_ethdev.c  | 104 ++--------------
 drivers/net/ice/ice_ethdev.h  |   1 +
 drivers/net/ice/ice_rxtx.c    |  49 ++++----
 drivers/net/ice/ice_rxtx.h    |   1 +
 drivers/net/ice/rte_pmd_ice.h | 219 +++++-----------------------------
 drivers/net/ice/version.map   |  13 --
 6 files changed, 68 insertions(+), 319 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 51b99c6506..ec27089cfa 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -32,42 +32,6 @@ static const char * const ice_valid_args[] = {
 	NULL
 };
 
-static const struct rte_mbuf_dynfield ice_proto_xtr_metadata_param = {
-	.name = "ice_dynfield_proto_xtr_metadata",
-	.size = sizeof(uint32_t),
-	.align = __alignof__(uint32_t),
-	.flags = 0,
-};
-
-struct proto_xtr_ol_flag {
-	const struct rte_mbuf_dynflag param;
-	uint64_t *ol_flag;
-	bool required;
-};
-
-static bool ice_proto_xtr_hw_support[PROTO_XTR_MAX];
-
-static struct proto_xtr_ol_flag ice_proto_xtr_ol_flag_params[] = {
-	[PROTO_XTR_VLAN] = {
-		.param = { .name = "ice_dynflag_proto_xtr_vlan" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_vlan_mask },
-	[PROTO_XTR_IPV4] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv4" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv4_mask },
-	[PROTO_XTR_IPV6] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_mask },
-	[PROTO_XTR_IPV6_FLOW] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6_flow" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask },
-	[PROTO_XTR_TCP] = {
-		.param = { .name = "ice_dynflag_proto_xtr_tcp" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_tcp_mask },
-	[PROTO_XTR_IP_OFFSET] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ip_offset" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ip_offset_mask },
-};
-
 #define ICE_DFLT_OUTER_TAG_TYPE ICE_AQ_VSI_OUTER_TAG_VLAN_9100
 
 #define ICE_OS_DEFAULT_PKG_NAME		"ICE OS Default Package"
@@ -542,7 +506,7 @@ handle_proto_xtr_arg(__rte_unused const char *key, const char *value,
 }
 
 static void
-ice_check_proto_xtr_support(struct ice_hw *hw)
+ice_check_proto_xtr_support(struct ice_pf *pf, struct ice_hw *hw)
 {
 #define FLX_REG(val, fld, idx) \
 	(((val) & GLFLXP_RXDID_FLX_WRD_##idx##_##fld##_M) >> \
@@ -587,7 +551,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
 
 			if (FLX_REG(v, PROT_MDID, 4) == xtr_sets[i].protid_0 &&
 			    FLX_REG(v, RXDID_OPCODE, 4) == xtr_sets[i].opcode)
-				ice_proto_xtr_hw_support[i] = true;
+				pf->hw_proto_xtr_ena[i] = 1;
 		}
 
 		if (xtr_sets[i].protid_1 != ICE_PROT_ID_INVAL) {
@@ -595,7 +559,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
 
 			if (FLX_REG(v, PROT_MDID, 5) == xtr_sets[i].protid_1 &&
 			    FLX_REG(v, RXDID_OPCODE, 5) == xtr_sets[i].opcode)
-				ice_proto_xtr_hw_support[i] = true;
+				pf->hw_proto_xtr_ena[i] = 1;
 		}
 	}
 }
@@ -1429,9 +1393,6 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
 			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
-	const struct proto_xtr_ol_flag *ol_flag;
-	bool proto_xtr_enable = false;
-	int offset;
 	uint16_t i;
 
 	pf->proto_xtr = rte_zmalloc(NULL, pf->lan_nb_qps, 0);
@@ -1440,65 +1401,20 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
 		return;
 	}
 
+	ice_check_proto_xtr_support(pf, hw);
+
 	for (i = 0; i < pf->lan_nb_qps; i++) {
 		pf->proto_xtr[i] = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
 				   ad->devargs.proto_xtr[i] :
 				   ad->devargs.proto_xtr_dflt;
 
-		if (pf->proto_xtr[i] != PROTO_XTR_NONE) {
-			uint8_t type = pf->proto_xtr[i];
-
-			ice_proto_xtr_ol_flag_params[type].required = true;
-			proto_xtr_enable = true;
-		}
-	}
-
-	if (likely(!proto_xtr_enable))
-		return;
-
-	ice_check_proto_xtr_support(hw);
-
-	offset = rte_mbuf_dynfield_register(&ice_proto_xtr_metadata_param);
-	if (unlikely(offset == -1)) {
-		PMD_DRV_LOG(ERR,
-			    "Protocol extraction metadata is disabled in mbuf with error %d",
-			    -rte_errno);
-		return;
-	}
-
-	PMD_DRV_LOG(DEBUG,
-		    "Protocol extraction metadata offset in mbuf is : %d",
-		    offset);
-	rte_net_ice_dynfield_proto_xtr_metadata_offs = offset;
-
-	for (i = 0; i < RTE_DIM(ice_proto_xtr_ol_flag_params); i++) {
-		ol_flag = &ice_proto_xtr_ol_flag_params[i];
-
-		if (!ol_flag->required)
-			continue;
-
-		if (!ice_proto_xtr_hw_support[i]) {
+		if (pf->proto_xtr[i] != PROTO_XTR_NONE &&
+		    !pf->hw_proto_xtr_ena[pf->proto_xtr[i]]) {
 			PMD_DRV_LOG(ERR,
-				    "Protocol extraction type %u is not supported in hardware",
-				    i);
-			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-			break;
+				    "The Rx queue %u doesn't support protocol extraction type %u\n",
+				    i, pf->proto_xtr[i]);
+			pf->proto_xtr[i] = PROTO_XTR_NONE;
 		}
-
-		offset = rte_mbuf_dynflag_register(&ol_flag->param);
-		if (unlikely(offset == -1)) {
-			PMD_DRV_LOG(ERR,
-				    "Protocol extraction offload '%s' failed to register with error %d",
-				    ol_flag->param.name, -rte_errno);
-
-			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-			break;
-		}
-
-		PMD_DRV_LOG(DEBUG,
-			    "Protocol extraction offload '%s' offset in mbuf is : %d",
-			    ol_flag->param.name, offset);
-		*ol_flag->ol_flag = 1ULL << offset;
 	}
 }
 
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 05218af05e..a675eac209 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -457,6 +457,7 @@ struct ice_pf {
 	uint64_t old_rx_bytes;
 	uint64_t old_tx_bytes;
 	uint64_t supported_rxdid; /* bitmap for supported RXDID */
+	uint8_t hw_proto_xtr_ena[PROTO_XTR_MAX];
 };
 
 #define ICE_MAX_QUEUE_NUM  2048
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index f6291894cd..27f04a432f 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -15,16 +15,8 @@
 		PKT_TX_TCP_SEG |		 \
 		PKT_TX_OUTER_IP_CKSUM)
 
-/* Offset of mbuf dynamic field for protocol extraction data */
-int rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-
-/* Mask of mbuf dynamic flags for protocol extraction type */
-uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+#define ICE_DYNF_PROTO_XTR_METADATA(m) \
+	RTE_MBUF_DYNFIELD((m), rxq->xtr_metadata_off, uint32_t *)
 
 static inline uint8_t
 ice_proto_xtr_type_to_rxdid(uint8_t xtr_type)
@@ -104,7 +96,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v1(struct ice_rx_queue *rxq,
 		if (metadata) {
 			mb->ol_flags |= rxq->xtr_ol_flag;
 
-			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
+			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
 		}
 	}
 #endif
@@ -142,7 +134,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
 		if (metadata) {
 			mb->ol_flags |= rxq->xtr_ol_flag;
 
-			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
+			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
 		}
 	}
 #endif
@@ -151,34 +143,38 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
 static void
 ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
 {
+	struct rte_mbuf_dynfield metadata_param;
+	const char *flag_name = NULL;
+	int flag_off, metadata_off;
+
 	switch (rxdid) {
 	case ICE_RXDID_COMMS_AUX_VLAN:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_vlan_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV4:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv4_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV6:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV6_FLOW:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_TCP:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_tcp_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IP_OFFSET:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v2;
 		break;
 
@@ -192,8 +188,21 @@ ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
 		break;
 	}
 
-	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
-		rxq->xtr_ol_flag = 0;
+	if (!flag_name)
+		return;
+
+	flag_off = rte_mbuf_dynflag_lookup(flag_name, NULL);
+	if (flag_off == -1)
+		return;
+
+	metadata_off = rte_mbuf_dynfield_lookup
+				(RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+				 &metadata_param);
+	if (metadata_off == -1 || metadata_param.size != sizeof(uint32_t))
+		return;
+
+	rxq->xtr_metadata_off = metadata_off;
+	rxq->xtr_ol_flag = 1ULL << flag_off;
 }
 
 static enum ice_status
diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
index 23409d479a..53d0a609f3 100644
--- a/drivers/net/ice/ice_rxtx.h
+++ b/drivers/net/ice/ice_rxtx.h
@@ -85,6 +85,7 @@ struct ice_rx_queue {
 	bool q_set; /* indicate if rx queue has been configured */
 	bool rx_deferred_start; /* don't start this queue in dev start */
 	uint8_t proto_xtr; /* Protocol extraction from flexible descriptor */
+	int xtr_metadata_off; /* Protocol extraction offload metadata offset */
 	uint64_t xtr_ol_flag; /* Protocol extraction offload flag */
 	ice_rxd_to_pkt_fields_t rxd_to_pkt_fields; /* handle FlexiMD by RXDID */
 	ice_rx_release_mbufs_t rx_rel_mbufs;
diff --git a/drivers/net/ice/rte_pmd_ice.h b/drivers/net/ice/rte_pmd_ice.h
index 9a436a140b..df27b59d35 100644
--- a/drivers/net/ice/rte_pmd_ice.h
+++ b/drivers/net/ice/rte_pmd_ice.h
@@ -22,220 +22,55 @@
 extern "C" {
 #endif
 
-/**
- * The supported network protocol extraction metadata format.
- */
-union rte_net_ice_proto_xtr_metadata {
-	uint32_t metadata;
-
-	struct {
-		uint16_t data0;
-		uint16_t data1;
-	} raw;
-
-	struct {
-		uint16_t stag_vid:12,
-			 stag_dei:1,
-			 stag_pcp:3;
-		uint16_t ctag_vid:12,
-			 ctag_dei:1,
-			 ctag_pcp:3;
-	} vlan;
-
-	struct {
-		uint16_t protocol:8,
-			 ttl:8;
-		uint16_t tos:8,
-			 ihl:4,
-			 version:4;
-	} ipv4;
-
-	struct {
-		uint16_t hoplimit:8,
-			 nexthdr:8;
-		uint16_t flowhi4:4,
-			 tc:8,
-			 version:4;
-	} ipv6;
-
-	struct {
-		uint16_t flowlo16;
-		uint16_t flowhi4:4,
-			 tc:8,
-			 version:4;
-	} ipv6_flow;
-
-	struct {
-		uint16_t fin:1,
-			 syn:1,
-			 rst:1,
-			 psh:1,
-			 ack:1,
-			 urg:1,
-			 ece:1,
-			 cwr:1,
-			 res1:4,
-			 doff:4;
-		uint16_t rsvd;
-	} tcp;
-
-	uint32_t ip_ofs;
-};
-
-/* Offset of mbuf dynamic field for protocol extraction data */
-extern int rte_net_ice_dynfield_proto_xtr_metadata_offs;
-
-/* Mask of mbuf dynamic flags for protocol extraction type */
-extern uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+#ifndef __INTEL_FXP_RX_DESC_METADATA__
+#define __INTEL_FXP_RX_DESC_METADATA__
 
 /**
- * The mbuf dynamic field pointer for protocol extraction metadata.
+ * The mbuf dynamic field for metadata extraction from NIC:
+ *  a). Extract 16b (2 Bytes) from the defined offset location of the specified
+ *      protocol in the packet.
+ *  b). Report the offset to the selected protocol.
  */
-#define RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m) \
-	RTE_MBUF_DYNFIELD((m), \
-			  rte_net_ice_dynfield_proto_xtr_metadata_offs, \
-			  uint32_t *)
+#define RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME \
+	"rte_pmd_dynfield_proto_xtr_metadata"
 
 /**
- * The mbuf dynamic flag for VLAN protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'vlan' specified.
+ * The mbuf dynamic flag for VLAN protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_VLAN \
-	(rte_net_ice_dynflag_proto_xtr_vlan_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME \
+	"rte_pmd_dynflag_proto_xtr_vlan"
 
 /**
- * The mbuf dynamic flag for IPv4 protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ipv4' specified.
+ * The mbuf dynamic flag for IPv4 protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV4 \
-	(rte_net_ice_dynflag_proto_xtr_ipv4_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv4"
 
 /**
- * The mbuf dynamic flag for IPv6 protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ipv6' specified.
+ * The mbuf dynamic flag for IPv6 protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6 \
-	(rte_net_ice_dynflag_proto_xtr_ipv6_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv6"
 
 /**
- * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata, it is
- * valid when dev_args 'proto_xtr' has 'ipv6_flow' specified.
+ * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW \
-	(rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv6_flow"
 
 /**
- * The mbuf dynamic flag for TCP protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'tcp' specified.
+ * The mbuf dynamic flag for TCP protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_TCP \
-	(rte_net_ice_dynflag_proto_xtr_tcp_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME \
+	"rte_pmd_dynflag_proto_xtr_tcp"
 
 /**
- * The mbuf dynamic flag for IP_OFFSET extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ip_offset' specified.
+  * The mbuf dynamic flag for IPv4 or IPv6 header offset report metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET \
-	(rte_net_ice_dynflag_proto_xtr_ip_offset_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME \
+	"rte_pmd_dynflag_proto_xtr_ip_offset"
 
-/**
- * Check if mbuf dynamic field for protocol extraction metadata is registered.
- *
- * @return
- *   True if registered, false otherwise.
- */
-__rte_experimental
-static __rte_always_inline int
-rte_net_ice_dynf_proto_xtr_metadata_avail(void)
-{
-	return rte_net_ice_dynfield_proto_xtr_metadata_offs != -1;
-}
-
-/**
- * Get the mbuf dynamic field for protocol extraction metadata.
- *
- * @param m
- *    The pointer to the mbuf.
- * @return
- *   The saved protocol extraction metadata.
- */
-__rte_experimental
-static __rte_always_inline uint32_t
-rte_net_ice_dynf_proto_xtr_metadata_get(struct rte_mbuf *m)
-{
-	return *RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m);
-}
-
-/**
- * Dump the mbuf dynamic field for protocol extraction metadata.
- *
- * @param m
- *    The pointer to the mbuf.
- */
-__rte_experimental
-static inline void
-rte_net_ice_dump_proto_xtr_metadata(struct rte_mbuf *m)
-{
-	union rte_net_ice_proto_xtr_metadata data;
-
-	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
-		return;
-
-	data.metadata = rte_net_ice_dynf_proto_xtr_metadata_get(m);
-
-	if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_VLAN)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],vlan,stag=%u:%u:%u,ctag=%u:%u:%u",
-		       data.raw.data0, data.raw.data1,
-		       data.vlan.stag_pcp,
-		       data.vlan.stag_dei,
-		       data.vlan.stag_vid,
-		       data.vlan.ctag_pcp,
-		       data.vlan.ctag_dei,
-		       data.vlan.ctag_vid);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV4)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv4,ver=%u,hdrlen=%u,tos=%u,ttl=%u,proto=%u",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv4.version,
-		       data.ipv4.ihl,
-		       data.ipv4.tos,
-		       data.ipv4.ttl,
-		       data.ipv4.protocol);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6,ver=%u,tc=%u,flow_hi4=0x%x,nexthdr=%u,hoplimit=%u",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv6.version,
-		       data.ipv6.tc,
-		       data.ipv6.flowhi4,
-		       data.ipv6.nexthdr,
-		       data.ipv6.hoplimit);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6_flow,ver=%u,tc=%u,flow=0x%x%04x",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv6_flow.version,
-		       data.ipv6_flow.tc,
-		       data.ipv6_flow.flowhi4,
-		       data.ipv6_flow.flowlo16);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_TCP)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],tcp,doff=%u,flags=%s%s%s%s%s%s%s%s",
-		       data.raw.data0, data.raw.data1,
-		       data.tcp.doff,
-		       data.tcp.cwr ? "C" : "",
-		       data.tcp.ece ? "E" : "",
-		       data.tcp.urg ? "U" : "",
-		       data.tcp.ack ? "A" : "",
-		       data.tcp.psh ? "P" : "",
-		       data.tcp.rst ? "R" : "",
-		       data.tcp.syn ? "S" : "",
-		       data.tcp.fin ? "F" : "");
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
-		printf(" - Protocol Offset:ip_offset=%u",
-		       data.ip_ofs);
-}
+#endif
 
 #ifdef __cplusplus
 }
diff --git a/drivers/net/ice/version.map b/drivers/net/ice/version.map
index 632a296a0c..4a76d1d52d 100644
--- a/drivers/net/ice/version.map
+++ b/drivers/net/ice/version.map
@@ -1,16 +1,3 @@
 DPDK_21 {
 	local: *;
 };
-
-EXPERIMENTAL {
-	global:
-
-	# added in 19.11
-	rte_net_ice_dynfield_proto_xtr_metadata_offs;
-	rte_net_ice_dynflag_proto_xtr_vlan_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-	rte_net_ice_dynflag_proto_xtr_tcp_mask;
-	rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
-};
-- 
2.29.0


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

* [dpdk-dev] [PATCH v2] net/ice: refactor the protocol extraction design
  2020-10-25  7:13 [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Haiyue Wang
@ 2020-10-26  6:50 ` Haiyue Wang
  2020-10-26  6:55 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Haiyue Wang @ 2020-10-26  6:50 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, jia.guo, qi.z.zhang, zhaoyan.chen, Haiyue Wang,
	Qiming Yang, Ray Kinsella, Neil Horman

Change the protocol extraction dynamic mbuf usage from regiser API to
lookup API, so the application can decide to read the metadata or not
at the run time, in other words, PMD will check this at Rx queue start
time.

This design makes the API simple now: it just needs to export the name
string, not the whole dynamic mbuf data objects.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v2: update the commit message, doc; and add the error handling for
    dynamic mbuf lookup. Also keep the metadata format defination.
---
 doc/guides/nics/ice.rst       |  16 ++--
 drivers/net/ice/ice_ethdev.c  | 117 +++++------------------
 drivers/net/ice/ice_ethdev.h  |   1 +
 drivers/net/ice/ice_rxtx.c    |  62 ++++++++----
 drivers/net/ice/ice_rxtx.h    |   1 +
 drivers/net/ice/rte_pmd_ice.h | 171 +++++++---------------------------
 drivers/net/ice/version.map   |  13 ---
 7 files changed, 106 insertions(+), 275 deletions(-)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index a2aea12333..9878b665b3 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -156,19 +156,17 @@ Runtime Config Options
    +----------------------------+----------------------------+
    |           IPHDR2           |           IPHDR1           |
    +============================+============================+
-   |       IPv6 HDR Offset      |       IPv4 HDR Offset      |
+   |         Reserved           |       IP Header Offset     |
    +----------------------------+----------------------------+
 
-  IPHDR1 - Outer/Single IPv4 Header offset.
+  IPHDR1 - Outer/Single IPv4/IPv6 Header offset.
 
-  IPHDR2 - Outer/Single IPv6 Header offset.
+  IPHDR2 - Reserved.
 
-  Use ``rte_net_ice_dynf_proto_xtr_metadata_get`` to access the protocol
-  extraction metadata, and use ``RTE_PKT_RX_DYNF_PROTO_XTR_*`` to get the
-  metadata type of ``struct rte_mbuf::ol_flags``.
-
-  The ``rte_net_ice_dump_proto_xtr_metadata`` routine shows how to
-  access the protocol extraction result in ``struct rte_mbuf``.
+  The dynamic mbuf field for metadata uses "rte_pmd_dynfield_proto_xtr_metadata"
+  name with 4 byte size. And the related dynamic mbuf flag uses the name format
+  "rte_pmd_dynflag_proto_xtr_*" which ends with the protocol extraction devargs
+  name such as "ip_offset".
 
 Driver compilation and testing
 ------------------------------
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 51b99c6506..9e7d71ae4d 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -32,42 +32,6 @@ static const char * const ice_valid_args[] = {
 	NULL
 };
 
-static const struct rte_mbuf_dynfield ice_proto_xtr_metadata_param = {
-	.name = "ice_dynfield_proto_xtr_metadata",
-	.size = sizeof(uint32_t),
-	.align = __alignof__(uint32_t),
-	.flags = 0,
-};
-
-struct proto_xtr_ol_flag {
-	const struct rte_mbuf_dynflag param;
-	uint64_t *ol_flag;
-	bool required;
-};
-
-static bool ice_proto_xtr_hw_support[PROTO_XTR_MAX];
-
-static struct proto_xtr_ol_flag ice_proto_xtr_ol_flag_params[] = {
-	[PROTO_XTR_VLAN] = {
-		.param = { .name = "ice_dynflag_proto_xtr_vlan" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_vlan_mask },
-	[PROTO_XTR_IPV4] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv4" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv4_mask },
-	[PROTO_XTR_IPV6] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_mask },
-	[PROTO_XTR_IPV6_FLOW] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6_flow" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask },
-	[PROTO_XTR_TCP] = {
-		.param = { .name = "ice_dynflag_proto_xtr_tcp" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_tcp_mask },
-	[PROTO_XTR_IP_OFFSET] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ip_offset" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ip_offset_mask },
-};
-
 #define ICE_DFLT_OUTER_TAG_TYPE ICE_AQ_VSI_OUTER_TAG_VLAN_9100
 
 #define ICE_OS_DEFAULT_PKG_NAME		"ICE OS Default Package"
@@ -542,7 +506,7 @@ handle_proto_xtr_arg(__rte_unused const char *key, const char *value,
 }
 
 static void
-ice_check_proto_xtr_support(struct ice_hw *hw)
+ice_check_proto_xtr_support(struct ice_pf *pf, struct ice_hw *hw)
 {
 #define FLX_REG(val, fld, idx) \
 	(((val) & GLFLXP_RXDID_FLX_WRD_##idx##_##fld##_M) >> \
@@ -587,7 +551,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
 
 			if (FLX_REG(v, PROT_MDID, 4) == xtr_sets[i].protid_0 &&
 			    FLX_REG(v, RXDID_OPCODE, 4) == xtr_sets[i].opcode)
-				ice_proto_xtr_hw_support[i] = true;
+				pf->hw_proto_xtr_ena[i] = 1;
 		}
 
 		if (xtr_sets[i].protid_1 != ICE_PROT_ID_INVAL) {
@@ -595,7 +559,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
 
 			if (FLX_REG(v, PROT_MDID, 5) == xtr_sets[i].protid_1 &&
 			    FLX_REG(v, RXDID_OPCODE, 5) == xtr_sets[i].opcode)
-				ice_proto_xtr_hw_support[i] = true;
+				pf->hw_proto_xtr_ena[i] = 1;
 		}
 	}
 }
@@ -1429,9 +1393,8 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
 			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
-	const struct proto_xtr_ol_flag *ol_flag;
-	bool proto_xtr_enable = false;
-	int offset;
+	uint8_t err_msgs[PROTO_XTR_MAX];
+	uint8_t proto_xtr_type;
 	uint16_t i;
 
 	pf->proto_xtr = rte_zmalloc(NULL, pf->lan_nb_qps, 0);
@@ -1440,65 +1403,27 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
 		return;
 	}
 
-	for (i = 0; i < pf->lan_nb_qps; i++) {
-		pf->proto_xtr[i] = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
-				   ad->devargs.proto_xtr[i] :
-				   ad->devargs.proto_xtr_dflt;
-
-		if (pf->proto_xtr[i] != PROTO_XTR_NONE) {
-			uint8_t type = pf->proto_xtr[i];
-
-			ice_proto_xtr_ol_flag_params[type].required = true;
-			proto_xtr_enable = true;
-		}
-	}
-
-	if (likely(!proto_xtr_enable))
-		return;
-
-	ice_check_proto_xtr_support(hw);
-
-	offset = rte_mbuf_dynfield_register(&ice_proto_xtr_metadata_param);
-	if (unlikely(offset == -1)) {
-		PMD_DRV_LOG(ERR,
-			    "Protocol extraction metadata is disabled in mbuf with error %d",
-			    -rte_errno);
-		return;
-	}
-
-	PMD_DRV_LOG(DEBUG,
-		    "Protocol extraction metadata offset in mbuf is : %d",
-		    offset);
-	rte_net_ice_dynfield_proto_xtr_metadata_offs = offset;
-
-	for (i = 0; i < RTE_DIM(ice_proto_xtr_ol_flag_params); i++) {
-		ol_flag = &ice_proto_xtr_ol_flag_params[i];
-
-		if (!ol_flag->required)
-			continue;
+	ice_check_proto_xtr_support(pf, hw);
 
-		if (!ice_proto_xtr_hw_support[i]) {
-			PMD_DRV_LOG(ERR,
-				    "Protocol extraction type %u is not supported in hardware",
-				    i);
-			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-			break;
-		}
+	memset(err_msgs, 0, sizeof(err_msgs));
 
-		offset = rte_mbuf_dynflag_register(&ol_flag->param);
-		if (unlikely(offset == -1)) {
-			PMD_DRV_LOG(ERR,
-				    "Protocol extraction offload '%s' failed to register with error %d",
-				    ol_flag->param.name, -rte_errno);
+	for (i = 0; i < pf->lan_nb_qps; i++) {
+		proto_xtr_type = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
+				 ad->devargs.proto_xtr[i] :
+				 ad->devargs.proto_xtr_dflt;
+
+		if (proto_xtr_type != PROTO_XTR_NONE &&
+		    !pf->hw_proto_xtr_ena[proto_xtr_type]) {
+			if (!err_msgs[proto_xtr_type]) {
+				PMD_DRV_LOG(ERR, "protocol extraction type %u is not supported in hardware\n",
+					    proto_xtr_type);
+				err_msgs[proto_xtr_type] = 1;
+			}
 
-			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-			break;
+			proto_xtr_type = PROTO_XTR_NONE;
 		}
 
-		PMD_DRV_LOG(DEBUG,
-			    "Protocol extraction offload '%s' offset in mbuf is : %d",
-			    ol_flag->param.name, offset);
-		*ol_flag->ol_flag = 1ULL << offset;
+		pf->proto_xtr[i] = proto_xtr_type;
 	}
 }
 
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 05218af05e..a675eac209 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -457,6 +457,7 @@ struct ice_pf {
 	uint64_t old_rx_bytes;
 	uint64_t old_tx_bytes;
 	uint64_t supported_rxdid; /* bitmap for supported RXDID */
+	uint8_t hw_proto_xtr_ena[PROTO_XTR_MAX];
 };
 
 #define ICE_MAX_QUEUE_NUM  2048
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index f6291894cd..4063aabfeb 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -15,16 +15,8 @@
 		PKT_TX_TCP_SEG |		 \
 		PKT_TX_OUTER_IP_CKSUM)
 
-/* Offset of mbuf dynamic field for protocol extraction data */
-int rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-
-/* Mask of mbuf dynamic flags for protocol extraction type */
-uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+#define ICE_DYNF_PROTO_XTR_METADATA(m) \
+	RTE_MBUF_DYNFIELD((m), rxq->xtr_metadata_off, uint32_t *)
 
 static inline uint8_t
 ice_proto_xtr_type_to_rxdid(uint8_t xtr_type)
@@ -104,7 +96,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v1(struct ice_rx_queue *rxq,
 		if (metadata) {
 			mb->ol_flags |= rxq->xtr_ol_flag;
 
-			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
+			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
 		}
 	}
 #endif
@@ -142,7 +134,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
 		if (metadata) {
 			mb->ol_flags |= rxq->xtr_ol_flag;
 
-			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
+			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
 		}
 	}
 #endif
@@ -151,34 +143,38 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
 static void
 ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
 {
+	struct rte_mbuf_dynfield metadata_param;
+	const char *flag_name = NULL;
+	int flag_off, metadata_off;
+
 	switch (rxdid) {
 	case ICE_RXDID_COMMS_AUX_VLAN:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_vlan_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV4:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv4_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV6:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV6_FLOW:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_TCP:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_tcp_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IP_OFFSET:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v2;
 		break;
 
@@ -192,8 +188,34 @@ ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
 		break;
 	}
 
-	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
-		rxq->xtr_ol_flag = 0;
+	if (!flag_name)
+		return;
+
+	flag_off = rte_mbuf_dynflag_lookup(flag_name, NULL);
+	if (flag_off == -1) {
+		PMD_DRV_LOG(WARNING, "failed to lookup the dynamic flag '%s' for Rx queue %u : %d",
+			    flag_name, rxq->queue_id, -rte_errno);
+		return;
+	}
+
+	metadata_off = rte_mbuf_dynfield_lookup
+				(RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+				 &metadata_param);
+	if (metadata_off == -1) {
+		PMD_DRV_LOG(WARNING, "failed to lookup the dynamic field '%s' for Rx queue %u : %d",
+			    RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+			    rxq->queue_id, -rte_errno);
+		return;
+	}
+	if (metadata_param.size != sizeof(uint32_t)) {
+		PMD_DRV_LOG(WARNING, "the dynamic field '%s' data size is not matched for Rx queue %u",
+			    RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+			    rxq->queue_id);
+		return;
+	}
+
+	rxq->xtr_metadata_off = metadata_off;
+	rxq->xtr_ol_flag = 1ULL << flag_off;
 }
 
 static enum ice_status
diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
index 23409d479a..53d0a609f3 100644
--- a/drivers/net/ice/ice_rxtx.h
+++ b/drivers/net/ice/ice_rxtx.h
@@ -85,6 +85,7 @@ struct ice_rx_queue {
 	bool q_set; /* indicate if rx queue has been configured */
 	bool rx_deferred_start; /* don't start this queue in dev start */
 	uint8_t proto_xtr; /* Protocol extraction from flexible descriptor */
+	int xtr_metadata_off; /* Protocol extraction offload metadata offset */
 	uint64_t xtr_ol_flag; /* Protocol extraction offload flag */
 	ice_rxd_to_pkt_fields_t rxd_to_pkt_fields; /* handle FlexiMD by RXDID */
 	ice_rx_release_mbufs_t rx_rel_mbufs;
diff --git a/drivers/net/ice/rte_pmd_ice.h b/drivers/net/ice/rte_pmd_ice.h
index 9a436a140b..f7304add40 100644
--- a/drivers/net/ice/rte_pmd_ice.h
+++ b/drivers/net/ice/rte_pmd_ice.h
@@ -14,18 +14,19 @@
  *
  */
 
-#include <stdio.h>
-#include <rte_mbuf.h>
-#include <rte_mbuf_dyn.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+#ifndef __INTEL_RX_FLEX_DESC_METADATA__
+#define __INTEL_RX_FLEX_DESC_METADATA__
+
 /**
  * The supported network protocol extraction metadata format.
  */
-union rte_net_ice_proto_xtr_metadata {
+union rte_pmd_proto_xtr_metadata {
 	uint32_t metadata;
 
 	struct {
@@ -82,160 +83,56 @@ union rte_net_ice_proto_xtr_metadata {
 	uint32_t ip_ofs;
 };
 
-/* Offset of mbuf dynamic field for protocol extraction data */
-extern int rte_net_ice_dynfield_proto_xtr_metadata_offs;
-
-/* Mask of mbuf dynamic flags for protocol extraction type */
-extern uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
-
-/**
- * The mbuf dynamic field pointer for protocol extraction metadata.
- */
-#define RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m) \
-	RTE_MBUF_DYNFIELD((m), \
-			  rte_net_ice_dynfield_proto_xtr_metadata_offs, \
-			  uint32_t *)
-
 /**
- * The mbuf dynamic flag for VLAN protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'vlan' specified.
- */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_VLAN \
-	(rte_net_ice_dynflag_proto_xtr_vlan_mask)
-
-/**
- * The mbuf dynamic flag for IPv4 protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ipv4' specified.
+ * The mbuf dynamic field metadata for protocol extraction from hardware:
+ *
+ *  a). Extract one word from the defined location of the specified
+ *      protocol in the packet.
+ *  b). Report the offset to the selected protocol type.
+ *
+ *  And the metadata can hold two of the above defined fields (in word), these
+ *  words are in host endian order.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV4 \
-	(rte_net_ice_dynflag_proto_xtr_ipv4_mask)
+#define RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME \
+	"rte_pmd_dynfield_proto_xtr_metadata"
 
 /**
- * The mbuf dynamic flag for IPv6 protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ipv6' specified.
+ * The mbuf dynamic flag for VLAN protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6 \
-	(rte_net_ice_dynflag_proto_xtr_ipv6_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME \
+	"rte_pmd_dynflag_proto_xtr_vlan"
 
 /**
- * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata, it is
- * valid when dev_args 'proto_xtr' has 'ipv6_flow' specified.
+ * The mbuf dynamic flag for IPv4 protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW \
-	(rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv4"
 
 /**
- * The mbuf dynamic flag for TCP protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'tcp' specified.
+ * The mbuf dynamic flag for IPv6 protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_TCP \
-	(rte_net_ice_dynflag_proto_xtr_tcp_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv6"
 
 /**
- * The mbuf dynamic flag for IP_OFFSET extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ip_offset' specified.
+ * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET \
-	(rte_net_ice_dynflag_proto_xtr_ip_offset_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv6_flow"
 
 /**
- * Check if mbuf dynamic field for protocol extraction metadata is registered.
- *
- * @return
- *   True if registered, false otherwise.
+ * The mbuf dynamic flag for TCP protocol extraction metadata type.
  */
-__rte_experimental
-static __rte_always_inline int
-rte_net_ice_dynf_proto_xtr_metadata_avail(void)
-{
-	return rte_net_ice_dynfield_proto_xtr_metadata_offs != -1;
-}
+#define RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME \
+	"rte_pmd_dynflag_proto_xtr_tcp"
 
 /**
- * Get the mbuf dynamic field for protocol extraction metadata.
- *
- * @param m
- *    The pointer to the mbuf.
- * @return
- *   The saved protocol extraction metadata.
+ * The mbuf dynamic flag for IPv4 or IPv6 header offset report metadata type.
  */
-__rte_experimental
-static __rte_always_inline uint32_t
-rte_net_ice_dynf_proto_xtr_metadata_get(struct rte_mbuf *m)
-{
-	return *RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m);
-}
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME \
+	"rte_pmd_dynflag_proto_xtr_ip_offset"
 
-/**
- * Dump the mbuf dynamic field for protocol extraction metadata.
- *
- * @param m
- *    The pointer to the mbuf.
- */
-__rte_experimental
-static inline void
-rte_net_ice_dump_proto_xtr_metadata(struct rte_mbuf *m)
-{
-	union rte_net_ice_proto_xtr_metadata data;
-
-	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
-		return;
-
-	data.metadata = rte_net_ice_dynf_proto_xtr_metadata_get(m);
-
-	if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_VLAN)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],vlan,stag=%u:%u:%u,ctag=%u:%u:%u",
-		       data.raw.data0, data.raw.data1,
-		       data.vlan.stag_pcp,
-		       data.vlan.stag_dei,
-		       data.vlan.stag_vid,
-		       data.vlan.ctag_pcp,
-		       data.vlan.ctag_dei,
-		       data.vlan.ctag_vid);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV4)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv4,ver=%u,hdrlen=%u,tos=%u,ttl=%u,proto=%u",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv4.version,
-		       data.ipv4.ihl,
-		       data.ipv4.tos,
-		       data.ipv4.ttl,
-		       data.ipv4.protocol);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6,ver=%u,tc=%u,flow_hi4=0x%x,nexthdr=%u,hoplimit=%u",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv6.version,
-		       data.ipv6.tc,
-		       data.ipv6.flowhi4,
-		       data.ipv6.nexthdr,
-		       data.ipv6.hoplimit);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6_flow,ver=%u,tc=%u,flow=0x%x%04x",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv6_flow.version,
-		       data.ipv6_flow.tc,
-		       data.ipv6_flow.flowhi4,
-		       data.ipv6_flow.flowlo16);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_TCP)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],tcp,doff=%u,flags=%s%s%s%s%s%s%s%s",
-		       data.raw.data0, data.raw.data1,
-		       data.tcp.doff,
-		       data.tcp.cwr ? "C" : "",
-		       data.tcp.ece ? "E" : "",
-		       data.tcp.urg ? "U" : "",
-		       data.tcp.ack ? "A" : "",
-		       data.tcp.psh ? "P" : "",
-		       data.tcp.rst ? "R" : "",
-		       data.tcp.syn ? "S" : "",
-		       data.tcp.fin ? "F" : "");
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
-		printf(" - Protocol Offset:ip_offset=%u",
-		       data.ip_ofs);
-}
+#endif /* __INTEL_RX_FLEX_DESC_METADATA__ */
 
 #ifdef __cplusplus
 }
diff --git a/drivers/net/ice/version.map b/drivers/net/ice/version.map
index 632a296a0c..4a76d1d52d 100644
--- a/drivers/net/ice/version.map
+++ b/drivers/net/ice/version.map
@@ -1,16 +1,3 @@
 DPDK_21 {
 	local: *;
 };
-
-EXPERIMENTAL {
-	global:
-
-	# added in 19.11
-	rte_net_ice_dynfield_proto_xtr_metadata_offs;
-	rte_net_ice_dynflag_proto_xtr_vlan_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-	rte_net_ice_dynflag_proto_xtr_tcp_mask;
-	rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
-};
-- 
2.29.0


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

* [dpdk-dev] [PATCH v3] net/ice: refactor the protocol extraction design
  2020-10-25  7:13 [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Haiyue Wang
  2020-10-26  6:50 ` [dpdk-dev] [PATCH v2] net/ice: refactor the protocol extraction design Haiyue Wang
@ 2020-10-26  6:55 ` Haiyue Wang
  2020-10-27  1:35   ` Zhang, Qi Z
  2020-10-26 10:22 ` [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Olivier Matz
  2020-10-27  1:23 ` [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name Haiyue Wang
  3 siblings, 1 reply; 8+ messages in thread
From: Haiyue Wang @ 2020-10-26  6:55 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, jia.guo, qi.z.zhang, zhaoyan.chen, Haiyue Wang,
	Qiming Yang, Ray Kinsella, Neil Horman

Change the protocol extraction dynamic mbuf usage from register API to
lookup API, so the application can decide to read the metadata or not
at the run time, in other words, PMD will check this at Rx queue start
time.

This design makes the API simple now: it just needs to export the name
string, not the whole dynamic mbuf data objects.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v3: Fix 'regiser' typo in commit message.

v2: update the commit message, doc; and add the error handling for
    dynamic mbuf lookup. Also keep the metadata format defination.
---
 doc/guides/nics/ice.rst       |  16 ++--
 drivers/net/ice/ice_ethdev.c  | 117 +++++------------------
 drivers/net/ice/ice_ethdev.h  |   1 +
 drivers/net/ice/ice_rxtx.c    |  62 ++++++++----
 drivers/net/ice/ice_rxtx.h    |   1 +
 drivers/net/ice/rte_pmd_ice.h | 171 +++++++---------------------------
 drivers/net/ice/version.map   |  13 ---
 7 files changed, 106 insertions(+), 275 deletions(-)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index a2aea12333..9878b665b3 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -156,19 +156,17 @@ Runtime Config Options
    +----------------------------+----------------------------+
    |           IPHDR2           |           IPHDR1           |
    +============================+============================+
-   |       IPv6 HDR Offset      |       IPv4 HDR Offset      |
+   |         Reserved           |       IP Header Offset     |
    +----------------------------+----------------------------+
 
-  IPHDR1 - Outer/Single IPv4 Header offset.
+  IPHDR1 - Outer/Single IPv4/IPv6 Header offset.
 
-  IPHDR2 - Outer/Single IPv6 Header offset.
+  IPHDR2 - Reserved.
 
-  Use ``rte_net_ice_dynf_proto_xtr_metadata_get`` to access the protocol
-  extraction metadata, and use ``RTE_PKT_RX_DYNF_PROTO_XTR_*`` to get the
-  metadata type of ``struct rte_mbuf::ol_flags``.
-
-  The ``rte_net_ice_dump_proto_xtr_metadata`` routine shows how to
-  access the protocol extraction result in ``struct rte_mbuf``.
+  The dynamic mbuf field for metadata uses "rte_pmd_dynfield_proto_xtr_metadata"
+  name with 4 byte size. And the related dynamic mbuf flag uses the name format
+  "rte_pmd_dynflag_proto_xtr_*" which ends with the protocol extraction devargs
+  name such as "ip_offset".
 
 Driver compilation and testing
 ------------------------------
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 51b99c6506..9e7d71ae4d 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -32,42 +32,6 @@ static const char * const ice_valid_args[] = {
 	NULL
 };
 
-static const struct rte_mbuf_dynfield ice_proto_xtr_metadata_param = {
-	.name = "ice_dynfield_proto_xtr_metadata",
-	.size = sizeof(uint32_t),
-	.align = __alignof__(uint32_t),
-	.flags = 0,
-};
-
-struct proto_xtr_ol_flag {
-	const struct rte_mbuf_dynflag param;
-	uint64_t *ol_flag;
-	bool required;
-};
-
-static bool ice_proto_xtr_hw_support[PROTO_XTR_MAX];
-
-static struct proto_xtr_ol_flag ice_proto_xtr_ol_flag_params[] = {
-	[PROTO_XTR_VLAN] = {
-		.param = { .name = "ice_dynflag_proto_xtr_vlan" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_vlan_mask },
-	[PROTO_XTR_IPV4] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv4" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv4_mask },
-	[PROTO_XTR_IPV6] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_mask },
-	[PROTO_XTR_IPV6_FLOW] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6_flow" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask },
-	[PROTO_XTR_TCP] = {
-		.param = { .name = "ice_dynflag_proto_xtr_tcp" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_tcp_mask },
-	[PROTO_XTR_IP_OFFSET] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ip_offset" },
-		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ip_offset_mask },
-};
-
 #define ICE_DFLT_OUTER_TAG_TYPE ICE_AQ_VSI_OUTER_TAG_VLAN_9100
 
 #define ICE_OS_DEFAULT_PKG_NAME		"ICE OS Default Package"
@@ -542,7 +506,7 @@ handle_proto_xtr_arg(__rte_unused const char *key, const char *value,
 }
 
 static void
-ice_check_proto_xtr_support(struct ice_hw *hw)
+ice_check_proto_xtr_support(struct ice_pf *pf, struct ice_hw *hw)
 {
 #define FLX_REG(val, fld, idx) \
 	(((val) & GLFLXP_RXDID_FLX_WRD_##idx##_##fld##_M) >> \
@@ -587,7 +551,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
 
 			if (FLX_REG(v, PROT_MDID, 4) == xtr_sets[i].protid_0 &&
 			    FLX_REG(v, RXDID_OPCODE, 4) == xtr_sets[i].opcode)
-				ice_proto_xtr_hw_support[i] = true;
+				pf->hw_proto_xtr_ena[i] = 1;
 		}
 
 		if (xtr_sets[i].protid_1 != ICE_PROT_ID_INVAL) {
@@ -595,7 +559,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
 
 			if (FLX_REG(v, PROT_MDID, 5) == xtr_sets[i].protid_1 &&
 			    FLX_REG(v, RXDID_OPCODE, 5) == xtr_sets[i].opcode)
-				ice_proto_xtr_hw_support[i] = true;
+				pf->hw_proto_xtr_ena[i] = 1;
 		}
 	}
 }
@@ -1429,9 +1393,8 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
 			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
-	const struct proto_xtr_ol_flag *ol_flag;
-	bool proto_xtr_enable = false;
-	int offset;
+	uint8_t err_msgs[PROTO_XTR_MAX];
+	uint8_t proto_xtr_type;
 	uint16_t i;
 
 	pf->proto_xtr = rte_zmalloc(NULL, pf->lan_nb_qps, 0);
@@ -1440,65 +1403,27 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
 		return;
 	}
 
-	for (i = 0; i < pf->lan_nb_qps; i++) {
-		pf->proto_xtr[i] = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
-				   ad->devargs.proto_xtr[i] :
-				   ad->devargs.proto_xtr_dflt;
-
-		if (pf->proto_xtr[i] != PROTO_XTR_NONE) {
-			uint8_t type = pf->proto_xtr[i];
-
-			ice_proto_xtr_ol_flag_params[type].required = true;
-			proto_xtr_enable = true;
-		}
-	}
-
-	if (likely(!proto_xtr_enable))
-		return;
-
-	ice_check_proto_xtr_support(hw);
-
-	offset = rte_mbuf_dynfield_register(&ice_proto_xtr_metadata_param);
-	if (unlikely(offset == -1)) {
-		PMD_DRV_LOG(ERR,
-			    "Protocol extraction metadata is disabled in mbuf with error %d",
-			    -rte_errno);
-		return;
-	}
-
-	PMD_DRV_LOG(DEBUG,
-		    "Protocol extraction metadata offset in mbuf is : %d",
-		    offset);
-	rte_net_ice_dynfield_proto_xtr_metadata_offs = offset;
-
-	for (i = 0; i < RTE_DIM(ice_proto_xtr_ol_flag_params); i++) {
-		ol_flag = &ice_proto_xtr_ol_flag_params[i];
-
-		if (!ol_flag->required)
-			continue;
+	ice_check_proto_xtr_support(pf, hw);
 
-		if (!ice_proto_xtr_hw_support[i]) {
-			PMD_DRV_LOG(ERR,
-				    "Protocol extraction type %u is not supported in hardware",
-				    i);
-			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-			break;
-		}
+	memset(err_msgs, 0, sizeof(err_msgs));
 
-		offset = rte_mbuf_dynflag_register(&ol_flag->param);
-		if (unlikely(offset == -1)) {
-			PMD_DRV_LOG(ERR,
-				    "Protocol extraction offload '%s' failed to register with error %d",
-				    ol_flag->param.name, -rte_errno);
+	for (i = 0; i < pf->lan_nb_qps; i++) {
+		proto_xtr_type = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
+				 ad->devargs.proto_xtr[i] :
+				 ad->devargs.proto_xtr_dflt;
+
+		if (proto_xtr_type != PROTO_XTR_NONE &&
+		    !pf->hw_proto_xtr_ena[proto_xtr_type]) {
+			if (!err_msgs[proto_xtr_type]) {
+				PMD_DRV_LOG(ERR, "protocol extraction type %u is not supported in hardware\n",
+					    proto_xtr_type);
+				err_msgs[proto_xtr_type] = 1;
+			}
 
-			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-			break;
+			proto_xtr_type = PROTO_XTR_NONE;
 		}
 
-		PMD_DRV_LOG(DEBUG,
-			    "Protocol extraction offload '%s' offset in mbuf is : %d",
-			    ol_flag->param.name, offset);
-		*ol_flag->ol_flag = 1ULL << offset;
+		pf->proto_xtr[i] = proto_xtr_type;
 	}
 }
 
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 05218af05e..a675eac209 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -457,6 +457,7 @@ struct ice_pf {
 	uint64_t old_rx_bytes;
 	uint64_t old_tx_bytes;
 	uint64_t supported_rxdid; /* bitmap for supported RXDID */
+	uint8_t hw_proto_xtr_ena[PROTO_XTR_MAX];
 };
 
 #define ICE_MAX_QUEUE_NUM  2048
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index f6291894cd..4063aabfeb 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -15,16 +15,8 @@
 		PKT_TX_TCP_SEG |		 \
 		PKT_TX_OUTER_IP_CKSUM)
 
-/* Offset of mbuf dynamic field for protocol extraction data */
-int rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
-
-/* Mask of mbuf dynamic flags for protocol extraction type */
-uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
-uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+#define ICE_DYNF_PROTO_XTR_METADATA(m) \
+	RTE_MBUF_DYNFIELD((m), rxq->xtr_metadata_off, uint32_t *)
 
 static inline uint8_t
 ice_proto_xtr_type_to_rxdid(uint8_t xtr_type)
@@ -104,7 +96,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v1(struct ice_rx_queue *rxq,
 		if (metadata) {
 			mb->ol_flags |= rxq->xtr_ol_flag;
 
-			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
+			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
 		}
 	}
 #endif
@@ -142,7 +134,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
 		if (metadata) {
 			mb->ol_flags |= rxq->xtr_ol_flag;
 
-			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
+			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
 		}
 	}
 #endif
@@ -151,34 +143,38 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
 static void
 ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
 {
+	struct rte_mbuf_dynfield metadata_param;
+	const char *flag_name = NULL;
+	int flag_off, metadata_off;
+
 	switch (rxdid) {
 	case ICE_RXDID_COMMS_AUX_VLAN:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_vlan_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV4:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv4_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV6:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IPV6_FLOW:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_TCP:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_tcp_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
 		break;
 
 	case ICE_RXDID_COMMS_AUX_IP_OFFSET:
-		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
+		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME;
 		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v2;
 		break;
 
@@ -192,8 +188,34 @@ ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
 		break;
 	}
 
-	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
-		rxq->xtr_ol_flag = 0;
+	if (!flag_name)
+		return;
+
+	flag_off = rte_mbuf_dynflag_lookup(flag_name, NULL);
+	if (flag_off == -1) {
+		PMD_DRV_LOG(WARNING, "failed to lookup the dynamic flag '%s' for Rx queue %u : %d",
+			    flag_name, rxq->queue_id, -rte_errno);
+		return;
+	}
+
+	metadata_off = rte_mbuf_dynfield_lookup
+				(RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+				 &metadata_param);
+	if (metadata_off == -1) {
+		PMD_DRV_LOG(WARNING, "failed to lookup the dynamic field '%s' for Rx queue %u : %d",
+			    RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+			    rxq->queue_id, -rte_errno);
+		return;
+	}
+	if (metadata_param.size != sizeof(uint32_t)) {
+		PMD_DRV_LOG(WARNING, "the dynamic field '%s' data size is not matched for Rx queue %u",
+			    RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
+			    rxq->queue_id);
+		return;
+	}
+
+	rxq->xtr_metadata_off = metadata_off;
+	rxq->xtr_ol_flag = 1ULL << flag_off;
 }
 
 static enum ice_status
diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
index 23409d479a..53d0a609f3 100644
--- a/drivers/net/ice/ice_rxtx.h
+++ b/drivers/net/ice/ice_rxtx.h
@@ -85,6 +85,7 @@ struct ice_rx_queue {
 	bool q_set; /* indicate if rx queue has been configured */
 	bool rx_deferred_start; /* don't start this queue in dev start */
 	uint8_t proto_xtr; /* Protocol extraction from flexible descriptor */
+	int xtr_metadata_off; /* Protocol extraction offload metadata offset */
 	uint64_t xtr_ol_flag; /* Protocol extraction offload flag */
 	ice_rxd_to_pkt_fields_t rxd_to_pkt_fields; /* handle FlexiMD by RXDID */
 	ice_rx_release_mbufs_t rx_rel_mbufs;
diff --git a/drivers/net/ice/rte_pmd_ice.h b/drivers/net/ice/rte_pmd_ice.h
index 9a436a140b..f7304add40 100644
--- a/drivers/net/ice/rte_pmd_ice.h
+++ b/drivers/net/ice/rte_pmd_ice.h
@@ -14,18 +14,19 @@
  *
  */
 
-#include <stdio.h>
-#include <rte_mbuf.h>
-#include <rte_mbuf_dyn.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+#ifndef __INTEL_RX_FLEX_DESC_METADATA__
+#define __INTEL_RX_FLEX_DESC_METADATA__
+
 /**
  * The supported network protocol extraction metadata format.
  */
-union rte_net_ice_proto_xtr_metadata {
+union rte_pmd_proto_xtr_metadata {
 	uint32_t metadata;
 
 	struct {
@@ -82,160 +83,56 @@ union rte_net_ice_proto_xtr_metadata {
 	uint32_t ip_ofs;
 };
 
-/* Offset of mbuf dynamic field for protocol extraction data */
-extern int rte_net_ice_dynfield_proto_xtr_metadata_offs;
-
-/* Mask of mbuf dynamic flags for protocol extraction type */
-extern uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
-extern uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
-
-/**
- * The mbuf dynamic field pointer for protocol extraction metadata.
- */
-#define RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m) \
-	RTE_MBUF_DYNFIELD((m), \
-			  rte_net_ice_dynfield_proto_xtr_metadata_offs, \
-			  uint32_t *)
-
 /**
- * The mbuf dynamic flag for VLAN protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'vlan' specified.
- */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_VLAN \
-	(rte_net_ice_dynflag_proto_xtr_vlan_mask)
-
-/**
- * The mbuf dynamic flag for IPv4 protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ipv4' specified.
+ * The mbuf dynamic field metadata for protocol extraction from hardware:
+ *
+ *  a). Extract one word from the defined location of the specified
+ *      protocol in the packet.
+ *  b). Report the offset to the selected protocol type.
+ *
+ *  And the metadata can hold two of the above defined fields (in word), these
+ *  words are in host endian order.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV4 \
-	(rte_net_ice_dynflag_proto_xtr_ipv4_mask)
+#define RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME \
+	"rte_pmd_dynfield_proto_xtr_metadata"
 
 /**
- * The mbuf dynamic flag for IPv6 protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ipv6' specified.
+ * The mbuf dynamic flag for VLAN protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6 \
-	(rte_net_ice_dynflag_proto_xtr_ipv6_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME \
+	"rte_pmd_dynflag_proto_xtr_vlan"
 
 /**
- * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata, it is
- * valid when dev_args 'proto_xtr' has 'ipv6_flow' specified.
+ * The mbuf dynamic flag for IPv4 protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW \
-	(rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv4"
 
 /**
- * The mbuf dynamic flag for TCP protocol extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'tcp' specified.
+ * The mbuf dynamic flag for IPv6 protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_TCP \
-	(rte_net_ice_dynflag_proto_xtr_tcp_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv6"
 
 /**
- * The mbuf dynamic flag for IP_OFFSET extraction metadata, it is valid
- * when dev_args 'proto_xtr' has 'ip_offset' specified.
+ * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata type.
  */
-#define RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET \
-	(rte_net_ice_dynflag_proto_xtr_ip_offset_mask)
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME \
+	"rte_pmd_dynflag_proto_xtr_ipv6_flow"
 
 /**
- * Check if mbuf dynamic field for protocol extraction metadata is registered.
- *
- * @return
- *   True if registered, false otherwise.
+ * The mbuf dynamic flag for TCP protocol extraction metadata type.
  */
-__rte_experimental
-static __rte_always_inline int
-rte_net_ice_dynf_proto_xtr_metadata_avail(void)
-{
-	return rte_net_ice_dynfield_proto_xtr_metadata_offs != -1;
-}
+#define RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME \
+	"rte_pmd_dynflag_proto_xtr_tcp"
 
 /**
- * Get the mbuf dynamic field for protocol extraction metadata.
- *
- * @param m
- *    The pointer to the mbuf.
- * @return
- *   The saved protocol extraction metadata.
+ * The mbuf dynamic flag for IPv4 or IPv6 header offset report metadata type.
  */
-__rte_experimental
-static __rte_always_inline uint32_t
-rte_net_ice_dynf_proto_xtr_metadata_get(struct rte_mbuf *m)
-{
-	return *RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m);
-}
+#define RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME \
+	"rte_pmd_dynflag_proto_xtr_ip_offset"
 
-/**
- * Dump the mbuf dynamic field for protocol extraction metadata.
- *
- * @param m
- *    The pointer to the mbuf.
- */
-__rte_experimental
-static inline void
-rte_net_ice_dump_proto_xtr_metadata(struct rte_mbuf *m)
-{
-	union rte_net_ice_proto_xtr_metadata data;
-
-	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
-		return;
-
-	data.metadata = rte_net_ice_dynf_proto_xtr_metadata_get(m);
-
-	if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_VLAN)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],vlan,stag=%u:%u:%u,ctag=%u:%u:%u",
-		       data.raw.data0, data.raw.data1,
-		       data.vlan.stag_pcp,
-		       data.vlan.stag_dei,
-		       data.vlan.stag_vid,
-		       data.vlan.ctag_pcp,
-		       data.vlan.ctag_dei,
-		       data.vlan.ctag_vid);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV4)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv4,ver=%u,hdrlen=%u,tos=%u,ttl=%u,proto=%u",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv4.version,
-		       data.ipv4.ihl,
-		       data.ipv4.tos,
-		       data.ipv4.ttl,
-		       data.ipv4.protocol);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6,ver=%u,tc=%u,flow_hi4=0x%x,nexthdr=%u,hoplimit=%u",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv6.version,
-		       data.ipv6.tc,
-		       data.ipv6.flowhi4,
-		       data.ipv6.nexthdr,
-		       data.ipv6.hoplimit);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6_flow,ver=%u,tc=%u,flow=0x%x%04x",
-		       data.raw.data0, data.raw.data1,
-		       data.ipv6_flow.version,
-		       data.ipv6_flow.tc,
-		       data.ipv6_flow.flowhi4,
-		       data.ipv6_flow.flowlo16);
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_TCP)
-		printf(" - Protocol Extraction:[0x%04x:0x%04x],tcp,doff=%u,flags=%s%s%s%s%s%s%s%s",
-		       data.raw.data0, data.raw.data1,
-		       data.tcp.doff,
-		       data.tcp.cwr ? "C" : "",
-		       data.tcp.ece ? "E" : "",
-		       data.tcp.urg ? "U" : "",
-		       data.tcp.ack ? "A" : "",
-		       data.tcp.psh ? "P" : "",
-		       data.tcp.rst ? "R" : "",
-		       data.tcp.syn ? "S" : "",
-		       data.tcp.fin ? "F" : "");
-	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
-		printf(" - Protocol Offset:ip_offset=%u",
-		       data.ip_ofs);
-}
+#endif /* __INTEL_RX_FLEX_DESC_METADATA__ */
 
 #ifdef __cplusplus
 }
diff --git a/drivers/net/ice/version.map b/drivers/net/ice/version.map
index 632a296a0c..4a76d1d52d 100644
--- a/drivers/net/ice/version.map
+++ b/drivers/net/ice/version.map
@@ -1,16 +1,3 @@
 DPDK_21 {
 	local: *;
 };
-
-EXPERIMENTAL {
-	global:
-
-	# added in 19.11
-	rte_net_ice_dynfield_proto_xtr_metadata_offs;
-	rte_net_ice_dynflag_proto_xtr_vlan_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv4_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv6_mask;
-	rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
-	rte_net_ice_dynflag_proto_xtr_tcp_mask;
-	rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
-};
-- 
2.29.0


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

* Re: [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction
  2020-10-25  7:13 [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Haiyue Wang
  2020-10-26  6:50 ` [dpdk-dev] [PATCH v2] net/ice: refactor the protocol extraction design Haiyue Wang
  2020-10-26  6:55 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
@ 2020-10-26 10:22 ` Olivier Matz
  2020-10-26 11:29   ` Wang, Haiyue
  2020-10-27  1:23 ` [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name Haiyue Wang
  3 siblings, 1 reply; 8+ messages in thread
From: Olivier Matz @ 2020-10-26 10:22 UTC (permalink / raw)
  To: Haiyue Wang
  Cc: dev, ferruh.yigit, jia.guo, qi.z.zhang, zhaoyan.chen,
	Qiming Yang, Ray Kinsella, Neil Horman

Hi Haiyue,

On Sun, Oct 25, 2020 at 03:13:52PM +0800, Haiyue Wang wrote:
> Current dynamic mbuf design is that the driver will register the needed
> field and flags at the device probing time, this will make iavf PMD use
> different names to register the dynamic mbuf field and flags, but both
> of them use the exactly same protocol extraction metadata.
> 
> This will run out of the limited dynamic mbuf resource, meanwhile, the
> application has to handle the dynamic mbuf separately.
> 
> For making things simple and consistent, refactor dynamic mbuf in data
> extraction handling: the PMD just lookups the same name at the queue
> setup time after the application registers it.
> 
> In other words, make the dynamic mbuf string name as API, not the data
> object which is defined in each PMD.

In case the dynamic mbuf field is shared by several PMDs, it seems to
be indeed a better solution.

Currently, the "union rte_pmd_proto_xtr_metadata" is still defined in
rte_pmd_ice.h. Will it be the same for iavf, and will it be factorized
somewhere? However I don't know where could be a good place.

There is already lib/librte_mbuf/rte_mbuf_dyn.h which is the place to
centralize the name and description of dynamic fields/flags used in
libraries. But I think neither structure definitions nor PMD-specific
flags should go there too. I'd prefer to have them in
drivers/net/<common-something>, but I'm not sure it is possible.

Also, it is difficult from the patch to see the impact it has on an
application that was using these metadata. Should we have an example
of use?

Thanks,
Olivier


> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  drivers/net/ice/ice_ethdev.c  | 104 ++--------------
>  drivers/net/ice/ice_ethdev.h  |   1 +
>  drivers/net/ice/ice_rxtx.c    |  49 ++++----
>  drivers/net/ice/ice_rxtx.h    |   1 +
>  drivers/net/ice/rte_pmd_ice.h | 219 +++++-----------------------------
>  drivers/net/ice/version.map   |  13 --
>  6 files changed, 68 insertions(+), 319 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index 51b99c6506..ec27089cfa 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -32,42 +32,6 @@ static const char * const ice_valid_args[] = {
>  	NULL
>  };
>  
> -static const struct rte_mbuf_dynfield ice_proto_xtr_metadata_param = {
> -	.name = "ice_dynfield_proto_xtr_metadata",
> -	.size = sizeof(uint32_t),
> -	.align = __alignof__(uint32_t),
> -	.flags = 0,
> -};
> -
> -struct proto_xtr_ol_flag {
> -	const struct rte_mbuf_dynflag param;
> -	uint64_t *ol_flag;
> -	bool required;
> -};
> -
> -static bool ice_proto_xtr_hw_support[PROTO_XTR_MAX];
> -
> -static struct proto_xtr_ol_flag ice_proto_xtr_ol_flag_params[] = {
> -	[PROTO_XTR_VLAN] = {
> -		.param = { .name = "ice_dynflag_proto_xtr_vlan" },
> -		.ol_flag = &rte_net_ice_dynflag_proto_xtr_vlan_mask },
> -	[PROTO_XTR_IPV4] = {
> -		.param = { .name = "ice_dynflag_proto_xtr_ipv4" },
> -		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv4_mask },
> -	[PROTO_XTR_IPV6] = {
> -		.param = { .name = "ice_dynflag_proto_xtr_ipv6" },
> -		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_mask },
> -	[PROTO_XTR_IPV6_FLOW] = {
> -		.param = { .name = "ice_dynflag_proto_xtr_ipv6_flow" },
> -		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask },
> -	[PROTO_XTR_TCP] = {
> -		.param = { .name = "ice_dynflag_proto_xtr_tcp" },
> -		.ol_flag = &rte_net_ice_dynflag_proto_xtr_tcp_mask },
> -	[PROTO_XTR_IP_OFFSET] = {
> -		.param = { .name = "ice_dynflag_proto_xtr_ip_offset" },
> -		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ip_offset_mask },
> -};
> -
>  #define ICE_DFLT_OUTER_TAG_TYPE ICE_AQ_VSI_OUTER_TAG_VLAN_9100
>  
>  #define ICE_OS_DEFAULT_PKG_NAME		"ICE OS Default Package"
> @@ -542,7 +506,7 @@ handle_proto_xtr_arg(__rte_unused const char *key, const char *value,
>  }
>  
>  static void
> -ice_check_proto_xtr_support(struct ice_hw *hw)
> +ice_check_proto_xtr_support(struct ice_pf *pf, struct ice_hw *hw)
>  {
>  #define FLX_REG(val, fld, idx) \
>  	(((val) & GLFLXP_RXDID_FLX_WRD_##idx##_##fld##_M) >> \
> @@ -587,7 +551,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
>  
>  			if (FLX_REG(v, PROT_MDID, 4) == xtr_sets[i].protid_0 &&
>  			    FLX_REG(v, RXDID_OPCODE, 4) == xtr_sets[i].opcode)
> -				ice_proto_xtr_hw_support[i] = true;
> +				pf->hw_proto_xtr_ena[i] = 1;
>  		}
>  
>  		if (xtr_sets[i].protid_1 != ICE_PROT_ID_INVAL) {
> @@ -595,7 +559,7 @@ ice_check_proto_xtr_support(struct ice_hw *hw)
>  
>  			if (FLX_REG(v, PROT_MDID, 5) == xtr_sets[i].protid_1 &&
>  			    FLX_REG(v, RXDID_OPCODE, 5) == xtr_sets[i].opcode)
> -				ice_proto_xtr_hw_support[i] = true;
> +				pf->hw_proto_xtr_ena[i] = 1;
>  		}
>  	}
>  }
> @@ -1429,9 +1393,6 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
>  			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>  	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct ice_hw *hw = ICE_PF_TO_HW(pf);
> -	const struct proto_xtr_ol_flag *ol_flag;
> -	bool proto_xtr_enable = false;
> -	int offset;
>  	uint16_t i;
>  
>  	pf->proto_xtr = rte_zmalloc(NULL, pf->lan_nb_qps, 0);
> @@ -1440,65 +1401,20 @@ ice_init_proto_xtr(struct rte_eth_dev *dev)
>  		return;
>  	}
>  
> +	ice_check_proto_xtr_support(pf, hw);
> +
>  	for (i = 0; i < pf->lan_nb_qps; i++) {
>  		pf->proto_xtr[i] = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
>  				   ad->devargs.proto_xtr[i] :
>  				   ad->devargs.proto_xtr_dflt;
>  
> -		if (pf->proto_xtr[i] != PROTO_XTR_NONE) {
> -			uint8_t type = pf->proto_xtr[i];
> -
> -			ice_proto_xtr_ol_flag_params[type].required = true;
> -			proto_xtr_enable = true;
> -		}
> -	}
> -
> -	if (likely(!proto_xtr_enable))
> -		return;
> -
> -	ice_check_proto_xtr_support(hw);
> -
> -	offset = rte_mbuf_dynfield_register(&ice_proto_xtr_metadata_param);
> -	if (unlikely(offset == -1)) {
> -		PMD_DRV_LOG(ERR,
> -			    "Protocol extraction metadata is disabled in mbuf with error %d",
> -			    -rte_errno);
> -		return;
> -	}
> -
> -	PMD_DRV_LOG(DEBUG,
> -		    "Protocol extraction metadata offset in mbuf is : %d",
> -		    offset);
> -	rte_net_ice_dynfield_proto_xtr_metadata_offs = offset;
> -
> -	for (i = 0; i < RTE_DIM(ice_proto_xtr_ol_flag_params); i++) {
> -		ol_flag = &ice_proto_xtr_ol_flag_params[i];
> -
> -		if (!ol_flag->required)
> -			continue;
> -
> -		if (!ice_proto_xtr_hw_support[i]) {
> +		if (pf->proto_xtr[i] != PROTO_XTR_NONE &&
> +		    !pf->hw_proto_xtr_ena[pf->proto_xtr[i]]) {
>  			PMD_DRV_LOG(ERR,
> -				    "Protocol extraction type %u is not supported in hardware",
> -				    i);
> -			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
> -			break;
> +				    "The Rx queue %u doesn't support protocol extraction type %u\n",
> +				    i, pf->proto_xtr[i]);
> +			pf->proto_xtr[i] = PROTO_XTR_NONE;
>  		}
> -
> -		offset = rte_mbuf_dynflag_register(&ol_flag->param);
> -		if (unlikely(offset == -1)) {
> -			PMD_DRV_LOG(ERR,
> -				    "Protocol extraction offload '%s' failed to register with error %d",
> -				    ol_flag->param.name, -rte_errno);
> -
> -			rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
> -			break;
> -		}
> -
> -		PMD_DRV_LOG(DEBUG,
> -			    "Protocol extraction offload '%s' offset in mbuf is : %d",
> -			    ol_flag->param.name, offset);
> -		*ol_flag->ol_flag = 1ULL << offset;
>  	}
>  }
>  
> diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
> index 05218af05e..a675eac209 100644
> --- a/drivers/net/ice/ice_ethdev.h
> +++ b/drivers/net/ice/ice_ethdev.h
> @@ -457,6 +457,7 @@ struct ice_pf {
>  	uint64_t old_rx_bytes;
>  	uint64_t old_tx_bytes;
>  	uint64_t supported_rxdid; /* bitmap for supported RXDID */
> +	uint8_t hw_proto_xtr_ena[PROTO_XTR_MAX];
>  };
>  
>  #define ICE_MAX_QUEUE_NUM  2048
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> index f6291894cd..27f04a432f 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -15,16 +15,8 @@
>  		PKT_TX_TCP_SEG |		 \
>  		PKT_TX_OUTER_IP_CKSUM)
>  
> -/* Offset of mbuf dynamic field for protocol extraction data */
> -int rte_net_ice_dynfield_proto_xtr_metadata_offs = -1;
> -
> -/* Mask of mbuf dynamic flags for protocol extraction type */
> -uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
> -uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
> -uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
> -uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
> -uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
> -uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
> +#define ICE_DYNF_PROTO_XTR_METADATA(m) \
> +	RTE_MBUF_DYNFIELD((m), rxq->xtr_metadata_off, uint32_t *)
>  
>  static inline uint8_t
>  ice_proto_xtr_type_to_rxdid(uint8_t xtr_type)
> @@ -104,7 +96,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v1(struct ice_rx_queue *rxq,
>  		if (metadata) {
>  			mb->ol_flags |= rxq->xtr_ol_flag;
>  
> -			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
> +			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
>  		}
>  	}
>  #endif
> @@ -142,7 +134,7 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
>  		if (metadata) {
>  			mb->ol_flags |= rxq->xtr_ol_flag;
>  
> -			*RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
> +			*ICE_DYNF_PROTO_XTR_METADATA(mb) = metadata;
>  		}
>  	}
>  #endif
> @@ -151,34 +143,38 @@ ice_rxd_to_pkt_fields_by_comms_aux_v2(struct ice_rx_queue *rxq,
>  static void
>  ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
>  {
> +	struct rte_mbuf_dynfield metadata_param;
> +	const char *flag_name = NULL;
> +	int flag_off, metadata_off;
> +
>  	switch (rxdid) {
>  	case ICE_RXDID_COMMS_AUX_VLAN:
> -		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_vlan_mask;
> +		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME;
>  		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
>  		break;
>  
>  	case ICE_RXDID_COMMS_AUX_IPV4:
> -		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv4_mask;
> +		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME;
>  		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
>  		break;
>  
>  	case ICE_RXDID_COMMS_AUX_IPV6:
> -		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_mask;
> +		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME;
>  		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
>  		break;
>  
>  	case ICE_RXDID_COMMS_AUX_IPV6_FLOW:
> -		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
> +		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME;
>  		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
>  		break;
>  
>  	case ICE_RXDID_COMMS_AUX_TCP:
> -		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_tcp_mask;
> +		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME;
>  		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v1;
>  		break;
>  
>  	case ICE_RXDID_COMMS_AUX_IP_OFFSET:
> -		rxq->xtr_ol_flag = rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
> +		flag_name = RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME;
>  		rxq->rxd_to_pkt_fields = ice_rxd_to_pkt_fields_by_comms_aux_v2;
>  		break;
>  
> @@ -192,8 +188,21 @@ ice_select_rxd_to_pkt_fields_handler(struct ice_rx_queue *rxq, uint32_t rxdid)
>  		break;
>  	}
>  
> -	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
> -		rxq->xtr_ol_flag = 0;
> +	if (!flag_name)
> +		return;
> +
> +	flag_off = rte_mbuf_dynflag_lookup(flag_name, NULL);
> +	if (flag_off == -1)
> +		return;
> +
> +	metadata_off = rte_mbuf_dynfield_lookup
> +				(RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME,
> +				 &metadata_param);
> +	if (metadata_off == -1 || metadata_param.size != sizeof(uint32_t))
> +		return;
> +
> +	rxq->xtr_metadata_off = metadata_off;
> +	rxq->xtr_ol_flag = 1ULL << flag_off;
>  }
>  
>  static enum ice_status
> diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
> index 23409d479a..53d0a609f3 100644
> --- a/drivers/net/ice/ice_rxtx.h
> +++ b/drivers/net/ice/ice_rxtx.h
> @@ -85,6 +85,7 @@ struct ice_rx_queue {
>  	bool q_set; /* indicate if rx queue has been configured */
>  	bool rx_deferred_start; /* don't start this queue in dev start */
>  	uint8_t proto_xtr; /* Protocol extraction from flexible descriptor */
> +	int xtr_metadata_off; /* Protocol extraction offload metadata offset */
>  	uint64_t xtr_ol_flag; /* Protocol extraction offload flag */
>  	ice_rxd_to_pkt_fields_t rxd_to_pkt_fields; /* handle FlexiMD by RXDID */
>  	ice_rx_release_mbufs_t rx_rel_mbufs;
> diff --git a/drivers/net/ice/rte_pmd_ice.h b/drivers/net/ice/rte_pmd_ice.h
> index 9a436a140b..df27b59d35 100644
> --- a/drivers/net/ice/rte_pmd_ice.h
> +++ b/drivers/net/ice/rte_pmd_ice.h
> @@ -22,220 +22,55 @@
>  extern "C" {
>  #endif
>  
> -/**
> - * The supported network protocol extraction metadata format.
> - */
> -union rte_net_ice_proto_xtr_metadata {
> -	uint32_t metadata;
> -
> -	struct {
> -		uint16_t data0;
> -		uint16_t data1;
> -	} raw;
> -
> -	struct {
> -		uint16_t stag_vid:12,
> -			 stag_dei:1,
> -			 stag_pcp:3;
> -		uint16_t ctag_vid:12,
> -			 ctag_dei:1,
> -			 ctag_pcp:3;
> -	} vlan;
> -
> -	struct {
> -		uint16_t protocol:8,
> -			 ttl:8;
> -		uint16_t tos:8,
> -			 ihl:4,
> -			 version:4;
> -	} ipv4;
> -
> -	struct {
> -		uint16_t hoplimit:8,
> -			 nexthdr:8;
> -		uint16_t flowhi4:4,
> -			 tc:8,
> -			 version:4;
> -	} ipv6;
> -
> -	struct {
> -		uint16_t flowlo16;
> -		uint16_t flowhi4:4,
> -			 tc:8,
> -			 version:4;
> -	} ipv6_flow;
> -
> -	struct {
> -		uint16_t fin:1,
> -			 syn:1,
> -			 rst:1,
> -			 psh:1,
> -			 ack:1,
> -			 urg:1,
> -			 ece:1,
> -			 cwr:1,
> -			 res1:4,
> -			 doff:4;
> -		uint16_t rsvd;
> -	} tcp;
> -
> -	uint32_t ip_ofs;
> -};
> -
> -/* Offset of mbuf dynamic field for protocol extraction data */
> -extern int rte_net_ice_dynfield_proto_xtr_metadata_offs;
> -
> -/* Mask of mbuf dynamic flags for protocol extraction type */
> -extern uint64_t rte_net_ice_dynflag_proto_xtr_vlan_mask;
> -extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv4_mask;
> -extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_mask;
> -extern uint64_t rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
> -extern uint64_t rte_net_ice_dynflag_proto_xtr_tcp_mask;
> -extern uint64_t rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
> +#ifndef __INTEL_FXP_RX_DESC_METADATA__
> +#define __INTEL_FXP_RX_DESC_METADATA__
>  
>  /**
> - * The mbuf dynamic field pointer for protocol extraction metadata.
> + * The mbuf dynamic field for metadata extraction from NIC:
> + *  a). Extract 16b (2 Bytes) from the defined offset location of the specified
> + *      protocol in the packet.
> + *  b). Report the offset to the selected protocol.
>   */
> -#define RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m) \
> -	RTE_MBUF_DYNFIELD((m), \
> -			  rte_net_ice_dynfield_proto_xtr_metadata_offs, \
> -			  uint32_t *)
> +#define RTE_PMD_DYNFIELD_PROTO_XTR_METADATA_NAME \
> +	"rte_pmd_dynfield_proto_xtr_metadata"
>  
>  /**
> - * The mbuf dynamic flag for VLAN protocol extraction metadata, it is valid
> - * when dev_args 'proto_xtr' has 'vlan' specified.
> + * The mbuf dynamic flag for VLAN protocol extraction metadata type.
>   */
> -#define RTE_PKT_RX_DYNF_PROTO_XTR_VLAN \
> -	(rte_net_ice_dynflag_proto_xtr_vlan_mask)
> +#define RTE_PMD_DYNFLAG_PROTO_XTR_VLAN_NAME \
> +	"rte_pmd_dynflag_proto_xtr_vlan"
>  
>  /**
> - * The mbuf dynamic flag for IPv4 protocol extraction metadata, it is valid
> - * when dev_args 'proto_xtr' has 'ipv4' specified.
> + * The mbuf dynamic flag for IPv4 protocol extraction metadata type.
>   */
> -#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV4 \
> -	(rte_net_ice_dynflag_proto_xtr_ipv4_mask)
> +#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV4_NAME \
> +	"rte_pmd_dynflag_proto_xtr_ipv4"
>  
>  /**
> - * The mbuf dynamic flag for IPv6 protocol extraction metadata, it is valid
> - * when dev_args 'proto_xtr' has 'ipv6' specified.
> + * The mbuf dynamic flag for IPv6 protocol extraction metadata type.
>   */
> -#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6 \
> -	(rte_net_ice_dynflag_proto_xtr_ipv6_mask)
> +#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_NAME \
> +	"rte_pmd_dynflag_proto_xtr_ipv6"
>  
>  /**
> - * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata, it is
> - * valid when dev_args 'proto_xtr' has 'ipv6_flow' specified.
> + * The mbuf dynamic flag for IPv6 with flow protocol extraction metadata type.
>   */
> -#define RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW \
> -	(rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask)
> +#define RTE_PMD_DYNFLAG_PROTO_XTR_IPV6_FLOW_NAME \
> +	"rte_pmd_dynflag_proto_xtr_ipv6_flow"
>  
>  /**
> - * The mbuf dynamic flag for TCP protocol extraction metadata, it is valid
> - * when dev_args 'proto_xtr' has 'tcp' specified.
> + * The mbuf dynamic flag for TCP protocol extraction metadata type.
>   */
> -#define RTE_PKT_RX_DYNF_PROTO_XTR_TCP \
> -	(rte_net_ice_dynflag_proto_xtr_tcp_mask)
> +#define RTE_PMD_DYNFLAG_PROTO_XTR_TCP_NAME \
> +	"rte_pmd_dynflag_proto_xtr_tcp"
>  
>  /**
> - * The mbuf dynamic flag for IP_OFFSET extraction metadata, it is valid
> - * when dev_args 'proto_xtr' has 'ip_offset' specified.
> +  * The mbuf dynamic flag for IPv4 or IPv6 header offset report metadata type.
>   */
> -#define RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET \
> -	(rte_net_ice_dynflag_proto_xtr_ip_offset_mask)
> +#define RTE_PMD_DYNFLAG_PROTO_XTR_IP_OFFSET_NAME \
> +	"rte_pmd_dynflag_proto_xtr_ip_offset"
>  
> -/**
> - * Check if mbuf dynamic field for protocol extraction metadata is registered.
> - *
> - * @return
> - *   True if registered, false otherwise.
> - */
> -__rte_experimental
> -static __rte_always_inline int
> -rte_net_ice_dynf_proto_xtr_metadata_avail(void)
> -{
> -	return rte_net_ice_dynfield_proto_xtr_metadata_offs != -1;
> -}
> -
> -/**
> - * Get the mbuf dynamic field for protocol extraction metadata.
> - *
> - * @param m
> - *    The pointer to the mbuf.
> - * @return
> - *   The saved protocol extraction metadata.
> - */
> -__rte_experimental
> -static __rte_always_inline uint32_t
> -rte_net_ice_dynf_proto_xtr_metadata_get(struct rte_mbuf *m)
> -{
> -	return *RTE_NET_ICE_DYNF_PROTO_XTR_METADATA(m);
> -}
> -
> -/**
> - * Dump the mbuf dynamic field for protocol extraction metadata.
> - *
> - * @param m
> - *    The pointer to the mbuf.
> - */
> -__rte_experimental
> -static inline void
> -rte_net_ice_dump_proto_xtr_metadata(struct rte_mbuf *m)
> -{
> -	union rte_net_ice_proto_xtr_metadata data;
> -
> -	if (!rte_net_ice_dynf_proto_xtr_metadata_avail())
> -		return;
> -
> -	data.metadata = rte_net_ice_dynf_proto_xtr_metadata_get(m);
> -
> -	if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_VLAN)
> -		printf(" - Protocol Extraction:[0x%04x:0x%04x],vlan,stag=%u:%u:%u,ctag=%u:%u:%u",
> -		       data.raw.data0, data.raw.data1,
> -		       data.vlan.stag_pcp,
> -		       data.vlan.stag_dei,
> -		       data.vlan.stag_vid,
> -		       data.vlan.ctag_pcp,
> -		       data.vlan.ctag_dei,
> -		       data.vlan.ctag_vid);
> -	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV4)
> -		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv4,ver=%u,hdrlen=%u,tos=%u,ttl=%u,proto=%u",
> -		       data.raw.data0, data.raw.data1,
> -		       data.ipv4.version,
> -		       data.ipv4.ihl,
> -		       data.ipv4.tos,
> -		       data.ipv4.ttl,
> -		       data.ipv4.protocol);
> -	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6)
> -		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6,ver=%u,tc=%u,flow_hi4=0x%x,nexthdr=%u,hoplimit=%u",
> -		       data.raw.data0, data.raw.data1,
> -		       data.ipv6.version,
> -		       data.ipv6.tc,
> -		       data.ipv6.flowhi4,
> -		       data.ipv6.nexthdr,
> -		       data.ipv6.hoplimit);
> -	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW)
> -		printf(" - Protocol Extraction:[0x%04x:0x%04x],ipv6_flow,ver=%u,tc=%u,flow=0x%x%04x",
> -		       data.raw.data0, data.raw.data1,
> -		       data.ipv6_flow.version,
> -		       data.ipv6_flow.tc,
> -		       data.ipv6_flow.flowhi4,
> -		       data.ipv6_flow.flowlo16);
> -	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_TCP)
> -		printf(" - Protocol Extraction:[0x%04x:0x%04x],tcp,doff=%u,flags=%s%s%s%s%s%s%s%s",
> -		       data.raw.data0, data.raw.data1,
> -		       data.tcp.doff,
> -		       data.tcp.cwr ? "C" : "",
> -		       data.tcp.ece ? "E" : "",
> -		       data.tcp.urg ? "U" : "",
> -		       data.tcp.ack ? "A" : "",
> -		       data.tcp.psh ? "P" : "",
> -		       data.tcp.rst ? "R" : "",
> -		       data.tcp.syn ? "S" : "",
> -		       data.tcp.fin ? "F" : "");
> -	else if (m->ol_flags & RTE_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET)
> -		printf(" - Protocol Offset:ip_offset=%u",
> -		       data.ip_ofs);
> -}
> +#endif
>  
>  #ifdef __cplusplus
>  }
> diff --git a/drivers/net/ice/version.map b/drivers/net/ice/version.map
> index 632a296a0c..4a76d1d52d 100644
> --- a/drivers/net/ice/version.map
> +++ b/drivers/net/ice/version.map
> @@ -1,16 +1,3 @@
>  DPDK_21 {
>  	local: *;
>  };
> -
> -EXPERIMENTAL {
> -	global:
> -
> -	# added in 19.11
> -	rte_net_ice_dynfield_proto_xtr_metadata_offs;
> -	rte_net_ice_dynflag_proto_xtr_vlan_mask;
> -	rte_net_ice_dynflag_proto_xtr_ipv4_mask;
> -	rte_net_ice_dynflag_proto_xtr_ipv6_mask;
> -	rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask;
> -	rte_net_ice_dynflag_proto_xtr_tcp_mask;
> -	rte_net_ice_dynflag_proto_xtr_ip_offset_mask;
> -};
> -- 
> 2.29.0
> 

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

* Re: [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction
  2020-10-26 10:22 ` [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Olivier Matz
@ 2020-10-26 11:29   ` Wang, Haiyue
  0 siblings, 0 replies; 8+ messages in thread
From: Wang, Haiyue @ 2020-10-26 11:29 UTC (permalink / raw)
  To: Olivier Matz
  Cc: dev, Yigit, Ferruh, Guo, Jia, Zhang, Qi Z, Chen, Zhaoyan, Yang,
	Qiming, Ray Kinsella, Neil Horman

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Monday, October 26, 2020 18:22
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Guo, Jia <jia.guo@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chen, Zhaoyan <zhaoyan.chen@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> Subject: Re: [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction
> 
> Hi Haiyue,
> 
> On Sun, Oct 25, 2020 at 03:13:52PM +0800, Haiyue Wang wrote:
> > Current dynamic mbuf design is that the driver will register the needed
> > field and flags at the device probing time, this will make iavf PMD use
> > different names to register the dynamic mbuf field and flags, but both
> > of them use the exactly same protocol extraction metadata.
> >
> > This will run out of the limited dynamic mbuf resource, meanwhile, the
> > application has to handle the dynamic mbuf separately.
> >
> > For making things simple and consistent, refactor dynamic mbuf in data
> > extraction handling: the PMD just lookups the same name at the queue
> > setup time after the application registers it.
> >
> > In other words, make the dynamic mbuf string name as API, not the data
> > object which is defined in each PMD.
> 
> In case the dynamic mbuf field is shared by several PMDs, it seems to
> be indeed a better solution.
> 
> Currently, the "union rte_pmd_proto_xtr_metadata" is still defined in
> rte_pmd_ice.h. Will it be the same for iavf, and will it be factorized
> somewhere? However I don't know where could be a good place.
> 
> There is already lib/librte_mbuf/rte_mbuf_dyn.h which is the place to
> centralize the name and description of dynamic fields/flags used in
> libraries. But I think neither structure definitions nor PMD-specific
> flags should go there too. I'd prefer to have them in
> drivers/net/<common-something>, but I'm not sure it is possible.

May be new 'lib/librte_mbuf/rte_mbuf_dyn_pmd.h' for all PMDs specific ?
So that the application knows exactly how many *dynamic* things. Also,
a new API to query the dynamic information + dev_ops may be introduced
in next release cycle, then 'rte_pmd_mlx5_get_dyn_flag_names' can be
removed. And the application will be clean.

Currently, we use " #define __INTEL_RX_FLEX_DESC_METADATA__ " to fix the
duplicated definition, but the application have to include the two header
files like "rte_pmd_ice.h" / "rte_pmd_iavf.h"

> 
> Also, it is difficult from the patch to see the impact it has on an
> application that was using these metadata. Should we have an example
> of use?
> 

Thanks your link in previous mail:
http://inbox.dpdk.org/dev/20191030165626.w3flq5wdpitpsv2v@platinum/

Original patch uses: Solution 1, provide static inline helpers to access to the
dyn fields/flags

Now now: Solution 2, without global variable export and helpers: the application
calls rte_mbuf_dynfield_register(&rte_pmd_ice_proto_xtr_metadata_param) to get
the offset, and store it privately.

https://patchwork.dpdk.org/patch/82165/
In v3 patch, I kept the metadata format, and rename it to be more generic:
'union rte_pmd_proto_xtr_metadata', but no dump function as the original design.

> Thanks,
> Olivier
> 
> 
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> > 2.29.0
> >

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

* [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name
  2020-10-25  7:13 [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Haiyue Wang
                   ` (2 preceding siblings ...)
  2020-10-26 10:22 ` [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Olivier Matz
@ 2020-10-27  1:23 ` Haiyue Wang
  2020-10-27  1:42   ` Zhang, Qi Z
  3 siblings, 1 reply; 8+ messages in thread
From: Haiyue Wang @ 2020-10-27  1:23 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, jia.guo, qi.z.zhang, zhaoyan.chen, olivier.matz,
	Haiyue Wang, Qiming Yang

Rename the dynamic mbuf name to 'intel_pmd_xxx' format, so that the
Intel PMD which has the protocol extraction feature will share the
same dynamic field/flags space in mbuf.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v4: "who register, who use", even the PMD has the duplicated code with
    different name space, the defined API will help the application to
    focus on the function itself that the PMD provides.

v3: Fix 'regiser' typo in commit message.

v2: update the commit message, doc; and add the error handling for
    dynamic mbuf lookup. Also keep the metadata format defination.
---
 drivers/net/ice/ice_ethdev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 3483f99897..d51f3faba4 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -33,7 +33,7 @@ static const char * const ice_valid_args[] = {
 };
 
 static const struct rte_mbuf_dynfield ice_proto_xtr_metadata_param = {
-	.name = "ice_dynfield_proto_xtr_metadata",
+	.name = "intel_pmd_dynfield_proto_xtr_metadata",
 	.size = sizeof(uint32_t),
 	.align = __alignof__(uint32_t),
 	.flags = 0,
@@ -49,22 +49,22 @@ static bool ice_proto_xtr_hw_support[PROTO_XTR_MAX];
 
 static struct proto_xtr_ol_flag ice_proto_xtr_ol_flag_params[] = {
 	[PROTO_XTR_VLAN] = {
-		.param = { .name = "ice_dynflag_proto_xtr_vlan" },
+		.param = { .name = "intel_pmd_dynflag_proto_xtr_vlan" },
 		.ol_flag = &rte_net_ice_dynflag_proto_xtr_vlan_mask },
 	[PROTO_XTR_IPV4] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv4" },
+		.param = { .name = "intel_pmd_dynflag_proto_xtr_ipv4" },
 		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv4_mask },
 	[PROTO_XTR_IPV6] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6" },
+		.param = { .name = "intel_pmd_dynflag_proto_xtr_ipv6" },
 		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_mask },
 	[PROTO_XTR_IPV6_FLOW] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ipv6_flow" },
+		.param = { .name = "intel_pmd_dynflag_proto_xtr_ipv6_flow" },
 		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ipv6_flow_mask },
 	[PROTO_XTR_TCP] = {
-		.param = { .name = "ice_dynflag_proto_xtr_tcp" },
+		.param = { .name = "intel_pmd_dynflag_proto_xtr_tcp" },
 		.ol_flag = &rte_net_ice_dynflag_proto_xtr_tcp_mask },
 	[PROTO_XTR_IP_OFFSET] = {
-		.param = { .name = "ice_dynflag_proto_xtr_ip_offset" },
+		.param = { .name = "intel_pmd_dynflag_proto_xtr_ip_offset" },
 		.ol_flag = &rte_net_ice_dynflag_proto_xtr_ip_offset_mask },
 };
 
-- 
2.29.0


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

* Re: [dpdk-dev] [PATCH v3] net/ice: refactor the protocol extraction design
  2020-10-26  6:55 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
@ 2020-10-27  1:35   ` Zhang, Qi Z
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2020-10-27  1:35 UTC (permalink / raw)
  To: Wang, Haiyue, dev
  Cc: Yigit, Ferruh, Guo, Jia, Chen, Zhaoyan, Yang, Qiming,
	Ray Kinsella, Neil Horman



> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Monday, October 26, 2020 2:55 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Guo, Jia <jia.guo@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Chen, Zhaoyan
> <zhaoyan.chen@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>; Yang,
> Qiming <qiming.yang@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil
> Horman <nhorman@tuxdriver.com>
> Subject: [PATCH v3] net/ice: refactor the protocol extraction design
> 
> Change the protocol extraction dynamic mbuf usage from register API to
> lookup API, so the application can decide to read the metadata or not at the
> run time, in other words, PMD will check this at Rx queue start time.
> 
> This design makes the API simple now: it just needs to export the name string,
> not the whole dynamic mbuf data objects.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

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

* Re: [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name
  2020-10-27  1:23 ` [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name Haiyue Wang
@ 2020-10-27  1:42   ` Zhang, Qi Z
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2020-10-27  1:42 UTC (permalink / raw)
  To: Wang, Haiyue, dev
  Cc: Yigit, Ferruh, Guo, Jia, Chen, Zhaoyan, olivier.matz, Yang, Qiming



> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Tuesday, October 27, 2020 9:23 AM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Guo, Jia <jia.guo@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Chen, Zhaoyan
> <zhaoyan.chen@intel.com>; olivier.matz@6wind.com; Wang, Haiyue
> <haiyue.wang@intel.com>; Yang, Qiming <qiming.yang@intel.com>
> Subject: [PATCH v4] net/ice: rename the dynamic mbuf name
> 
> Rename the dynamic mbuf name to 'intel_pmd_xxx' format, so that the Intel
> PMD which has the protocol extraction feature will share the same dynamic
> field/flags space in mbuf.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel after revert v3.

Thanks
Qi

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

end of thread, other threads:[~2020-10-27  1:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-25  7:13 [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Haiyue Wang
2020-10-26  6:50 ` [dpdk-dev] [PATCH v2] net/ice: refactor the protocol extraction design Haiyue Wang
2020-10-26  6:55 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
2020-10-27  1:35   ` Zhang, Qi Z
2020-10-26 10:22 ` [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Olivier Matz
2020-10-26 11:29   ` Wang, Haiyue
2020-10-27  1:23 ` [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name Haiyue Wang
2020-10-27  1:42   ` Zhang, Qi Z

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).