[dpdk-dev] [PATCH 1/5] net/enic: fix action reordering

[dpdk-dev] [PATCH 1/5] net/enic: fix action reordering

[John Daley]

From: Hyong Youb Kim <hyonkim@cisco.com>

The current implementation produces wrong ordering for several cases
like these:

1. mark, decap, steer
Current: steer, mark, decap
Correct: mark, steer, decap

2. decap, steer, steer
Current: steer, steer, decap
Correct: steer, decap, steer

Simplify the logic and swap 1st steer and decap.
Also, allow just one decap action per flow.

Fixes: ea7768b5bba8 ("net/enic: add flow implementation based on Flow Manager API")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Signed-off-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_fm_flow.c | 63 +++++++++++++++------------------
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c
index d815f369ed..8d715fc436 100644
--- a/drivers/net/enic/enic_fm_flow.c
+++ b/drivers/net/enic/enic_fm_flow.c
@@ -870,46 +870,36 @@ enic_fm_append_action_op(struct enic_flowman *fm,
 	return 0;
 }
 
-/* Steer operations need to appear before other ops */
+/* NIC requires that 1st steer appear before decap.
+ * Correct example: steer, decap, steer, steer, ...
+ */
 static void
 enic_fm_reorder_action_op(struct enic_flowman *fm)
 {
-	struct fm_action_op *dst, *dst_head, *src, *src_head;
+	struct fm_action_op *op, *steer, *decap;
+	struct fm_action_op tmp_op;
 
 	ENICPMD_FUNC_TRACE();
-	/* Move steer ops to the front. */
-	src = fm->action.fma_action_ops;
-	src_head = src;
-	dst = fm->action_tmp.fma_action_ops;
-	dst_head = dst;
-	/* Copy steer ops to tmp */
-	while (src->fa_op != FMOP_END) {
-		if (src->fa_op == FMOP_RQ_STEER) {
-			ENICPMD_LOG(DEBUG, "move op: %ld -> dst %ld",
-				    (long)(src - src_head),
-				    (long)(dst - dst_head));
-			*dst = *src;
-			dst++;
-		}
-		src++;
-	}
-	/* Then append non-steer ops */
-	src = src_head;
-	while (src->fa_op != FMOP_END) {
-		if (src->fa_op != FMOP_RQ_STEER) {
-			ENICPMD_LOG(DEBUG, "move op: %ld -> dst %ld",
-				    (long)(src - src_head),
-				    (long)(dst - dst_head));
-			*dst = *src;
-			dst++;
-		}
-		src++;
+	/* Find 1st steer and decap */
+	op = fm->action.fma_action_ops;
+	steer = NULL;
+	decap = NULL;
+	while (op->fa_op != FMOP_END) {
+		if (!decap && op->fa_op == FMOP_DECAP_NOSTRIP)
+			decap = op;
+		else if (!steer && op->fa_op == FMOP_RQ_STEER)
+			steer = op;
+		op++;
+	}
+	/* If decap is before steer, swap */
+	if (steer && decap && decap < steer) {
+		op = fm->action.fma_action_ops;
+		ENICPMD_LOG(DEBUG, "swap decap %ld <-> steer %ld",
+			    (long)(decap - op), (long)(steer - op));
+		tmp_op = *decap;
+		*decap = *steer;
+		*steer = tmp_op;
 	}
-	/* Copy END */
-	*dst = *src;
-	/* Finally replace the original action with the reordered one */
-	memcpy(fm->action.fma_action_ops, fm->action_tmp.fma_action_ops,
-	       sizeof(fm->action.fma_action_ops));
 }
 
 /* VXLAN decap is done via flowman compound action */
@@ -1100,6 +1090,7 @@ enic_fm_copy_action(struct enic_flowman *fm,
 		PASSTHRU = 1 << 2,
 		COUNT = 1 << 3,
 		ENCAP = 1 << 4,
+		DECAP = 1 << 5,
 	};
 	struct fm_tcam_match_entry *fmt;
 	struct fm_action_op fm_op;
@@ -1282,6 +1273,10 @@ enic_fm_copy_action(struct enic_flowman *fm,
 			break;
 		}
 		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP: {
+			if (overlap & DECAP)
+				goto unsupported;
+			overlap |= DECAP;
+
 			ret = enic_fm_copy_vxlan_decap(fm, fmt, actions,
 				error);
 			if (ret != 0)
-- 
2.22.0

[dpdk-dev] [PATCH 2/5] net/enic: flow manager API update

[John Daley]

Update the VIC Flow Manager API. The extentions will allow support for:
  - Decap and strip VLAN
  - Remove outer VLAN
  - Set Egress port
  - Set VLAN when replicating encapped packets
  - RSS queue ranges on outer header

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 drivers/net/enic/base/vnic_flowman.h | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/base/vnic_flowman.h b/drivers/net/enic/base/vnic_flowman.h
index 49f743f5fb..81e2cff1b0 100644
--- a/drivers/net/enic/base/vnic_flowman.h
+++ b/drivers/net/enic/base/vnic_flowman.h
@@ -236,6 +236,20 @@ enum {
 	FMOP_SET_OVLAN,
 	/* Decap when vlan_strip is off */
 	FMOP_DECAP_NOSTRIP,
+	/* Decap and strip VLAN */
+	FMOP_DECAP_STRIP,
+	/* Remove outer VLAN */
+	FMOP_POP_VLAN,
+	/* Set Egress port */
+	FMOP_SET_EGPORT,
+	/* Steer to an RQ without entering EMIT state */
+	FMOP_RQ_STEER_ONLY,
+	/* Set VLAN when replicating encapped packets */
+	FMOP_SET_ENCAP_VLAN,
+	/* Enter EMIT state */
+	FMOP_EMIT,
+	/* Enter MODIFY state */
+	FMOP_MODIFY,
 	FMOP_OP_MAX,
 };
 
@@ -260,12 +274,16 @@ struct fm_action_op {
 			uint8_t template_len;
 		} __rte_packed encap;
 		struct {
-			uint32_t rq_index;
+			uint16_t rq_index;
+			uint16_t rq_count;
 			uint64_t vnic_handle;
 		} __rte_packed rq_steer;
 		struct {
 			uint16_t vlan;
 		} __rte_packed ovlan;
+		struct {
+			uint16_t vlan;
+		} __rte_packed set_encap_vlan;
 		struct {
 			uint16_t mark;
 		} __rte_packed mark;
@@ -278,6 +296,9 @@ struct fm_action_op {
 		struct {
 			uint64_t handle;
 		} __rte_packed exact;
+		struct {
+			uint32_t egport;
+		} __rte_packed set_egport;
 	} __rte_packed;
 } __rte_packed;
 
-- 
2.22.0

[dpdk-dev] [PATCH 3/5] net/enic: change Rx queue ordering to enable RSS action

[John Daley]

Each RTE RQ is represented on enic as a Start Of Packet (SOP) queue and
and overflow queue (DATA). There were arranged SOP0/DATA0, SOP1/DATA1,...
but need to be arranged SOP0, SOP1,..., DATA0, DATA1... so that
rte_flow RSS queue ranges work.

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 drivers/net/enic/enic.h        | 13 +++++++------
 drivers/net/enic/enic_ethdev.c |  2 +-
 drivers/net/enic/enic_main.c   | 13 ++++++-------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index c9901faf5f..a95e51eea8 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -221,25 +221,26 @@ static inline uint32_t enic_mtu_to_max_rx_pktlen(uint32_t mtu)
 /* Get the CQ index from a Start of Packet(SOP) RQ index */
 static inline unsigned int enic_sop_rq_idx_to_cq_idx(unsigned int sop_idx)
 {
-	return sop_idx / 2;
+	return sop_idx;
 }
 
 /* Get the RTE RQ index from a Start of Packet(SOP) RQ index */
 static inline unsigned int enic_sop_rq_idx_to_rte_idx(unsigned int sop_idx)
 {
-	return sop_idx / 2;
+	return sop_idx;
 }
 
 /* Get the Start of Packet(SOP) RQ index from a RTE RQ index */
 static inline unsigned int enic_rte_rq_idx_to_sop_idx(unsigned int rte_idx)
 {
-	return rte_idx * 2;
+	return rte_idx;
 }
 
 /* Get the Data RQ index from a RTE RQ index */
-static inline unsigned int enic_rte_rq_idx_to_data_idx(unsigned int rte_idx)
+static inline unsigned int enic_rte_rq_idx_to_data_idx(unsigned int rte_idx,
+						       struct enic *enic)
 {
-	return rte_idx * 2 + 1;
+	return enic->rq_count + rte_idx;
 }
 
 static inline unsigned int enic_vnic_rq_count(struct enic *enic)
@@ -253,7 +254,7 @@ static inline unsigned int enic_cq_rq(__rte_unused struct enic *enic, unsigned i
 	 * completion queue, so the completion queue number is no
 	 * longer the same as the rq number.
 	 */
-	return rq / 2;
+	return rq;
 }
 
 static inline unsigned int enic_cq_wq(struct enic *enic, unsigned int wq)
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index a7a178e41b..32d5397f85 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -940,7 +940,7 @@ static void enicpmd_dev_rxq_info_get(struct rte_eth_dev *dev,
 
 	ENICPMD_FUNC_TRACE();
 	sop_queue_idx = enic_rte_rq_idx_to_sop_idx(rx_queue_id);
-	data_queue_idx = enic_rte_rq_idx_to_data_idx(rx_queue_id);
+	data_queue_idx = enic_rte_rq_idx_to_data_idx(rx_queue_id, enic);
 	rq_sop = &enic->rq[sop_queue_idx];
 	rq_data = &enic->rq[data_queue_idx]; /* valid if data_queue_enable */
 	qinfo->mp = rq_sop->mp;
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 78e2dd133c..7942b0df6b 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -222,13 +222,12 @@ void enic_init_vnic_resources(struct enic *enic)
 			error_interrupt_enable,
 			error_interrupt_offset);
 
-		data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(index)];
+		data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(index, enic)];
 		if (data_rq->in_use)
 			vnic_rq_init(data_rq,
 				     cq_idx,
 				     error_interrupt_enable,
 				     error_interrupt_offset);
