[dpdk-dev] [PATCH 1/3] net/enic: fix flow API memory leak

DPDK patches and discussions
 help / color / mirror / Atom feed

* [dpdk-dev] [PATCH 1/3] net/enic: fix flow API memory leak
@ 2018-09-28  1:58 John Daley
  2018-09-28  1:58 ` [dpdk-dev] [PATCH 2/3] net/enic: support for flow counter action John Daley
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: John Daley @ 2018-09-28  1:58 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, John Daley, stable, Hyong Youb Kim

rte_flow structures were not being freed when destroyed or flushed.

Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index 0cf04aefd..9b612f1d5 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -1532,6 +1532,7 @@ enic_flow_destroy(struct rte_eth_dev *dev, struct rte_flow *flow,
 	enic_flow_del_filter(enic, flow->enic_filter_id, error);
 	LIST_REMOVE(flow, next);
 	rte_spinlock_unlock(&enic->flows_lock);
+	rte_free(flow);
 	return 0;
 }
 
@@ -1555,6 +1556,7 @@ enic_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 		flow = LIST_FIRST(&enic->flows);
 		enic_flow_del_filter(enic, flow->enic_filter_id, error);
 		LIST_REMOVE(flow, next);
+		rte_free(flow);
 	}
 	rte_spinlock_unlock(&enic->flows_lock);
 	return 0;
-- 
2.16.2

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

* [dpdk-dev] [PATCH 2/3] net/enic: support for flow counter action
  2018-09-28  1:58 [dpdk-dev] [PATCH 1/3] net/enic: fix flow API memory leak John Daley
@ 2018-09-28  1:58 ` John Daley
  2018-09-28  1:58 ` [dpdk-dev] [PATCH 3/3] doc: update enic guide for flow API " John Daley
  2018-09-28  3:08 ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak John Daley
  2 siblings, 0 replies; 7+ messages in thread
From: John Daley @ 2018-09-28  1:58 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, John Daley

Support counter action for 1400 series adapters.

The adapter API for allocating and freeing counters is independent of
the adapter match/action API. If the filter action is requested, a
counter is first allocated and then assigned to the filter, and when
the filter is deleted, the counter must also be deleted.

Counters are DMAd to pre-allocated consistent memory periodically,
controlled by the define VNIC_FLOW_COUNTER_UPDATE_MSECS. The default is
100 milliseconds.

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 drivers/net/enic/base/vnic_dev.c    |  93 +++++++++++++++++++
 drivers/net/enic/base/vnic_dev.h    |   8 ++
 drivers/net/enic/base/vnic_devcmd.h |  57 +++++++++++-
 drivers/net/enic/enic.h             |   3 +
 drivers/net/enic/enic_flow.c        | 178 ++++++++++++++++++++++++++++++++----
 drivers/net/enic/enic_main.c        |  11 ++-
 drivers/net/enic/enic_res.c         |   6 +-
 7 files changed, 331 insertions(+), 25 deletions(-)

diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index 16e8814a6..04285b68b 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -57,6 +57,8 @@ struct vnic_dev {
 	void (*free_consistent)(void *priv,
 		size_t size, void *vaddr,
 		dma_addr_t dma_handle);
+	struct vnic_counter_counts *flow_counters;
+	dma_addr_t flow_counters_pa;
 };
 
 #define VNIC_MAX_RES_HDR_SIZE \
@@ -64,6 +66,8 @@ struct vnic_dev {
 	sizeof(struct vnic_resource) * RES_TYPE_MAX)
 #define VNIC_RES_STRIDE	128
 
+#define VNIC_MAX_FLOW_COUNTERS 2048
+
 void *vnic_dev_priv(struct vnic_dev *vdev)
 {
 	return vdev->priv;
@@ -611,6 +615,23 @@ int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats)
 	return vnic_dev_cmd(vdev, CMD_STATS_DUMP, &a0, &a1, wait);
 }
 
+/*
+ * Configure counter DMA
+ */
+int vnic_dev_counter_dma_cfg(struct vnic_dev *vdev, u32 period, u32 counter_idx)
+{
+	u64 args[3];
+	int wait = 1000;
+
+	if (!vdev->flow_counters || (counter_idx >= VNIC_MAX_FLOW_COUNTERS))
+		return -ENOMEM;
+
+	args[0] = counter_idx + 1;
+	args[1] = vdev->flow_counters_pa;
+	args[2] = period;
+	return vnic_dev_cmd_args(vdev, CMD_COUNTER_DMA_CONFIG, args, 3, wait);
+}
+
 int vnic_dev_close(struct vnic_dev *vdev)
 {
 	u64 a0 = 0, a1 = 0;
@@ -939,6 +960,23 @@ int vnic_dev_alloc_stats_mem(struct vnic_dev *vdev)
 	return vdev->stats == NULL ? -ENOMEM : 0;
 }
 
+/*
+ * Initialize for up to VNIC_MAX_FLOW_COUNTERS
+ */
+int vnic_dev_alloc_counter_mem(struct vnic_dev *vdev)
+{
+	char name[NAME_MAX];
+	static u32 instance;
+
+	snprintf((char *)name, sizeof(name), "vnic_flow_ctrs-%u", instance++);
+	vdev->flow_counters = vdev->alloc_consistent(vdev->priv,
+					     sizeof(struct vnic_counter_counts)
+					     * VNIC_MAX_FLOW_COUNTERS,
+					     &vdev->flow_counters_pa,
+					     (u8 *)name);
+	return vdev->flow_counters == NULL ? -ENOMEM : 0;
+}
+
 void vnic_dev_unregister(struct vnic_dev *vdev)
 {
 	if (vdev) {
@@ -951,6 +989,15 @@ void vnic_dev_unregister(struct vnic_dev *vdev)
 			vdev->free_consistent(vdev->priv,
 				sizeof(struct vnic_stats),
 				vdev->stats, vdev->stats_pa);
+		if (vdev->flow_counters) {
+			/* turn off counter DMAs before freeing memory */
+			vnic_dev_counter_dma_cfg(vdev, 0, 0);
+
+			vdev->free_consistent(vdev->priv,
+				sizeof(struct vnic_counter_counts)
+				* VNIC_MAX_FLOW_COUNTERS,
+				vdev->flow_counters, vdev->flow_counters_pa);
+		}
 		if (vdev->fw_info)
 			vdev->free_consistent(vdev->priv,
 				sizeof(struct vnic_devcmd_fw_info),
@@ -1094,3 +1141,49 @@ int vnic_dev_capable_vxlan(struct vnic_dev *vdev)
 		(a1 & (FEATURE_VXLAN_IPV6 | FEATURE_VXLAN_MULTI_WQ)) ==
 		(FEATURE_VXLAN_IPV6 | FEATURE_VXLAN_MULTI_WQ);
 }
+
+bool vnic_dev_counter_alloc(struct vnic_dev *vdev, uint32_t *idx)
+{
+	u64 a0 = 0;
+	u64 a1 = 0;
+	int wait = 1000;
+
+	if (vnic_dev_cmd(vdev, CMD_COUNTER_ALLOC, &a0, &a1, wait))
+		return false;
+	*idx = (uint32_t)a0;
+	return true;
+}
+
+bool vnic_dev_counter_free(struct vnic_dev *vdev, uint32_t idx)
+{
+	u64 a0 = idx;
+	u64 a1 = 0;
+	int wait = 1000;
+
+	return vnic_dev_cmd(vdev, CMD_COUNTER_FREE, &a0, &a1,
+			    wait) == 0;
+}
+
+bool vnic_dev_counter_query(struct vnic_dev *vdev, uint32_t idx,
+			    bool reset, uint64_t *packets, uint64_t *bytes)
+{
+	u64 a0 = idx;
+	u64 a1 = reset ? 1 : 0;
+	int wait = 1000;
+
+	if (vdev->flow_counters) {
+		/* Using counter DMA API, so counters avail in host memory */
+		*packets = vdev->flow_counters[idx].vcc_packets;
+		*bytes = vdev->flow_counters[idx].vcc_bytes;
+		if (reset)
+			if (vnic_dev_cmd(vdev, CMD_COUNTER_QUERY, &a0, &a1,
+			    wait))
+				return false;
+	} else {
+		if (vnic_dev_cmd(vdev, CMD_COUNTER_QUERY, &a0, &a1, wait))
+			return false;
+		*packets = a0;
+		*bytes = a1;
+	}
+	return true;
+}
diff --git a/drivers/net/enic/base/vnic_dev.h b/drivers/net/enic/base/vnic_dev.h
index 270a47bd2..63751d8c5 100644
--- a/drivers/net/enic/base/vnic_dev.h
+++ b/drivers/net/enic/base/vnic_dev.h
@@ -118,6 +118,8 @@ int vnic_dev_spec(struct vnic_dev *vdev, unsigned int offset, size_t size,
 	void *value);
 int vnic_dev_stats_clear(struct vnic_dev *vdev);
 int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats);
+int vnic_dev_counter_dma_cfg(struct vnic_dev *vdev, u32 period,
+			     u32 counter_idx);
 int vnic_dev_hang_notify(struct vnic_dev *vdev);
 int vnic_dev_packet_filter(struct vnic_dev *vdev, int directed, int multicast,
 	int broadcast, int promisc, int allmulti);
@@ -170,6 +172,7 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
 	unsigned int num_bars);
 struct rte_pci_device *vnic_dev_get_pdev(struct vnic_dev *vdev);
 int vnic_dev_alloc_stats_mem(struct vnic_dev *vdev);
+int vnic_dev_alloc_counter_mem(struct vnic_dev *vdev);
 int vnic_dev_cmd_init(struct vnic_dev *vdev, int fallback);
 int vnic_dev_get_size(void);
 int vnic_dev_int13(struct vnic_dev *vdev, u64 arg, u32 op);
@@ -187,4 +190,9 @@ int vnic_dev_overlay_offload_ctrl(struct vnic_dev *vdev,
 int vnic_dev_overlay_offload_cfg(struct vnic_dev *vdev, u8 overlay,
 	u16 vxlan_udp_port_number);
 int vnic_dev_capable_vxlan(struct vnic_dev *vdev);
+bool vnic_dev_counter_alloc(struct vnic_dev *vdev, uint32_t *idx);
+bool vnic_dev_counter_free(struct vnic_dev *vdev, uint32_t idx);
+bool vnic_dev_counter_query(struct vnic_dev *vdev, uint32_t idx,
+			    bool reset, uint64_t *packets, uint64_t *bytes);
+
 #endif /* _VNIC_DEV_H_ */
diff --git a/drivers/net/enic/base/vnic_devcmd.h b/drivers/net/enic/base/vnic_devcmd.h
index fffe307e0..0efcee2ff 100644
--- a/drivers/net/enic/base/vnic_devcmd.h
+++ b/drivers/net/enic/base/vnic_devcmd.h
@@ -598,6 +598,47 @@ enum vnic_devcmd_cmd {
 	 *                       a3 = bitmask of supported actions
 	 */
 	CMD_ADD_ADV_FILTER = _CMDC(_CMD_DIR_RW, _CMD_VTYPE_ENET, 77),
+
+	/*
+	 * Allocate a counter for use with CMD_ADD_FILTER
+	 * out:(u32) a0 = counter index
+	 */
+	CMD_COUNTER_ALLOC = _CMDC(_CMD_DIR_READ, _CMD_VTYPE_ENET, 85),
+
+	/*
+	 * Free a counter
+	 * in: (u32) a0 = counter_id
+	 */
+	CMD_COUNTER_FREE = _CMDC(_CMD_DIR_WRITE, _CMD_VTYPE_ENET, 86),
+
+	/*
+	 * Read a counter
+	 * in: (u32) a0 = counter_id
+	 *     (u32) a1 = clear counter if non-zero
+	 * out:(u64) a0 = packet count
+	 *     (u64) a1 = byte count
+	 */
+	CMD_COUNTER_QUERY = _CMDC(_CMD_DIR_RW, _CMD_VTYPE_ENET, 87),
+
+	/*
+	 * Configure periodic counter DMA.  This will trigger an immediate
+	 * DMA of the counters (unless period == 0), and then schedule a DMA
+	 * of the counters every <period> seconds until disdabled.
+	 * Each new COUNTER_DMA_CONFIG will override all previous commands on
+	 * this vnic.
+	 * Setting a2 (period) = 0 will disable periodic DMAs
+	 * If a0 (num_counters) != 0, an immediate DMA will always be done,
+	 * irrespective of the value in a2.
+	 * in: (u32) a0 = number of counters to DMA
+	 *     (u64) a1 = host target DMA address
+	 *     (u32) a2 = DMA period in milliseconds (0 to disable)
+	 */
+	CMD_COUNTER_DMA_CONFIG = _CMDC(_CMD_DIR_WRITE, _CMD_VTYPE_ENET, 88),
+
+	/*
+	 * Clear all counters on a vnic
+	 */
+	CMD_COUNTER_CLEAR_ALL = _CMDC(_CMD_DIR_NONE, _CMD_VTYPE_ENET, 89),
 };
 
 /* Modes for exchanging advanced filter capabilities. The modes supported by
@@ -863,9 +904,11 @@ struct filter_action {
 #define FILTER_ACTION_RQ_STEERING_FLAG	(1 << 0)
 #define FILTER_ACTION_FILTER_ID_FLAG	(1 << 1)
 #define FILTER_ACTION_DROP_FLAG		(1 << 2)
+#define FILTER_ACTION_COUNTER_FLAG      (1 << 3)
 #define FILTER_ACTION_V2_ALL		(FILTER_ACTION_RQ_STEERING_FLAG \
+					 | FILTER_ACTION_FILTER_ID_FLAG \
 					 | FILTER_ACTION_DROP_FLAG \
-					 | FILTER_ACTION_FILTER_ID_FLAG)
+					 | FILTER_ACTION_COUNTER_FLAG)
 
 /* Version 2 of filter action must be a strict extension of struct filter_action
  * where the first fields exactly match in size and meaning.
@@ -875,7 +918,8 @@ struct filter_action_v2 {
 	u32 rq_idx;
 	u32 flags;                     /* use FILTER_ACTION_XXX_FLAG defines */
 	u16 filter_id;
-	uint8_t reserved[32];         /* for future expansion */
+	u32 counter_index;
+	uint8_t reserved[28];         /* for future expansion */
 } __attribute__((packed));
 
 /* Specifies the filter type. */
@@ -1122,4 +1166,13 @@ typedef enum {
 	GRPINTR_UPD_VECT,
 } grpintr_subcmd_t;
 
+/*
+ * Structure for counter DMA
+ * (DMAed by CMD_COUNTER_DMA_CONFIG)
+ */
+struct vnic_counter_counts {
+	u64 vcc_packets;
+	u64 vcc_bytes;
+};
+
 #endif /* _VNIC_DEVCMD_H_ */
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 7c27bd513..775cd5d55 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -38,6 +38,7 @@
 #define ENIC_PAGE_SIZE          4096
 #define PAGE_ROUND_UP(x) \
 	((((unsigned long)(x)) + ENIC_PAGE_SIZE-1) & (~(ENIC_PAGE_SIZE-1)))
+#define VNIC_FLOW_COUNTER_UPDATE_MSECS 100
 
 #define ENICPMD_VFIO_PATH          "/dev/vfio/vfio"
 /*#define ENIC_DESC_COUNT_MAKE_ODD (x) do{if ((~(x)) & 1) { (x)--; } }while(0)*/
@@ -94,6 +95,7 @@ struct rte_flow {
 	LIST_ENTRY(rte_flow) next;
 	u16 enic_filter_id;
 	struct filter_v2 enic_filter;
+	int counter_idx; /* NIC allocated counter index (-1 = invalid) */
 };
 
 /* Per-instance private data structure */
@@ -165,6 +167,7 @@ struct enic {
 	rte_spinlock_t mtu_lock;
 
 	LIST_HEAD(enic_flows, rte_flow) flows;
+	int max_flow_counter;
 	rte_spinlock_t flows_lock;
 
 	/* RSS */
diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index 9b612f1d5..04fc351b3 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -289,6 +289,15 @@ static const enum rte_flow_action_type enic_supported_actions_v2_drop[] = {
 	RTE_FLOW_ACTION_TYPE_END,
 };
 
+static const enum rte_flow_action_type enic_supported_actions_v2_count[] = {
+	RTE_FLOW_ACTION_TYPE_QUEUE,
+	RTE_FLOW_ACTION_TYPE_MARK,
+	RTE_FLOW_ACTION_TYPE_FLAG,
+	RTE_FLOW_ACTION_TYPE_DROP,
+	RTE_FLOW_ACTION_TYPE_COUNT,
+	RTE_FLOW_ACTION_TYPE_END,
+};
+
 /** Action capabilities indexed by NIC version information */
 static const struct enic_action_cap enic_action_cap[] = {
 	[FILTER_ACTION_RQ_STEERING_FLAG] = {
@@ -303,6 +312,10 @@ static const struct enic_action_cap enic_action_cap[] = {
 		.actions = enic_supported_actions_v2_drop,
 		.copy_fn = enic_copy_action_v2,
 	},
+	[FILTER_ACTION_COUNTER_FLAG] = {
+		.actions = enic_supported_actions_v2_count,
+		.copy_fn = enic_copy_action_v2,
+	},
 };
 
 static int
@@ -1068,6 +1081,10 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
 			enic_action->flags |= FILTER_ACTION_DROP_FLAG;
 			break;
 		}
+		case RTE_FLOW_ACTION_TYPE_COUNT: {
+			enic_action->flags |= FILTER_ACTION_COUNTER_FLAG;
+			break;
+		}
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			continue;
 		default:
@@ -1112,7 +1129,9 @@ enic_get_action_cap(struct enic *enic)
 	uint8_t actions;
 
 	actions = enic->filter_actions;
-	if (actions & FILTER_ACTION_DROP_FLAG)
+	if (actions & FILTER_ACTION_COUNTER_FLAG)
+		ea = &enic_action_cap[FILTER_ACTION_COUNTER_FLAG];
+	else if (actions & FILTER_ACTION_DROP_FLAG)
 		ea = &enic_action_cap[FILTER_ACTION_DROP_FLAG];
 	else if (actions & FILTER_ACTION_FILTER_ID_FLAG)
 		ea = &enic_action_cap[FILTER_ACTION_FILTER_ID_FLAG];
@@ -1395,8 +1414,10 @@ enic_flow_add_filter(struct enic *enic, struct filter_v2 *enic_filter,
 		   struct rte_flow_error *error)
 {
 	struct rte_flow *flow;
-	int ret;
-	u16 entry;
+	int err;
+	uint16_t entry;
+	int ctr_idx;
+	int last_max_flow_ctr;
 
 	FLOW_TRACE();
 
@@ -1407,20 +1428,64 @@ enic_flow_add_filter(struct enic *enic, struct filter_v2 *enic_filter,
 		return NULL;
 	}
 
+	flow->counter_idx = -1;
+	last_max_flow_ctr = -1;
+	if (enic_action->flags & FILTER_ACTION_COUNTER_FLAG) {
+		if (!vnic_dev_counter_alloc(enic->vdev, (uint32_t *)&ctr_idx)) {
+			rte_flow_error_set(error, ENOMEM,
+					   RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+					   NULL, "cannot allocate counter");
+			goto unwind_flow_alloc;
+		}
+		flow->counter_idx = ctr_idx;
+		enic_action->counter_index = ctr_idx;
+
+		/* If index is the largest, increase the counter DMA size */
+		if (ctr_idx > enic->max_flow_counter) {
+			err = vnic_dev_counter_dma_cfg(enic->vdev,
+						 VNIC_FLOW_COUNTER_UPDATE_MSECS,
+						 ctr_idx);
+			if (err) {
+				rte_flow_error_set(error, -err,
+					   RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+					   NULL, "counter DMA config failed");
+				goto unwind_ctr_alloc;
+			}
+			last_max_flow_ctr = enic->max_flow_counter;
+			enic->max_flow_counter = ctr_idx;
+		}
+	}
+
 	/* entry[in] is the queue id, entry[out] is the filter Id for delete */
 	entry = enic_action->rq_idx;
-	ret = vnic_dev_classifier(enic->vdev, CLSF_ADD, &entry, enic_filter,
+	err = vnic_dev_classifier(enic->vdev, CLSF_ADD, &entry, enic_filter,
 				  enic_action);
-	if (!ret) {
-		flow->enic_filter_id = entry;
-		flow->enic_filter = *enic_filter;
-	} else {
-		rte_flow_error_set(error, ret, RTE_FLOW_ERROR_TYPE_HANDLE,
+	if (err) {
+		rte_flow_error_set(error, -err, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "vnic_dev_classifier error");
-		rte_free(flow);
-		return NULL;
+		goto unwind_ctr_dma_cfg;
 	}
+
+	flow->enic_filter_id = entry;
+	flow->enic_filter = *enic_filter;
+
 	return flow;
+
+/* unwind if there are errors */
+unwind_ctr_dma_cfg:
+	if (last_max_flow_ctr != -1) {
+		/* reduce counter DMA size */
+		vnic_dev_counter_dma_cfg(enic->vdev,
+					 VNIC_FLOW_COUNTER_UPDATE_MSECS,
+					 last_max_flow_ctr);
+		enic->max_flow_counter = last_max_flow_ctr;
+	}
+unwind_ctr_alloc:
+	if (flow->counter_idx != -1)
+		vnic_dev_counter_free(enic->vdev, ctr_idx);
+unwind_flow_alloc:
+	rte_free(flow);
+	return NULL;
 }
 
 /**
@@ -1435,18 +1500,29 @@ enic_flow_add_filter(struct enic *enic, struct filter_v2 *enic_filter,
  * @param error[out]
  */
 static int
-enic_flow_del_filter(struct enic *enic, u16 filter_id,
+enic_flow_del_filter(struct enic *enic, struct rte_flow *flow,
 		   struct rte_flow_error *error)
 {
-	int ret;
+	u16 filter_id;
+	int err;
 
 	FLOW_TRACE();
 
-	ret = vnic_dev_classifier(enic->vdev, CLSF_DEL, &filter_id, NULL, NULL);
-	if (!ret)
-		rte_flow_error_set(error, ret, RTE_FLOW_ERROR_TYPE_HANDLE,
+	filter_id = flow->enic_filter_id;
+	err = vnic_dev_classifier(enic->vdev, CLSF_DEL, &filter_id, NULL, NULL);
+	if (err) {
+		rte_flow_error_set(error, -err, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "vnic_dev_classifier failed");
-	return ret;
+		return -err;
+	}
+
+	if (flow->counter_idx != -1) {
+		if (!vnic_dev_counter_free(enic->vdev, flow->counter_idx))
+			dev_err(enic, "counter free failed, idx: %d\n",
+				flow->counter_idx);
+		flow->counter_idx = -1;
+	}
+	return 0;
 }
 
 /*
@@ -1529,7 +1605,7 @@ enic_flow_destroy(struct rte_eth_dev *dev, struct rte_flow *flow,
 	FLOW_TRACE();
 
 	rte_spinlock_lock(&enic->flows_lock);
-	enic_flow_del_filter(enic, flow->enic_filter_id, error);
+	enic_flow_del_filter(enic, flow, error);
 	LIST_REMOVE(flow, next);
 	rte_spinlock_unlock(&enic->flows_lock);
 	rte_free(flow);
@@ -1554,7 +1630,7 @@ enic_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 
 	while (!LIST_EMPTY(&enic->flows)) {
 		flow = LIST_FIRST(&enic->flows);
-		enic_flow_del_filter(enic, flow->enic_filter_id, error);
+		enic_flow_del_filter(enic, flow, error);
 		LIST_REMOVE(flow, next);
 		rte_free(flow);
 	}
@@ -1562,6 +1638,69 @@ enic_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	return 0;
 }
 
+static int
+enic_flow_query_count(struct rte_eth_dev *dev,
+		      struct rte_flow *flow, void *data,
+		      struct rte_flow_error *error)
+{
+	struct enic *enic = pmd_priv(dev);
+	struct rte_flow_query_count *query;
+	uint64_t packets, bytes;
+
+	FLOW_TRACE();
+
+	if (flow->counter_idx == -1) {
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL,
+					  "flow does not have counter");
+	}
+	query = (struct rte_flow_query_count *)data;
+	if (!vnic_dev_counter_query(enic->vdev, flow->counter_idx,
+				    !!query->reset, &packets, &bytes)) {
+		return rte_flow_error_set
+			(error, EINVAL,
+			 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+			 NULL,
+			 "cannot read counter");
+	}
+	query->hits_set = 1;
+	query->bytes_set = 1;
+	query->hits = packets;
+	query->bytes = bytes;
+	return 0;
+}
+
+static int
+enic_flow_query(struct rte_eth_dev *dev,
+		struct rte_flow *flow,
+		const struct rte_flow_action *actions,
+		void *data,
+		struct rte_flow_error *error)
+{
+	int ret = 0;
+
+	FLOW_TRACE();
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_VOID:
+			break;
+		case RTE_FLOW_ACTION_TYPE_COUNT:
+			ret = enic_flow_query_count(dev, flow, data, error);
+			break;
+		default:
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  actions,
+						  "action not supported");
+		}
+		if (ret < 0)
+			return ret;
+	}
+	return 0;
+}
+
 /**
  * Flow callback registration.
  *
@@ -1572,4 +1711,5 @@ const struct rte_flow_ops enic_flow_ops = {
 	.create = enic_flow_create,
 	.destroy = enic_flow_destroy,
 	.flush = enic_flow_flush,
+	.query = enic_flow_query,
 };
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index af29f9d90..ea6cddbd3 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1647,6 +1647,7 @@ static int enic_dev_init(struct enic *enic)
 
 	LIST_INIT(&enic->flows);
 	rte_spinlock_init(&enic->flows_lock);
+	enic->max_flow_counter = -1;
 
 	/* set up link status checking */
 	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
@@ -1729,14 +1730,20 @@ int enic_probe(struct enic *enic)
 		enic_free_consistent);
 
 	/*
-	 * Allocate the consistent memory for stats upfront so both primary and
-	 * secondary processes can dump stats.
+	 * Allocate the consistent memory for stats and counters upfront so
+	 * both primary and secondary processes can dump stats.
 	 */
 	err = vnic_dev_alloc_stats_mem(enic->vdev);
 	if (err) {
 		dev_err(enic, "Failed to allocate cmd memory, aborting\n");
 		goto err_out_unregister;
 	}
+	err = vnic_dev_alloc_counter_mem(enic->vdev);
+	if (err) {
+		dev_err(enic, "Failed to allocate counter memory, aborting\n");
+		goto err_out_unregister;
+	}
+
 	/* Issue device open to get device in known state */
 	err = enic_dev_open(enic);
 	if (err) {
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 84486cace..28ae823f4 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -85,7 +85,7 @@ int enic_get_vnic_config(struct enic *enic)
 	vnic_dev_capable_udp_rss_weak(enic->vdev, &enic->nic_cfg_chk,
 				      &enic->udp_rss_weak);
 
-	dev_info(enic, "Flow api filter mode: %s Actions: %s%s%s\n",
+	dev_info(enic, "Flow api filter mode: %s Actions: %s%s%s%s\n",
 		((enic->flow_filter_mode == FILTER_DPDK_1) ? "DPDK" :
 		((enic->flow_filter_mode == FILTER_USNIC_IP) ? "USNIC" :
 		((enic->flow_filter_mode == FILTER_IPV4_5TUPLE) ? "5TUPLE" :
@@ -95,7 +95,9 @@ int enic_get_vnic_config(struct enic *enic)
 		((enic->filter_actions & FILTER_ACTION_FILTER_ID_FLAG) ?
 		 "tag " : ""),
 		((enic->filter_actions & FILTER_ACTION_DROP_FLAG) ?
-		 "drop " : ""));
+		 "drop " : ""),
+		((enic->filter_actions & FILTER_ACTION_COUNTER_FLAG) ?
+		 "count " : ""));
 
 	c->wq_desc_count =
 		min_t(u32, ENIC_MAX_WQ_DESCS,
-- 
2.16.2

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

* [dpdk-dev] [PATCH 3/3] doc: update enic guide for flow API counter action
  2018-09-28  1:58 [dpdk-dev] [PATCH 1/3] net/enic: fix flow API memory leak John Daley
  2018-09-28  1:58 ` [dpdk-dev] [PATCH 2/3] net/enic: support for flow counter action John Daley
