DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure
@ 2016-10-12 21:12 John Daley
  2016-10-12 21:12 ` [dpdk-dev] [PATCH 2/2] net/enic: trivial function name changes John Daley
  2016-10-19  9:42 ` [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure Bruce Richardson
  0 siblings, 2 replies; 3+ messages in thread
From: John Daley @ 2016-10-12 21:12 UTC (permalink / raw)
  To: bruce.richardson; +Cc: dev, John Daley

The incorrect completion queue corresponding to an RQ would be
freed if multiple Rx queues are in use and the MTU is changed,
or an Rx queue is released. This could lead to a segmentation fault
when the device is disabled or even in the Rx or Tx paths.

The index of the completion queue corresponding to a RQ needed
to be adjusted after Rx scatter was introduced.

Fixes: 856d7ba7ed22 ("net/enic: support scattered Rx")

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Nelson Escobar <neescoba@cisco.com>
---
 drivers/net/enic/enic.h      | 5 +++++
 drivers/net/enic/enic_main.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 4ea4e4a..13a4b31 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -170,6 +170,11 @@ struct enic {
 
 };
 
+/* 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;
+}
 static inline unsigned int enic_rq_sop(unsigned int sop_rq)
 {
 	return sop_rq / 2;
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 622b317..65a8307 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -541,7 +541,7 @@ void enic_free_rq(void *rxq)
 	if (rq_data->in_use)
 		vnic_rq_free(rq_data);
 
-	vnic_cq_free(&enic->cq[rq_sop->index]);
+	vnic_cq_free(&enic->cq[enic_sop_rq_idx_to_cq_idx(rq_sop->index)]);
 
 	rq_sop->in_use = 0;
 	rq_data->in_use = 0;
-- 
2.10.0

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

* [dpdk-dev] [PATCH 2/2] net/enic: trivial function name changes
  2016-10-12 21:12 [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure John Daley
@ 2016-10-12 21:12 ` John Daley
  2016-10-19  9:42 ` [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure Bruce Richardson
  1 sibling, 0 replies; 3+ messages in thread
From: John Daley @ 2016-10-12 21:12 UTC (permalink / raw)
  To: bruce.richardson; +Cc: dev, John Daley

The function names for converting between RQ indexes known to
the RTE code and intenal RQ indexes for primary Start of Packet
(SOP) queues and spill-over (Data) queues was unclear and
confusing.

Clarify with more explicit function names.

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Nelson Escobar <neescoba@cisco.com>
---
 drivers/net/enic/enic.h        | 16 ++++++++-----
 drivers/net/enic/enic_clsf.c   |  2 +-
 drivers/net/enic/enic_ethdev.c |  6 ++---
 drivers/net/enic/enic_main.c   | 51 ++++++++++++++++++++++++------------------
 4 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 13a4b31..45b27a5 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -175,19 +175,23 @@ static inline unsigned int enic_sop_rq_idx_to_cq_idx(unsigned int sop_idx)
 {
 	return sop_idx / 2;
 }
-static inline unsigned int enic_rq_sop(unsigned int sop_rq)
+
+/* 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_rq / 2;
+	return sop_idx / 2;
 }
 
-static inline unsigned int enic_sop_rq(unsigned int rq)
+/* 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 rq * 2;
+	return rte_idx * 2;
 }
 
-static inline unsigned int enic_data_rq(unsigned int rq)
+/* 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)
 {
-	return rq * 2 + 1;
+	return rte_idx * 2 + 1;
 }
 
 static inline unsigned int enic_vnic_rq_count(struct enic *enic)
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index c709be9..724957a 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -384,7 +384,7 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_eth_fdir_filter *params)
 	}
 
 	/* Get the enicpmd RQ from the DPDK Rx queue */
-	queue = enic_sop_rq(params->action.rx_queue);
+	queue = enic_rte_rq_idx_to_sop_idx(params->action.rx_queue);
 
 	if (!enic->rq[queue].in_use)
 		return -EINVAL;
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 4d24bbd..18bb356 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -154,7 +154,7 @@ static int enicpmd_dev_setup_intr(struct enic *enic)
 		return 0;
 	/* check start of packet (SOP) RQs only in case scatter is disabled. */
 	for (index = 0; index < enic->rq_count; index++) {
-		if (!enic->rq[enic_sop_rq(index)].ctrl)
+		if (!enic->rq[enic_rte_rq_idx_to_sop_idx(index)].ctrl)
 			break;
 	}
 	if (enic->rq_count != index)
@@ -277,7 +277,7 @@ static uint32_t enicpmd_dev_rx_queue_count(struct rte_eth_dev *dev,
 		return 0;
 	}
 
-	rq_num = enic_sop_rq(rx_queue_id);
+	rq_num = enic_rte_rq_idx_to_sop_idx(rx_queue_id);
 	cq = &enic->cq[enic_cq_rq(enic, rq_num)];
 	cq_idx = cq->to_clean;
 
@@ -313,7 +313,7 @@ static int enicpmd_dev_rx_queue_setup(struct rte_eth_dev *eth_dev,
 	}
 
 	eth_dev->data->rx_queues[queue_idx] =
-		(void *)&enic->rq[enic_sop_rq(queue_idx)];
+		(void *)&enic->rq[enic_rte_rq_idx_to_sop_idx(queue_idx)];
 
 	ret = enic_alloc_rq(enic, queue_idx, socket_id, mp, nb_desc);
 	if (ret) {
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 65a8307..7689c3c 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -241,14 +241,14 @@ void enic_init_vnic_resources(struct enic *enic)
 	struct vnic_rq *data_rq;
 
 	for (index = 0; index < enic->rq_count; index++) {
-		cq_idx = enic_cq_rq(enic, enic_sop_rq(index));
+		cq_idx = enic_cq_rq(enic, enic_rte_rq_idx_to_sop_idx(index));
 
-		vnic_rq_init(&enic->rq[enic_sop_rq(index)],
+		vnic_rq_init(&enic->rq[enic_rte_rq_idx_to_sop_idx(index)],
 			cq_idx,
 			error_interrupt_enable,
 			error_interrupt_offset);
 
-		data_rq = &enic->rq[enic_data_rq(index)];
+		data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(index)];
 		if (data_rq->in_use)
 			vnic_rq_init(data_rq,
 				     cq_idx,
@@ -462,17 +462,17 @@ int enic_enable(struct enic *enic)
 
 	for (index = 0; index < enic->rq_count; index++) {
 		err = enic_alloc_rx_queue_mbufs(enic,
-			&enic->rq[enic_sop_rq(index)]);
+			&enic->rq[enic_rte_rq_idx_to_sop_idx(index)]);
 		if (err) {
 			dev_err(enic, "Failed to alloc sop RX queue mbufs\n");
 			return err;
 		}
 		err = enic_alloc_rx_queue_mbufs(enic,
-			&enic->rq[enic_data_rq(index)]);
+			&enic->rq[enic_rte_rq_idx_to_data_idx(index)]);
 		if (err) {
 			/* release the allocated mbufs for the sop rq*/
 			enic_rxmbuf_queue_release(enic,
-				&enic->rq[enic_sop_rq(index)]);
+				&enic->rq[enic_rte_rq_idx_to_sop_idx(index)]);
 
 			dev_err(enic, "Failed to alloc data RX queue mbufs\n");
 			return err;
@@ -569,8 +569,10 @@ int enic_stop_wq(struct enic *enic, uint16_t queue_idx)
 
 void enic_start_rq(struct enic *enic, uint16_t queue_idx)
 {
-	struct vnic_rq *rq_sop = &enic->rq[enic_sop_rq(queue_idx)];
-	struct vnic_rq *rq_data = &enic->rq[rq_sop->data_queue_idx];
+	struct vnic_rq *rq_sop;
+	struct vnic_rq *rq_data;
+	rq_sop = &enic->rq[enic_rte_rq_idx_to_sop_idx(queue_idx)];
+	rq_data = &enic->rq[rq_sop->data_queue_idx];
 	struct rte_eth_dev *eth_dev = enic->rte_dev;
 
 	if (rq_data->in_use)
@@ -584,8 +586,10 @@ int enic_stop_rq(struct enic *enic, uint16_t queue_idx)
 {
 	int ret1 = 0, ret2 = 0;
 	struct rte_eth_dev *eth_dev = enic->rte_dev;
-	struct vnic_rq *rq_sop = &enic->rq[enic_sop_rq(queue_idx)];
-	struct vnic_rq *rq_data = &enic->rq[rq_sop->data_queue_idx];
+	struct vnic_rq *rq_sop;
+	struct vnic_rq *rq_data;
+	rq_sop = &enic->rq[enic_rte_rq_idx_to_sop_idx(queue_idx)];
+	rq_data = &enic->rq[rq_sop->data_queue_idx];
 
 	ret2 = vnic_rq_disable(rq_sop);
 	rte_mb();
@@ -606,8 +610,8 @@ int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
 	uint16_t nb_desc)
 {
 	int rc;
-	uint16_t sop_queue_idx = enic_sop_rq(queue_idx);
-	uint16_t data_queue_idx = enic_data_rq(queue_idx);
+	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);
 	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;
@@ -963,7 +967,7 @@ static int enic_set_rsscpu(struct enic *enic, u8 rss_hash_bits)
 
 	for (i = 0; i < (1 << rss_hash_bits); i++)
 		(*rss_cpu_buf_va).cpu[i / 4].b[i % 4] =
-			enic_sop_rq(i % enic->rq_count);
+			enic_rte_rq_idx_to_sop_idx(i % enic->rq_count);
 
 	err = enic_set_rss_cpu(enic,
 		rss_cpu_buf_pa,
@@ -1115,8 +1119,8 @@ enic_reinit_rq(struct enic *enic, unsigned int rq_idx)
 	unsigned int cq_idx = enic_cq_rq(enic, rq_idx);
 	int rc = 0;
 
-	sop_rq = &enic->rq[enic_sop_rq(rq_idx)];
-	data_rq = &enic->rq[enic_data_rq(rq_idx)];
+	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)];
 
 	vnic_cq_clean(&enic->cq[cq_idx]);
 	vnic_cq_init(&enic->cq[cq_idx],
@@ -1132,12 +1136,14 @@ enic_reinit_rq(struct enic *enic, unsigned int rq_idx)
 		     0 /* cq_message_addr */);
 
 
-	vnic_rq_init_start(sop_rq, enic_cq_rq(enic, enic_sop_rq(rq_idx)),
-			   0, sop_rq->ring.desc_count - 1, 1, 0);
+	vnic_rq_init_start(sop_rq, enic_cq_rq(enic,
+			   enic_rte_rq_idx_to_sop_idx(rq_idx)), 0,
+			   sop_rq->ring.desc_count - 1, 1, 0);
 	if (data_rq->in_use) {
 		vnic_rq_init_start(data_rq,
-				   enic_cq_rq(enic, enic_data_rq(rq_idx)),
-				   0, data_rq->ring.desc_count - 1, 1, 0);
+				   enic_cq_rq(enic,
+				   enic_rte_rq_idx_to_data_idx(rq_idx)), 0,
+				   data_rq->ring.desc_count - 1, 1, 0);
 	}
 
 	rc = enic_alloc_rx_queue_mbufs(enic, sop_rq);
@@ -1210,7 +1216,8 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
 	for (rq_idx = 0; rq_idx < enic->rq_count * 2; rq_idx++) {
 		rq = &enic->rq[rq_idx];
 		if (rq->is_sop && rq->in_use) {
-			rc = enic_stop_rq(enic, enic_rq_sop(rq_idx));
+			rc = enic_stop_rq(enic,
+					  enic_sop_rq_idx_to_rte_idx(rq_idx));
 			if (rc) {
 				dev_err(enic, "Failed to stop Rq %u\n", rq_idx);
 				goto set_mtu_done;
@@ -1232,7 +1239,7 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
 
 	/* free and reallocate RQs with the new MTU */
 	for (rq_idx = 0; rq_idx < enic->rq_count; rq_idx++) {
-		rq = &enic->rq[enic_sop_rq(rq_idx)];
+		rq = &enic->rq[enic_rte_rq_idx_to_sop_idx(rq_idx)];
 
 		enic_free_rq(rq);
 		rc = enic_alloc_rq(enic, rq_idx, rq->socket_id, rq->mp,
@@ -1258,7 +1265,7 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
 
 	/* restart Rx traffic */
 	for (rq_idx = 0; rq_idx < enic->rq_count; rq_idx++) {
-		rq = &enic->rq[enic_sop_rq(rq_idx)];
+		rq = &enic->rq[enic_rte_rq_idx_to_sop_idx(rq_idx)];
 		if (rq->is_sop && rq->in_use)
 			enic_start_rq(enic, rq_idx);
 	}
-- 
2.10.0

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

* Re: [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure
  2016-10-12 21:12 [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure John Daley
  2016-10-12 21:12 ` [dpdk-dev] [PATCH 2/2] net/enic: trivial function name changes John Daley
@ 2016-10-19  9:42 ` Bruce Richardson
  1 sibling, 0 replies; 3+ messages in thread
From: Bruce Richardson @ 2016-10-19  9:42 UTC (permalink / raw)
  To: John Daley; +Cc: dev

On Wed, Oct 12, 2016 at 02:12:02PM -0700, John Daley wrote:
> The incorrect completion queue corresponding to an RQ would be
> freed if multiple Rx queues are in use and the MTU is changed,
> or an Rx queue is released. This could lead to a segmentation fault
> when the device is disabled or even in the Rx or Tx paths.
> 
> The index of the completion queue corresponding to a RQ needed
> to be adjusted after Rx scatter was introduced.
> 
> Fixes: 856d7ba7ed22 ("net/enic: support scattered Rx")
> 
> Signed-off-by: John Daley <johndale@cisco.com>
> Reviewed-by: Nelson Escobar <neescoba@cisco.com>
> ---
Patchset applied to dpdk-next-net/rel_16_11

/Bruce

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

end of thread, other threads:[~2016-10-19  9:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-12 21:12 [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure John Daley
2016-10-12 21:12 ` [dpdk-dev] [PATCH 2/2] net/enic: trivial function name changes John Daley
2016-10-19  9:42 ` [dpdk-dev] [PATCH 1/2] net/enic: fix crash on MTU update or rxq reconfigure Bruce Richardson

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