-
 		vnic_cq_init(&enic->cq[cq_idx],
 			0 /* flow_control_enable */,
 			1 /* color_enable */,
@@ -620,7 +619,7 @@ int enic_enable(struct enic *enic)
 			return err;
 		}
 		err = enic_alloc_rx_queue_mbufs(enic,
-			&enic->rq[enic_rte_rq_idx_to_data_idx(index)]);
+			&enic->rq[enic_rte_rq_idx_to_data_idx(index, enic)]);
 		if (err) {
 			/* release the allocated mbufs for the sop rq*/
 			enic_rxmbuf_queue_release(enic,
@@ -808,7 +807,7 @@ int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
 {
 	int rc;
 	uint16_t sop_queue_idx = enic_rte_rq_idx_to_sop_idx(queue_idx);
-	uint16_t data_queue_idx = enic_rte_rq_idx_to_data_idx(queue_idx);
+	uint16_t data_queue_idx = enic_rte_rq_idx_to_data_idx(queue_idx, enic);
 	struct vnic_rq *rq_sop = &enic->rq[sop_queue_idx];
 	struct vnic_rq *rq_data = &enic->rq[data_queue_idx];
 	unsigned int mbuf_size, mbufs_per_pkt;
@@ -1475,7 +1474,7 @@ enic_reinit_rq(struct enic *enic, unsigned int rq_idx)
 	int rc = 0;
 
 	sop_rq = &enic->rq[enic_rte_rq_idx_to_sop_idx(rq_idx)];
-	data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(rq_idx)];
+	data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(rq_idx, enic)];
 	cq_idx = rq_idx;
 
 	vnic_cq_clean(&enic->cq[cq_idx]);
@@ -1498,8 +1497,8 @@ enic_reinit_rq(struct enic *enic, unsigned int rq_idx)
 	if (data_rq->in_use) {
 		vnic_rq_init_start(data_rq,
 				   enic_cq_rq(enic,
-				   enic_rte_rq_idx_to_data_idx(rq_idx)), 0,
-				   data_rq->ring.desc_count - 1, 1, 0);
+				   enic_rte_rq_idx_to_data_idx(rq_idx, enic)),
+				   0, data_rq->ring.desc_count - 1, 1, 0);
 	}
 
 	rc = enic_alloc_rx_queue_mbufs(enic, sop_rq);