@ 2018-09-28  1:58 ` John Daley
  2018-09-28  3:08 ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak John Daley
  2 siblings, 0 replies; 7+ messages in thread
From: John Daley @ 2018-09-28  1:58 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, John Daley

Add counter action support

Signed-off-by: John Daley <johndale@cisco.com>
---
 doc/guides/nics/enic.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/nics/enic.rst b/doc/guides/nics/enic.rst
index 438a83d5f..86941fdb2 100644
--- a/doc/guides/nics/enic.rst
+++ b/doc/guides/nics/enic.rst
@@ -260,6 +260,12 @@ Generic Flow API is supported. The baseline support is:
   - Selectors: 'is', 'spec' and 'mask'. 'last' is not supported
   - In total, up to 64 bytes of mask is allowed across all headers
 
+- **1400 and later series VICS with advanced filters enabled**
+
+  All the above plus:
+
+  - Action: count
+
 More features may be added in future firmware and new versions of the VIC.
 Please refer to the release notes.
 
-- 
2.16.2

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

* [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak
  2018-09-28  1:58 [dpdk-dev] [PATCH 1/3] net/enic: fix flow API memory leak John Daley
  2018-09-28  1:58 ` [dpdk-dev] [PATCH 2/3] net/enic: support for flow counter action John Daley
  2018-09-28  1:58 ` [dpdk-dev] [PATCH 3/3] doc: update enic guide for flow API " John Daley
@ 2018-09-28  3:08 ` John Daley
  2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 2/3] net/enic: support for flow counter action John Daley
                     ` (2 more replies)
  2 siblings, 3 replies; 7+ messages in thread
From: John Daley @ 2018-09-28  3:08 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, John Daley, stable, Hyong Youb Kim

rte_flow structures were not being freed when destroyed or flushed.

Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Signed-off-by: John Daley <johndale@cisco.com>
---
v2: fix signoff

 drivers/net/enic/enic_flow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index 0cf04aefd..9b612f1d5 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -1532,6 +1532,7 @@ enic_flow_destroy(struct rte_eth_dev *dev, struct rte_flow *flow,
 	enic_flow_del_filter(enic, flow->enic_filter_id, error);
 	LIST_REMOVE(flow, next);
 	rte_spinlock_unlock(&enic->flows_lock);
+	rte_free(flow);
 	return 0;
 }
 
@@ -1555,6 +1556,7 @@ enic_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 		flow = LIST_FIRST(&enic->flows);
 		enic_flow_del_filter(enic, flow->enic_filter_id, error);
 		LIST_REMOVE(flow, next);
+		rte_free(flow);
 	}
 	rte_spinlock_unlock(&enic->flows_lock);
 	return 0;
-- 
2.16.2

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

* [dpdk-dev] [PATCH v2 2/3] net/enic: support for flow counter action
  2018-09-28  3:08 ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak John Daley
@ 2018-09-28  3:08   ` John Daley
  2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 3/3] doc: update enic guide for flow API " John Daley
  2018-10-02 14:49   ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak Ferruh Yigit
  2 siblings, 0 replies; 7+ messages in thread
From: John Daley @ 2018-09-28  3:08 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, John Daley

Support counter action for 1400 series adapters.

The adapter API for allocating and freeing counters is independent of
the adapter match/action API. If the filter action is requested, a
counter is first allocated and then assigned to the filter, and when
the filter is deleted, the counter must also be deleted.

Counters are DMAd to pre-allocated consistent memory periodically,
controlled by the define VNIC_FLOW_COUNTER_UPDATE_MSECS. The default is
100 milliseconds.

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Hyong Youb Kim <hyonkim@cisco.com>
---
v2: fix parens (coding style)

 drivers/net/enic/base/vnic_dev.c    |  93 +++++++++++++++++++
 drivers/net/enic/base/vnic_dev.h    |   8 ++
 drivers/net/enic/base/vnic_devcmd.h |  57 +++++++++++-
 drivers/net/enic/enic.h             |   3 +
 drivers/net/enic/enic_flow.c        | 178 ++++++++++++++++++++++++++++++++----
 drivers/net/enic/enic_main.c        |  11 ++-
 drivers/net/enic/enic_res.c         |   6 +-
 7 files changed, 331 insertions(+), 25 deletions(-)

diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index 16e8814a6..1a3656f87 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -57,6 +57,8 @@ struct vnic_dev {
 	void (*free_consistent)(void *priv,
 		size_t size, void *vaddr,
 		dma_addr_t dma_handle);
+	struct vnic_counter_counts *flow_counters;
+	dma_addr_t flow_counters_pa;
 };
 
 #define VNIC_MAX_RES_HDR_SIZE \