-- 
2.22.0

[dpdk-dev] [PATCH 4/5] net/enic: support flow API RSS ranges on outer headers

[John Daley]

Support rte_flow RSS action on outer headers (level 0). RSS ranges on
the non-default port is OK.
Restrictions:
 - The RETA is ignored. The hash function is simply applied across
   the RSS queue range.
 - The queues used in the RSS group must be sequential.
 - There is a performance hit if the number of queues is not a power
   of 2.

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 doc/guides/nics/enic.rst        |  4 +++
 drivers/net/enic/enic_fm_flow.c | 56 ++++++++++++++++++++++-----------
 2 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/doc/guides/nics/enic.rst b/doc/guides/nics/enic.rst
index 65e536d422..aa4fdc0e39 100644
--- a/doc/guides/nics/enic.rst
+++ b/doc/guides/nics/enic.rst
@@ -459,6 +459,10 @@ PKT_RX_VLAN_STRIPPED mbuf flags would not be set. This mode is enabled with the
     packets and then receive them normally. These require 1400 series VIC adapters
     and latest firmware.
   - RAW items are limited to matching UDP tunnel headers like VXLAN.
+  - For 1400 VICs, all flows using the RSS action on a port use same hash
+    configuration. The RETA is ignored. The queues used in the RSS group must be
+    sequential. There is a performance hit if the number of queues is not a power of 2.
+    Only level 0 (outer header) RSS is allowed.
 
 - **Statistics**
 
diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c
index 8d715fc436..86efeffc64 100644
--- a/drivers/net/enic/enic_fm_flow.c
+++ b/drivers/net/enic/enic_fm_flow.c
@@ -1174,8 +1174,8 @@ enic_fm_copy_action(struct enic_flowman *fm,
 				actions->conf;
 
 			/*
-			 * If other fate kind is set, fail.  Multiple
-			 * queue actions are ok.
+			 * If fate other than QUEUE or RSS, fail. Multiple
+			 * rss and queue actions are ok.
 			 */
 			if ((overlap & FATE) && first_rq)
 				goto unsupported;
@@ -1185,6 +1185,7 @@ enic_fm_copy_action(struct enic_flowman *fm,
 			fm_op.fa_op = FMOP_RQ_STEER;
 			fm_op.rq_steer.rq_index =
 				enic_rte_rq_idx_to_sop_idx(queue->index);
+			fm_op.rq_steer.rq_count = 1;
 			fm_op.rq_steer.vnic_handle = vnic_h;
 			ret = enic_fm_append_action_op(fm, &fm_op, error);
 			if (ret)
@@ -1219,27 +1220,44 @@ enic_fm_copy_action(struct enic_flowman *fm,
 			uint16_t i;
 
 			/*
-			 * Hardware does not support general RSS actions, but
-			 * we can still support the dummy one that is used to
-			 * "receive normally".
+			 * If fate other than QUEUE or RSS, fail. Multiple
+			 * rss and queue actions are ok.
+			 */
+			if ((overlap & FATE) && first_rq)
+				goto unsupported;
+			first_rq = false;
+			overlap |= FATE;
+
+			/*
+			 * Hardware only supports RSS actions on outer level
+			 * with default type and function. Queues must be
+			 * sequential.
 			 */
 			allow = rss->func == RTE_ETH_HASH_FUNCTION_DEFAULT &&
-				rss->level == 0 &&
-				(rss->types == 0 ||
-				 rss->types == enic->rss_hf) &&
-				rss->queue_num == enic->rq_count &&
-				rss->key_len == 0;
-			/* Identity queue map is ok */
-			for (i = 0; i < rss->queue_num; i++)
-				allow = allow && (i == rss->queue[i]);
+				rss->level == 0 && (rss->types == 0 ||
+				rss->types == enic->rss_hf) &&
+				rss->queue_num <= enic->rq_count &&
+				rss->queue[rss->queue_num - 1] < enic->rq_count;
+
+
+			/* Identity queue map needs to be sequential */
+			for (i = 1; i < rss->queue_num; i++)
+				allow = allow && (rss->queue[i] ==
+					rss->queue[i - 1] + 1);
 			if (!allow)
 				goto unsupported;
-			if (overlap & FATE)
-				goto unsupported;
-			/* Need MARK or FLAG */
-			if (!(overlap & MARK))
-				goto unsupported;
-			overlap |= FATE;
+
+			memset(&fm_op, 0, sizeof(fm_op));
+			fm_op.fa_op = FMOP_RQ_STEER;
+			fm_op.rq_steer.rq_index =
+				enic_rte_rq_idx_to_sop_idx(rss->queue[0]);
+			fm_op.rq_steer.rq_count = rss->queue_num;
+			fm_op.rq_steer.vnic_handle = vnic_h;
+			ret = enic_fm_append_action_op(fm, &fm_op, error);
+			if (ret)
+				return ret;
+			ENICPMD_LOG(DEBUG, "create QUEUE action rq: %u",
+				    fm_op.rq_steer.rq_index);
 			break;
 		}
 		case RTE_FLOW_ACTION_TYPE_PORT_ID: {
-- 
2.22.0

[dpdk-dev] [PATCH 5/5] net/enic: allow multiple mark and flag actions

[John Daley]

1400 series adapters support multiple MARK and FLAG action types.
  e.g.: mark id 10 / queue index 2 / mark id 11 / queue index 3

Remove the restriction in the Flow Manager implementation.

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 drivers/net/enic/enic_fm_flow.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c
index 86efeffc64..6ee0224372 100644
--- a/drivers/net/enic/enic_fm_flow.c
+++ b/drivers/net/enic/enic_fm_flow.c
@@ -1086,11 +1086,10 @@ enic_fm_copy_action(struct enic_flowman *fm,
 {
 	enum {
 		FATE = 1 << 0,
-		MARK = 1 << 1,
+		DECAP = 1 << 1,
 		PASSTHRU = 1 << 2,
 		COUNT = 1 << 3,
 		ENCAP = 1 << 4,
-		DECAP = 1 << 5,
 	};
 	struct fm_tcam_match_entry *fmt;
 	struct fm_action_op fm_op;
@@ -1141,9 +1140,6 @@ enic_fm_copy_action(struct enic_flowman *fm,
 			const struct rte_flow_action_mark *mark =
 				actions->conf;
 
-			if (overlap & MARK)
-				goto unsupported;
-			overlap |= MARK;
 			if (mark->id >= ENIC_MAGIC_FILTER_ID - 1)
 				return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION,
@@ -1157,9 +1153,6 @@ enic_fm_copy_action(struct enic_flowman *fm,
 			break;
 		}
 		case RTE_FLOW_ACTION_TYPE_FLAG: {
-			if (overlap & MARK)
-				goto unsupported;
-			overlap |= MARK;
 			/* ENIC_MAGIC_FILTER_ID is reserved for flagging */
 			memset(&fm_op, 0, sizeof(fm_op));
 			fm_op.fa_op = FMOP_MARK;
-- 
2.22.0

Re: [dpdk-dev] [PATCH 1/5] net/enic: fix action reordering

[Ferruh Yigit]

On 4/15/2020 2:06 AM, John Daley wrote:
> From: Hyong Youb Kim <hyonkim@cisco.com>
> 
> The current implementation produces wrong ordering for several cases
> like these:
> 
> 1. mark, decap, steer
> Current: steer, mark, decap
> Correct: mark, steer, decap
> 
> 2. decap, steer, steer
> Current: steer, steer, decap
> Correct: steer, decap, steer
> 
> Simplify the logic and swap 1st steer and decap.
> Also, allow just one decap action per flow.
> 
> Fixes: ea7768b5bba8 ("net/enic: add flow implementation based on Flow Manager API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> Signed-off-by: John Daley <johndale@cisco.com>

Series applied to dpdk-next-net/master, thanks.