@@ -64,6 +66,8 @@ struct vnic_dev {
 	sizeof(struct vnic_resource) * RES_TYPE_MAX)
 #define VNIC_RES_STRIDE	128
 
+#define VNIC_MAX_FLOW_COUNTERS 2048
+
 void *vnic_dev_priv(struct vnic_dev *vdev)
 {
 	return vdev->priv;
@@ -611,6 +615,23 @@ int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats)
 	return vnic_dev_cmd(vdev, CMD_STATS_DUMP, &a0, &a1, wait);
 }
 
+/*
+ * Configure counter DMA
+ */
+int vnic_dev_counter_dma_cfg(struct vnic_dev *vdev, u32 period, u32 counter_idx)
+{
+	u64 args[3];
+	int wait = 1000;
+
+	if (!vdev->flow_counters || counter_idx >= VNIC_MAX_FLOW_COUNTERS)
+		return -ENOMEM;
+
+	args[0] = counter_idx + 1;
+	args[1] = vdev->flow_counters_pa;
+	args[2] = period;
+	return vnic_dev_cmd_args(vdev, CMD_COUNTER_DMA_CONFIG, args, 3, wait);
+}
+
 int vnic_dev_close(struct vnic_dev *vdev)
 {
 	u64 a0 = 0, a1 = 0;
@@ -939,6 +960,23 @@ int vnic_dev_alloc_stats_mem(struct vnic_dev *vdev)
 	return vdev->stats == NULL ? -ENOMEM : 0;
 }
 
+/*
+ * Initialize for up to VNIC_MAX_FLOW_COUNTERS
+ */
+int vnic_dev_alloc_counter_mem(struct vnic_dev *vdev)
+{
+	char name[NAME_MAX];
+	static u32 instance;
+
+	snprintf((char *)name, sizeof(name), "vnic_flow_ctrs-%u", instance++);
+	vdev->flow_counters = vdev->alloc_consistent(vdev->priv,
+					     sizeof(struct vnic_counter_counts)
+					     * VNIC_MAX_FLOW_COUNTERS,
+					     &vdev->flow_counters_pa,
+					     (u8 *)name);
+	return vdev->flow_counters == NULL ? -ENOMEM : 0;
+}
+
 void vnic_dev_unregister(struct vnic_dev *vdev)
 {
 	if (vdev) {
@@ -951,6 +989,15 @@ void vnic_dev_unregister(struct vnic_dev *vdev)
 			vdev->free_consistent(vdev->priv,
 				sizeof(struct vnic_stats),
 				vdev->stats, vdev->stats_pa);
+		if (vdev->flow_counters) {
+			/* turn off counter DMAs before freeing memory */
+			vnic_dev_counter_dma_cfg(vdev, 0, 0);
+
+			vdev->free_consistent(vdev->priv,
+				sizeof(struct vnic_counter_counts)
+				* VNIC_MAX_FLOW_COUNTERS,
+				vdev->flow_counters, vdev->flow_counters_pa);
+		}
 		if (vdev->fw_info)
 			vdev->free_consistent(vdev->priv,
 				sizeof(struct vnic_devcmd_fw_info),
@@ -1094,3 +1141,49 @@ int vnic_dev_capable_vxlan(struct vnic_dev *vdev)
 		(a1 & (FEATURE_VXLAN_IPV6 | FEATURE_VXLAN_MULTI_WQ)) ==
 		(FEATURE_VXLAN_IPV6 | FEATURE_VXLAN_MULTI_WQ);
 }
+
+bool vnic_dev_counter_alloc(struct vnic_dev *vdev, uint32_t *idx)
+{
+	u64 a0 = 0;
+	u64 a1 = 0;
+	int wait = 1000;
+
+	if (vnic_dev_cmd(vdev, CMD_COUNTER_ALLOC, &a0, &a1, wait))
+		return false;
+	*idx = (uint32_t)a0;
+	return true;
+}
+
+bool vnic_dev_counter_free(struct vnic_dev *vdev, uint32_t idx)
+{
+	u64 a0 = idx;
+	u64 a1 = 0;
+	int wait = 1000;
+
+	return vnic_dev_cmd(vdev, CMD_COUNTER_FREE, &a0, &a1,
+			    wait) == 0;
+}
+
+bool vnic_dev_counter_query(struct vnic_dev *vdev, uint32_t idx,
+			    bool reset, uint64_t *packets, uint64_t *bytes)
+{
+	u64 a0 = idx;
+	u64 a1 = reset ? 1 : 0;
+	int wait = 1000;
+
+	if (vdev->flow_counters) {
+		/* Using counter DMA API, so counters avail in host memory */
+		*packets = vdev->flow_counters[idx].vcc_packets;
+		*bytes = vdev->flow_counters[idx].vcc_bytes;
+		if (reset)
+			if (vnic_dev_cmd(vdev, CMD_COUNTER_QUERY, &a0, &a1,
+			    wait))
+				return false;
+	} else {
+		if (vnic_dev_cmd(vdev, CMD_COUNTER_QUERY, &a0, &a1, wait))
+			return false;
+		*packets = a0;
+		*bytes = a1;
+	}
+	return true;
+}
diff --git a/drivers/net/enic/base/vnic_dev.h b/drivers/net/enic/base/vnic_dev.h
index 270a47bd2..63751d8c5 100644
--- a/drivers/net/enic/base/vnic_dev.h
+++ b/drivers/net/enic/base/vnic_dev.h
@@ -118,6 +118,8 @@ int vnic_dev_spec(struct vnic_dev *vdev, unsigned int offset, size_t size,
 	void *value);
 int vnic_dev_stats_clear(struct vnic_dev *vdev);
 int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats);
+int vnic_dev_counter_dma_cfg(struct vnic_dev *vdev, u32 period,
+			     u32 counter_idx);
 int vnic_dev_hang_notify(struct vnic_dev *vdev);
 int vnic_dev_packet_filter(struct vnic_dev *vdev, int directed, int multicast,
 	int broadcast, int promisc, int allmulti);
@@ -170,6 +172,7 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
 	unsigned int num_bars);
 struct rte_pci_device *vnic_dev_get_pdev(struct vnic_dev *vdev);
 int vnic_dev_alloc_stats_mem(struct vnic_dev *vdev);
+int vnic_dev_alloc_counter_mem(struct vnic_dev *vdev);
 int vnic_dev_cmd_init(struct vnic_dev *vdev, int fallback);
 int vnic_dev_get_size(void);
 int vnic_dev_int13(struct vnic_dev *vdev, u64 arg, u32 op);
@@ -187,4 +190,9 @@ int vnic_dev_overlay_offload_ctrl(struct vnic_dev *vdev,
 int vnic_dev_overlay_offload_cfg(struct vnic_dev *vdev, u8 overlay,
 	u16 vxlan_udp_port_number);
 int vnic_dev_capable_vxlan(struct vnic_dev *vdev);
+bool vnic_dev_counter_alloc(struct vnic_dev *vdev, uint32_t *idx);
+bool vnic_dev_counter_free(struct vnic_dev *vdev, uint32_t idx);
+bool vnic_dev_counter_query(struct vnic_dev *vdev, uint32_t idx,
+			    bool reset, uint64_t *packets, uint64_t *bytes);
+
 #endif /* _VNIC_DEV_H_ */
diff --git a/drivers/net/enic/base/vnic_devcmd.h b/drivers/net/enic/base/vnic_devcmd.h
index fffe307e0..0efcee2ff 100644
--- a/drivers/net/enic/base/vnic_devcmd.h
+++ b/drivers/net/enic/base/vnic_devcmd.h
@@ -598,6 +598,47 @@ enum vnic_devcmd_cmd {
 	 *                       a3 = bitmask of supported actions
 	 */
 	CMD_ADD_ADV_FILTER = _CMDC(_CMD_DIR_RW, _CMD_VTYPE_ENET, 77),
+
+	/*
+	 * Allocate a counter for use with CMD_ADD_FILTER
+	 * out:(u32) a0 = counter index
+	 */
+	CMD_COUNTER_ALLOC = _CMDC(_CMD_DIR_READ, _CMD_VTYPE_ENET, 85),
+
+	/*
+	 * Free a counter
+	 * in: (u32) a0 = counter_id
+	 */
+	CMD_COUNTER_FREE = _CMDC(_CMD_DIR_WRITE, _CMD_VTYPE_ENET, 86),
+
+	/*
+	 * Read a counter
+	 * in: (u32) a0 = counter_id
+	 *     (u32) a1 = clear counter if non-zero
+	 * out:(u64) a0 = packet count
+	 *     (u64) a1 = byte count
+	 */
+	CMD_COUNTER_QUERY = _CMDC(_CMD_DIR_RW, _CMD_VTYPE_ENET, 87),
+
+	/*
+	 * Configure periodic counter DMA.  This will trigger an immediate
+	 * DMA of the counters (unless period == 0), and then schedule a DMA
+	 * of the counters every <period> seconds until disdabled.
+	 * Each new COUNTER_DMA_CONFIG will override all previous commands on
+	 * this vnic.
+	 * Setting a2 (period) = 0 will disable periodic DMAs
+	 * If a0 (num_counters) != 0, an immediate DMA will always be done,
+	 * irrespective of the value in a2.
+	 * in: (u32) a0 = number of counters to DMA
+	 *     (u64) a1 = host target DMA address
+	 *     (u32) a2 = DMA period in milliseconds (0 to disable)
+	 */
+	CMD_COUNTER_DMA_CONFIG = _CMDC(_CMD_DIR_WRITE, _CMD_VTYPE_ENET, 88),
+
+	/*
+	 * Clear all counters on a vnic
+	 */
+	CMD_COUNTER_CLEAR_ALL = _CMDC(_CMD_DIR_NONE, _CMD_VTYPE_ENET, 89),
 };
 
 /* Modes for exchanging advanced filter capabilities. The modes supported by
@@ -863,9 +904,11 @@ struct filter_action {
 #define FILTER_ACTION_RQ_STEERING_FLAG	(1 << 0)
 #define FILTER_ACTION_FILTER_ID_FLAG	(1 << 1)
 #define FILTER_ACTION_DROP_FLAG		(1 << 2)
+#define FILTER_ACTION_COUNTER_FLAG      (1 << 3)
 #define FILTER_ACTION_V2_ALL		(FILTER_ACTION_RQ_STEERING_FLAG \
+					 | FILTER_ACTION_FILTER_ID_FLAG \
 					 | FILTER_ACTION_DROP_FLAG \
-					 | FILTER_ACTION_FILTER_ID_FLAG)
+					 | FILTER_ACTION_COUNTER_FLAG)
 
 /* Version 2 of filter action must be a strict extension of struct filter_action
  * where the first fields exactly match in size and meaning.
@@ -875,7 +918,8 @@ struct filter_action_v2 {
 	u32 rq_idx;
 	u32 flags;                     /* use FILTER_ACTION_XXX_FLAG defines */
 	u16 filter_id;
-	uint8_t reserved[32];         /* for future expansion */
+	u32 counter_index;
+	uint8_t reserved[28];         /* for future expansion */
 } __attribute__((packed));
 
 /* Specifies the filter type. */
@@ -1122,4 +1166,13 @@ typedef enum {
 	GRPINTR_UPD_VECT,
 } grpintr_subcmd_t;
 
+/*
+ * Structure for counter DMA
+ * (DMAed by CMD_COUNTER_DMA_CONFIG)
+ */
+struct vnic_counter_counts {
+	u64 vcc_packets;
+	u64 vcc_bytes;
+};
+
 #endif /* _VNIC_DEVCMD_H_ */
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 7c27bd513..775cd5d55 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -38,6 +38,7 @@
 #define ENIC_PAGE_SIZE          4096
 #define PAGE_ROUND_UP(x) \
 	((((unsigned long)(x)) + ENIC_PAGE_SIZE-1) & (~(ENIC_PAGE_SIZE-1)))
+#define VNIC_FLOW_COUNTER_UPDATE_MSECS 100
 
 #define ENICPMD_VFIO_PATH          "/dev/vfio/vfio"
 /*#define ENIC_DESC_COUNT_MAKE_ODD (x) do{if ((~(x)) & 1) { (x)--; } }while(0)*/
@@ -94,6 +95,7 @@ struct rte_flow {
 	LIST_ENTRY(rte_flow) next;
 	u16 enic_filter_id;
 	struct filter_v2 enic_filter;
+	int counter_idx; /* NIC allocated counter index (-1 = invalid) */
 };
 
 /* Per-instance private data structure */
@@ -165,6 +167,7 @@ struct enic {
 	rte_spinlock_t mtu_lock;
 
 	LIST_HEAD(enic_flows, rte_flow) flows;
+	int max_flow_counter;
 	rte_spinlock_t flows_lock;
 
 	/* RSS */
diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index 9b612f1d5..04fc351b3 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -289,6 +289,15 @@ static const enum rte_flow_action_type enic_supported_actions_v2_drop[] = {
 	RTE_FLOW_ACTION_TYPE_END,
 };
 
+static const enum rte_flow_action_type enic_supported_actions_v2_count[] = {
+	RTE_FLOW_ACTION_TYPE_QUEUE,
+	RTE_FLOW_ACTION_TYPE_MARK,
+	RTE_FLOW_ACTION_TYPE_FLAG,
+	RTE_FLOW_ACTION_TYPE_DROP,
+	RTE_FLOW_ACTION_TYPE_COUNT,
+	RTE_FLOW_ACTION_TYPE_END,
+};
+
 /** Action capabilities indexed by NIC version information */
 static const struct enic_action_cap enic_action_cap[] = {
 	[FILTER_ACTION_RQ_STEERING_FLAG] = {
@@ -303,6 +312,10 @@ static const struct enic_action_cap enic_action_cap[] = {
 		.actions = enic_supported_actions_v2_drop,
 		.copy_fn = enic_copy_action_v2,
 	},
+	[FILTER_ACTION_COUNTER_FLAG] = {
+		.actions = enic_supported_actions_v2_count,
+		.copy_fn = enic_copy_action_v2,
+	},
 };
 
 static int
@@ -1068,6 +1081,10 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
 			enic_action->flags |= FILTER_ACTION_DROP_FLAG;
 			break;
 		}
+		case RTE_FLOW_ACTION_TYPE_COUNT: {
+			enic_action->flags |= FILTER_ACTION_COUNTER_FLAG;
+			break;
+		}
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			continue;
 		default:
@@ -1112,7 +1129,9 @@ enic_get_action_cap(struct enic *enic)
 	uint8_t actions;
 
 	actions = enic->filter_actions;
-	if (actions & FILTER_ACTION_DROP_FLAG)
+	if (actions & FILTER_ACTION_COUNTER_FLAG)
+		ea = &enic_action_cap[FILTER_ACTION_COUNTER_FLAG];
+	else if (actions & FILTER_ACTION_DROP_FLAG)
 		ea = &enic_action_cap[FILTER_ACTION_DROP_FLAG];
 	else if (actions & FILTER_ACTION_FILTER_ID_FLAG)
 		ea = &enic_action_cap[FILTER_ACTION_FILTER_ID_FLAG];
@@ -1395,8 +1414,10 @@ enic_flow_add_filter(struct enic *enic, struct filter_v2 *enic_filter,
 		   struct rte_flow_error *error)
 {
 	struct rte_flow *flow;
-	int ret;
-	u16 entry;
+	int err;
+	uint16_t entry;
+	int ctr_idx;
+	int last_max_flow_ctr;
 
 	FLOW_TRACE();
 
@@ -1407,20 +1428,64 @@ enic_flow_add_filter(struct enic *enic, struct filter_v2 *enic_filter,
 		return NULL;
 	}
 
+	flow->counter_idx = -1;
+	last_max_flow_ctr = -1;
+	if (enic_action->flags & FILTER_ACTION_COUNTER_FLAG) {
+		if (!vnic_dev_counter_alloc(enic->vdev, (uint32_t *)&ctr_idx)) {
+			rte_flow_error_set(error, ENOMEM,
+					   RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+					   NULL, "cannot allocate counter");
+			goto unwind_flow_alloc;
+		}
+		flow->counter_idx = ctr_idx;
+		enic_action->counter_index = ctr_idx;
+
+		/* If index is the largest, increase the counter DMA size */
+		if (ctr_idx > enic->max_flow_counter) {
+			err = vnic_dev_counter_dma_cfg(enic->vdev,
+						 VNIC_FLOW_COUNTER_UPDATE_MSECS,
+						 ctr_idx);
+			if (err) {
+				rte_flow_error_set(error, -err,
+					   RTE_FLOW_ERROR_TYPE_ACTION_CONF,
+					   NULL, "counter DMA config failed");
+				goto unwind_ctr_alloc;
+			}
+			last_max_flow_ctr = enic->max_flow_counter;
+			enic->max_flow_counter = ctr_idx;
+		}
+	}
+
 	/* entry[in] is the queue id, entry[out] is the filter Id for delete */
 	entry = enic_action->rq_idx;
-	ret = vnic_dev_classifier(enic->vdev, CLSF_ADD, &entry, enic_filter,
+	err = vnic_dev_classifier(enic->vdev, CLSF_ADD, &entry, enic_filter,
 				  enic_action);
-	if (!ret) {
-		flow->enic_filter_id = entry;
-		flow->enic_filter = *enic_filter;
-	} else {
-		rte_flow_error_set(error, ret, RTE_FLOW_ERROR_TYPE_HANDLE,
+	if (err) {
+		rte_flow_error_set(error, -err, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "vnic_dev_classifier error");
-		rte_free(flow);
-		return NULL;
+		goto unwind_ctr_dma_cfg;
 	}
+
+	flow->enic_filter_id = entry;
+	flow->enic_filter = *enic_filter;
+
 	return flow;
+
+/* unwind if there are errors */
+unwind_ctr_dma_cfg:
+	if (last_max_flow_ctr != -1) {
+		/* reduce counter DMA size */
+		vnic_dev_counter_dma_cfg(enic->vdev,
+					 VNIC_FLOW_COUNTER_UPDATE_MSECS,
+					 last_max_flow_ctr);
+		enic->max_flow_counter = last_max_flow_ctr;
+	}
+unwind_ctr_alloc:
+	if (flow->counter_idx != -1)
+		vnic_dev_counter_free(enic->vdev, ctr_idx);
+unwind_flow_alloc:
+	rte_free(flow);
+	return NULL;
 }
 
 /**
@@ -1435,18 +1500,29 @@ enic_flow_add_filter(struct enic *enic, struct filter_v2 *enic_filter,
  * @param error[out]
  */
 static int
-enic_flow_del_filter(struct enic *enic, u16 filter_id,
+enic_flow_del_filter(struct enic *enic, struct rte_flow *flow,
 		   struct rte_flow_error *error)
 {
-	int ret;
+	u16 filter_id;
+	int err;
 
 	FLOW_TRACE();
 
-	ret = vnic_dev_classifier(enic->vdev, CLSF_DEL, &filter_id, NULL, NULL);
-	if (!ret)
-		rte_flow_error_set(error, ret, RTE_FLOW_ERROR_TYPE_HANDLE,
+	filter_id = flow->enic_filter_id;
+	err = vnic_dev_classifier(enic->vdev, CLSF_DEL, &filter_id, NULL, NULL);
+	if (err) {
+		rte_flow_error_set(error, -err, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "vnic_dev_classifier failed");
-	return ret;
+		return -err;
+	}
+
+	if (flow->counter_idx != -1) {
+		if (!vnic_dev_counter_free(enic->vdev, flow->counter_idx))
+			dev_err(enic, "counter free failed, idx: %d\n",
+				flow->counter_idx);
+		flow->counter_idx = -1;
+	}
+	return 0;
 }
 
 /*
@@ -1529,7 +1605,7 @@ enic_flow_destroy(struct rte_eth_dev *dev, struct rte_flow *flow,
 	FLOW_TRACE();
 
 	rte_spinlock_lock(&enic->flows_lock);
-	enic_flow_del_filter(enic, flow->enic_filter_id, error);
+	enic_flow_del_filter(enic, flow, error);
 	LIST_REMOVE(flow, next);
 	rte_spinlock_unlock(&enic->flows_lock);
 	rte_free(flow);
@@ -1554,7 +1630,7 @@ enic_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 
 	while (!LIST_EMPTY(&enic->flows)) {
 		flow = LIST_FIRST(&enic->flows);
-		enic_flow_del_filter(enic, flow->enic_filter_id, error);
+		enic_flow_del_filter(enic, flow, error);
 		LIST_REMOVE(flow, next);
 		rte_free(flow);
 	}
@@ -1562,6 +1638,69 @@ enic_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	return 0;
 }
 
+static int
+enic_flow_query_count(struct rte_eth_dev *dev,
+		      struct rte_flow *flow, void *data,
+		      struct rte_flow_error *error)
+{
+	struct enic *enic = pmd_priv(dev);
+	struct rte_flow_query_count *query;
+	uint64_t packets, bytes;
+
+	FLOW_TRACE();
+
+	if (flow->counter_idx == -1) {
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL,
+					  "flow does not have counter");
+	}
+	query = (struct rte_flow_query_count *)data;
+	if (!vnic_dev_counter_query(enic->vdev, flow->counter_idx,
+				    !!query->reset, &packets, &bytes)) {
+		return rte_flow_error_set
+			(error, EINVAL,
+			 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+			 NULL,
+			 "cannot read counter");
+	}
+	query->hits_set = 1;
+	query->bytes_set = 1;
+	query->hits = packets;
+	query->bytes = bytes;
+	return 0;
+}
+
+static int
+enic_flow_query(struct rte_eth_dev *dev,
+		struct rte_flow *flow,
+		const struct rte_flow_action *actions,
+		void *data,
+		struct rte_flow_error *error)
+{
+	int ret = 0;
+
+	FLOW_TRACE();
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_VOID:
+			break;
+		case RTE_FLOW_ACTION_TYPE_COUNT:
+			ret = enic_flow_query_count(dev, flow, data, error);
+			break;
+		default:
+			return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  actions,
+						  "action not supported");
+		}
+		if (ret < 0)
+			return ret;
+	}
+	return 0;
+}
+
 /**
  * Flow callback registration.
  *
@@ -1572,4 +1711,5 @@ const struct rte_flow_ops enic_flow_ops = {
 	.create = enic_flow_create,
 	.destroy = enic_flow_destroy,
 	.flush = enic_flow_flush,
+	.query = enic_flow_query,
 };
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index af29f9d90..ea6cddbd3 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1647,6 +1647,7 @@ static int enic_dev_init(struct enic *enic)
 
 	LIST_INIT(&enic->flows);
 	rte_spinlock_init(&enic->flows_lock);
+	enic->max_flow_counter = -1;
 
 	/* set up link status checking */
 	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
@@ -1729,14 +1730,20 @@ int enic_probe(struct enic *enic)
 		enic_free_consistent);
 
 	/*
-	 * Allocate the consistent memory for stats upfront so both primary and
-	 * secondary processes can dump stats.
+	 * Allocate the consistent memory for stats and counters upfront so
+	 * both primary and secondary processes can dump stats.
 	 */
 	err = vnic_dev_alloc_stats_mem(enic->vdev);
 	if (err) {
 		dev_err(enic, "Failed to allocate cmd memory, aborting\n");
 		goto err_out_unregister;
 	}
+	err = vnic_dev_alloc_counter_mem(enic->vdev);
+	if (err) {
+		dev_err(enic, "Failed to allocate counter memory, aborting\n");
+		goto err_out_unregister;
+	}
+
 	/* Issue device open to get device in known state */
 	err = enic_dev_open(enic);
 	if (err) {
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 84486cace..28ae823f4 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -85,7 +85,7 @@ int enic_get_vnic_config(struct enic *enic)
 	vnic_dev_capable_udp_rss_weak(enic->vdev, &enic->nic_cfg_chk,
 				      &enic->udp_rss_weak);
 
-	dev_info(enic, "Flow api filter mode: %s Actions: %s%s%s\n",
+	dev_info(enic, "Flow api filter mode: %s Actions: %s%s%s%s\n",
 		((enic->flow_filter_mode == FILTER_DPDK_1) ? "DPDK" :
 		((enic->flow_filter_mode == FILTER_USNIC_IP) ? "USNIC" :
 		((enic->flow_filter_mode == FILTER_IPV4_5TUPLE) ? "5TUPLE" :
@@ -95,7 +95,9 @@ int enic_get_vnic_config(struct enic *enic)
 		((enic->filter_actions & FILTER_ACTION_FILTER_ID_FLAG) ?
 		 "tag " : ""),
 		((enic->filter_actions & FILTER_ACTION_DROP_FLAG) ?
-		 "drop " : ""));
+		 "drop " : ""),
+		((enic->filter_actions & FILTER_ACTION_COUNTER_FLAG) ?
+		 "count " : ""));
 
 	c->wq_desc_count =
 		min_t(u32, ENIC_MAX_WQ_DESCS,
-- 
2.16.2

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

* [dpdk-dev] [PATCH v2 3/3] doc: update enic guide for flow API counter action
  2018-09-28  3:08 ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak John Daley
  2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 2/3] net/enic: support for flow counter action John Daley
@ 2018-09-28  3:08   ` John Daley
  2018-10-02 14:49   ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak Ferruh Yigit
  2 siblings, 0 replies; 7+ messages in thread
From: John Daley @ 2018-09-28  3:08 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, John Daley

Add counter action support

Signed-off-by: John Daley <johndale@cisco.com>
---
 doc/guides/nics/enic.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/nics/enic.rst b/doc/guides/nics/enic.rst
index 438a83d5f..86941fdb2 100644
--- a/doc/guides/nics/enic.rst
+++ b/doc/guides/nics/enic.rst
@@ -260,6 +260,12 @@ Generic Flow API is supported. The baseline support is:
   - Selectors: 'is', 'spec' and 'mask'. 'last' is not supported
   - In total, up to 64 bytes of mask is allowed across all headers
 
+- **1400 and later series VICS with advanced filters enabled**
+
+  All the above plus:
+
+  - Action: count
+
 More features may be added in future firmware and new versions of the VIC.
 Please refer to the release notes.
 
-- 
2.16.2

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

* Re: [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak
  2018-09-28  3:08 ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak John Daley
  2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 2/3] net/enic: support for flow counter action John Daley
  2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 3/3] doc: update enic guide for flow API " John Daley
@ 2018-10-02 14:49   ` Ferruh Yigit
  2 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2018-10-02 14:49 UTC (permalink / raw)
  To: John Daley; +Cc: dev, stable, Hyong Youb Kim

On 9/28/2018 4:08 AM, John Daley wrote:
> rte_flow structures were not being freed when destroyed or flushed.
> 
> Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
> 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.

(Patch 3/3 merged into 2/3 while merging, to keep relevant documentation and
implementation is same patch.)

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

end of thread, other threads:[~2018-10-02 14:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-28  1:58 [dpdk-dev] [PATCH 1/3] net/enic: fix flow API memory leak John Daley
2018-09-28  1:58 ` [dpdk-dev] [PATCH 2/3] net/enic: support for flow counter action John Daley
2018-09-28  1:58 ` [dpdk-dev] [PATCH 3/3] doc: update enic guide for flow API " John Daley
2018-09-28  3:08 ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak John Daley
2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 2/3] net/enic: support for flow counter action John Daley
2018-09-28  3:08   ` [dpdk-dev] [PATCH v2 3/3] doc: update enic guide for flow API " John Daley
2018-10-02 14:49   ` [dpdk-dev] [PATCH v2 1/3] net/enic: fix flow API memory leak Ferruh Yigit

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